From cc98878160e4a10809b1a3405006a47b9a690203 Mon Sep 17 00:00:00 2001 From: Bhanu Pulluri Date: Fri, 4 Oct 2024 00:32:27 -0400 Subject: [PATCH] Add null pointer check and testcase for snapsync tasks Signed-off-by: Bhanu Pulluri --- .../request/StorageRangeDataRequest.java | 4 ++++ .../sync/snapsync/CompleteTaskStepTest.java | 4 ++-- .../sync/snapsync/PersistDataStepTest.java | 23 +++++++++++++++++-- .../eth/sync/snapsync/TaskGenerator.java | 14 ++++++++--- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java index 14f92e2c3f9..146e0f095e8 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java @@ -180,6 +180,10 @@ public Stream getChildRequests( final StackTrie.TaskElement taskElement = stackTrie.getElement(startKeyHash); + if (null == taskElement) { + return Stream.empty(); + } + findNewBeginElementInRange(storageRoot, taskElement.proofs(), taskElement.keys(), endKeyHash) .ifPresent( missingRightElement -> { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/CompleteTaskStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/CompleteTaskStepTest.java index 2bfc58a3b8a..b78f2b10e89 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/CompleteTaskStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/CompleteTaskStepTest.java @@ -116,7 +116,7 @@ public void shouldMarkStorageTrieNodeTaskCompleteIfItDoesNotHaveDataAndExpired() @Test public void shouldMarkSnapsyncTaskCompleteWhenData() { - final List> requests = TaskGenerator.createAccountRequest(true); + final List> requests = TaskGenerator.createAccountRequest(true, false); requests.stream() .map(StubTask.class::cast) .forEach( @@ -132,7 +132,7 @@ public void shouldMarkSnapsyncTaskCompleteWhenData() { @Test public void shouldMarkSnapsyncTaskAsFailedWhenNoData() { - final List> requests = TaskGenerator.createAccountRequest(false); + final List> requests = TaskGenerator.createAccountRequest(false, false); requests.stream() .map(StubTask.class::cast) .forEach( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java index 0364dc75814..8cfa7e6cefa 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java @@ -59,7 +59,7 @@ public void setUp() { @Test public void shouldPersistDataWhenPresent() { - final List> tasks = TaskGenerator.createAccountRequest(true); + final List> tasks = TaskGenerator.createAccountRequest(true, false); final List> result = persistDataStep.persist(tasks); assertThat(result).isSameAs(tasks); @@ -69,7 +69,7 @@ public void shouldPersistDataWhenPresent() { @Test public void shouldSkipPersistDataWhenNoData() { - final List> tasks = TaskGenerator.createAccountRequest(false); + final List> tasks = TaskGenerator.createAccountRequest(false, false); final List> result = persistDataStep.persist(tasks); assertThat(result).isSameAs(tasks); @@ -80,6 +80,25 @@ public void shouldSkipPersistDataWhenNoData() { .isEmpty(); } + @Test + public void shouldHandleNullTaskElementInTrie() { + // Create a StorageRangeDataRequest where taskElement might be null or incomplete + List> tasks = TaskGenerator.createAccountRequest(false, true); + + try { + List> 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> tasks) { tasks.forEach( task -> { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java index 0e3915bc777..0b3a17cccff 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java @@ -44,7 +44,8 @@ public class TaskGenerator { - public static List> createAccountRequest(final boolean withData) { + public static List> createAccountRequest( + final boolean withData, final boolean withNullTaskElement) { final BonsaiWorldStateKeyValueStorage worldStateKeyValueStorage = new BonsaiWorldStateKeyValueStorage( @@ -91,7 +92,8 @@ public static List> createAccountRequest(final boolean wit rootHash, accountHash, stateTrieAccountValue.getStorageRoot(), - withData); + withData, + withNullTaskElement); final BytecodeRequest bytecodeRequest = createBytecodeDataRequest( worldStateKeyValueStorage, @@ -112,7 +114,8 @@ private static StorageRangeDataRequest createStorageRangeDataRequest( 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 @@ private static StorageRangeDataRequest createStorageRangeDataRequest( 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; }