Refactor NullNode and remove NullLeafNode class (#73)

* Refactor NullNode and remove NullLeafNode class Signed-off-by: Luis Pinto <[email protected]> Remove NullLeafNode and use NullNode instead Signed-off-by: Luis Pinto <[email protected]> * fixup! Refactor NullNode and remove NullLeafNode class rename nullLeafNode to newNullLeafNode in NullNode Signed-off-by: Luis Pinto <[email protected]> --------- Signed-off-by: Luis Pinto <[email protected]>
hyperledger · Oct 2, 2024 · e103275 · e103275
1 parent 69cd7e7
commit e103275
Show file tree

Hide file tree

Showing 19 changed files with 112 additions and 277 deletions.
diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/VerkleTrieBatchHasher.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/VerkleTrieBatchHasher.java
@@ -24,7 +24,6 @@
 import org.hyperledger.besu.ethereum.trie.verkle.node.InternalNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.LeafNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.Node;
-import org.hyperledger.besu.ethereum.trie.verkle.node.NullLeafNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.NullNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.StemNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.StoredNode;
@@ -65,8 +64,7 @@ public class VerkleTrieBatchHasher {
   public void addNodeToBatch(final Optional<Bytes> maybeLocation, final Node<?> node) {
     maybeLocation.ifPresent(
         location -> {
-          if ((node instanceof NullNode<?> || node instanceof NullLeafNode<?>)
-              && !location.isEmpty()) {
+          if (node instanceof NullNode<?> && !location.isEmpty()) {
             updatedNodes.remove(location);
           } else {
             updatedNodes.put(location, node);

diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/factory/StoredNodeFactory.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/factory/StoredNodeFactory.java
@@ -21,7 +21,6 @@
 import org.hyperledger.besu.ethereum.trie.verkle.node.InternalNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.LeafNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.Node;
-import org.hyperledger.besu.ethereum.trie.verkle.node.NullLeafNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.NullNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.StemNode;
 import org.hyperledger.besu.ethereum.trie.verkle.node.StoredInternalNode;
@@ -52,9 +51,6 @@ 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>();
-
   private final NodeLoader nodeLoader;
   private final Function<Bytes, V> valueDeserializer;
   private final Boolean areCommitmentsCompressed;
@@ -182,7 +178,7 @@ private InternalNode<V> createInternalNode(
     List<Node<V>> children = new ArrayList<>(nChild);
     for (int i = 0; i < nChild; i++) {
       if (scalars.get(i).compareTo(Bytes32.ZERO) == 0) {
-        children.add(new NullNode<V>());
+        children.add(NullNode.nullNode());
       } else {
         if (indices.containsKey((byte) i)) {
           children.add(
@@ -227,7 +223,7 @@ StemNode<V> decodeStemNode(Bytes stem, Bytes encodedValues, Bytes32 hash) {
     List<Node<V>> children = new ArrayList<>(nChild);
     for (int i = 0; i < nChild; i++) {
       if (values.get(i) == Bytes.EMPTY) {
-        children.add(nullLeafNode);
+        children.add(NullNode.newNullLeafNode());
       } else {
         children.add(
             createLeafNode(

diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/BranchNode.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/BranchNode.java
@@ -89,7 +89,7 @@ public BranchNode(final Bytes location) {
         new ArrayList<>() {
           {
             for (int i = 0; i < maxChild(); i++) {
-              add(new NullNode<>());
+              add(NullNode.nullNode());
             }
           }
         });
@@ -236,7 +236,7 @@ public String print() {
   public String toDot(Boolean showNullNodes) {
     StringBuilder result =
         new StringBuilder()
-            .append(getClass().getSimpleName())
+            .append(getName())
             .append(getLocation().orElse(Bytes.EMPTY))
             .append(" [label=\"B: ")
             .append(getLocation().orElse(Bytes.EMPTY))
@@ -247,10 +247,10 @@ public String toDot(Boolean showNullNodes) {
 
     for (Node<V> child : getChildren()) {
       String edgeString =
-          getClass().getSimpleName()
+          getName()
               + getLocation().orElse(Bytes.EMPTY)
               + " -> "
-              + child.getClass().getSimpleName()
+              + child.getName()
               + child.getLocation().orElse(Bytes.EMPTY)
               + "\n";
 

diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/InternalNode.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/InternalNode.java
@@ -196,7 +196,7 @@ public String print() {
   public String toDot(Boolean showNullNodes) {
     StringBuilder result =
         new StringBuilder()
-            .append(getClass().getSimpleName())
+            .append(getName())
             .append(getLocation().orElse(Bytes.EMPTY))
             .append(" [label=\"I: ")
             .append(getLocation().orElse(Bytes.EMPTY))
@@ -206,10 +206,10 @@ public String toDot(Boolean showNullNodes) {
 
     for (Node<V> child : getChildren()) {
       String edgeString =
-          getClass().getSimpleName()
+          getName()
               + getLocation().orElse(Bytes.EMPTY)
               + " -> "
-              + child.getClass().getSimpleName()
+              + child.getName()
               + child.getLocation().orElse(Bytes.EMPTY)
               + "\n";
 

diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/LeafNode.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/LeafNode.java
@@ -176,14 +176,14 @@ public String print() {
   public String toDot(Boolean showNullNodes) {
     Bytes locationBytes = getLocation().orElse(Bytes.EMPTY);
 
-    return getClass().getSimpleName()
+    return getName()
         + locationBytes
         + " [label=\"L: "
         + locationBytes
         + "\nSuffix: "
         + Bytes.of(locationBytes.get(locationBytes.size() - 1))
         + "\"]\n"
-        + getClass().getSimpleName()
+        + getName()
         + locationBytes
         + " -> "
         + "Value"

diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/Node.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/Node.java
@@ -210,6 +210,15 @@ public boolean isPersisted() {
    */
   public abstract String toDot(Boolean showNullNodes);
 
+  /**
+   * Gets the name for the DOT representation for the Node.
+   *
+   * @return name for the DOT representation for the Node.
+   */
+  public String getName() {
+    return getClass().getSimpleName();
+  }
+
   /**
    * Generates DOT representation for the Node.
    *

diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/NullLeafNode.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/NullLeafNode.java
diff --git a/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/NullNode.java b/src/main/java/org/hyperledger/besu/ethereum/trie/verkle/node/NullNode.java
@@ -32,10 +32,17 @@
  */
 public class NullNode<V> extends Node<V> {
 
-  public NullNode() {
+  private boolean isLeaf;
+
+  private NullNode() {
     super(false, true);
   }
 
+  private NullNode(Optional<V> previousValue) {
+    super(false, true);
+    this.previous = previousValue;
+  }
+
   /**
    * Accepts a visitor for path-based operations on the node.
    *
@@ -95,14 +102,18 @@ public void markDirty() {
     dirty = true;
   }
 
+  public boolean isLeaf() {
+    return isLeaf;
+  }
+
   /**
    * Get a string representation of the `NullNode`.
    *
    * @return A string representation indicating that it is a "NULL" node.
    */
   @Override
   public String print() {
-    return "[NULL]";
+    return isLeaf() ? "[NULL-LEAF]" : "[NULL]";
   }
 
   /**
@@ -115,12 +126,44 @@ public String toDot(Boolean showRepeatingEdges) {
     if (!showRepeatingEdges) {
       return "";
     }
-    String result =
-        getClass().getSimpleName()
-            + getLocation().orElse(Bytes.EMPTY)
-            + " [label=\"NL: "
-            + getLocation().orElse(Bytes.EMPTY)
-            + "\"]\n";
-    return result;
+    return getName()
+        + getLocation().orElse(Bytes.EMPTY)
+        + " [label=\"NL: "
+        + getLocation().orElse(Bytes.EMPTY)
+        + "\"]\n";
+  }
+
+  @Override
+  public String getName() {
+    return isLeaf() ? "NullLeafNode" : "NullNode";
+  }
+
+  @SuppressWarnings("rawtypes")
+  private static NullNode nullLeafNode;
+
+  @SuppressWarnings("rawtypes")
+  private static NullNode nullNode;
+
+  @SuppressWarnings("unchecked")
+  public static <T> NullNode<T> nullNode() {
+    if (nullNode == null) {
+      nullNode = new NullNode<>();
+    }
+    return nullNode;
+  }
+
+  @SuppressWarnings("unchecked")
+  public static <T> NullNode<T> newNullLeafNode() {
+    if (nullLeafNode == null) {
+      nullLeafNode = new NullNode<>();
+      nullLeafNode.isLeaf = true;
+    }
+    return nullLeafNode;
+  }
+
+  public static <T> NullNode<T> newNullLeafNode(Optional<T> previousValue) {
+    NullNode<T> nullLeafNode = new NullNode<>(previousValue);
+    nullLeafNode.isLeaf = true;
+    return nullLeafNode;
   }
 }