Skip to content

Fix on-disk consensus context format#5598

Merged
mergify[bot] merged 2 commits intosigp:unstablefrom
michaelsproul:on-disk-consensus-context
Apr 19, 2024
Merged

Fix on-disk consensus context format#5598
mergify[bot] merged 2 commits intosigp:unstablefrom
michaelsproul:on-disk-consensus-context

Conversation

@michaelsproul
Copy link
Member

Issue Addressed

Fix a regression in unstable that causes logs of the form:

ERRO Failed to maintain availability cache error: DecodeError(OffsetOutOfBounds(1157628160)), service: beacon

This was due to the addition of fields to ConsensusContext in the single-pass epoch processing PR. ConsensusContext is now part of our on-disk database schema via DietAvailabilityPendingExecutedBlock:

pub struct DietAvailabilityPendingExecutedBlock<E: EthSpec> {
#[ssz(with = "ssz_tagged_signed_beacon_block_arc")]
block: Arc<SignedBeaconBlock<E>>,
state_root: Hash256,
#[ssz(with = "ssz_tagged_signed_beacon_block")]
parent_block: SignedBeaconBlock<E, BlindedPayload<E>>,
parent_eth1_finalization_data: Eth1FinalizationData,
confirmed_state_roots: Vec<Hash256>,
consensus_context: ConsensusContext<E>,
payload_verification_outcome: PayloadVerificationOutcome,
}

Proposed Changes

  • Define a new OnDiskConsensusContext struct which keeps only the previous fields.
  • Convert between in-memory consensus contexts and the stable on-disk format as necessary.

@michaelsproul michaelsproul added bug Something isn't working ready-for-review The code is ready for review database deneb v5.2.0 Q2 2024 labels Apr 18, 2024
@michaelsproul michaelsproul mentioned this pull request Apr 18, 2024
4 tasks
parent_eth1_finalization_data: Eth1FinalizationData,
confirmed_state_roots: Vec<Hash256>,
consensus_context: ConsensusContext<E>,
consensus_context: OnDiskConsensusContext,
Copy link
Member

Choose a reason for hiding this comment

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

DietAvailabilityPendingExecutedBlock is constructued on every block, and since OnDiskConsensusContext drops indexed_attestations, that cache would be dropped every block.

Could fix this easily by keeping the indexed_attestations field in OnDiskConsensusContext with #[ssz(skip_serializing, skip_deserializing)].

Copy link
Member Author

Choose a reason for hiding this comment

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

Great catch, just pushed a fix

@realbigsean realbigsean added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Apr 18, 2024
@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Apr 19, 2024
Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelsproul
Copy link
Member Author

@Mergifyio queue

@mergify
Copy link

mergify bot commented Apr 19, 2024

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 5a9e973

mergify bot added a commit that referenced this pull request Apr 19, 2024
@mergify mergify bot merged commit 5a9e973 into sigp:unstable Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working database deneb ready-for-review The code is ready for review v5.2.0 Q2 2024

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants