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

Blob storage module #510

Merged
merged 9 commits into from
Jul 24, 2023
Merged

Blob storage module #510

merged 9 commits into from
Jul 24, 2023

Conversation

citizen-stig
Copy link
Member

@citizen-stig citizen-stig commented Jul 18, 2023

Description

This PR introduces a new module "sov-blob-storage" for storing Data Access (DA) layer blobs within the Rollup state. The addition of this module serves as a prerequisite for sequencing with soft confirmations. The default state transition function (STF) will utilize this module to manage deferred blocks.

The module consists of two key methods:

    pub fn store_blobs<B: BlobTransactionTrait>(
        &self,
        block_number: u64,
        blobs: &[B],
        working_set: &mut WorkingSet<C::Storage>,
    ) -> anyhow::Result<()>
    
    pub fn take_blobs_for_block_number<B: BlobTransactionTrait>(
        &self,
        block_number: u64,
        working_set: &mut WorkingSet<C::Storage>,
    ) -> Vec<B>

The State Transition Function (STF) now has the flexibility to decide which blobs to execute and when.

Consider a situation where the STF receives a blob from the preferred sequencer in block 100. It can use the store_blob function to hold onto deferred blobs, and later retrieve these blobs at the next block or after several blocks.

Points to Consider

This PR mainly focuses on the public API for this module. The current implementation is straightforward and does not contain much business logic.

One key consideration is the "defer period", which refers to the number of blocks for which blobs will be deferred. Currently, this value is hard-coded in the STF, but should it be stored in the state instead?

Also, how strict should this module be? Should it limit the number of blobs that can be stored or track the total size of stored blobs? Should it permit the insertion of blobs out of order?

Linked Issues

Testing

Integration tests have been added

Docs

#![deny(missing_docs)]

@citizen-stig citizen-stig self-assigned this Jul 18, 2023
@citizen-stig citizen-stig marked this pull request as ready for review July 18, 2023 13:52
@citizen-stig citizen-stig changed the title WIP: blob storage module Blob storage module Jul 18, 2023
@preston-evans98
Copy link
Member

preston-evans98 commented Jul 19, 2023

It seems like there's some potential for type confusion here - is the stored blob a serialized BlobTransaction, or is it just the blob data? Maybe it would be more natural to change from storing a blob as a raw Vec<u8> to storing it as a concrete StoredBlob type which contains the data (probably as a Vec<u8>) and the sender?

@citizen-stig
Copy link
Member Author

It seems like there's some potential for type confusion here - is the stored blob a serialized BlobTransaction, or is it just the blob data? Maybe it would be more natural to change from storing a blob as a raw Vec<u8> to storing it as a concrete StoredBlob type which contains the data (probably as a Vec<u8>) and the sender?

Yes, I've done via introducing BlobTransactionTrait generic, so STF can just pass it without thinking twice.

@citizen-stig citizen-stig force-pushed the feature/blob-storage-module branch from 5694bec to a21c70b Compare July 20, 2023 14:12
@citizen-stig citizen-stig requested a review from bkolad July 20, 2023 14:47
Copy link
Member

@preston-evans98 preston-evans98 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for addressing the comments @citizen-stig!

@citizen-stig citizen-stig merged commit f7164ee into nightly Jul 24, 2023
@citizen-stig citizen-stig deleted the feature/blob-storage-module branch July 24, 2023 06:46
@citizen-stig citizen-stig linked an issue Jul 25, 2023 that may be closed by this pull request
4 tasks
preston-evans98 pushed a commit that referenced this pull request Sep 14, 2023
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.

Based Sequencing with Soft Confirmations
3 participants