apollo_batcher,apollo_consensus_orchestrator: consensus over partial block hash #12451
Conversation
|
This change is |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| })?, | ||
| }, | ||
| )); | ||
| } |
There was a problem hiding this comment.
Setting cache before commit causes panic on retry
Medium Severity
self.prev_proposal_commitment is now set before commit_proposal writes to storage, whereas the old code set it after. If commit_proposal fails and returns an error, prev_proposal_commitment already holds Some((height, ...)) for the uncommitted height. On a retry of decision_reached for the same height, get_parent_proposal_commitment finds this stale entry with h == height but needs prev_height == height - 1, triggering the assert_eq! and panicking.
Additional Locations (1)
ArielElp
force-pushed
the
ariel/consensus_over_partial_block_hash
branch
from
52ea950 to
542b7ae
Compare
ArielElp
force-pushed
the
ariel/partial_block_hash_definition
branch
from
71a6a74 to
9d1db12
Compare
ArielElp
force-pushed
the
ariel/consensus_over_partial_block_hash
branch
from
542b7ae to
5b8cbdd
Compare
|
Artifacts upload workflows: |
ArielElp
force-pushed
the
ariel/consensus_over_partial_block_hash
branch
from
5b8cbdd to
f9dab0e
Compare
ArielElp
force-pushed
the
ariel/partial_block_hash_definition
branch
from
9d1db12 to
fc53903
Compare
ArielElp
force-pushed
the
ariel/consensus_over_partial_block_hash
branch
from
f9dab0e to
ea9afdf
Compare
ArielElp
force-pushed
the
ariel/partial_block_hash_definition
branch
from
fc53903 to
e529641
Compare
| state_diff.deprecated_declared_classes = Vec::new(); | ||
| })?; | ||
| let components = | ||
| components.expect("Missing partial block hash components for previous height."); |
There was a problem hiding this comment.
Panic on cache miss for ParentHash blocks
Low Severity
When prev_proposal_commitment is None (e.g. after node restart), get_parent_proposal_commitment falls back to reading from storage. For blocks stored via StorageCommitmentBlockHash::ParentHash (pre-0.13.2 synced blocks), get_parent_hash_and_partial_block_hash_components returns None for the components, and the .expect(...) panics. The old code gracefully handled all block types by reading the state diff with get_state_diff and computing the hash from it.
2 participants
Note
Medium Risk
Touches consensus-critical identifiers used to correlate proposals between batcher and orchestrator; mismatches or storage gaps in partial hash components could break proposal validation/parent linkage despite largely mechanical refactors.
Overview
Switches consensus proposal identification from a
state_diff_commitmentto a partial block hash (ProposalCommitment.partial_block_hash) acrossapollo_batcher,apollo_batcher_types, and the consensus orchestrator.Batchernow caches and serves the parent proposal commitment by computingPartialBlockHashfrom storedPartialBlockHashComponents(viaget_parent_hash_and_partial_block_hash_components) and setsprev_proposal_commitmentduring commit only when partial components are present; block building also derives the commitment fromPartialBlockHashComponentsinstead of reading the state-diff commitment directly. Tests are updated accordingly, including new storage-reader expectations for retrieving partial block hash components.Written by Cursor Bugbot for commit ea9afdf. This will update automatically on new commits. Configure here.