Skip to content

chore(trie): Remove redundant generic lifetime for InitializationJob #632

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.

Sign up for GitHub

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

Merged
emhane merged 3 commits into from
Jan 27, 2026
Merged

chore(trie): Remove redundant generic lifetime for InitializationJob #632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1aa7471
Remove redundant generic lifetime for InitializationJob
emhane Jan 26, 2026
62a520c
Drive-by: fix lint
emhane Jan 26, 2026
2025aab
Merge branch 'unstable' into emhane/rm-redundant-generic-lifetime-ini…
emhane Jan 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/optimism/cli/src/commands/op_proofs/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl<C: ChainSpecParser<ChainSpec = OpChainSpec>> InitCommand<C> {
provider_factory.database_provider_ro()?.disable_long_read_transaction_safety();
let db_tx = db_provider.into_tx();

InitializationJob::new(storage.clone(), &db_tx).run(best_number, best_hash).await?;
InitializationJob::new(storage.clone(), db_tx).run(best_number, best_hash).await?;
}

info!(
Expand Down
8 changes: 2 additions & 6 deletions crates/optimism/cli/src/commands/op_proofs/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use clap::{Parser, Subcommand};
use reth_cli::chainspec::ChainSpecParser;
use reth_cli_commands::common::CliNodeTypes;
use reth_optimism_chainspec::OpChainSpec;
use reth_optimism_primitives::OpPrimitives;
use std::sync::Arc;
Expand All @@ -19,12 +20,7 @@ pub struct Command<C: ChainSpecParser> {

impl<C: ChainSpecParser<ChainSpec = OpChainSpec>> Command<C> {
/// Execute `op-proofs` command
pub async fn execute<
N: reth_cli_commands::common::CliNodeTypes<
ChainSpec = C::ChainSpec,
Primitives = OpPrimitives,
>,
>(
pub async fn execute<N: CliNodeTypes<ChainSpec = C::ChainSpec, Primitives = OpPrimitives>>(
self,
) -> eyre::Result<()> {
match self.command {
Expand Down
47 changes: 21 additions & 26 deletions crates/optimism/trie/src/initialize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::{
};
use alloy_eips::BlockNumHash;
use alloy_primitives::B256;
use derive_more::Constructor;
use reth_db::{
cursor::{DbCursorRO, DbDupCursorRO},
tables,
Expand All @@ -28,10 +29,10 @@ const INITIALIZE_STORAGE_THRESHOLD: usize = 100000;
const INITIALIZE_LOG_THRESHOLD: usize = 100000;

/// Initialization job for external storage.
#[derive(Debug)]
pub struct InitializationJob<'a, Tx: DbTx, S: OpProofsStore + Send> {
#[derive(Debug, Constructor)]
pub struct InitializationJob<Tx: DbTx, S: OpProofsStore + Send> {
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

OpProofsStore already requires Send + Sync (see crates/optimism/trie/src/api.rs), so the additional + Send bound on S here is redundant. Dropping it would keep the new type signature as minimal as the PR intends.

Suggested change
pub struct InitializationJob<Tx: DbTx, S: OpProofsStore + Send> {
pub struct InitializationJob<Tx: DbTx, S: OpProofsStore> {

Copilot uses AI. Check for mistakes.
storage: S,
tx: &'a Tx,
tx: Tx,
}

/// Macro to generate simple cursor iterators for tables
Expand Down Expand Up @@ -133,15 +134,14 @@ impl CompletionEstimatable for StoredNibbles {
/// Initialize a table from a source iterator to a storage function. Handles batching and logging.
async fn initialize<
S: Iterator<Item = Result<(Key, Value), DatabaseError>>,
F: Future<Output = Result<(), OpProofsStorageError>> + Send,
Key: CompletionEstimatable + Clone + 'static,
Value: Clone + 'static,
>(
name: &str,
source: S,
storage_threshold: usize,
log_threshold: usize,
save_fn: impl Fn(Vec<(Key, Value)>) -> F,
save_fn: impl AsyncFnOnce(Vec<(Key, Value)>) -> Result<(), OpProofsStorageError> + Send + Copy,
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

initialize calls save_fn multiple times (batch flush + final flush), but the bound is AsyncFnOnce and relies on Copy to permit repeated calls. This is easy to misread and also restricts closures that capture non-Copy state. Consider switching the bound to AsyncFn/AsyncFnMut (and dropping Copy) so the type signature matches the actual call pattern, or otherwise document why AsyncFnOnce + Copy is required here.

Suggested change
save_fn: impl AsyncFnOnce(Vec<(Key, Value)>) -> Result<(), OpProofsStorageError> + Send + Copy,
mut save_fn: impl AsyncFnMut(Vec<(Key, Value)>) -> Result<(), OpProofsStorageError> + Send,

Copilot uses AI. Check for mistakes.
) -> Result<u64, OpProofsStorageError> {
let mut entries = Vec::new();

Expand Down Expand Up @@ -204,14 +204,9 @@ async fn initialize<
Ok(total_entries)
}

impl<'a, Tx: DbTx + Sync, S: OpProofsStore + OpProofsInitialStateStore + Send>
InitializationJob<'a, Tx, S>
impl<Tx: DbTx + Sync, S: OpProofsStore + OpProofsInitialStateStore + Send>
InitializationJob<Tx, S>
{
/// Create a new initialization job.
pub const fn new(storage: S, tx: &'a Tx) -> Self {
Self { storage, tx }
}

/// Save mapping of hashed addresses to accounts to storage.
async fn save_hashed_accounts(
&self,
Expand Down Expand Up @@ -517,7 +512,7 @@ mod tests {

// Run initialization
let tx = db.tx().unwrap();
let job = InitializationJob::new(storage.clone(), &tx);
let job = InitializationJob::new(storage.clone(), tx);
job.initialize_hashed_accounts(None).await.unwrap();

// Verify data was stored (will be in sorted order)
Expand Down Expand Up @@ -568,7 +563,7 @@ mod tests {

// Run initialization
let tx = db.tx().unwrap();
let job = InitializationJob::new(storage.clone(), &tx);
let job = InitializationJob::new(storage.clone(), tx);
job.initialize_hashed_storages(None).await.unwrap();

// Verify data was stored for addr1
Expand Down Expand Up @@ -616,7 +611,7 @@ mod tests {

// Run initialization
let tx = db.tx().unwrap();
let job = InitializationJob::new(storage.clone(), &tx);
let job = InitializationJob::new(storage.clone(), tx);
job.initialize_accounts_trie(None).await.unwrap();

// Verify data was stored
Expand Down Expand Up @@ -675,7 +670,7 @@ mod tests {

// Run initialization
let tx = db.tx().unwrap();
let job = InitializationJob::new(storage.clone(), &tx);
let job = InitializationJob::new(storage.clone(), tx);
job.initialize_storages_trie(None).await.unwrap();

// Verify data was stored for addr1
Expand Down Expand Up @@ -752,7 +747,7 @@ mod tests {

// Run full initialization
let tx = db.tx().unwrap();
let job = InitializationJob::new(storage.clone(), &tx);
let job = InitializationJob::new(storage.clone(), tx);
let best_number = 100;
let best_hash = B256::repeat_byte(0x42);

Expand Down Expand Up @@ -796,7 +791,7 @@ mod tests {
storage.commit_initial_state().await.expect("commit anchor");

let tx = db.tx().unwrap();
let job = InitializationJob::new(storage.clone(), &tx);
let job = InitializationJob::new(storage.clone(), tx);

// Run initialization - should skip
job.run(100, B256::repeat_byte(0x42)).await.unwrap();
Expand Down Expand Up @@ -843,7 +838,7 @@ mod tests {
// Initialization #1
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_hashed_accounts(None).await.unwrap();
}

Expand All @@ -869,7 +864,7 @@ mod tests {
// Initialization #2 (restart)
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_hashed_accounts(Some(k2)).await.unwrap();
}

Expand Down Expand Up @@ -933,7 +928,7 @@ mod tests {
// Initialization #1
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_hashed_storages(None).await.unwrap();
}

Expand All @@ -958,7 +953,7 @@ mod tests {
// Initialization #2
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_hashed_storages(Some(HashedStorageKey::new(a2, s21))).await.unwrap();
}

Expand Down Expand Up @@ -1023,7 +1018,7 @@ mod tests {
// Initialization #1
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_accounts_trie(None).await.unwrap();
}

Expand All @@ -1044,7 +1039,7 @@ mod tests {
// Initialization #2
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_accounts_trie(Some(p2.clone())).await.unwrap();
}

Expand Down Expand Up @@ -1104,7 +1099,7 @@ mod tests {
// Initialization #1
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_storages_trie(None).await.unwrap();
}

Expand Down Expand Up @@ -1133,7 +1128,7 @@ mod tests {
// Initialization #2
{
let tx = db.tx().unwrap();
let job = InitializationJob::new(store.clone(), &tx);
let job = InitializationJob::new(store.clone(), tx);
job.initialize_storages_trie(Some(StorageTrieKey::new(a2, StoredNibbles::from(n2.0))))
.await
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/trie/tests/live.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ where
{
let provider = provider_factory.db_ref();
let tx = provider.tx()?;
let initialization_job = InitializationJob::new(storage.clone(), &tx);
let initialization_job = InitializationJob::new(storage.clone(), tx);
initialization_job.run(last_block_number, last_block_hash).await?;
}

Expand Down
Loading