Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add null pointer check and testcase for snapsync tasks #7724

Merged
merged 3 commits into from
Oct 11, 2024
+38 −7
Merged

Add null pointer check and testcase for snapsync tasks #7724

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc98878
Add null pointer check and testcase for snapsync tasks
Oct 4, 2024
bad33cc
Merge branch 'main' into 6113-snapsync
macfarla Oct 10, 2024
9ea174b
Merge branch 'main' into 6113-snapsync
macfarla Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ public Stream<SnapDataRequest> getChildRequests(

final StackTrie.TaskElement taskElement = stackTrie.getElement(startKeyHash);

if (null == taskElement) {
return Stream.empty();
}

findNewBeginElementInRange(storageRoot, taskElement.proofs(), taskElement.keys(), endKeyHash)
.ifPresent(
missingRightElement -> {
Expand Down
4 changes: 2 additions & 2 deletions ...h/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/CompleteTaskStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public void shouldMarkStorageTrieNodeTaskCompleteIfItDoesNotHaveDataAndExpired()

@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(
Expand All @@ -132,7 +132,7 @@ public void shouldMarkSnapsyncTaskCompleteWhenData() {

@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(
Expand Down
23 changes: 21 additions & 2 deletions ...th/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void setUp() {

@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);
Expand Down Expand Up @@ -105,7 +105,7 @@ public void shouldPersistTrieNodeHealDataOnlyOnce() {

@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);
Expand All @@ -116,6 +116,25 @@ public void shouldSkipPersistDataWhenNoData() {
.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 -> {
Expand Down
14 changes: 11 additions & 3 deletions ...reum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/TaskGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@

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(
Expand Down Expand Up @@ -91,7 +92,8 @@ public static List<Task<SnapDataRequest>> createAccountRequest(final boolean wit
rootHash,
accountHash,
stateTrieAccountValue.getStorageRoot(),
withData);
withData,
withNullTaskElement);
final BytecodeRequest bytecodeRequest =
createBytecodeDataRequest(
worldStateKeyValueStorage,
Expand All @@ -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(
Expand Down Expand Up @@ -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;
}

Expand Down
Loading