chore: merge development into installer branch #3279
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The recovery task broke out of its monitoring loop before getting the `UtxoScannerEvent::Completed` event. This PR just moves that break statement so that the final completed callback is made. Also Ignore `test_store_and_forward_send_tx` due to it being flakey on CI and the functionality is covered by Cucumber tests.
## Description The recovery task broke out of its monitoring loop before getting the `UtxoScannerEvent::Completed` event. This PR just moves that break statement so that the final completed callback is made. ## How Has This Been Tested? Wallet clients will need to test this ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
…cript The `wallet_import_utxo` FFI function in LibWallet just used defaults for a number of the new UTXO fields when importing a faucet UTXO. The Faucet UTXO provided by the client is just the spending key and amount. The `metadata_signature` and `sender_offset_public_key` can both remain as default values as they are not used in spending an UTXO. A Nop script is assumed and the spending key is used as the `script_private_key`. The final update is that the `input_data` it set as the public key of the spending key. To test that the base node is happy for an UTXO imported in this way to be spent a Cucumber test is provided which imports a UTXO into a wallet and zeroes out the `metadata_signature` and`sender_offset_public_key` and updates the `input_data` and `script_private_key` in the same way as described above. This imported Faucet utxo is then successfully spent to another wallet.
…riScript (tari-project#3139) ## Description The `wallet_import_utxo` FFI function in LibWallet just used defaults for a number of the new UTXO fields when importing a faucet UTXO. The Faucet UTXO provided by the client is just the spending key and amount. The `metadata_signature` and `sender_offset_public_key` can both remain as default values as they are not used in spending an UTXO. A Nop script is assumed and the spending key is used as the `script_private_key`. The final update is that the `input_data` it set as the public key of the spending key. To test that the base node is happy for an UTXO imported in this way to be spent a Cucumber test is provided which imports a UTXO into a wallet and zeroes out the `metadata_signature` and`sender_offset_public_key` and updates the `input_data` and `script_private_key` in the same way as described above. This imported Faucet utxo is then successfully spent to another wallet. ## How Has This Been Tested? Cucumber test provided ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
The current architecture of the wallet is that the AppState contains a cache of the current state that the UI uses to draw from and a second instance of the data that is updated in the background when events are received from the wallet services without interfering with drawing. When the background data has been updated by the event monitoring thread it flips a flag telling the UI that the cache has been invalidated so when the drawing is done on a Tick event the UI thread will clone the background data into the cache for future drawing calls. A problem was found where when a large number of transactions were being processed by the wallet the UI would become unresponsive. The reason for this is that with a large amount of transactions there is quite a lot of AppState that is copied from the background into the UI cache which could take 300-400ms and this cache was being invalidated very often as the transactions are being handled by the wallet services. This would mean that a Cache copy occurred after every single draw cycle and would block the processing of Key events for 300-400ms. This PR proposes a solution of introducing a cache update cooldown period, initially set to 2 seconds, so that if a cache update has occurred the soonest that the next update can occur is 2 seconds later giving the UI thread time to handle key events. In normal wallet operation state update events do not occur that often so this approach will allow the cache update to occur as soon as the cache is invalidated but will force a fast subsequent update to wait at least 2 seconds. In the mean time the background data can be worked on in the background thread.
<!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> We sometimes find that transactions created by the cucumber transaction builder fail lmdb insertion by duplicate excess. This increases the private key range for the private keys increasing the range of the excess. This should decrease duplicate entries. ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
<!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> When a transaction is not found on `GetTransactionInfo` the GRPC call will return a transaction with "junk" data including a status that is set to complete. But the not_found flag is set to false, which is also the default GRPC value for boolean. This removes the `not_found` flag and adds a status, `not found` to avoid confusion. ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
Update proxy.rs WIP Added stratum configuration to miner. Mostly implemented stratum. Mostly implemented stratum controller. Partially implemented stratum miner. Rebased to latest dev Import PR#3006 Rebased to latest dev and updated version to 0.9.0 Fixed tari_stratum_ffi tests Clippy and cargo-fmt Bug fixes Return blockheader as json object. Retrieve recipients from params instead of directly from body of request. Fix bug in GetHeaderByHeight Update stratum miner to receive blockheader instead of block clippy update Update Implemented keepalive Bug fix for transfer results Implemented stratum error code response handling in tari_mining_node Rebase fix Update stratum.rs Update stratum.rs Review Comments Update and Fixes Added ResumeJob to MinerMessage. Fixed disconnection bug where miner would not resume solves on reconnect. Added transcoder_host_address config variable to stop using proxy_host_address as it is already used by mm_proxy, this enables them both to be run simultaneously. Update cucumber config variables
- protocol notifications now have a set "safety" timeout. - add log for inbound comms pipeline concurrency usage
A bug was observed that now and then a Coinbase Transaction would not conclude its monitoring with this error: `ValueNotFound(CompletedTransaction(15645503741743694020))` This occurred when due to a small network reorg the miner would request a block for the same height. The new request would often have a different amount of fees which would mean this transaction needs to be cancelled in favour of the new transaction. However, with the transaction being cancelled the coinbase monitoring task will come around to checking the DB if the transaction is still there so it can continue to poll the base node and will get the ValueNotFound error. This is correct behaviour so should not have been logged as an error. The error case also means that the TransactionCancelled event is never fired which resulted in the console wallet UI not updating the state of that transaction which left it in the transaction list erroneous. So this PR handles that “Error” properly and sends the event before ending the coinbase monitoring task.
increase key length
This PR adds the tari_stratum_ffi library and tari_stratum_transcoder application for use with Miningcore. This PR also expands tari_mining_node with stratum support to interact with Miningcore. See here for the Tari implementation in Miningcore: Source: https://github.com/StriderDM/miningcore/tree/tari Compare: coinfoundry/miningcore@master...StriderDM:tari Edit: Functional, will need refactoring
…roject#3143) <!--- Provide a general summary of your changes in the Title above --> <!--- Describe your changes in detail --> - protocol notifications now have a set "safety" timeout. - add log for inbound comms pipeline concurrency usage <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> Reports of inbound messaging being blocked. This will shed some light. The logs show that a single client node is excessively trying to create multiple `t/bn-wallet//1` (rpc) sessions. <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> Log observed in base node <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
Moved stratum_transcoder config variables to its' own section. Fixed bug in defaults for stratum_transcoder. Updated example config and cucumber variable to reflect changes in configuration. Added stratum mode configuration variables for tari_mining_node in sample config, commented out by default.
…#3146) ## Description The current architecture of the wallet is that the AppState contains a cache of the current state that the UI uses to draw from and a second instance of the data that is updated in the background when events are received from the wallet services without interfering with drawing. When the background data has been updated by the event monitoring thread it flips a flag telling the UI that the cache has been invalidated so when the drawing is done on a Tick event the UI thread will clone the background data into the cache for future drawing calls. A problem was found where when a large number of transactions were being processed by the wallet the UI would become unresponsive. The reason for this is that with a large amount of transactions there is quite a lot of AppState that is copied from the background into the UI cache which could take 300-400ms and this cache was being invalidated very often as the transactions are being handled by the wallet services. This would mean that a Cache copy occurred after every single draw cycle and would block the processing of Key events for 300-400ms. This PR proposes a solution of introducing a cache update cooldown period, initially set to 2 seconds, so that if a cache update has occurred the soonest that the next update can occur is 2 seconds later giving the UI thread time to handle key events. In normal wallet operation state update events do not occur that often so this approach will allow the cache update to occur as soon as the cache is invalidated but will force a fast subsequent update to wait at least 2 seconds. In the mean time the background data can be worked on in the background thread. ## How Has This Been Tested? Manually Tested ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
…me height (tari-project#3151) ## Description A bug was observed that now and then a Coinbase Transaction would not conclude its monitoring with this error: `ValueNotFound(CompletedTransaction(15645503741743694020))` This occurred when due to a small network reorg the miner would request a block for the same height. The new request would often have a different amount of fees which would mean this transaction needs to be cancelled in favour of the new transaction. However, with the transaction being cancelled the coinbase monitoring task will come around to checking the DB if the transaction is still there so it can continue to poll the base node and will get the ValueNotFound error. This is correct behaviour so should not have been logged as an error. The error case also means that the TransactionCancelled event is never fired which resulted in the console wallet UI not updating the state of that transaction which left it in the transaction list erroneous. So this PR handles that “Error” properly and sends the event before ending the coinbase monitoring task. ## How Has This Been Tested? unit tests ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
<!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> - Use the GRPC client connection given `WalletProcess` - Outputs error details in webhook in recovery cron job - Adds very basic mocha tests ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> Recovery daily is failing even though it succeeds. ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
…reate` (tari-project#3249) Description --- ### Note: This is a breaking change to LibWallet FFI Currently if a wallet recovery was in progress and the wallet was shutdown the next time that wallet is start by an FFI client using the ‘wallet_create’ method there is no way for the FFI client to know that the recovery should be continued. The wallet is able to resume the recovery from where it left off and it should so as not to lose funds but the FFI client must restart the recovery process with the same seed words. The FFI client has to do the restarting so that it can provide the callback through which the process is monitored. Furthermore, the wallet does not respond to P2P transaction negotiation message if a recovery process is in progress so it is important that an FFI client completes any outstanding recoveries ASAP. How Has This Been Tested? --- untested in the backend.
Description --- Fixed the base_node_service_config not being initialized with values from the config file. Motivation and Context --- See above How Has This Been Tested? --- System level testing
…oject#3252) Description --- Add the —exit flag to the Cucumber CI commands to force Cucumber to end when the tests are completed. This doesn’t solve the issue where something is keeping the Cucumber process running due to a poor shutdown though. How Has This Been Tested? --- N/A
Description --- This Request for Comment (RFC) aims to describe Tari's ergonomic approach to securing funds in a hot wallet. The focus is on mobile wallets, but the strategy described here is equally applicable to console or desktop wallets. Motivation and Context --- This philosophy has been partially implemented in Aurora already but has not been captured in community documentation before. How Has This Been Tested? --- N/A
Description --- Add tracing to comms to debug timings via the `--tracing-enabled` flag Motivation and Context --- It's currently difficult to understand the timings of network calls and errors in the application. How Has This Been Tested? --- Tested manually
<!--- Provide a general summary of your changes in the Title above -->
## Description
<!--- Describe your changes in detail -->
This provides a audit removing and increasing security over the following db methods from the WriteOperation enum:
```rust
InsertChainOrphanBlock(Arc<ChainBlock>),
InsertInput {
header_hash: HashOutput,
input: Box<TransactionInput>,
mmr_position: u32,
},
InsertKernel {
header_hash: HashOutput,
kernel: Box<TransactionKernel>,
mmr_position: u32,
},
InsertOutput {
header_hash: HashOutput,
output: Box<TransactionOutput>,
mmr_position: u32,
},
DeleteHeader(u64),
DeleteOrphanChainTip(HashOutput),
InsertOrphanChainTip(HashOutput),
SetBestBlock {
height: u64,
hash: HashOutput,
accumulated_difficulty: u128,
},
SetPruningHorizonConfig(u64),
SetPrunedHeight {
height: u64,
kernel_sum: Commitment,
utxo_sum: Commitment,
},
```
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
This synced to tip, and passed all unit tests
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
* [x] I'm merging against the `development` branch.
* [ ] I have squashed my commits into a single commit.
… of rounds (tari-project#3211) <!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> Use the dedup cache hit count to allow certain duplicate messages through a configurable number of times. ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> ~~This improves mempool synchronization.~~ Implements gossip repropagation that could be used for some message types in future. ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> New unit test. More manual system tests need to be done ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> * [x] I'm merging against the `development` branch. * [x] I have squashed my commits into a single commit.
Description
Refactored WalletFFI.feature into a working state, tested locally.
Further refactoring and dead code removal would be beneficial.
Motivation and Context
Necessary to get WalletFFI.feature working.
How Has This Been Tested?
Tested locally, each scenario tested with:
`./node_modules/.bin/cucumber-js --name "${scenario_name}"`
…ri-project#3237) Description --- - Adds a periodic check of the connection status and attempts a reconnect if no longer connected. Previously it was assumed that this can be done lazily because some caller will always call `obtain_base_node_wallet_rpc_client`, but this may not be the case. A periodic check is added. - Clean up some state checking to use the wallet connectivity service. Motivation and Context --- Improves snappiness of the connectivity and chain state updates in the wallet How Has This Been Tested? --- Manually on the console wallet + existing tests
Description --- Send transactions to all connected peers as we do with block propagation Motivation and Context --- Alternative to tari-project#3211. How Has This Been Tested? --- Existing tests / single line code change
…3253) Description --- This adds in random transactions spending each other and unused coin bases to fill in blocks. Motivation and Context --- This is to allow us to test more thoroughly with all blocks having transactions and not just blocks that were explicitly created with transactions. These are limited to 10 transactions per block to not make it too slow at the current validation speeds. This might be revisited in a later stage. How Has This Been Tested? --- Manually confirmed that the blocks do have the transactions in them. Ran all cucumber tests with the flags: critical and not broken and not flaky
…oject#3255) Description --- This PR changes the peer metadata push to listing mode speed to push every time it receives a chain metadata ping or pong message. Motivation and Context --- This is introduced to allow a node to switch faster and not wait till it received it all the pings and pongs from a node. How Has This Been Tested? --- Run all unit tests and manually ran node.
Description --- The escape sequence was eating up the string "Starting recovery at height: ". How Has This Been Tested? --- Manually/visually.
Description --- This PR adds support for the Igor testnet to the repo. This involves adding Igor to the Network Enum, adding a Igor generic block and adding a config file with the details of 4 Igor seed nodes (still to be rolled out) Motivation and Context --- We need a second testnet to test network switching How Has This Been Tested? --- Manually ran the network to generate seed nodes details.
Description --- - upgrades to tokio 1.10 - upgrade multiaddr to 1.13 - updates select loops to use tokio::select! - updates to use tokio mpsc and oneshot channels - remove max_threads config - removed tari_wallet dependency from tari base node - moved emoji id library out of tari wallet into tari core (in order to remove dependency on `tari_wallet` for tari base node) - Wait for bootstrap with mempool sync moved to the initializer - Unit and integration test fixup - Upgraded following crates that use or are required by tokio 1: `bytes`, `prost`, `tonic`, `reqwest`, `hyper`, `trust-dns-client` ~~Include changes from tari-project#3237 merged Motivation and Context --- Tokio runtime is perhaps the most critical dependency we have and was very out of date (was 0.2.x). This PR takes advantage of bug fixes and optimisations of tokio 1.10. How Has This Been Tested? --- - Existing unit and integration tests run and pass - Existing cucumber tests pass - Ran all tari applications (base node, console wallet, miner, mm proxy, stratum transcoder) - Ran a washing machine test on two upgraded wallets connected to an upgraded base node
…oject#3264) Description --- When entering the `synchonize_headers` function, a chain of headers has been downloaded and validated but not committed. If there less than 1000 (not equal to as before), the function can exit without streaming more as there are no more to send. This PR correctly handles the case where the node is exactly 1000 headers behind by: (1) correcting the off-by-one "no further headers to download" conditional and (2) commiting headers before starting streaming if the PoW is stronger, in case no further headers would be streamed. Motivation and Context --- Header sync ends prematurely when receiving exactly 1000 "pre-sync" headers. How Has This Been Tested? --- Manually - Sync from scratch.
Description --- The mining_node relies on its stdout logging for output for the binary and a recent global update to the logging filtered out the debug and info messages to the std out. This PR updates the default logging config for the mining node so that debug and info messages are logged to stdout. How Has This Been Tested? --- Manually
…#3247) Description Network selection for applications Motivation and Context Allows network to be selected at application start How Has This Been Tested? Manually
Description --- Adds the ability to bypass rangeproof verification. Motivation and Context --- Warning: This should not be done by default as it can cause a fork. By default this should always be set to verify rangeproofs, but in some scenarios, you want to disable it to quickly download a chain or run on a slim device. The rangeproof verification also takes the majority of time when profiling, so by disabling it, we can monitor other performance bottlenecks How Has This Been Tested? --- Manually > Note that I disabled checking of rangeproofs during wallet sending because it adds little value to validate a rangeproof that you created
Description --- Added trace tag info into the liveness log messages for improved tracing of ping-pong messages Motivation and Context --- This will help to investigate why ping-pong messages are not robust when using a single forced sync peer. How Has This Been Tested? --- System-level testing
…sabled (tari-project#3270) Description --- Continuously checks for updates when auto_update_check_interval is disabled. Thanks @mikethetike. for finding it and for the fix Add check to if no auto update URIs are configured Motivation and Context --- Bug fix When check_interval is disabled, stream::empty() is used to disable the update checking, however it returns None continuously when polled, causing the update to continuously be checked. Also sets MissedTickBehaviour::Skip - which will prevent bursts of checks if intervals are missed How Has This Been Tested? --- Ran base node with auto_update_check_interval = 0 (or equivalently without this setting set)
…tari-project#3271) Description Code was moved to ffiInterface.js and updated. Mistakenly got re-included when fixing a conflict in a rebase. Motivation and Context --- How Has This Been Tested? ---
…i-project#3272) Description --- Subscribe to connectivity events before waiting for the state machine to bootstrap Motivation and Context --- Causes cucumber ` Scenario: Transactions are synced` to fail. Could cause mempool sync not to happen in some fairly unlikely but possible cases in base node. How Has This Been Tested? --- Cucumber Scenario: Transactions are synced passes
…es to tari_common_types (tari-project#3266) Description --- It moves emoji id and common types to tari_common_types Motivation and Context --- The main problem here was a dependency on tari_base_node -> tari_wallet for `EmojiId`. Then EmojiId references PublicKey, so ended up moving a whole bunch around. > Note: Hidden in all of this is feature to compile SQLite without having it installed as a lib How Has This Been Tested? --- Manually
Description --- Remove logging of errors from tracing instrument macros. Motivation and Context --- Was reported as making the base node unusable. Hopefully we are not swallowing important information, but probably the right choice How Has This Been Tested? --- Manually > ~~Note: This PR is based on tari-project#3266 to enable compilation without SQLite installed~~
Description --- We want to see the network in the base node status line. How Has This Been Tested? --- manually
stringhandler
approved these changes
Sep 1, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Mergey McMergeface
Motivation and Context
Merging
How Has This Been Tested?
Built and started syncing base node