Skip to content

Commit 4feef37

Browse files
haythemsellamihaythemsellami
committed
fix(database): make DatabaseCommit dyn-compatible by changing commit_iter to use &mut dyn Iterator
1 parent 373c38a commit 4feef37

2 files changed

Lines changed: 49 additions & 95 deletions

File tree

‎crates/database/interface/src/lib.rs‎

Lines changed: 48 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use core::convert::Infallible;
1010
use auto_impl::auto_impl;
1111
use primitives::{address, Address, HashMap, StorageKey, StorageValue, B256, U256};
1212
use state::{Account, AccountInfo, Bytecode};
13-
use std::{boxed::Box, vec::Vec};
13+
use std::vec::Vec;
1414

1515
/// Address with all `0xff..ff` in it. Used for testing.
1616
pub const FFADDRESS: Address = address!("0xffffffffffffffffffffffffffffffffffffffff");
@@ -87,12 +87,9 @@ pub trait Database {
8787
///
8888
/// # Dyn Compatibility
8989
///
90-
/// This trait is dyn-compatible. The `commit_iter` method has a `Self: Sized` bound
91-
/// which excludes it from the trait's vtable, allowing `dyn DatabaseCommit` to work.
92-
///
93-
/// Note: We manually implement this trait for `&mut T` and `Box<T>` instead of using
94-
/// `#[auto_impl]` because auto_impl generates impls for `T: ?Sized` that try to delegate
95-
/// `commit_iter` to the inner type, which fails when `T` is `?Sized` (like `dyn DatabaseCommit`).
90+
/// This trait is dyn-compatible. The `commit_iter` method uses `&mut dyn Iterator`
91+
/// which allows it to be called on trait objects while remaining in the vtable.
92+
#[auto_impl(&mut, Box)]
9693
pub trait DatabaseCommit {
9794
/// Commit changes to the database.
9895
fn commit(&mut self, changes: HashMap<Address, Account>);
@@ -105,78 +102,27 @@ pub trait DatabaseCommit {
105102
///
106103
/// # Dyn Compatibility
107104
///
108-
/// This method requires `Self: Sized` because it uses `impl Trait` in argument position,
109-
/// which is not dyn-compatible. Without this bound, the entire `DatabaseCommit` trait
110-
/// would become not dyn-compatible, breaking code that uses `dyn DatabaseCommit` or
111-
/// traits that extend it (e.g., Foundry's `DatabaseExt`).
112-
///
113-
/// The `Sized` bound excludes this method from the trait's vtable while keeping the
114-
/// trait itself dyn-compatible. Code using trait objects should use [`commit`](Self::commit)
115-
/// instead, collecting the iterator into a `HashMap` first if needed.
116-
fn commit_iter(&mut self, changes: impl IntoIterator<Item = (Address, Account)>)
117-
where
118-
Self: Sized,
119-
{
120-
let changes: HashMap<Address, Account> = changes.into_iter().collect();
105+
/// This method uses `&mut dyn Iterator` to remain object-safe and callable on trait objects.
106+
/// For ergonomic usage with `impl IntoIterator`, use the inherent method
107+
/// `commit_from_iter` on `dyn DatabaseCommit`.
108+
fn commit_iter(&mut self, changes: &mut dyn Iterator<Item = (Address, Account)>) {
109+
let changes: HashMap<Address, Account> = changes.collect();
121110
self.commit(changes);
122111
}
123112
}
124113

125114
/// Inherent implementation for `dyn DatabaseCommit` trait objects.
126115
///
127-
/// This provides `commit_from_iter` for trait objects, which can't use the trait's
128-
/// `commit_iter` method because it requires `Self: Sized`. The inherent method
129-
/// collects the iterator into a `HashMap` and delegates to `commit()`.
116+
/// This provides `commit_from_iter` as an ergonomic wrapper around the trait's
117+
/// `commit_iter` method, accepting `impl IntoIterator` for convenience.
130118
impl dyn DatabaseCommit {
131119
/// Commit changes to the database with an iterator.
132120
///
133-
/// This is an inherent method on `dyn DatabaseCommit` that provides the same
134-
/// functionality as the trait's `commit_iter` method, but works with trait objects.
121+
/// This is an ergonomic wrapper that accepts `impl IntoIterator` and delegates
122+
/// to the trait's [`commit_iter`](DatabaseCommit::commit_iter) method.
135123
#[inline]
136124
pub fn commit_from_iter(&mut self, changes: impl IntoIterator<Item = (Address, Account)>) {
137-
let changes: HashMap<Address, Account> = changes.into_iter().collect();
138-
self.commit(changes);
139-
}
140-
}
141-
142-
/// Manual implementation for `&mut T` where `T: DatabaseCommit + ?Sized`.
143-
///
144-
/// We can't use `#[auto_impl]` because it would generate code that tries to call
145-
/// `T::commit_iter` which requires `T: Sized`, but `T` might be `?Sized` here.
146-
impl<T: DatabaseCommit + ?Sized> DatabaseCommit for &mut T {
147-
#[inline]
148-
fn commit(&mut self, changes: HashMap<Address, Account>) {
149-
(**self).commit(changes)
150-
}
151-
152-
#[inline]
153-
fn commit_iter(&mut self, changes: impl IntoIterator<Item = (Address, Account)>)
154-
where
155-
Self: Sized,
156-
{
157-
// Don't delegate to T::commit_iter since T might be ?Sized.
158-
// Instead, collect and call commit directly.
159-
let changes: HashMap<Address, Account> = changes.into_iter().collect();
160-
(**self).commit(changes);
161-
}
162-
}
163-
164-
/// Manual implementation for `Box<T>` where `T: DatabaseCommit + ?Sized`.
165-
impl<T: DatabaseCommit + ?Sized> DatabaseCommit for Box<T> {
166-
#[inline]
167-
fn commit(&mut self, changes: HashMap<Address, Account>) {
168-
(**self).commit(changes)
169-
}
170-
171-
#[inline]
172-
fn commit_iter(&mut self, changes: impl IntoIterator<Item = (Address, Account)>)
173-
where
174-
Self: Sized,
175-
{
176-
// Don't delegate to T::commit_iter since T might be ?Sized.
177-
// Instead, collect and call commit directly.
178-
let changes: HashMap<Address, Account> = changes.into_iter().collect();
179-
(**self).commit(changes);
125+
self.commit_iter(&mut changes.into_iter())
180126
}
181127
}
182128

@@ -306,9 +252,8 @@ pub trait DatabaseCommitExt: Database + DatabaseCommit {
306252
&mut self,
307253
balances: impl IntoIterator<Item = (Address, u128)>,
308254
) -> Result<(), Self::Error> {
309-
// Make transition and update cache state.
310-
// Collect directly to HashMap to avoid double collection.
311-
let transitions: HashMap<Address, Account> = balances
255+
// Make transition and update cache state
256+
let transitions = balances
312257
.into_iter()
313258
.map(|(address, balance)| {
314259
let mut original_account = match self.basic(address)? {
@@ -322,9 +267,10 @@ pub trait DatabaseCommitExt: Database + DatabaseCommit {
322267
original_account.mark_touch();
323268
Ok((address, original_account))
324269
})
325-
.collect::<Result<_, _>>()?;
270+
// Unfortunately must collect here to short circuit on error
271+
.collect::<Result<Vec<_>, _>>()?;
326272

327-
self.commit(transitions);
273+
self.commit_iter(&mut transitions.into_iter());
328274
Ok(())
329275
}
330276

@@ -335,25 +281,24 @@ pub trait DatabaseCommitExt: Database + DatabaseCommit {
335281
&mut self,
336282
addresses: impl IntoIterator<Item = Address>,
337283
) -> Result<Vec<u128>, Self::Error> {
338-
// Make transition and update cache state.
339-
// We need to collect both transitions (HashMap) and balances (Vec).
284+
// Make transition and update cache state
340285
let addresses_iter = addresses.into_iter();
341286
let (lower, _) = addresses_iter.size_hint();
342-
let mut transitions = HashMap::with_capacity_and_hasher(lower, Default::default());
343-
let mut balances = Vec::with_capacity(lower);
344-
345-
for address in addresses_iter {
346-
let mut original_account = match self.basic(address)? {
347-
Some(acc_info) => Account::from(acc_info),
348-
None => Account::new_not_existing(0),
349-
};
350-
let balance = core::mem::take(&mut original_account.info.balance);
351-
original_account.mark_touch();
352-
transitions.insert(address, original_account);
353-
balances.push(balance.try_into().unwrap());
354-
}
287+
let mut transitions = Vec::with_capacity(lower);
288+
let balances = addresses_iter
289+
.map(|address| {
290+
let mut original_account = match self.basic(address)? {
291+
Some(acc_info) => Account::from(acc_info),
292+
None => Account::new_not_existing(0),
293+
};
294+
let balance = core::mem::take(&mut original_account.info.balance);
295+
original_account.mark_touch();
296+
transitions.push((address, original_account));
297+
Ok(balance.try_into().unwrap())
298+
})
299+
.collect::<Result<Vec<_>, _>>()?;
355300

356-
self.commit(transitions);
301+
self.commit_iter(&mut transitions.into_iter());
357302
Ok(balances)
358303
}
359304
}
@@ -384,22 +329,31 @@ mod tests {
384329

385330
let mut db = MockDb { commits: vec![] };
386331

387-
// Test via trait method (requires Sized) - works on concrete types
388-
db.commit_iter(vec![]);
332+
// Test commit_iter on concrete types
333+
let items: Vec<(Address, Account)> = vec![];
334+
db.commit_iter(&mut items.into_iter());
389335
assert_eq!(db.commits.len(), 1);
390336

391-
// Test via dyn - commit() works on trait objects
337+
// Test commit() on trait objects
392338
{
393339
let db_dyn: &mut dyn DatabaseCommit = &mut db;
394340
db_dyn.commit(HashMap::default());
395341
}
396342
assert_eq!(db.commits.len(), 2);
397343

398-
// For commit_iter on dyn, use the inherent impl's commit_from_iter
344+
// Test commit_iter on trait objects (now works directly!)
399345
{
400346
let db_dyn: &mut dyn DatabaseCommit = &mut db;
401-
db_dyn.commit_from_iter(vec![]);
347+
let items: Vec<(Address, Account)> = vec![];
348+
db_dyn.commit_iter(&mut items.into_iter());
402349
}
403350
assert_eq!(db.commits.len(), 3);
351+
352+
// Test ergonomic commit_from_iter on trait objects
353+
{
354+
let db_dyn: &mut dyn DatabaseCommit = &mut db;
355+
db_dyn.commit_from_iter(vec![]);
356+
}
357+
assert_eq!(db.commits.len(), 4);
404358
}
405359
}

‎crates/database/src/states/state.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ impl<DB: Database> DatabaseCommit for State<DB> {
392392
}
393393
}
394394

395-
fn commit_iter(&mut self, changes: impl IntoIterator<Item = (Address, Account)>) {
395+
fn commit_iter(&mut self, changes: &mut dyn Iterator<Item = (Address, Account)>) {
396396
let transitions = self
397397
.cache
398398
.apply_evm_state_iter(changes, |address, account| {

0 commit comments

Comments
 (0)