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

chore(sequencer)!: put blocks and deposits to non-verified storage (ENG-812) #1525

Merged
merged 10 commits into from
Sep 26, 2024

Conversation

Fraser999
Copy link
Contributor

@Fraser999 Fraser999 commented Sep 19, 2024

Summary

This changes where sequencer blocks and the TracePrefixed native asset are stored, and also persists Deposits to non-verifiable storage.

Background

Sequencer blocks are currently written to verified storage, which is unnecessary as all of their constituent data is already available there.

We want to avoid the cost of storing full blocks in verified storage, but still have the ability to provide a third party with a full block if requested. To further assist with this, deposits will also be written to non-verified storage.

It also seemed appropriate to store the TracePrefixed native asset in verified storage, since this is something which all validators should reach consensus over.

Changes

  • SequencerBlocks are now stored in non-verifiable storage.
  • Deposits are now stored in non-verifiable storage.
  • The native asset is now stored in verifiable storage.

Testing

Existing unit tests have been updated as required to assert these changes.

Breaking Changelist

  • The set of data being written to verified storage has changed, which is a breaking change in terms of on-disk data and generation of state root hashes. Otherwise no APIs have changed.

Related Issues

Closes #1493.

@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Sep 19, 2024
@Fraser999 Fraser999 changed the title chore(sequencer)!: put blocks and deposits to non-verified storage chore(sequencer)!: put blocks and deposits to non-verified storage (ENG-812) Sep 20, 2024
Copy link
Member

Choose a reason for hiding this comment

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

@noot Can I ask you to double check these keys? I think I recall (but might be mistaken) that you wanted plaintext/human readable keys.

I don't think we ever formalized this, but I would like you to give your explicit approval here.

@Fraser999 Fraser999 enabled auto-merge September 23, 2024 19:12
@Fraser999 Fraser999 added this pull request to the merge queue Sep 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 23, 2024
Copy link
Collaborator

@noot noot left a comment

Choose a reason for hiding this comment

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

changes based on our call:

  • store deposits for each block in a different storage key, separated by block number.
  • these can be put in the nonverifiable store instead of the ephemeral object store for simplifying the end of block logic

Copy link
Collaborator

@noot noot left a comment

Choose a reason for hiding this comment

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

looks good to me!

@Fraser999 Fraser999 enabled auto-merge September 26, 2024 12:27
@Fraser999 Fraser999 added this pull request to the merge queue Sep 26, 2024
Merged via the queue into main with commit bda4ffc Sep 26, 2024
43 checks passed
@Fraser999 Fraser999 deleted the fraser/blocks-to-non-verified-storage branch September 26, 2024 12:40
bharath-123 pushed a commit that referenced this pull request Sep 26, 2024
…NG-812) (#1525)

## Summary
This changes where sequencer blocks and the `TracePrefixed` native asset
are stored, and also persists `Deposit`s to non-verifiable storage.

## Background
Sequencer blocks are currently written to verified storage, which is
unnecessary as all of their constituent data is already available there.

We want to avoid the cost of storing full blocks in verified storage,
but still have the ability to provide a third party with a full block if
requested. To further assist with this, deposits will also be written to
non-verified storage.

It also seemed appropriate to store the `TracePrefixed` native asset in
verified storage, since this is something which all validators should
reach consensus over.

## Changes
- `SequencerBlock`s are now stored in non-verifiable storage.
- `Deposit`s are now stored in non-verifiable storage.
- The native asset is now stored in verifiable storage.

## Testing
Existing unit tests have been updated as required to assert these
changes.

## Breaking Changelist
- The set of data being written to verified storage has changed, which
is a breaking change in terms of on-disk data and generation of state
root hashes. Otherwise no APIs have changed.

## Related Issues
Closes #1493.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check verifiable storage usage
4 participants