diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java index bfccede676d..8d0add16f98 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java @@ -456,8 +456,9 @@ MessageData constructGetBytecodesResponse(final MessageData message) { } else { Optional optCode = worldStateStorageCoordinator.getCode(Hash.wrap(codeHash), null); if (optCode.isPresent()) { - if (sumListBytes(codeBytes) + optCode.get().size() > maxResponseBytes - || stopWatch.getTime() > StatefulPredicate.MAX_MILLIS_PER_REQUEST) { + if (!codeBytes.isEmpty() + && (sumListBytes(codeBytes) + optCode.get().size() > maxResponseBytes + || stopWatch.getTime() > StatefulPredicate.MAX_MILLIS_PER_REQUEST)) { break; } codeBytes.add(optCode.get()); @@ -511,8 +512,9 @@ MessageData constructGetTrieNodesResponse(final MessageData message) { var optStorage = storage.getTrieNodeUnsafe(CompactEncoding.decode(triePath.get(0))); if (optStorage.isPresent()) { - if (sumListBytes(trieNodes) + optStorage.get().size() > maxResponseBytes - || stopWatch.getTime() > StatefulPredicate.MAX_MILLIS_PER_REQUEST) { + if (!trieNodes.isEmpty() + && (sumListBytes(trieNodes) + optStorage.get().size() > maxResponseBytes + || stopWatch.getTime() > StatefulPredicate.MAX_MILLIS_PER_REQUEST)) { break; } trieNodes.add(optStorage.get()); @@ -536,7 +538,9 @@ MessageData constructGetTrieNodesResponse(final MessageData message) { storage.getTrieNodeUnsafe( Bytes.concatenate(accountPrefix, CompactEncoding.decode(path))); if (optStorage.isPresent()) { - if (sumListBytes(trieNodes) + optStorage.get().size() > maxResponseBytes) { + if (!trieNodes.isEmpty() + && sumListBytes(trieNodes) + optStorage.get().size() + > maxResponseBytes) { break; } trieNodes.add(optStorage.get()); @@ -614,11 +618,14 @@ public boolean test(final Pair pair) { return false; } + var hasNoRecords = recordLimit.get() == 0; var underRecordLimit = recordLimit.addAndGet(1) <= MAX_ENTRIES_PER_REQUEST; var underByteLimit = byteLimit.accumulateAndGet(0, (cur, __) -> cur + encodingSizeAccumulator.apply(pair)) < maxResponseBytesFudgeFactor; - if (underRecordLimit && underByteLimit) { + // Only enforce limits when we have at least 1 record as the snapsync spec + // requires at least 1 record must be returned + if (hasNoRecords || (underRecordLimit && underByteLimit)) { return true; } else { shouldContinue.set(false); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServerTest.java index d7d3fc52751..1b4cc0618cf 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServerTest.java @@ -221,6 +221,37 @@ public void assertAccountLimitRangeResponse() { assertThat(assertIsValidAccountRangeProof(Hash.ZERO, rangeData)).isTrue(); } + @Test + public void assertAccountLimitRangeResponse_atLeastOneAccount() { + List randomLoad = IntStream.range(1, 4096).boxed().collect(Collectors.toList()); + Collections.shuffle(randomLoad); + randomLoad.stream() + .forEach( + i -> + insertTestAccounts( + createTestAccount( + Bytes.concatenate( + Bytes.fromHexString("0x40"), + Bytes.fromHexStringLenient(Integer.toHexString(i * 256))) + .toHexString()))); + + final BytesValueRLPOutput tmp = new BytesValueRLPOutput(); + tmp.startList(); + tmp.writeBytes(storageTrie.getRootHash()); + tmp.writeBytes(Hash.ZERO); + tmp.writeBytes(HASH_LAST); + tmp.writeBigIntegerScalar(BigInteger.ZERO); + tmp.endList(); + var tinyRangeLimit = new GetAccountRangeMessage(tmp.encoded()).wrapMessageData(BigInteger.ZERO); + + var rangeData = + getAndVerifyAccountRangeData( + (AccountRangeMessage) snapServer.constructGetAccountRangeResponse(tinyRangeLimit), 1); + + // assert proofs are valid for the requested range + assertThat(assertIsValidAccountRangeProof(Hash.ZERO, rangeData)).isTrue(); + } + @Test public void assertLastEmptyRange() { // When our final range request is empty, no next account is possible, @@ -353,6 +384,42 @@ public void assertStorageLimitRangeResponse() { .isTrue(); } + @Test + public void assertStorageLimitRangeResponse_atLeastOneSlot() { + insertTestAccounts(acct1, acct2, acct3, acct4); + + final BytesValueRLPOutput tmp = new BytesValueRLPOutput(); + tmp.startList(); + tmp.writeBigIntegerScalar(BigInteger.ONE); + tmp.writeBytes(storageTrie.getRootHash()); + tmp.writeList( + List.of(acct3.addressHash, acct4.addressHash), + (hash, rlpOutput) -> rlpOutput.writeBytes(hash)); + tmp.writeBytes(Hash.ZERO); + tmp.writeBytes(HASH_LAST); + tmp.writeBigIntegerScalar(BigInteger.ZERO); + tmp.endList(); + var tinyRangeLimit = new GetStorageRangeMessage(tmp.encoded()); + + var rangeData = + (StorageRangeMessage) snapServer.constructGetStorageRangeResponse(tinyRangeLimit); + + // assert proofs are valid for the requested range + assertThat(rangeData).isNotNull(); + var slotsData = rangeData.slotsData(false); + assertThat(slotsData).isNotNull(); + assertThat(slotsData.slots()).isNotNull(); + assertThat(slotsData.slots().size()).isEqualTo(1); + var firstAccountStorages = slotsData.slots().first(); + // expecting to see complete 10 slot storage for acct3 + assertThat(firstAccountStorages.size()).isEqualTo(1); + assertThat(slotsData.proofs().size()).isNotEqualTo(0); + + assertThat( + assertIsValidStorageProof(acct4, Hash.ZERO, firstAccountStorages, slotsData.proofs())) + .isTrue(); + } + @Test public void assertAccountTriePathRequest() { insertTestAccounts(acct1, acct2, acct3, acct4); @@ -416,6 +483,39 @@ public void assertAccountTrieLimitRequest() { assertThat(trieNodes.size()).isEqualTo(accountNodeLimit * 90 / 100); } + @Test + public void assertAccountTrieLimitRequest_atLeastOneTrieNode() { + insertTestAccounts(acct1, acct2, acct3, acct4); + + var partialPathToAcct1 = Bytes.fromHexString("0x01"); // first nibble is 1 + var partialPathToAcct2 = CompactEncoding.bytesToPath(acct2.addressHash).slice(0, 1); + var partialPathToAcct3 = Bytes.fromHexString("0x03"); // first nibble is 1 + var partialPathToAcct4 = Bytes.fromHexString("0x04"); // first nibble is 1 + final BytesValueRLPOutput tmp = new BytesValueRLPOutput(); + tmp.startList(); + tmp.writeBigIntegerScalar(BigInteger.ONE); + tmp.writeBytes(storageTrie.getRootHash()); + tmp.writeList( + List.of( + List.of(partialPathToAcct4), + List.of(partialPathToAcct3), + List.of(partialPathToAcct2), + List.of(partialPathToAcct1)), + (path, rlpOutput) -> + rlpOutput.writeList(path, (b, subRlpOutput) -> subRlpOutput.writeBytes(b))); + tmp.writeBigIntegerScalar(BigInteger.ZERO); + tmp.endList(); + + var trieNodeRequest = + (TrieNodesMessage) + snapServer.constructGetTrieNodesResponse(new GetTrieNodesMessage(tmp.encoded())); + + assertThat(trieNodeRequest).isNotNull(); + List trieNodes = trieNodeRequest.nodes(false); + assertThat(trieNodes).isNotNull(); + assertThat(trieNodes.size()).isEqualTo(1); + } + @Test public void assertStorageTriePathRequest() { insertTestAccounts(acct1, acct2, acct3, acct4); @@ -468,6 +568,37 @@ public void assertStorageTrieLimitRequest() { assertThat(trieNodes.size()).isEqualTo(trieNodeLimit * 90 / 100); } + @Test + public void assertStorageTrieLimitRequest_atLeastOneTrieNode() { + insertTestAccounts(acct1, acct2, acct3, acct4); + + var pathToSlot11 = CompactEncoding.encode(Bytes.fromHexStringLenient("0x0101")); + var pathToSlot12 = CompactEncoding.encode(Bytes.fromHexStringLenient("0x0102")); + var pathToSlot1a = CompactEncoding.encode(Bytes.fromHexStringLenient("0x010A")); // not present + + final BytesValueRLPOutput tmp = new BytesValueRLPOutput(); + tmp.startList(); + tmp.writeBigIntegerScalar(BigInteger.ONE); + tmp.writeBytes(storageTrie.getRootHash()); + tmp.writeList( + List.of( + List.of(acct3.addressHash, pathToSlot11, pathToSlot12, pathToSlot1a), + List.of(acct4.addressHash, pathToSlot11, pathToSlot12, pathToSlot1a)), + (path, rlpOutput) -> + rlpOutput.writeList(path, (b, subRlpOutput) -> subRlpOutput.writeBytes(b))); + tmp.writeBigIntegerScalar(BigInteger.ZERO); + tmp.endList(); + + var trieNodeRequest = + (TrieNodesMessage) + snapServer.constructGetTrieNodesResponse(new GetTrieNodesMessage(tmp.encoded())); + + assertThat(trieNodeRequest).isNotNull(); + List trieNodes = trieNodeRequest.nodes(false); + assertThat(trieNodes).isNotNull(); + assertThat(trieNodes.size()).isEqualTo(1); + } + @Test public void assertCodePresent() { insertTestAccounts(acct1, acct2, acct3, acct4); @@ -506,6 +637,29 @@ public void assertCodeLimitRequest() { assertThat(codes.codes().size()).isEqualTo(codeLimit * 90 / 100); } + @Test + public void assertCodeLimitRequest_atLeastOneByteCode() { + insertTestAccounts(acct1, acct2, acct3, acct4); + + final BytesValueRLPOutput tmp = new BytesValueRLPOutput(); + tmp.startList(); + tmp.writeBigIntegerScalar(BigInteger.ONE); + tmp.writeList( + List.of(acct3.accountValue.getCodeHash(), acct4.accountValue.getCodeHash()), + (hash, rlpOutput) -> rlpOutput.writeBytes(hash)); + tmp.writeBigIntegerScalar(BigInteger.ZERO); + tmp.endList(); + + var codeRequest = + (ByteCodesMessage) + snapServer.constructGetBytecodesResponse(new GetByteCodesMessage(tmp.encoded())); + + assertThat(codeRequest).isNotNull(); + ByteCodesMessage.ByteCodes codes = codeRequest.bytecodes(false); + assertThat(codes).isNotNull(); + assertThat(codes.codes().size()).isEqualTo(1); + } + static SnapTestAccount createTestAccount(final String hexAddr) { return new SnapTestAccount( Hash.wrap(Bytes32.rightPad(Bytes.fromHexString(hexAddr))),