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.

Sign up for GitHub

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

Jump to bottom

fix: multi committee transaction #768

Merged
merged 1 commit into from
Nov 23, 2023
Merged

fix: multi committee transaction #768

merged 1 commit into from
Nov 23, 2023

Conversation

Cifko
Copy link
Contributor

@Cifko Cifko commented Nov 14, 2023

Description

This fixes two bugs:

  • every VN now distributes the transaction after execution (we can maybe limit this to the VNs that have the TxReceipt, because that's the shard where is it always distributed).
  • VNs is no longer trying to delete substates that are in foreign committees.

Motivation and Context

How Has This Been Tested?

Running dan-testing with 2 committees and several transactions.

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

Same as above.

Breaking Changes

  • None
  • Requires data directory to be deleted
  • Other - Older VNs are currently not working 100% properly and they need to be recompiled.

sdbondi
sdbondi reviewed Nov 14, 2023
View reviewed changes
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to only DOWNing/UPing local substates. Have some questions on the other change.

applications/tari_validator_node/src/consensus/state_manager.rs Show resolved Hide resolved
applications/tari_validator_node/src/p2p/services/mempool/service.rs Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 14, 2023 •
edited
Loading

Test Results (CI)

171 tests  ±0   171 ✔️ ±0   1h 43m 32s ⏱️ + 31m 14s
  48 suites ±0       0 💤 ±0 
    2 files   ±0       0 ❌ ±0 

Results for commit 3e56745. ± Comparison against base commit 6d48db4.

♻️ This comment has been updated with latest results.

@Cifko Cifko force-pushed the fix-multi-committee-tx branch from f4b5e2c to 6c156b9 Compare November 15, 2023 07:30
@Cifko Cifko force-pushed the fix-multi-committee-tx branch 2 times, most recently from da7251f to f4841a4 Compare November 16, 2023 13:42
@Cifko Cifko force-pushed the fix-multi-committee-tx branch 2 times, most recently from e0831ae to 2672f2a Compare November 21, 2023 11:49
@Cifko Cifko force-pushed the fix-multi-committee-tx branch from 2672f2a to 3e56745 Compare November 22, 2023 20:21
@sdbondi sdbondi added this pull request to the merge queue Nov 23, 2023
Merged via the queue into tari-project:development with commit 42452d4 Nov 23, 2023
9 checks passed
@Cifko Cifko deleted the fix-multi-committee-tx branch November 27, 2023 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stringhandler stringhandler stringhandler approved these changes

@sdbondi sdbondi sdbondi approved these changes

Assignees
No one assigned
Labels
P-acks_required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Cifko @sdbondi @stringhandler @ghpbot-tari-project