Fix stateroot issue #72
Conversation
Signed-off-by: Karim Taam <[email protected]>
|
| @@ -52,6 +52,9 @@ enum NodeType { | |||
| * @param <V> The type of values stored in Verkle Trie nodes. | |||
| */ | |||
| public class StoredNodeFactory<V> implements NodeFactory<V> { | |||
|
|
|||
| private final NullLeafNode<V> nullLeafNode = new NullLeafNode<V>(); | |||
There was a problem hiding this comment.
We always create a new VerkleTrie on the caller side so I think it would be best if this field is static
There was a problem hiding this comment.
I used a non static field in order to have the same type as StoredNodeFactory ( V )
| this.location = Optional.of(location); | ||
| this.children = new ArrayList<>(); | ||
| for (int i = 0; i < maxChild(); i++) { | ||
| final NullNode<V> nullNode = new NullNode<V>(); | ||
| nullNode.markDirty(); | ||
| children.add(nullNode); |
There was a problem hiding this comment.
can't you use this() to avoid repetition like you did on LeafNode?
| @@ -29,7 +29,7 @@ | |||
| * @param <V> The type of node values. | |||
| */ | |||
| public class GetVisitor<V> implements PathNodeVisitor<V> { | |||
| private final Node<V> NULL_NODE_RESULT = new NullLeafNode<>(); | |||
| public final Node<V> NULL_NODE_RESULT = new NullLeafNode<>(); | |||
There was a problem hiding this comment.
can't reuse the one from StoredNodeFactory? Maybe move it into a location with constants or something
There was a problem hiding this comment.
same as the first comment don't found a way to have a static field and keep V type
| /** | ||
| * Constructs a new BranchNode with optional location and path, initializing children to | ||
| * NullNodes. | ||
| * | ||
| * @param location The optional location in the tree. | ||
| */ | ||
| public BranchNode(final Bytes location) { | ||
| super(false, false); | ||
| super(false, true); | ||
| this.location = Optional.of(location); | ||
| this.children = new ArrayList<>(); | ||
| for (int i = 0; i < maxChild(); i++) { | ||
| final NullNode<V> nullNode = new NullNode<V>(); |
There was a problem hiding this comment.
What's the difference between NullNode and NullLeafNode It seems to me that they are mostly the same apart from that NullLeafNode's ability to store a previous value.
There was a problem hiding this comment.
it's mainly that , it's also a way to have a specific logic in the PutVisitor for NullNode and NullLeafNode
There was a problem hiding this comment.
As discussed offline: I was trying to do a refactoring to remove NullLeafNode and provide the same functionality but I'm getting some errors so I'll back out my last commit to not block you on this one
Signed-off-by: Karim Taam <[email protected]>
lu-pinto
force-pushed
the
feature/fix-stateroot-issue
branch
from
1bfa22e
to
4919430
Compare
PR description
Update code in order to fix state root issues when we reload a trie from the database ; add also some cleaning of the code
Fixed Issue(s)