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

chore: merge development branch into feature-dan #4895

Merged
merged 54 commits into from
Nov 7, 2022

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Nov 7, 2022

Description

Merges development into feature-dan

Cifko and others added 30 commits October 6, 2022 09:03
Description
---
Split rewind DbTx into smaller pieces.

How Has This Been Tested?
---
I did rewind on 20000+ (empty) blocks.
Description
---
The base node errored when reading the `block_sync_trigger = 5` setting
```
ExitError { exit_code: ConfigError, details: Some("Invalid value for `base_node`: unknown field `block_sync_trigger`, expected one of `override_from`, `unconfirmed_pool`, `reorg_pool`, `service`") }
```

Motivation and Context
---
Reading default config settings should not cause an error

How Has This Been Tested?
---
System level testing
Description
---
Removed stray unwrap in liveness service

Motivation and Context
---
Caused a base node to panic in stress test conditions.

```
thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: DhtOutboundError(RequesterReplyChannelClosed)', base_layer\p2p\src\services\liveness\service.rs:164:71
```

How Has This Been Tested?
---
Tests pass
…de/serialization (tari-project#4791)

Description
---
- Uses tari script encoding (equivalent to consensus encoding) for `ExecutionStack` serde impl
- Rename as_bytes to to_bytes as per rust convention.
- adds migration to fix execution stack encoding in db

Motivation and Context
---
Resolves tari-project#4790 

How Has This Been Tested?
---
Added test to alert if breaking changes occur with serde serialization for execution stack.
Manual testing in progress
Description
---
Transaction service sql db queries must handle  `DieselError(DatabaseError(__Unknown, "database is locked"))`. This PR attempts
  to remove situations where that error may occur under highly busy async cirumstances, specifically:
- Combine find and update/write type queries into one.
- Add sql transactions around complex tasks.

_**Note:** Partial resolution for tari-project#4731._

Motivation and Context
---
See above.

How Has This Been Tested?
---
- Passed unit tests.
- Passed cucumber tests.
- ~~**TODO:**~~ System level tests under stress conditions.
Description
---

This moves the nonce to the front of the hashing order when hashing for the sha3 difficulty. 
This is done so that mining cannot cache part most the header and only load the nonce in. This forces the miner to hash the complete header each time the nonce chances. 

Motivation and Context
---

Fixes: tari-project#4767 

How Has This Been Tested?
---
Unit tests all pass.
Description
---
- Ignores nanos for `stored_at` field in StoredMessages
- Uses direct u32 <-> i32 conversion
- Improve error message if attempting to store an expired message
- Discard expired messages immediately
- Debug log when remote client closes the connection in RPC server

Motivation and Context
---
- Nano conversion will fail when >= 2_000_000_000, nanos are not important to preserve so we ignore them (set to zero)
- u32 to/from i32 conversion does not lose any data as both are 32-bit, only used as i32 in the database 
- 'The message was not valid for store and forward' occurs if the message has expired, this PR uses a more descriptive error message for this specific case.
- Expired messages should be discarded immediately
- Early close "errors" on the rpc server simply indicate that the client went away, which is expected and not something that the server controls, and so is logged at debug level 

How Has This Been Tested?
---
Manually,
Description
---
Adds conditional to only increase database size if migration is required

Motivation and Context
---
A new database (cucumber, functional tests) has no inputs and so migration is not required.
Ref tari-project#4791 

How Has This Been Tested?
---
Description
---
Removes unused function in miner

Motivation and Context
---
Clippy

How Has This Been Tested?
---
No clippy error
Description
---
* remove auto update tests from cucumber
* rename some tests to be prefixed with `test_`
* simplified two cucumber tests by removing steps

Motivation and Context
---
The auto update tests have an external dependency, which makes it hard to test reliably. They were marked as broken, so I rather removed them.
There were two steps in the `list_height` and `list_headers` tests that created base nodes. Upon inspection of the logs, these base nodes never synced to the height of 5 and were  not checked in the test, so were pretty useless and just slowed the test down 

How Has This Been Tested?
---
npm test
tari-project#4742)

Description
---
Added an `m-of-n` multisig TariScript that returns the aggregate public key of the signatories if successful and fails otherwise. 

This is useful if the aggregate public key of the signatories is also the script public key, where signatories would work together to create an aggregate script signature using their individual script private keys.

Motivation and Context
---
To enhance the practicality of the  `m-of-n` multisig TariScript.

How Has This Been Tested?
---
Unit tests

Co-Authored-By: SW van Heerden [email protected]
…#4819)

Description
---
- adds socket-level liveness checks
- adds configuration to enable liveness checks (currently enabled by default in base node, disabled in wallet)
- update status line to display liveness status

Motivation and Context
---
Allows us to gain visibility on the base latency of the transport without including overhead of the noise socket and yamux

How Has This Been Tested?
---
Manually
…oject#4802)

Description
---
- checks for edge-case which prevents an unnecessary full candidate block request when block is empty.

Motivation and Context
---
A full block request for empty block is not necessary as we already have all the information required to construct the candidate block. This check was missing from the branch where the candidate block is not the next tip block.

How Has This Been Tested?
---
Description
---
Removes this autoupdate DNS publishing script. 

Motivation and Context
---
It was not being used and is in the git history if we need to find it. 
It also had an alert in https://github.com/tari-project/tari/security/dependabot/202

How Has This Been Tested?
---
Description
---
Finally updates log4rs to remove a vuln in `traitobject`

Motivation and Context
---
https://rustsec.org/advisories/RUSTSEC-2020-0027

How Has This Been Tested?
---
* doc: add a docstring to the check_sig method

* Update infrastructure/tari_script/src/script.rs

Co-authored-by: Byron Hambly <[email protected]>

Co-authored-by: Byron Hambly <[email protected]>
Allow tests to be run individually, i.e. `cargo test -p tari_wallet` in the wallet.

The main reason this didn't work was that the wallet excluded the `base_node` feature from `tari_core`. I've added it as a dev-dependency, but not sure if it breaks the libwallet build
* fix(dht): fix over allocation for encryupted messages

* fix: always add padding bytes

Co-authored-by: Cayle Sharrock <[email protected]>
* improve m-of-n opcode docs

* Apply suggestions from code review

Co-authored-by: Cayle Sharrock <[email protected]>
Description
---
List-connections command prints wallets only if there are base node connections.

How Has This Been Tested?
---
Manually.
Description
---
This adds more detailed error mapping for the FFI.

Motivation and Context
---
On sentry we get error 999, which is the general mapping for all error types without any other error logging with it. 
This is an attempt to get back more information to narrow down which error is causing the 999 without any logging.
SWvheerden and others added 23 commits October 24, 2022 09:12
)

* Push u64 back for feedback into tms validation

* update description

* Update base_layer/wallet/src/transaction_service/handle.rs

Co-authored-by: Cayle Sharrock <[email protected]>

* re-format display

Co-authored-by: Cayle Sharrock <[email protected]>
Description
---
Zeroizes authenticated encryption keys (via the `AuthenticatedCipherKey` struct) on drop. Fixes [issue 4842](tari-project#4842).

Motivation and Context
---
Authenticated encryption (AEAD) keys are intended to be zeroized on drop, but the relevant macros are not applied. This work adds the macros.

How Has This Been Tested?
---
Manually tested that `Zeroize` was not supported previously, and is supported now.
Make some tests run on every PR
…roject#4838)

Description
---
- parses network in cli using Network::from_str
- fixes bug where valid network string 'esme' would break config overrides

Motivation and Context
---
Bug fix, using --network esme would result in different configuration from --network esmeralda

How Has This Been Tested?
---
Manually, `--network esme`
Description
---
Replaces `clear_on_drop` usage with `zeroize`

Motivation and Context
---
`zeroize` is preferred in the tari codebase
`clear_on_drop` is still listed as a dependency in `rpgp`, though AFAICS is unused. In the current master branch, it has been removed, so once `rpgp` releases that, the clear_on_drop dependency will be gone completely.

How Has This Been Tested?
---
Build passes
Description
---
This adds in an opcode versioning that is allowed.
Refactors the validation to ensure transaction versioning info is check by blocks as well. 

Motivation and Context
---
We might need to add in new opcodes as hard forks. This adds the opcode version as part of the consensus code so we can track allowed opcodes. 

How Has This Been Tested?
---
Unit tests

Fixes: tari-project#4821
…ct#4851)

Description
---
Added a static lifetime to `emission_amounts` calculation. This is needed if the `pub fn emission_amounts` will be called from an external client.

Motivation and Context
---
The `&'[u64]` return value was at odds with the `emission_decay: &'static [u64]` definition.

How Has This Been Tested?
---
Unit tests
External client
Description
---
adds txo/kernel version checks to async validator

Motivation and Context
---
Version checks were missing from async validator

Ref tari-project#4836 

How Has This Been Tested?
---
Manually: Rewind a few blocks and resync
…ct#4855)

Description
---
- uses plug emoji for unreachable
- puts reachability/liveness status before randomx output

Motivation and Context
---
‼ is a little harsh looking

How Has This Been Tested?
---
Manually
…-project#4857)

Description
GHA have update environment variable storage

Motivation and Context
Remove CI/build warnings

How Has This Been Tested?
Builds in local fork
Description
---
Orphans should be deleted from the orphan database if they exist. 

Motivation and Context
---
If the block is added to the main chain, it should not exist in the orphan database as well. It will also remove the orphan tips if it removes the block from the pool. 

How Has This Been Tested?
---
Unit test and manually running

fixes: tari-project#4867
Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v3)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

Description:
Revert resolver from 2 to 1 for ARM64 builds

Motivation and Context:
Fix ARM64 binary builds

How Has This Been Tested?
Build locally in fork
* feat: impl final tari pow algorithm

Tari's independent proof-of-work algorithm is very straightforward.

Calculate the _triple hash_ of the following input data:
 - Nonce (8 bytes)
 - Tari mining hash (32 bytes)
 - PoW record (for Sha-3x, this is always a single byte of value 1)

That is, the nonce in little-endian format, mining hash and the PoW record are chained together and hashed by the
Keccak Sha3-256 algorithm. The result is hashed again, and this result is hashed a third time. The result of the third
hash is compared to the target value of the current block difficulty.

A triple hash is selected to keep the requirements on hardware miners (FPGAs, ASICs) fairly low. But we also want to
avoid making the proof-of-work immediately "NiceHashable". There are several coins that already use a single or
double SHA3 hash, and we'd like to avoid having that hashrate immediately deployable against Tari.

This PR also stabilises RFC-0131

* fix: mining_test_difficulty

* feat: update to version for PoW (tari-project#4875)

Description
---
Updates the header version to change the PoW so we don't have to reset the chain

Motivation and Context
---
See: tari-project#4862 

How Has This Been Tested?
---

Co-authored-by: SW van Heerden <[email protected]>
…ject#4877)

Description
---
- fixes possible panic in `Peer::offline_since` 
- offline_at has second resolution

Motivation and Context
---
Possible panic when converting chrono Duration to std Duration
second resolution is good enough for offline_since

How Has This Been Tested?
---
Updated unit test for this case
Description:
Remove selective hack that reverted resolver from 1 to 2. Fix ARM64 builds by removing env overrides in Cross.toml breaking openssl-sys

Motivation and Context:
Make ARM64 builds standard

How Has This Been Tested?
Build test in locally and in local fork
Description
Split out each choco windows package install/upgrade. Seems to be a new bug in choco.
Added a debug log upload, which should only be run on failure.
Also switched out zip for 7zip, as it's packaged with al runners. Had zip installs in the past.
Switched out Ubuntu apt-get install with scripts/install_ubuntu_dependencies.sh as a re-used script.
Update rust-cache action.

Motivation and Context
Fix Windows binary builds

How Has This Been Tested?
Test in local fork, builds complete.
Description
---
Better description of seed-words with example tari-project#4796
* development: (52 commits)
  chore: better help for seed-words command (tari-project#4885)
  fix(ci): resolve windows binary builds (tari-project#4883)
  fix(ci): correct ARM64 builds (tari-project#4876)
  fix(comms/peer_manager): fix possible panic in offline calc (tari-project#4877)
  feat!: impl final tari pow algorithm (tari-project#4862)
  fix(ci): selectively revert resolver for arm64 builds (tari-project#4871)
  chore(deps): bump actions/checkout from 2 to 3 (tari-project#4873)
  fix: delete orphans if they exist (tari-project#4868)
  chore: replace manual implementation of char methods (tari-project#4864)
  chore: fix potentially buggy split of string into lines (tari-project#4863)
  fix(ci): update GHA set-output plus dependabot schedule for GHA (tari-project#4857)
  fix(base-node): use less harsh emoji for unreachable node (tari-project#4855)
  fix(core): add txo version checks to async validator (tari-project#4852)
  feat: add static lifetime to emission amounts calculation (tari-project#4851)
  v0.38.8
  feat: add opcode versions (tari-project#4836)
  fix: remove clear_on_drop dependency (tari-project#4848)
  fix(base-node): use Network::from_str to parse network in cli (tari-project#4838)
  ci: remove circleci
  test: add cucumber critical (tari-project#4823)
  ...
@CjS77 CjS77 added the CR-too_long Changes Requested - Your PR is too long label Nov 7, 2022
@stringhandler stringhandler merged commit 1c3eff1 into tari-project:feature-dan Nov 7, 2022
@sdbondi sdbondi deleted the merge-dev-feature-dan branch November 7, 2022 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR-too_long Changes Requested - Your PR is too long
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants