Skip to content

fix: use StateCommitment#17790

Closed
robinsdan wants to merge 1 commit intoparadigmxyz:mainfrom
robinsdan:main
Closed

fix: use StateCommitment#17790
robinsdan wants to merge 1 commit intoparadigmxyz:mainfrom
robinsdan:main

Conversation

@robinsdan
Copy link
Contributor

The current implementation ignored the StateCommitment constraint

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Aug 11, 2025
@robinsdan robinsdan changed the title use StateCommitment fix: use StateCommitment Aug 11, 2025
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

i dont understand what this fixes, can you elaborate

does this make sense @Rjected

@robinsdan
Copy link
Contributor Author

@mattsse Wasn't the introduction of the StateCommitment trait intended to abstract the implementation of the Database? However, the current implementation uses a fixed type, StateRoot, StorageRoot, etc, which assumes that StateCommitment is equivalent to MerklePatriciaTrie.

pub struct MerklePatriciaTrie;
impl StateCommitment for MerklePatriciaTrie {
type StateRoot<'a, TX: DbTx + 'a> =
StateRoot<DatabaseTrieCursorFactory<'a, TX>, DatabaseHashedCursorFactory<'a, TX>>;
type StorageRoot<'a, TX: DbTx + 'a> =
StorageRoot<DatabaseTrieCursorFactory<'a, TX>, DatabaseHashedCursorFactory<'a, TX>>;
type StateProof<'a, TX: DbTx + 'a> =
Proof<DatabaseTrieCursorFactory<'a, TX>, DatabaseHashedCursorFactory<'a, TX>>;
type StateWitness<'a, TX: DbTx + 'a> =
TrieWitness<DatabaseTrieCursorFactory<'a, TX>, DatabaseHashedCursorFactory<'a, TX>>;
type KeyHasher = KeccakKeyHasher;
}

@Rjected
Copy link
Member

Rjected commented Aug 11, 2025

The intention of this abstraction was to allow customization of the state root algorithm yes, but I think it has failed at doing that because there are too many parts of the code which assume MPT and the abstraction only handles certain db related traits. I am going to close this PR as I think it makes the code more verbose, but I would instead support removing the StateCommitment trait

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants