use Bytes.EMPTY instead of null to indicate empty keys/values (#9519)

Signed-off-by: lukelee-sl <[email protected]>
hashgraph · Oct 26, 2023 · 276a5e1 · 276a5e1
1 parent bb31850
commit 276a5e1
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 29 deletions.
diff --git a/...src/main/java/com/hedera/node/app/service/contract/impl/infra/IterableStorageManager.java b/...src/main/java/com/hedera/node/app/service/contract/impl/infra/IterableStorageManager.java
@@ -28,7 +28,6 @@
 import com.hedera.node.app.service.contract.impl.state.StorageSizeChange;
 import com.hedera.pbj.runtime.io.buffer.Bytes;
 import edu.umd.cs.findbugs.annotations.NonNull;
-import edu.umd.cs.findbugs.annotations.Nullable;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -99,7 +98,9 @@ public void persistChanges(
                 enhancement
                         .operations()
                         .updateStorageMetadata(
-                                change.contractNumber(), firstKeys.get(change.contractNumber()), change.netChange());
+                                change.contractNumber(),
+                                firstKeys.getOrDefault(change.contractNumber(), Bytes.EMPTY),
+                                change.netChange());
             }
         });
     }
@@ -110,10 +111,12 @@ public void persistChanges(
      * @param contractNumber the contract number
      * @return the first storage key for the contract or null if none exists.
      */
-    @Nullable
+    @NonNull
     private Bytes contractFirstKeyOf(@NonNull final Enhancement enhancement, long contractNumber) {
         final var account = enhancement.nativeOperations().getAccount(contractNumber);
-        return account != null && account.firstContractStorageKey() != null ? account.firstContractStorageKey() : null;
+        return account != null && account.firstContractStorageKey() != null
+                ? account.firstContractStorageKey()
+                : Bytes.EMPTY;
     }
 
     /**
@@ -125,10 +128,10 @@ private Bytes contractFirstKeyOf(@NonNull final Enhancement enhancement, long co
      * @param key The slot key to remove
      * @return the new first key in the linked list of storage for the given contract
      */
-    @Nullable
+    @NonNull
     private Bytes removeAccessedValue(
             @NonNull final ContractStateStore store,
-            @Nullable final Bytes firstContractKey,
+            @NonNull final Bytes firstContractKey,
             long contractNumber,
             @NonNull final Bytes key) {
         try {
@@ -139,7 +142,7 @@ private Bytes removeAccessedValue(
             final var nextKey = slotValue.nextKey();
             final var prevKey = slotValue.previousKey();
 
-            if (nextKey != null) {
+            if (!nextKey.equals(Bytes.EMPTY)) {
                 // Look up the next slot value
                 final var nextSlotKey = newSlotKeyFor(contractNumber, nextKey);
                 final var nextValue = slotValueFor(store, true, nextSlotKey, "Missing next key ");
@@ -149,7 +152,7 @@ private Bytes removeAccessedValue(
                         nextValue.copyBuilder().previousKey(prevKey).build();
                 store.putSlot(nextSlotKey, newNextValue);
             }
-            if (prevKey != null) {
+            if (!prevKey.equals(Bytes.EMPTY)) {
                 // Look up the previous slot value
                 final var prevSlotKey = newSlotKeyFor(contractNumber, prevKey);
                 final var prevValue = slotValueFor(store, true, prevSlotKey, "Missing previous key ");
@@ -181,10 +184,10 @@ private Bytes removeAccessedValue(
      * @param newKey The slot key to insert
      * @return the new first key in the linked list of storage for the given contract
      */
-    @Nullable
+    @NonNull
     private Bytes insertAccessedValue(
             @NonNull final ContractStateStore store,
-            @Nullable final Bytes firstContractKey,
+            @NonNull final Bytes firstContractKey,
             @NonNull final Bytes newValue,
             long contractNumber,
             @NonNull final Bytes newKey) {
@@ -196,7 +199,7 @@ private Bytes insertAccessedValue(
             final var newSlotKey = newSlotKeyFor(contractNumber, newKey);
             final var newSlotValue = SlotValue.newBuilder()
                     .value(newValue)
-                    .previousKey(null)
+                    .previousKey(Bytes.EMPTY)
                     .nextKey(firstContractKey)
                     .build();
             store.putSlot(newSlotKey, newSlotValue);

diff --git a/...ice-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/StorageAccess.java b/...ice-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/StorageAccess.java
@@ -30,7 +30,7 @@
  *
  * @param key the key of the access
  * @param value the value read or overwritten
- * @param writtenValue if not null, the overwriting value
+ * @param writtenValue if not Bytes.EMPTY, the overwriting value
  */
 public record StorageAccess(@NonNull UInt256 key, @NonNull UInt256 value, @Nullable UInt256 writtenValue) {
     public StorageAccess {

diff --git a/...java/com/hedera/node/app/service/contract/impl/test/infra/IterableStorageManagerTest.java b/...java/com/hedera/node/app/service/contract/impl/test/infra/IterableStorageManagerTest.java
@@ -71,15 +71,15 @@ class IterableStorageManagerTest {
     void rewriteUpdatesKvCountStorageMetadataOnly() {
         final var sizeChanges = List.of(
                 new StorageSizeChange(CONTRACT_1, 2, 3),
-                new StorageSizeChange(2L, 3, 2),
+                new StorageSizeChange(CONTRACT_2, 3, 2),
                 new StorageSizeChange(3L, 4, 4));
 
         given(enhancement.operations()).willReturn(hederaOperations);
         subject.persistChanges(enhancement, List.of(), sizeChanges, store);
 
-        verify(hederaOperations).updateStorageMetadata(CONTRACT_1, null, 1);
-        verify(hederaOperations).updateStorageMetadata(CONTRACT_2, null, -1);
-        verify(hederaOperations, never()).updateStorageMetadata(CONTRACT_2, null, 0);
+        verify(hederaOperations).updateStorageMetadata(CONTRACT_1, Bytes.EMPTY, 1);
+        verify(hederaOperations).updateStorageMetadata(CONTRACT_2, Bytes.EMPTY, -1);
+        verify(hederaOperations, never()).updateStorageMetadata(CONTRACT_2, Bytes.EMPTY, 0);
     }
 
     @Test
@@ -103,7 +103,8 @@ void removesLastSlotsWithZeroValues() {
         given(account.firstContractStorageKey()).willReturn(BYTES_1);
         given(enhancement.operations()).willReturn(hederaOperations);
         // Deleting the last slot contract storage for CONTRACT_2
-        given(store.getSlotValue(new SlotKey(CONTRACT_2, BYTES_1))).willReturn(new SlotValue(Bytes.EMPTY, null, null));
+        given(store.getSlotValue(new SlotKey(CONTRACT_2, BYTES_1)))
+                .willReturn(new SlotValue(Bytes.EMPTY, Bytes.EMPTY, Bytes.EMPTY));
 
         subject.persistChanges(enhancement, accesses, sizeChanges, store);
 
@@ -113,8 +114,8 @@ void removesLastSlotsWithZeroValues() {
         verifyNoMoreInteractions(store);
 
         // Model call to modify metadata for CONTRACT_2.
-        // The new first key is null as the last slot for the contract was deleted.
-        verify(hederaOperations).updateStorageMetadata(2L, null, -1);
+        // The new first key is Bytes.EMPTY as the last slot for the contract was deleted.
+        verify(hederaOperations).updateStorageMetadata(2L, Bytes.EMPTY, -1);
         verifyNoMoreInteractions(hederaOperations);
     }
 
@@ -130,14 +131,15 @@ void removeFirstSlot() {
         given(account.firstContractStorageKey()).willReturn(BYTES_1);
         given(enhancement.operations()).willReturn(hederaOperations);
         // Deleting the first slot
-        given(store.getSlotValue(new SlotKey(CONTRACT_1, BYTES_1))).willReturn(new SlotValue(BYTES_1, null, BYTES_2));
+        given(store.getSlotValue(new SlotKey(CONTRACT_1, BYTES_1)))
+                .willReturn(new SlotValue(BYTES_1, Bytes.EMPTY, BYTES_2));
         given(store.getSlotValueForModify(new SlotKey(CONTRACT_1, BYTES_2)))
                 .willReturn(new SlotValue(BYTES_2, BYTES_1, BYTES_3));
 
         subject.persistChanges(enhancement, accesses, sizeChanges, store);
 
         // Model deleting the first contract storage
-        verify(store).putSlot(new SlotKey(CONTRACT_1, BYTES_2), new SlotValue(BYTES_2, null, BYTES_3));
+        verify(store).putSlot(new SlotKey(CONTRACT_1, BYTES_2), new SlotValue(BYTES_2, Bytes.EMPTY, BYTES_3));
         verify(store).removeSlot(new SlotKey(CONTRACT_1, BYTES_1));
         // The new first key is BYTES_2 as the first slot for the contract was deleted.
         verify(hederaOperations).updateStorageMetadata(1L, BYTES_2, -1);
@@ -160,14 +162,14 @@ void removeSecondSlot() {
         given(store.getSlotValue(new SlotKey(CONTRACT_1, BYTES_2)))
                 .willReturn(new SlotValue(BYTES_2, BYTES_1, BYTES_3));
         given(store.getSlotValueForModify(new SlotKey(CONTRACT_1, BYTES_1)))
-                .willReturn(new SlotValue(BYTES_1, null, BYTES_2));
+                .willReturn(new SlotValue(BYTES_1, Bytes.EMPTY, BYTES_2));
         given(store.getSlotValueForModify(new SlotKey(CONTRACT_1, BYTES_3)))
                 .willReturn(new SlotValue(BYTES_3, BYTES_2, BYTES_3));
 
         subject.persistChanges(enhancement, accesses, sizeChanges, store);
 
         // Model deleting the second contract storage
-        verify(store).putSlot(new SlotKey(CONTRACT_1, BYTES_1), new SlotValue(BYTES_1, null, BYTES_3));
+        verify(store).putSlot(new SlotKey(CONTRACT_1, BYTES_1), new SlotValue(BYTES_1, Bytes.EMPTY, BYTES_3));
         verify(store).putSlot(new SlotKey(CONTRACT_1, BYTES_3), new SlotValue(BYTES_3, BYTES_1, BYTES_3));
         verify(store).removeSlot(new SlotKey(CONTRACT_1, BYTES_2));
         // The new first key is BYTES_1 as before running the test
@@ -206,7 +208,7 @@ void insertSlotIntoEmptyStorage() {
 
         given(enhancement.nativeOperations()).willReturn(hederaNativeOperations);
         given(hederaNativeOperations.getAccount(CONTRACT_1)).willReturn(account);
-        given(account.firstContractStorageKey()).willReturn(null);
+        given(account.firstContractStorageKey()).willReturn(Bytes.EMPTY);
         given(enhancement.operations()).willReturn(hederaOperations);
 
         // Insert into the second slot
@@ -216,7 +218,7 @@ void insertSlotIntoEmptyStorage() {
         verify(store)
                 .putSlot(
                         new SlotKey(CONTRACT_1, BYTES_2),
-                        new SlotValue(ConversionUtils.tuweniToPbjBytes(UInt256.MAX_VALUE), null, null));
+                        new SlotValue(ConversionUtils.tuweniToPbjBytes(UInt256.MAX_VALUE), Bytes.EMPTY, Bytes.EMPTY));
 
         // The new first key is BYTES_2
         verify(hederaOperations).updateStorageMetadata(1L, BYTES_2, 1);
@@ -243,7 +245,7 @@ void insertSlotIntoExistingStorage() {
         verify(store)
                 .putSlot(
                         new SlotKey(CONTRACT_1, BYTES_2),
-                        new SlotValue(ConversionUtils.tuweniToPbjBytes(UInt256.MAX_VALUE), null, BYTES_1));
+                        new SlotValue(ConversionUtils.tuweniToPbjBytes(UInt256.MAX_VALUE), Bytes.EMPTY, BYTES_1));
 
         // The new first key is BYTES_2
         verify(hederaOperations).updateStorageMetadata(1L, BYTES_2, 1);

diff --git a/...ce-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiImpl.java b/...ce-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiImpl.java
@@ -250,7 +250,7 @@ public ContractChangeSummary summarizeContractChanges() {
      */
     @Override
     public void updateStorageMetadata(
-            @NonNull final AccountID accountId, @Nullable final Bytes firstKey, final int netChangeInSlotsUsed) {
+            @NonNull final AccountID accountId, @NonNull final Bytes firstKey, final int netChangeInSlotsUsed) {
         requireNonNull(firstKey);
         requireNonNull(accountId);
         final var target = requireNonNull(accountStore.get(accountId));

diff --git a/...ra-token-service/src/main/java/com/hedera/node/app/service/token/api/TokenServiceApi.java b/...ra-token-service/src/main/java/com/hedera/node/app/service/token/api/TokenServiceApi.java
@@ -140,10 +140,10 @@ void assertValidStakingElection(
      * Updates the storage metadata for the given contract.
      *
      * @param accountId the id of the contract
-     * @param firstKey       the first key in the storage linked list, null if the storage is empty
+     * @param firstKey       the first key in the storage linked list, empty if the storage is empty
      * @param netChangeInSlotsUsed      the net change in the number of storage slots used by the contract
      */
-    void updateStorageMetadata(@NonNull AccountID accountId, @Nullable Bytes firstKey, int netChangeInSlotsUsed);
+    void updateStorageMetadata(@NonNull AccountID accountId, @NonNull Bytes firstKey, int netChangeInSlotsUsed);
 
     /**
      * Charges the payer the given network fee, and records that fee in the given record builder.