core/state, eth/protocols, trie, triedb/pathdb: remove unused error from trie Commit #29869
Conversation
jwasinger
requested review from
karalabe,
holiman and
rjl493456442
as code owners
trie/verkle.go
Outdated
| } | ||
| func (t *VerkleTrie) Commit(_ bool) (common.Hash, *trienode.NodeSet) { | ||
| root := t.root.(*verkle.InternalNode) | ||
| nodes, _ := root.BatchSerialize() |
There was a problem hiding this comment.
I think that this doesn't need to return an error, and opened an upstream PR.
There was a problem hiding this comment.
If it doesn't get accepted, we can just ignore the error return here IMO. afaict, indicates an internal logic error that we should not experience under normal operation and can't reasonably recover from.
There was a problem hiding this comment.
Added a panic if an error is returned here. We assume by this point, that the Trie we are committing is well-formed.
TBQH, I don't fully understand the underlying logic of what is happening here, but it seems clear that this can only return an error if the underlying tree is somehow corrupted or the code logic is wrong, both of which should be unrecoverable IMO. The underlying error originates here.
CC @gballet if there is something I'm missing here.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Test are failing, looks like you accidently push the changes of test submdule. Please revert it. |
|
Pls rebase, I guess gary's PR broke things |
jwasinger
force-pushed
the
trie-commit-no-error
branch
from
ebf2e2a
to
215ed94
Compare
trie/verkle.go
Outdated
| nodes, err := root.BatchSerialize() | ||
| if err != nil { | ||
| return common.Hash{}, nil, fmt.Errorf("serializing tree nodes: %s", err) | ||
| // error return from this function indicates error in the code logic |
There was a problem hiding this comment.
Nitpick, please capitalise the first letter of the sentence :P
…rom trie Commit (ethereum#29869) * core/state, eth/protocols, trie, triedb/pathdb: remove unused error return from trie Commit * move set back to account-trie-update block scoping for easier readability * address review * undo tests submodule change * trie: panic if BatchSerialize returns an error in Verkle trie Commit * trie: verkle comment nitpicks --------- Co-authored-by: Péter Szilágyi <peterke@gmail.com>
The
Commitoperation performs the commitment computation, computing the changed node-set, internal intermediate roots, and new state root. There's no reason that it should fail and return an error.