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 into feature-dan #4815

Merged
merged 20 commits into from
Oct 19, 2022

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Oct 18, 2022

Description

Merged development branch into feature-dan

Cifko and others added 13 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
@sdbondi sdbondi marked this pull request as ready for review October 18, 2022 05:27
@sdbondi sdbondi marked this pull request as draft October 19, 2022 08:10
hansieodendaal and others added 4 commits October 19, 2022 10:26
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 swvheerden@gmail.com
…#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?
---
* development:
  fix(core): dont request full non-tip block if block is empty (tari-project#4802)
  feat(comms): adds periodic socket-level liveness checks (tari-project#4819)
  feat: add multisig script that returns aggregate of signed public keys (tari-project#4742)
  fix(core): increase sync timeouts (tari-project#4800)
  fix(comms/rpc): measures client-side latency to first message received (tari-project#4817)
  fix(core): periodically commit large transaction in prune_to_height (tari-project#4805)
  feat: add deepsource config
  v0.38.7
  test: remove cucumber tests, simplify others (tari-project#4794)
  fix(miner): clippy error (tari-project#4793)
  fix(core): only resize db if migration is required (tari-project#4792)
  v0.38.6
  fix(dht): remove some invalid saf failure cases (tari-project#4787)
  feat: move nonce to first in sha hash (tari-project#4778)
  feat: optimize transaction service queries (tari-project#4775)
  fix(tari-script): use tari script encoding for execution stack serde de/serialization (tari-project#4791)
  fix(p2p/liveness): remove fallible unwrap (tari-project#4784)
  fix: fix config.toml bug (tari-project#4780)
  fix: batch rewind operations (tari-project#4752)
@CjS77 CjS77 added the CR-too_long Changes Requested - Your PR is too long label Oct 19, 2022
@sdbondi sdbondi assigned sdbondi and unassigned sdbondi Oct 19, 2022
@sdbondi sdbondi marked this pull request as ready for review October 19, 2022 09:35
@sdbondi
Copy link
Member Author

sdbondi commented Oct 19, 2022

ACK

@CjS77 CjS77 added P-reviews_required Process - Requires a review from a lead maintainer to be merged P-merge Process - Queued for merging labels Oct 19, 2022
@CjS77 CjS77 removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 19, 2022
@CjS77 CjS77 merged commit ce35b65 into tari-project:feature-dan Oct 19, 2022
@sdbondi sdbondi deleted the merge-dev branch October 19, 2022 12:06
sdbondi added a commit to sdbondi/tari that referenced this pull request Nov 14, 2022
* feature-dan: (21 commits)
  fix(core)!: remove unused get_committees call from base node (tari-project#4880)
  fix: correct value for validator_node_timeout consensus constant in localnet (tari-project#4879)
  feat: add block height to input request to get network consensus constants (tari-project#4856)
  fix: remove unused config for validator node (tari-project#4849)
  feat: add missing fields to grpc consensus constants interface (tari-project#4845)
  chore: merge development into feature-dan (tari-project#4815)
  refactor: split tari_base_node and tari_console_wallet into a lib component (tari-project#4818)
  fix(core)!: adds utxo and block info to get_template_registrations request (tari-project#4789)
  fix: computation of vn mmr (tari-project#4772)
  fix(wallet/grpc): add transaction id and template_address to template_reg response (tari-project#4788)
  fix: fix get shard key (tari-project#4744)
  feat(core): store and fetch templates from lmdb (tari-project#4726)
  fix: fix validator node registration logic (tari-project#4718)
  feat(base_node_grpc_client): add getActiveValidatorNodes method (tari-project#4719)
  fix after merge
  fix(core): bring validator node MR inline with other merkle root code (tari-project#4692)
  feat(core): add validator registration sidechain feature (tari-project#4690)
  fix merge issues
  feat: add grpc to get shard key for public key (tari-project#4654)
  feat: add validator node registration (tari-project#4507)
  ...
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 P-merge Process - Queued for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants