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

Remove blocks as CSM input #657

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft

Conversation

delbonis
Copy link
Contributor

@delbonis delbonis commented Feb 6, 2025

Description

The goal of this PR (which is related to the big STF refactors I'm slowly merging in) is to remove the CSM tracking of the chain tip and center the FCM as the source of truth on that. The CSM will only have L1 messages as inputs and will be mainly concerned about checkpoints and L1 finalization.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

These are all sorta related.

Copy link
Contributor

github-actions bot commented Feb 6, 2025

Commit: 5090a4d

SP1 Performance Test Results

program cycles success
BTC_BLOCKSPACE 30,370,596
EL_BLOCK 97,975
CL_BLOCK 94,015
L1_BATCH 30,419,814
L2_BATCH 3,566
CHECKPOINT 25,699

@delbonis
Copy link
Contributor Author

delbonis commented Feb 13, 2025

Since there's some loose ends in the bookkeeping without the CSM being aware of the chain tip, I'm extending this refactor to change some things about how checkpoints are tracked in the CSM and how it works more generally.

  • Sync events are primarily just updates to the L1 tip and genesis. Checkpoints are no longer separate messages. The CSM does all validation, deriving checkpoints from L1 manifests.
  • The "actual" ClientState, which tracks and is used to validate checkpoints, is this new InternalState structure corresponding to L1 blocks by height. These are a pure function of the previous InternalState and an input L1 block. When we get a new block we create a new instance, when it reorgs we discard invalid ones.
    • The instance corresponding to the chain tip is the source of truth on what the last "confirmed" checkpoint is, as of the chain tip. The instance that corresponds to whatever the "buried" L1 block is determines what the finalized checkpoint is.
    • We keep in the ClientState a list of the last several InternalStates so we can access them easily.
  • Some refactoring relating to how we validate checkpoints.
  • L1 block assembly ignores the client state more, fetches the L1 blocks we include directly from the L1 database manager.

In the short term, I need some eyes on this to make sure I'm not missing anything important. There's a LOT of TODOs, mostly to remove code that's no longer relevant, or reconnect some pieces that had to be temporarily disconnected/stubbed. I'm running into some issues where functions have multiple responsibilities or struct/field names are ambiguous, so I have to reverse engineer some things from context and it's likely I may be subtly misinterpreting some things.

The plan (in a future PR) is to eventually (1) move the InternalState type out and onto disk as its own named entries, supersceding the ClientState as it exists now, and then change the CSM logic so most of the bookkeeping that happens persistently by way of processing sequential SyncEvents to instead just happen on the fly in memory. This approach is much more durable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant