Add null pointer check and testcase for snapsync tasks (#7724)

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>
Co-authored-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java

@@ -179,6 +179,10 @@ public class StorageRangeDataRequest extends SnapDataRequest {
 
     final StackTrie.TaskElement taskElement = stackTrie.getElement(startKeyHash);
 
+    if (null == taskElement) {
+      return Stream.empty();
+    }
+
     findNewBeginElementInRange(storageRoot, taskElement.proofs(), taskElement.keys(), endKeyHash)
         .ifPresent(
             missingRightElement -> {

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/CompleteTaskStepTest.java

@@ -116,7 +116,7 @@ public class CompleteTaskStepTest {
 
   @Test
   public void shouldMarkSnapsyncTaskCompleteWhenData() {
-    final List<Task<SnapDataRequest>> requests = TaskGenerator.createAccountRequest(true);
+    final List<Task<SnapDataRequest>> requests = TaskGenerator.createAccountRequest(true, false);
     requests.stream()
         .map(StubTask.class::cast)
         .forEach(
@@ -132,7 +132,7 @@ public class CompleteTaskStepTest {
 
   @Test
   public void shouldMarkSnapsyncTaskAsFailedWhenNoData() {
-    final List<Task<SnapDataRequest>> requests = TaskGenerator.createAccountRequest(false);
+    final List<Task<SnapDataRequest>> requests = TaskGenerator.createAccountRequest(false, false);
     requests.stream()
         .map(StubTask.class::cast)
         .forEach(

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java

@@ -68,7 +68,7 @@ public class PersistDataStepTest {
 
   @Test
   public void shouldPersistDataWhenPresent() {
-    final List<Task<SnapDataRequest>> tasks = TaskGenerator.createAccountRequest(true);
+    final List<Task<SnapDataRequest>> tasks = TaskGenerator.createAccountRequest(true, false);
     final List<Task<SnapDataRequest>> result = persistDataStep.persist(tasks);
 
     assertThat(result).isSameAs(tasks);
@@ -105,7 +105,7 @@ public class PersistDataStepTest {
 
   @Test
   public void shouldSkipPersistDataWhenNoData() {
-    final List<Task<SnapDataRequest>> tasks = TaskGenerator.createAccountRequest(false);
+    final List<Task<SnapDataRequest>> tasks = TaskGenerator.createAccountRequest(false, false);
     final List<Task<SnapDataRequest>> result = persistDataStep.persist(tasks);
 
     assertThat(result).isSameAs(tasks);
@@ -116,6 +116,25 @@ public class PersistDataStepTest {
         .isEmpty();
   }
 
+  @Test
+  public void shouldHandleNullTaskElementInTrie() {
+    // Create a StorageRangeDataRequest where taskElement might be null or incomplete
+    List<Task<SnapDataRequest>> tasks = TaskGenerator.createAccountRequest(false, true);
+
+    try {
+      List<Task<SnapDataRequest>> result = persistDataStep.persist(tasks);
+
+      // check for proper handling of null taskElement
+      assertThat(result).isSameAs(tasks);
+      assertThat(result)
+          .isNotNull(); // Make sure the result isn't null even with the bad taskElement
+    } catch (NullPointerException e) {
+      fail(
+          "NullPointerException occurred during persist step, taskElement might be null: "
+              + e.getMessage());
+    }
+  }
+
   private void assertDataPersisted(final List<Task<SnapDataRequest>> tasks) {
     tasks.forEach(
         task -> {

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java

@@ -44,7 +44,8 @@ import org.apache.tuweni.bytes.Bytes32;
 
 public class TaskGenerator {
 
-  public static List<Task<SnapDataRequest>> createAccountRequest(final boolean withData) {
+  public static List<Task<SnapDataRequest>> createAccountRequest(
+      final boolean withData, final boolean withNullTaskElement) {
 
     final BonsaiWorldStateKeyValueStorage worldStateKeyValueStorage =
         new BonsaiWorldStateKeyValueStorage(
@@ -91,7 +92,8 @@ public class TaskGenerator {
             rootHash,
             accountHash,
             stateTrieAccountValue.getStorageRoot(),
-            withData);
+            withData,
+            withNullTaskElement);
     final BytecodeRequest bytecodeRequest =
         createBytecodeDataRequest(
             worldStateKeyValueStorage,
@@ -112,7 +114,8 @@ public class TaskGenerator {
       final Hash rootHash,
       final Hash accountHash,
       final Bytes32 storageRoot,
-      final boolean withData) {
+      final boolean withData,
+      final boolean withNullTaskElement) {
 
     final RangeStorageEntriesCollector collector =
         RangeStorageEntriesCollector.createCollector(
@@ -140,6 +143,11 @@ public class TaskGenerator {
       request.setProofValid(true);
       request.addResponse(null, worldStateProofProvider, slots, new ArrayDeque<>());
     }
+
+    if (withNullTaskElement) {
+      // setting isValidProof to true to simulate a null task element.
+      request.setProofValid(true);
+    }
     return request;
   }