Skip to content

Snap Server: sumListBytes fudge factor removed #9855

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

Draft
macfarla wants to merge 4 commits into
base: main
Choose a base branch
from
+11 −15
Draft

Snap Server: sumListBytes fudge factor removed #9855

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
15c6bcc
ExceedingPredicate in SnapServer.java
macfarla Feb 19, 2026
d77995b
sumListBytes fudge factor removal
macfarla Feb 20, 2026
7166fd1
Merge branch 'main' of github.com:hyperledger/besu into hive-more
macfarla Feb 20, 2026
842c1e1
Merge branch 'main' of github.com:hyperledger/besu into hive-more
macfarla Apr 2, 2026
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
18 changes: 8 additions & 10 deletions ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,12 +478,11 @@ MessageData constructGetBytecodesResponse(final MessageData message) {
} else {
Optional<Bytes> optCode = worldStateStorageCoordinator.getCode(Hash.wrap(codeHash), null);
if (optCode.isPresent()) {
if (!codeBytes.isEmpty()
&& (sumListBytes(codeBytes) + optCode.get().size() > maxResponseBytes
|| stopWatch.getTime() > ResponseSizePredicate.MAX_MILLIS_PER_REQUEST)) {
codeBytes.add(optCode.get());
if (sumListBytes(codeBytes) > maxResponseBytes
|| stopWatch.getTime() > ResponseSizePredicate.MAX_MILLIS_PER_REQUEST) {
Comment on lines +540 to +541
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The add-then-check pattern may cause the last added bytecode to be included in the response even if it should be excluded. After breaking from the loop, consider removing the last element if it caused the limit to be exceeded: codeBytes.remove(codeBytes.size() - 1) before the break.

Suggested change
if (sumListBytes(codeBytes) > maxResponseBytes
|| stopWatch.getTime() > ResponseSizePredicate.MAX_MILLIS_PER_REQUEST) {
final boolean exceededSizeLimit = sumListBytes(codeBytes) > maxResponseBytes;
final boolean exceededTimeLimit =
stopWatch.getTime() > ResponseSizePredicate.MAX_MILLIS_PER_REQUEST;
if (exceededSizeLimit) {
codeBytes.remove(codeBytes.size() - 1);
}
if (exceededSizeLimit || exceededTimeLimit) {

Copilot uses AI. Check for mistakes.
break;
}
codeBytes.add(optCode.get());
}
}
}
Expand Down Expand Up @@ -598,8 +597,9 @@ && sumListBytes(trieNodes) + trieNode.size() > maxResponseBytes) {
}

/**
* Predicate that doesn't immediately stop when the delegate predicate returns false, but instead
* sets a flag to stop after the current element is processed.
* Predicate that delegates to another predicate and allows one element past the limit. This
* implements the snap protocol's soft limit behavior where the response should include the
* element that first exceeds the limit, ensuring progress and providing proof of completeness.
*/
static class ExceedingPredicate implements Predicate<Pair<Bytes32, Bytes>> {
private final Predicate<Pair<Bytes32, Bytes>> delegate;
Expand All @@ -611,8 +611,7 @@ public ExceedingPredicate(final Predicate<Pair<Bytes32, Bytes>> delegate) {

@Override
public boolean test(final Pair<Bytes32, Bytes> pair) {
final boolean result = delegate.test(pair);
return shouldContinue.getAndSet(result);
return shouldContinue.getAndSet(delegate.test(pair));
}

public boolean shouldGetMore() {
Expand Down Expand Up @@ -701,8 +700,7 @@ Hash getAccountStorageRoot(
}

private static int sumListBytes(final List<Bytes> listOfBytes) {
// TODO: remove hack, 10% is a fudge factor to account for the overhead of rlp encoding
return listOfBytes.stream().map(Bytes::size).reduce((a, b) -> a + b).orElse(0) * 11 / 10;
return listOfBytes.stream().map(Bytes::size).reduce((a, b) -> a + b).orElse(0);
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using reduce((a, b) -> a + b) creates unnecessary lambda overhead. Replace with reduce(Integer::sum) or use mapToInt(Bytes::size).sum() for better performance.

Suggested change
return listOfBytes.stream().map(Bytes::size).reduce((a, b) -> a + b).orElse(0);
return listOfBytes.stream().mapToInt(Bytes::size).sum();

Copilot uses AI. Check for mistakes.
}

private static String asLogHash(final Bytes32 hash) {
Expand Down
8 changes: 3 additions & 5 deletions ...reum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,7 @@ public void assertAccountTrieLimitRequest(final FlatDbMode flatDbMode) {
assertThat(trieNodeRequest).isNotNull();
List<Bytes> trieNodes = trieNodeRequest.nodes(false);
assertThat(trieNodes).isNotNull();
// TODO: adjust this assertion after sorting out the request fudge factor
assertThat(trieNodes.size()).isEqualTo(accountNodeLimit * 90 / 100);
assertThat(trieNodes.size()).isEqualTo(accountNodeLimit);
}

@ParameterizedTest
Expand Down Expand Up @@ -723,7 +722,7 @@ public void assertStorageTrieLimitRequest(final FlatDbMode flatDbMode) {
assertThat(trieNodeRequest).isNotNull();
List<Bytes> trieNodes = trieNodeRequest.nodes(false);
assertThat(trieNodes).isNotNull();
assertThat(trieNodes.size()).isEqualTo(3);
assertThat(trieNodes.size()).isEqualTo(4);
}

@ParameterizedTest
Expand Down Expand Up @@ -799,8 +798,7 @@ public void assertCodeLimitRequest(final FlatDbMode flatDbMode) {
assertThat(codeRequest).isNotNull();
ByteCodesMessage.ByteCodes codes = codeRequest.bytecodes(false);
assertThat(codes).isNotNull();
// TODO adjust this assertion after sorting out the request fudge factor
assertThat(codes.codes().size()).isEqualTo(codeLimit * 90 / 100);
assertThat(codes.codes().size()).isEqualTo(codeLimit);
}

@ParameterizedTest
Expand Down