[mfp] do not vote on potentially GC'ed blocks #24492
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mwtian
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mwtian
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mwtian
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mwtian
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mwtian
changed the title
mwtian
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mwtian
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mwtian
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
| }) | ||
| .join(", "); | ||
| let digest = self.rejected_transactions_digest(); | ||
| format!("{str}; digest: {digest}") |
There was a problem hiding this comment.
what is the purpose of the rejected transactions digest? Is it just to provide an identifier for the set of rejected transactions in a committed subdag? If so should the identifier be first in the debug statement?
| while let Some(block_ref) = targets.pop_front() { | ||
| // This is only correct with GC enabled. | ||
| // No need to collect and mark blocks at or below GC round. These blocks will not be included in new commits | ||
| // and do not need their transactions to be voted on. |
There was a problem hiding this comment.
nit: Maybe you can change how the comments are structured across the PR if you agree with this. Other than maybe the root block, technically the transactions in the causal history have already been voted on. The thing we are not doing now is counting votes of GCd blocks/transactions
| finalized_transactions | ||
| } | ||
|
|
||
| // Returns true if the pending block round can be GC'ed when proposing blocks in current commit. |
There was a problem hiding this comment.
why only "can be GC'ed" and not "has been GC'ed"?
| let skip_votes = curr_block_ref.author == curr_origin_ancestor_ref.author | ||
| && pending_block_ref.round < curr_origin_ancestor_ref.round | ||
| && !blocks_map.contains_key(curr_origin_ancestor_ref); | ||
| // When proposing the current block, if it is possible that the pending block has already been GC'ed, |
There was a problem hiding this comment.
nit: Instead of saying "proposing" block can we say "counting votes" or "counting tx votes", it seems clearer as to what we are doing here
Description
When proposing a block, only votes on blocks within DagState GC round at proposal time are included.
So when considering votes in CommitFinalizer, if GC round is <= R when proposing block B, any block with round <= R should not receive votes from B.
Test plan
CI
antithesis