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

fix: dockerfiles for base node and wallet #3263

Merged
merged 236 commits into from
Aug 30, 2021

Conversation

delta1
Copy link
Contributor

@delta1 delta1 commented Aug 30, 2021

Description

Merging to installer branch as it continues from there.

Get the base node and wallet containers in the docker rig working with tor docker container.

Motivation and Context

Dockerize

How Has This Been Tested?

cd buildtools/docker_rig
docker build -t quay.io/tarilabs/tor:latest -f tor.Dockerfile .
docker build -t quay.io/tarilabs/tari_base_node:latest -f base_node.Dockerfile ../../
docker build -t quay.io/tarilabs/tari_console_wallet:latest -f console_wallet.Dockerfile ../../
docker-compose up -d #starts tor, base node, and wallet

philipr-za and others added 30 commits July 28, 2021 16:20
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/[email protected]: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.
aviator-app bot and others added 25 commits August 25, 2021 11:13
…3244)

Description
---
Add status output to logs in non-interactive mode

Motivation and Context
---
Base node in non-interactive mode was logging status

How Has This Been Tested?
---
Manually run base node in non-interactive mode
Description
---

Add a github action to check PR titles follow conventional commit spec

Motivation and Context
---

Automate checks

How Has This Been Tested?
---

github actions
Description
---
- Removed OpenSSL installers and dependencies from Windows runtime
- Removed stdout as appender from console wallet's log4rs logger

Motivation and Context
---
- OpenSSL is not a Windows runtime dependency anymore
- The console wallet cannot handle log4rs logger messages to its stdout as it breaks the TUI

How Has This Been Tested?
---
Build the installer, ran an installer, ran all the executable components
Description
---
Add a new tab in the console wallet. In the new tab is a stdout log from the wallet. The stdout is redirected to a new logfile called "stdout.log"
I've added also some coloring, it's much easier to read the log file with some simple colors.

Motivation and Context
---
Easier to debug the wallet.

How Has This Been Tested?
---
Manually.
## Description
Add support for forcing sync to a seed node. Specify index for the node from peer_seeds list.

## How Has This Been Tested?
Manually.

## 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
Add cucumber test that tests forced sync to single node.

## How Has This Been Tested?
npm test -- --name "Force sync many nodes agains one peer"

## 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
Added multiple wallet recovery from peer

Motivation and Context
Additional tests

How Has This Been Tested?
Manually (npm test -- --name "Multiple Wallet recovery from seed node")
)

Description
Prompt user to create id if not found

Motivation and Context
Improvement to base node startup, specifically on first run.

How Has This Been Tested?
Manually
<!--- 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.
@delta1 delta1 changed the title fix: dockerfiles fix: dockerfiles for base node and wallet Aug 30, 2021
Byron Hambly added 2 commits August 30, 2021 13:22
Just to pass CI until clippy warnings in installer can be resolved
@stringhandler stringhandler merged commit 38579a1 into tari-project:installer Aug 30, 2021
@delta1 delta1 deleted the dockers branch August 30, 2021 12:45
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.

10 participants