Skip to content

Conversation

@alarso16
Copy link

@alarso16 alarso16 commented Jun 16, 2025

Why this should be merged

To allow more thorough handling of duplicate state roots (or any other info other users would like), additional information can be provided to a call of statedb.Commit. This change allows arbitrary types to be sent to triedb as well as the SnapshotTree. However, this is a breaking change for those using the functionality already, since the snapshot commit option is wrapped with another call.

How this works

See the edited libevm test for usage.

How this was tested

Edited test case to include TrieDBUpdateOption and ensures the payload is sent.

@alarso16 alarso16 requested a review from ARR4N June 16, 2025 19:13
@ARR4N
Copy link
Collaborator

ARR4N commented Jun 17, 2025

Using the same option type for both snapshot.Tree.Update() and triedb.backend.Update() risks accidentally propagating options to the wrong places. I think we should have 3 different option types, one for snapshots, one for trie DBs, and one for StateDB.Commit() as a carrier of the first two.

package stateconf

type SnapshotUpdateOption = options.Option[snapshotUpdateConfig] // unchanged

type snapshotUpdateConfig {
    payload any // unchanged; I don't love this being `any`, but that's a different issue
}

type TrieDBUpdateOption = options.Option[triedbUpdateConfig]

type triedbUpdateConfig {
    parentBlock *common.Hash // pointer to differentiate from no Option setting the value
}

func WithParentBlockHash(h common.Hash) TrieDBUpdateOption {
    return options.Func[triedbUpdateConfig](func(c *triedbUpdateConfig) {
        c.parentBlock = &h
    })
}

type StateDBCommitOption = options.Option[stateDBCommitConfig]

type stateDBCommitConfig struct {
    snaphotOpts []SnapshotUpdateOption
    triedbOpts []TrieDBUpdateOption
}

func WithSnapshotUpdateOpts(opts ... SnapshotUpdateOption) StateDBCommitOption {
    return options.Func[stateDBCommitConfig](func(c *stateDBCommitConfig) {
        // I don't like append() because there's no way to remove options, but that's a weakly held opinion
        c.snapshotOpts = opts
    })
}

func ExtractSnapshotUpdateOpts(opts ...StateDBCommitOption) []SnapshotUpdateOption {
    return options.As(opts...).snapshotOpts
}

// Similarly for WithTrieDBUpdateOpts() and ExtractTrieDBUpdateOpts().

And then used as:

package state

func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool, opts ...stateconf.StateDBCommitOption) (common.Hash, error) {
// ...
    s.db.TrieDB().Update(/* ... */, stateconf.ExtractTrieDBUpdateOpts(opts...)...)
// ...
    s.snaps.Update(/* ... */, stateconf.ExtractSnapshotUpdateOpts(opts...)...)
// ...
}

Rationale

At risk of being too philosophical about engineering, one of my favourite documents is Bumper-Sticker API Design. This sticker is one I regularly refer to:

APIs should be easy to use and hard to misuse. It should be easy to do simple things; possible to do complex things; and impossible, or at least difficult, to do wrong things.

The 3-type design makes it impossible to do wrong things.

PS

My code is quick and rough. Please include comments on exported types, also calling out behaviour that may be ambiguous (e.g. overwrite instead of append).

@alarso16 alarso16 marked this pull request as ready for review June 17, 2025 15:23
@alarso16 alarso16 requested a review from ARR4N June 18, 2025 13:57
@ARR4N ARR4N changed the title feat: Allow addition options on statedb.Commit feat: triedb.Database.Update options via statedb.Commit Jun 18, 2025
@alarso16 alarso16 enabled auto-merge (squash) June 18, 2025 17:52
@alarso16 alarso16 merged commit 9b97d60 into main Jun 18, 2025
12 checks passed
@alarso16 alarso16 deleted the alarso16/commit-options branch June 18, 2025 17:54
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.

2 participants