Skip to content

Conversation

@greged93
Copy link
Contributor

Introduces the providers crate, centralizing the provider traits and their implementations on the crate. Also provides the OnlineL1Provider and the DatabaseL1MessagesProvider implementations.

Comment on lines 35 to 37
block_timestamp: ActiveValue::Set(
batch_input.block_timestamp.try_into().expect("block number should fit in i64"),
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

timestamp of the L1 block is required in order to fetch the blobs for the block.

@greged93 greged93 marked this pull request as draft March 31, 2025 09:56
@greged93 greged93 force-pushed the feat/integrate-batch-changes branch from 32549df to eeb7a63 Compare April 1, 2025 15:05
@greged93 greged93 marked this pull request as ready for review April 1, 2025 15:18
@greged93 greged93 requested a review from frisitano April 1, 2025 15:18
Copy link
Collaborator

@frisitano frisitano 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, left some minor comments inline.

batch_commit.block_number.try_into().expect("block number should fit in i64"),
),
block_timestamp: ActiveValue::Set(
batch_commit.block_timestamp.try_into().expect("block number should fit in i64"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
batch_commit.block_timestamp.try_into().expect("block number should fit in i64"),
batch_commit.block_timestamp.try_into().expect("block timestamp should fit in i64"),

}

#[async_trait::async_trait]
impl<P: L1MessageProvider + Sync> L1BlobProvider for OnlineL1Provider<P> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need the L1MessageProvider trait bound on P for this impl?

.await
.map_err(Into::into)?
.ok_or(DerivationPipelineError::MissingL1Message)?;
let mut bytes = Vec::new();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let mut bytes = Vec::new();
let mut bytes = Vec::with_capacity(l1_message.eip2718_encoded_length());

@greged93 greged93 changed the base branch from feat/integrate-batch-changes to main April 3, 2025 13:57
@greged93 greged93 merged commit 8589790 into main Apr 3, 2025
12 checks passed
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.

3 participants