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(consensus)!: block diffs, exec on propose and locks improvement #1035

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented May 17, 2024

Description

  • Generate and persist block diff for an accepted block
  • generate a ProposedBlockChangeSet and persist for an accepted block
  • State store read transactions no longer require a mutable reference
  • Use PendingSubstateStore to track block changes and locks
  • Remove previous locking code
  • Allows previously conflicting local-only transactions to be sequenced using uncommitted outputs from other local only transactions as inputs (Local-Only-Rules)
  • Fix multiple issues resulting in test flakiness
  • Execute deferred transactions on propose and on prepare
  • Adds deferred transaction test (currently for local-only case only)
  • decide_what_to_vote is read-only
  • Unignored some previously-flaky consensus tests
  • Removed BlockTransactionExecutorBuilder trait
  • Updated block sync

Motivation and Context

This PR improves locking, substate management and allows local-only transaction concurrency.

It adds the following concepts:
BlockDiff - contains the substate changes (UP or DOWN) for a block. Is used to commit state in a commit block
PendingSubstateStore - tracks and validates changes to the substate store and decouples state from execution. It is used to generate a BlockDiff, track locks in the current block context, and to generate the state tree diff.
ProposedBlockChangeSet - contains all the pending changes to be committed when deciding to ACCEPT a new block
TransactionExecution - decouples execution results from a transaction record. These represent an execution within the context of a block. Once a transaction is finalized, the execution result is added to the final result in the transaction.

Possible issues:
Currently to fetch the correct transaction executions and locks for a block we need to limit the results to those that match the chain. This query will grow slower as the chain progresses. Bugs may still occur in the unhappy/malicious path.

How Has This Been Tested?

New deferred transaction consensus test, new unit tests, existing tests, removed ignore attribute from several consensus tests.

What process can a PR reviewer use to test or verify this change?

Submit transactions without specifying the version of the input. Submit local-only transactions concurrently that spend previous outputs from other local only transactions.

Breaking Changes

  • None
  • Requires data directory to be deleted
  • Other - Please specify

BREAKING CHANGE: database structure changed

Copy link

github-actions bot commented May 17, 2024

Test Results (CI)

541 tests  +9   541 ✅ +9   1h 36m 13s ⏱️ - 20m 0s
 63 suites ±0     0 💤 ±0 
  2 files   ±0     0 ❌ ±0 

Results for commit 766f8a0. ± Comparison against base commit 198e6ec.

This pull request removes 2 and adds 11 tests. Note that renamed tests count towards both.
consensus_tests ‑ consensus::leader_failure_output_conflict
tari_dan_common_types ‑ substate_address::tests::to_committee_shard
consensus_tests ‑ consensus::deferred_execution
consensus_tests ‑ consensus::foreign_shard_decides_to_abort
consensus_tests ‑ consensus::multi_shard_propose_blocks_with_new_transactions_until_all_committed
consensus_tests ‑ consensus::output_conflict_abort
consensus_tests ‑ substate_store::it_allows_down_then_up
consensus_tests ‑ substate_store::it_allows_locks_within_one_transaction
consensus_tests ‑ substate_store::it_allows_substate_up_for_v0
consensus_tests ‑ substate_store::it_disallows_more_than_one_write_lock_non_local_only
consensus_tests ‑ substate_store::it_fails_if_previous_version_is_not_down
tari_dan_common_types ‑ substate_address::tests::max_committees
…

♻️ This comment has been updated with latest results.

@sdbondi sdbondi force-pushed the consensus-block-diffs branch 2 times, most recently from 7d09313 to bc6f364 Compare May 17, 2024 13:09
@sdbondi sdbondi marked this pull request as ready for review May 17, 2024 13:09
@sdbondi sdbondi marked this pull request as draft May 17, 2024 14:05
@sdbondi sdbondi marked this pull request as ready for review May 20, 2024 04:39
@stringhandler stringhandler added this pull request to the merge queue May 20, 2024
Merged via the queue into tari-project:development with commit 9ef62bb May 20, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants