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.

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

refactor: chain storage stub for validator node #3525

Merged
merged 88 commits into from
Nov 3, 2021

Conversation

stringhandler
Copy link
Collaborator

No description provided.

stringhandler and others added 30 commits October 1, 2021 11:40
- 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
…ct-chain-metadata

* development:
  fix: fix recovery test reporting message (tari-project#3479)
  chore: improve cucumber tests to wait for broadcast (tari-project#3461)
  test: use TCP node for daily sync test (tari-project#3464)
hansieodendaal and others added 25 commits October 28, 2021 08:31
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
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
…ct-chain-metadata

* development: (32 commits)
  test: improve cucumber with wallets (tari-project#3507)
  feat: optimize pending transactions inbound query (tari-project#3500)
  test: add trace logs to wallet's base node monitor (tari-project#3502)
  fix: improve test Wallet should display transactions made (tari-project#3501)
  test: change timeouts in ci to reasonable values (tari-project#3494)
  fix: correct panic in tracing for comms (tari-project#3499)
  feat: optimize get transactions query (tari-project#3496)
  fix: fix config file whitespace issue when auto generated in windows (tari-project#3491)
  bump to rerun tests
  fix: improve responsiveness of wallet base node switching (tari-project#3488)
  feat: add decay_params method (tari-project#3454)
  Revert "macos-11"
  macos-11
  clean
  remove scripts after install
  install to tmp then use script to copy to home
  wip
  wip
  path
  wip
  ...
Description
---
Updates the windows installer ci build step, `iscc.exe` doesn't run the [precompile] step

Motivation and Context
---

How Has This Been Tested?
---
https://github.com/delta1/tari/runs/4020028249?check_suite_focus=true
…roject#3478)

Description
---
- 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)
- Ban 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 that claim to provide enough full
  blocks for _our_ pruning horizon (fixes cucumber test)

Motivation and Context
---
A peer may disrupt a base node by sending false chain metadata. This PR ensures that if the remote peer cannot provide
the claimed chain, it will get banned and the base node will continue in listening state immediately.

How Has This Been Tested?
---
Ran sync cucumber tests
Manually, by creating a node that inflates it's advertised accumulated Pow by a small amount and checking that the node is banned by the syncing node.
not entirely related: add block sync RPC unit tests
…ct#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 tari-project#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)
```
* 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.
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
…ari-project#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
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.
# Conflicts:
#	applications/tari_validator_node/src/dan_layer/dan_node.rs
#	applications/tari_validator_node/src/dan_layer/models/instruction_set.rs
#	applications/tari_validator_node/src/dan_layer/models/mod.rs
#	applications/tari_validator_node/src/dan_layer/models/tari_dan_payload.rs
#	applications/tari_validator_node/src/dan_layer/services/asset_processor.rs
#	applications/tari_validator_node/src/dan_layer/services/infrastructure_services/inbound_connection_service.rs
#	applications/tari_validator_node/src/dan_layer/services/payload_processor.rs
#	applications/tari_validator_node/src/dan_layer/services/payload_provider.rs
#	applications/tari_validator_node/src/dan_layer/storage/lmdb/mod.rs
#	applications/tari_validator_node/src/dan_layer/storage/mod.rs
#	applications/tari_validator_node/src/dan_layer/templates/editable_metadata_template.rs
#	applications/tari_validator_node/src/dan_layer/workers/consensus_worker.rs
#	applications/tari_validator_node/src/dan_layer/workers/states/decide_state.rs
#	applications/tari_validator_node/src/dan_layer/workers/states/prepare.rs
#	applications/tari_validator_node/src/dan_layer/workers/states/starting.rs
# Conflicts:
#	.github/workflows/ci.yml
#	Cargo.lock
#	base_layer/core/Cargo.toml
#	base_layer/core/src/transactions/transaction_protocol/sender_transaction_protocol_builder.rs
#	base_layer/wallet/Cargo.toml
#	base_layer/wallet/src/base_node_service/config.rs
#	base_layer/wallet/src/transaction_service/storage/database.rs
#	base_layer/wallet_ffi/src/callback_handler.rs
#	common/config/presets/console_wallet.toml
#	integration_tests/features/support/steps.js
#	integration_tests/features/support/world.js
@stringhandler stringhandler changed the title WIP Chain Storage for validator node refactor: chain storage stub for validator node Nov 3, 2021
@stringhandler stringhandler marked this pull request as ready for review November 3, 2021 06:07
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM? 🤷

@stringhandler stringhandler merged commit df25d03 into tari-project:validator-node Nov 3, 2021
@stringhandler stringhandler deleted the mb-v-next branch November 3, 2021 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants