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

feat(consensus): sequence transaction from foreign LocalPrepare/Accept #1120

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Aug 19, 2024

Description

  • sequence transaction from foreign LocalPrepare/Accept
  • park and request applicable missing transactions from foreign proposals
  • minor swarm improvements/fixes
  • evidence uses QC that justifies the block containing the transaction atom (not the parent of that block)

Motivation and Context

On receipt of a valid foreign proposal, a node may not be aware of all shard group applicable transactions in the following cases:

  • they were not propagated due to some communication failure
  • the shard group is only involved in outputs and were not initially part of the transaction

The node determines involvement in the transactions from a proposal by inspecting the evidence, which must contain at least one valid substate address applicable to the shard group. The transaction is requested as required and the foreign proposal message is unparked once all transactions are received.

How Has This Been Tested?

New consensus test for output-only involvement of a shard. Manually, swarm with 20 vns

Manually multi SG testing reveals that reliability is still poor, with SGs often getting stuck on LocalPrepared. This seems to be an issue with the broadcasting of proposals and is being investigated.

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

Multishard testing with swarm

Breaking Changes

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

@sdbondi sdbondi force-pushed the consensus-sequence-from-foreign-proposal branch 4 times, most recently from 769307e to 8d83588 Compare August 19, 2024 14:41
@sdbondi sdbondi mentioned this pull request Aug 19, 2024
3 tasks
Copy link

github-actions bot commented Aug 19, 2024

Test Results (CI)

550 tests  +1   550 ✅ +1   1h 40m 35s ⏱️ - 5m 55s
 58 suites ±0     0 💤 ±0 
  2 files   ±0     0 ❌ ±0 

Results for commit 61e2216. ± Comparison against base commit f5f3860.

This pull request removes 6 and adds 7 tests. Note that renamed tests count towards both.
consensus_tests ‑ consensus::foreign_shard_decides_to_abort
consensus_tests ‑ substate_store::it_allows_locks_within_one_transaction
tari_dan_storage ‑ consensus_models::command::export_bindings_evidence
tari_dan_storage ‑ consensus_models::command::export_bindings_shardevidence
tari_dan_storage ‑ consensus_models::foreign_proposal::export_bindings_foreignproposal
tari_dan_storage ‑ consensus_models::foreign_proposal::export_bindings_foreignproposalstate
consensus_tests ‑ consensus::foreign_shard_group_decides_to_abort
consensus_tests ‑ consensus::multishard_local_inputs_and_outputs_foreign_outputs
consensus_tests ‑ substate_store::it_allows_requesting_the_same_lock_within_one_transaction
tari_dan_storage ‑ consensus_models::evidence::export_bindings_evidence
tari_dan_storage ‑ consensus_models::evidence::export_bindings_shardevidence
tari_dan_storage ‑ consensus_models::evidence::tests::it_merges_two_evidences_together
tari_dan_storage ‑ consensus_models::foreign_proposal::export_bindings_foreignproposalatom

♻️ This comment has been updated with latest results.

@sdbondi sdbondi force-pushed the consensus-sequence-from-foreign-proposal branch 4 times, most recently from 35cd721 to bb7b1fc Compare August 20, 2024 13:08
github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2024
Description
---
fix(consensus)!: multi shard substate pledges
Version is not considered when calculating which preshard a substate
belongs to
Fix several race conditions in consensus
Adds a test for all local inputs, foreign outputs
Build transactions with inputs in tests
Fixed mempool gossip

Motivation and Context
---

Implements cross-shard exchange and pledges for inputs/outputs.

TODO: 
- output shard group does not sequence an unknown transaction from
foreign proposals (PR #1120)
- number of cucumbers scenarios do not include any inputs. This is now
completely invalid, so they have been ignored for now until we switch
them to use the wallet daemon.

How Has This Been Tested?
---
Manually - multi-shard-group test, existing tests, new tests

What process can a PR reviewer use to test or verify this change?
---
Submit transactions to a multi-shard-group network

Breaking Changes
---

- [ ] None
- [x] Requires data directory to be deleted
- [x] Other - Please specify

BREAKING CHANGE: SubstateAddress has increased in size by 4 bytes, new
commands
@ghpbot-tari-project ghpbot-tari-project added the P-conflicts The PR has merge conflicts that need to be resolved label Aug 21, 2024
@ghpbot-tari-project ghpbot-tari-project removed the P-conflicts The PR has merge conflicts that need to be resolved label Aug 21, 2024
@sdbondi sdbondi force-pushed the consensus-sequence-from-foreign-proposal branch 3 times, most recently from ad7d10d to 2acb91d Compare August 22, 2024 06:56
@sdbondi sdbondi marked this pull request as ready for review August 22, 2024 06:56
@sdbondi sdbondi force-pushed the consensus-sequence-from-foreign-proposal branch from 2acb91d to 8c8eeba Compare August 22, 2024 07:03
@sdbondi sdbondi marked this pull request as draft August 22, 2024 13:07
@sdbondi sdbondi force-pushed the consensus-sequence-from-foreign-proposal branch 5 times, most recently from 373f01a to f4b8722 Compare August 23, 2024 14:51
@sdbondi sdbondi marked this pull request as ready for review August 26, 2024 04:56
@sdbondi sdbondi force-pushed the consensus-sequence-from-foreign-proposal branch from f4b8722 to ff2893d Compare August 26, 2024 06:19
@sdbondi sdbondi merged commit 5329042 into tari-project:development Aug 27, 2024
12 checks passed
@sdbondi sdbondi deleted the consensus-sequence-from-foreign-proposal branch August 27, 2024 08:31
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.

2 participants