Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add caching and clippy annotations to CI #3518

Merged
merged 1 commit into from
Oct 31, 2021
Merged

feat: add caching and clippy annotations to CI #3518

merged 1 commit into from
Oct 31, 2021

Conversation

CjS77
Copy link
Collaborator

@CjS77 CjS77 commented Oct 31, 2021

  • Adds caching to the standard CI flow; this should drastically reduce build times when pushing small fixes to PRs by 90% or more.
  • Introduce clippy-check that puts annotations in the PR for identifying clippy warnings.
    • The motivation for this is that recent changes to clippy make warnings into hard errors, which shouldn't break an entire PR build (e.g. missing impl Default warning).
    • Fixing clippy errors are always "Good first issues", and having annotations in the code makes it clear where first-time contributors need to focus.

@CjS77 CjS77 mentioned this pull request Oct 31, 2021
Merged
@aviator-app aviator-app bot merged commit beacb9e into development Oct 31, 2021
@sdbondi sdbondi deleted the cache_ci branch November 1, 2021 04:17
stringhandler added a commit that referenced this pull request Nov 3, 2021
@stringhandler @sdbondi
@aviator-app @CjS77 @hansieodendaal @StriderDM @therustmonk @SWvheerden
refactor: chain storage stub for validator node (#3525)
df25d03 
* ci: run libwallet daily

* chore: update rust toolchain refs

* fix: ban peer that advertises higher PoW than able to provide

- Can only transition to `HeaderSync` if claimed chain metadata is
  advertised
- `HeaderSync` is now aware of the claimed `ChainMetadata`
- `HeaderSync` now assumes (invariant) that all sync peers have
  claimed a higher accumulated PoW and will ban them if the validated
  accumulated difficulty does not reach the claimed acc diff.
- Adds ban condition in `determine_sync_status` phase, if a peer is not
  able to improve on the local chain strength (because we know that in
  order to be selected for header sync it must have advertised a
  stronger chain)
- Adds ban condition if header sync completes but is less than the
  claimed PoW. This is not strictly necessary since they were still able
  to provide a stronger chain as per Nakamoto consensus, but could still
  indicate some malicious intent.
- If sync fails for all peers, the state machine continues rather than
  `WAITING`. This removes the disruption that false metadata can cause.
- fix `select_sync_peers` to include peers claim that provide a enough full
  blocks for _our_ pruning horison (fixes cucumber test)
  higher than the local pruned

* update osx zipper

* wip

* wip2

* wip3

* wip4

* wip

* wip

* wip

* wip

* wip

* yolo

* wip

* path

* wip

* wip

* install to tmp then use script to copy to home

* remove scripts after install

* Fix missing awaits in cucumber tests

* Increase timeouts for tip height waiting

* Fix silly mistake on cucumber step

* clean

* ci: delete versioning action (#3482)

Description
---
Removes the versioning action, it is no longer required.

* macos-11

* Revert "macos-11"

This reverts commit 6d9549e.

* ci: mark test 'pruned mode reorg past horizon' as flaky

* feat: add decay_params method (#3454)

Wondering how the emission schedule parameters are derived?

This provides a `decay_params` method that calculates the decay array parameters
used in EmissionSchedule.

The method serves as reference and means of determining the array
parameters.

* fix: improve responsiveness of wallet base node switching (#3488)

Description
---

- clear previous base node's state if a new base node is selected
- interrupt sleep in bn monitor early if base node has changed
- interrupt get_tip_info RPC call early if base node has changed
- dont trigger set peer events if same peer is set


Motivation and Context
---
When the user selects a base node, the UI could appear slow when a sleep is in progress in the base node monitor.
During this time, the previous node's latency and height are displayed, which also gives the impression of an unresponsive UI.

How Has This Been Tested?
---
Manually by switching base nodes

* fix after merge

* bump to rerun tests

* fix: fix config file whitespace issue when auto generated in windows (#3491)


Description
---
- Added whitespace at the end of each individual config file so that the generated file do not have file end and file beginning overlaps in a single line when generated for Windows. 
- Re-inserted banners for each section that were removed by a previous PR.

Motivation and Context
---
The generated `tari_config_example.toml` had overlapping information in joining lines.

How Has This Been Tested?
---
Generated the combined `tari_config_example.toml` file with the provided `generate_config.bat`.

* feat: optimize get transactions query (#3496)

Description
---
- Optimized the get transactions query (`broadcast_all_completed_transactions`) for transactions that need to be broadcast/rebroadcast by sending a single diesel SQL query that only returns the result, instead of multiple queries that return all the transactions in the database with filtering and selection in the Rust code.
- Added a new unit test 'test_get_tranactions_to_be_rebroadcast'.

Motivation and Context
---
It is much more efficient to have a SQL query perform the filtering upfront.

How Has This Been Tested?
---
Unit tests, cucumber tests.

* refactor: move payload processor to prepare stage

* fix: correct panic in tracing for comms (#3499)

Description
---
Fixes a panic for tracing,
Adds additional comments for viewing the Jaeger container after running the image in docker.

Motivation and Context
---
Without this PR, the following error is encountered when using the `--tracing-enabled` flag
```
thread 'tokio-runtime-worker' panicked at 'Span to follow not found, this is a bug', .cargo\registry\src\gh.neting.cc-1ecc6299db9ec823\tracing-opentelemetry-0.15.0\src\layer.rs:484:14
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'tokio-runtime-worker' panicked at 'Mutex poisoned: PoisonError { .. }', .cargo\registry\src\gh.neting.cc-1ecc6299db9ec823\tracing-subscriber-0.2.20\src\registry\sharded.rs:400:58
```

How Has This Been Tested?
---
Manually

* ci: fix windows installer build github action

* test: change timeouts in ci to reasonable values (#3494)

Description
---
Reduces timeouts to reasonable values. Used max values from reports of successful passes.

Motivation and Context
---
CI takes up to 4+ hours when issues happen and we have to wait when the full timeout expires.

How Has This Been Tested?
---
CI pass expected.

* fix: improve test Wallet should display transactions made (#3501)

Description
---
This test can have the wallets fail to talk to each other before sending begins which results in the wallets talking over SAF. This is a much slower communication method which can result in the step `And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100` failing to reach the broadcast stage. 
This PR moves the init step of Wallet_B much earlier to increase the changes that the two wallets should communicate directly.

* test: add trace logs to wallet's base node monitor (#3502)

Description
---
- Re-added trace logs to the console wallet's base node monitor to enable profiling of slow responses.
- Consistent fixed ms format rather than significant digits for log entries were used to enable easy creation of timing graphs with post-processing of the log files.

Motivation and Context
---
These trace logs are still needed for the improvement effort of the slow console wallet responses.

How Has This Been Tested?
---
Ran cargo clippy and cargo format.

* messy wip

* feat: optimize pending transactions inbound query (#3500)

Description
---
- Optimized pending inbound transaction query by doing filtering with SQL.
- Expanded the unit tests to also test the new query.

Motivation and Context
---
This is a part of a series of PRs to reduce the memory footprint of the console wallet.

How Has This Been Tested?
---
Unit tests, wallet cucumber tests

* fix: wallet grpc setting

* test: improve cucumber with wallets (#3507)

Description
---
It is beneficial to start all wallets as soon as possible in a cucumber test if the test logic allows it to improve wallet discovery finishing within the set time out limits.

Motivation and Context
---
Flaky cucumber tests with CI.

How Has This Been Tested?
---
Running cucumber on CI

* feat(wallet_ffi)!: add get_balance callback to wallet ffi (#3475)

Description
---
- Added get_balance callback to the wallet ffi callback handler that fires only if the balance has actually changed.
- Expanded the wallet ffi callback handler test framework to include a mock output manager request-response server.
- _**Update:** Added required methods and interfaces to the cucumber test framework._
- _**Update:** Fixed flaky wallet FFI cucumber tests._
- _**Update:** Fixed a bug in the wallet FFI cucumber test framework where the return type of `ref.types.ulonglong` did not correspond to the Rust return type and had a memory alignment problem._
- _**Update:** Fixed an issue whereby on a fast 8-core multi-tasking computer (e.g. AMD Ryzen 7 2700X) some of the wallet FFI cucumber tests did not complete properly after the test and went into an endless wait. The root cause of this issue has been traced down to incorrect use of synchronous calls to wallet FFI destroy methods where in actual fact those methods have async behaviour._
- _**Update:** Re-applied #3271._

~~The following anomaly exists when compiling the proposed `wallet_ffi/tests` module:~~
```
24 | use tari_wallet_ffi::callback_handler::CallbackHandler;
   |     ^^^^^^^^^^^^^^^ use of undeclared crate or module `tari_wallet_ffi`
```

_**Update**_ ~~Various code organizations have been tried, all with the same result. As an alternative, a working test and output manager service mock has been added into the test module in `callback_handler.rs`. Hopefully, the anomaly can be fixed. Duplicate code will be removed before the final commit.~~

Motivation and Context
---
- Mobile wallet efficiency.
- Resilient wallet FFI cucumber tests.

How Has This Been Tested?
---
- Expanded the current FFI `test_callback_handler` unit test.
- _**Update:** Ran all the wallet FFI cucumber tests to verify the new callback is working properly when using the wallet FFI library:_ 

```
2021-10-21T06:29:32.160Z callbackTransactionValidationComplete(9123501482775375388,0)
2021-10-21T06:29:32.161Z callbackBalanceUpdated: available = 0,  time locked = 0  pending incoming = 2000000 pending outgoing = 0
2021-10-21T06:29:32.263Z received Transaction with txID 14659183447022727953

...

2021-10-21T06:31:38.358Z Transaction with txID 14659183447022727953 was mined.
2021-10-21T06:31:38.358Z callbackBalanceUpdated: available = 2000000,  time locked = 2000000  pending incoming = 0 pending outgoing = 0
2021-10-21T06:31:38.359Z callbackTransactionValidationComplete(17755253868227079780,0)
```

* v0.12.0

* v0.12.0

* wip

* test: increase limit for cucumber

* ci: disable builds on ci

* feat: add caching and clippy annotations to CI (#3518)

* Adds caching to the standard CI flow; this should drastically reduce build times when pushing small fixes to PRs by 90% or more.
* Introduce clippy-check that puts annotations in the PR for identifying clippy warnings.
  * The motivation for this is that recent changes to clippy make warnings into hard errors, which shouldn't break an entire PR build (e.g. `missing impl Default` warning).
  * Fixing clippy errors are always "Good first issues", and having annotations in the code makes it clear where first-time contributors need to focus.

* test: add logs on non-passing tests (#3516)

Description
---
Change the test to add logs from `=== 'failed'` to `!== 'passed'`. 

Motivation and Context
---
When a step would timeout, it would not trigger the step to add logs

How Has This Been Tested?
---
Checking on ci

* test: fix stack overflow for noise::larger_writes test 1.57-nightly (#3515)

Description
---
Remove large slices from the stack in noise tests

Motivation and Context
---
Possible to have a stack overflow when running tests on new nightly

How Has This Been Tested?
---
Tests pass without overflow

* wip

* chore: update toolchain to nightly-2021-09-18 (#3514)

This seemingly innoucous update required several changes due to changes
in how the compiler interprets dead code.

* A few variables were renamed _x where it looked like x might be used
  in upcoming PRs. YAGNI maybe, but I gave the benefit of the doubt in
these cases.
* Some crates that had LOADS of dead code (wallet ffi crate I'm looking at
  you), I just changed the `deny(dead_code)` to `warn(dead_code)`
* And in some places, I just nuked the dead code, which tidied up some
  heavy code in the wallet in particular. Since the tests pass, I
presume this is fine and there isn't a weird new compiler bug that is
missing where the code is needed.

* wip

* fixes after compile

* refactor: move to dan_core

* chore: merge development

Co-authored-by: Byron Hambly <bizzle@tari.com>
Co-authored-by: Stanimal <sdbondi@users.noreply.github.com>
Co-authored-by: mergequeue[bot] <48659329+mergequeue[bot]@users.noreply.github.com>
Co-authored-by: Cayle Sharrock <CjS77@users.noreply.github.com>
Co-authored-by: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com>
Co-authored-by: David Main <51991544+StriderDM@users.noreply.github.com>
Co-authored-by: Denis Kolodin <DenisKolodin@gmail.com>
Co-authored-by: SW van Heerden <swvheerden@gmail.com>
stringhandler added a commit that referenced this pull request Nov 4, 2021
@stringhandler
v0.13.0
b42058a 
BREAKING CHANGES

* implement new CipherSeed and upgrade encryption KDF (#3505)

Features

* add a Rejected status to TransactionStatus ([#3512](#3512)) ([c65a01c](c65a01c))
* add caching and clippy annotations to CI ([#3518](#3518)) ([beacb9e](beacb9e))
* implement new CipherSeed and upgrade encryption KDF ([#3505](#3505)) ([ef4f84f](ef4f84f))

Bug Fixes

* edge case fix for integer pair iter ([#3508](#3508)) ([097e3e2](097e3e2))
* header sync must allow transition to archival/pruned if tip is behind ([#3520](#3520)) ([e028386](e028386))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdbondi sdbondi sdbondi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CjS77 @sdbondi