feat: introduce StateCommitment type

[frisitano]

Overview

This PR introduces StateCommitment and KeyHasher traits. The StateCommitment trait is integrated as an associated type in the NodeTypes trait. A implementation of the StateCommitment trait is provided for the MerklePatriciaTrie which represents the state commitment used in ethereum.

supports: #11830

[emhane]

very cool!

[Rjected]

Cool! I just have one question about the TX generic in the GAT

[Rjected]

I'm a bit confused as to why these require DbTx bounds? Is it just because these must implement the Database* traits?

[frisitano]

This is a consequence of the impl of DatabaseStateRoot on StateRoot. As we see below we have a DbTx bound on TX which is required for certain methods used in the implementation.

reth/crates/trie/db/src/state.rs

Lines 132 to 133 in 51594c9

	impl<'a, TX: DbTx> DatabaseStateRoot<'a, TX>
	for StateRoot<DatabaseTrieCursorFactory<'a, TX>, DatabaseHashedCursorFactory<'a, TX>>

As a consequence of this in the implementation of StateCommitment for the MerklePatriciaTrie we must also have a DbTx bound on the StateRoot type as seen below:

impl StateCommitment for MerklePatriciaTrie {
    type StateRoot<'a, TX: DbTx + 'a> =
        StateRoot<DatabaseTrieCursorFactory<'a, TX>, DatabaseHashedCursorFactory<'a, TX>>;
        ...
}

Rust does not allow for an impl of a trait to have more strict bounds than the trait itself and therefore me must propagate the bound to the trait itself. The above rationale is also applicable for the other types in StateCommitment.

[Rjected]

I just added this issue: #12216

note that it should not block any of this work, but eventually these traits could be simplified

[frisitano]

What is the preferred method or fixing conflicts? Rebase and force-push or merge upstream?

[mattsse]

do we expect that a Statecommitment instance will be stateful?

I still need to look at the other prs before we can make a decision on this direction, but I think this could work

[mattsse]

okay, after I looked at the other PRs I get it now.

this should work, I believe there'll likely be things we need to change down the road but this is the best we can do now

[frisitano]

No we wouldn't expect that a StateCommitment instance would be state-full.

I'm out of office until Monday. I will address feedback and rebase upstream upon my return.

[frisitano]

@joshieDo could you provide a review on this when you have time please? We have a number of PR's lined up that will work towards #7434 in an incremental fashion.

[rkrasiuk]

nice

[mattsse]

cool, lets get all of this over the line before we introduce a ton of additional conflicts