Skip to content

Commit 8308650

Browse files
Exclude empty requests from requests hash (#8036)
* exclude empty requests from requests hash Signed-off-by: Daniel Lehrner <[email protected]> * Exclude empty requests and prepend request type byte in engine_getPayloadV4 Fixes ethereum/execution-apis#599 change to EIP-7685 Signed-off-by: Simon Dudley <[email protected]> * fix test Signed-off-by: Daniel Lehrner <[email protected]> * fix prague tests: change request hash, reorder json elements Signed-off-by: Daniel Lehrner <[email protected]> * fix extracting of request for new payload Signed-off-by: Daniel Lehrner <[email protected]> * fix test by fixing request object creation Signed-off-by: Daniel Lehrner <[email protected]> * spotless Signed-off-by: Daniel Lehrner <[email protected]> * set request type to only one byte, fixed creation of request list in T8nExecutor Signed-off-by: Daniel Lehrner <[email protected]> * fix expected json output to exclude empty requests Signed-off-by: Daniel Lehrner <[email protected]> --------- Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Co-authored-by: Simon Dudley <[email protected]>
1 parent 11a4317 commit 8308650

File tree

14 files changed

+156
-95
lines changed

14 files changed

+156
-95
lines changed

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/CodeDelegationTransactionAcceptanceTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ public void shouldTransferAllEthOfAuthorizerToSponsor() throws IOException {
140140
*/
141141
@Test
142142
public void shouldCheckNonceAfterNonceIncreaseOfSender() throws IOException {
143-
final long GAS_LIMIT = 1000000L;
144-
cluster.verify(authorizer.balanceEquals(Amount.ether(90000)));
143+
final long GAS_LIMIT = 1_000_000L;
144+
cluster.verify(authorizer.balanceEquals(Amount.ether(90_000)));
145145

146146
final CodeDelegation authorization =
147147
org.hyperledger.besu.ethereum.core.CodeDelegation.builder()
@@ -159,7 +159,7 @@ public void shouldCheckNonceAfterNonceIncreaseOfSender() throws IOException {
159159
.type(TransactionType.DELEGATE_CODE)
160160
.chainId(BigInteger.valueOf(20211))
161161
.nonce(0)
162-
.maxPriorityFeePerGas(Wei.of(1000000000))
162+
.maxPriorityFeePerGas(Wei.of(1_000_000_000))
163163
.maxFeePerGas(Wei.fromHexString("0x02540BE400"))
164164
.gasLimit(GAS_LIMIT)
165165
.to(Address.fromHexStringStrict(authorizer.getAddress()))
@@ -209,7 +209,7 @@ public void shouldCheckNonceAfterNonceIncreaseOfSender() throws IOException {
209209
.nonce(2)
210210
.maxPriorityFeePerGas(Wei.of(10))
211211
.maxFeePerGas(Wei.of(100))
212-
.gasLimit(21000)
212+
.gasLimit(21_000)
213213
.to(Address.fromHexStringStrict(otherAccount.getAddress()))
214214
.value(Wei.ONE)
215215
.payload(Bytes.EMPTY)

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/PragueAcceptanceTestHelper.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ public void buildNewBlock() throws IOException {
9898
executionPayload.toString(), parentBeaconBlockRoot, executionRequests.toString()));
9999
try (final Response newPayloadResponse = newPayloadRequest.execute()) {
100100
assertThat(newPayloadResponse.code()).isEqualTo(200);
101+
102+
final String responseStatus =
103+
mapper.readTree(newPayloadResponse.body().string()).get("result").get("status").asText();
104+
assertThat(responseStatus).isEqualTo("VALID");
101105
}
102106

103107
final Call moveChainAheadRequest = createEngineCall(createForkChoiceRequest(newBlockHash));

datatypes/src/main/java/org/hyperledger/besu/datatypes/Hash.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,9 @@ public class Hash extends DelegatingBytes32 {
5252
public static final Hash EMPTY = hash(Bytes.EMPTY);
5353

5454
/**
55-
* Hash of empty requests or "0x6036c41849da9c076ed79654d434017387a88fb833c2856b32e18218b3341c5f"
55+
* Hash of empty requests or "0xe3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
5656
*/
57-
public static final Hash EMPTY_REQUESTS_HASH =
58-
Hash.wrap(
59-
sha256(
60-
Bytes.concatenate(
61-
sha256(Bytes.of(RequestType.DEPOSIT.getSerializedType())),
62-
sha256(Bytes.of(RequestType.WITHDRAWAL.getSerializedType())),
63-
sha256(Bytes.of(RequestType.CONSOLIDATION.getSerializedType())))));
57+
public static final Hash EMPTY_REQUESTS_HASH = Hash.wrap(sha256(Bytes.EMPTY));
6458

6559
/**
6660
* Instantiates a new Hash.

datatypes/src/test/java/org/hyperledger/besu/datatypes/HashTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ public void shouldGetExpectedValueForEmptyHash() {
3131
public void shouldGetExpectedValueForEmptyRequestsHash() {
3232
assertThat(Hash.EMPTY_REQUESTS_HASH)
3333
.isEqualTo(
34-
Hash.fromHexString(
35-
"0x6036c41849da9c076ed79654d434017387a88fb833c2856b32e18218b3341c5f"));
34+
Hash.fromHexString("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"));
3635
}
3736
}

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@
7171
import java.util.List;
7272
import java.util.Optional;
7373
import java.util.stream.Collectors;
74-
import java.util.stream.IntStream;
7574

7675
import io.vertx.core.Vertx;
7776
import io.vertx.core.json.Json;
@@ -596,8 +595,12 @@ private Optional<List<Request>> extractRequests(final Optional<List<String>> may
596595

597596
return maybeRequestsParam.map(
598597
requests ->
599-
IntStream.range(0, requests.size())
600-
.mapToObj(i -> new Request(RequestType.of(i), Bytes.fromHexString(requests.get(i))))
598+
requests.stream()
599+
.map(
600+
s -> {
601+
final Bytes request = Bytes.fromHexString(s);
602+
return new Request(RequestType.of(request.get(0)), request.slice(1));
603+
})
601604
.collect(Collectors.toList()));
602605
}
603606

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/BlockResultFactory.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ public EngineGetPayloadResultV4 payloadTransactionCompleteV4(final PayloadWrappe
167167
rqs ->
168168
rqs.stream()
169169
.sorted(Comparator.comparing(Request::getType))
170-
.map(r -> r.getData().toHexString())
170+
.filter(r -> !r.getData().isEmpty())
171+
.map(Request::getEncodedRequest)
172+
.map(Bytes::toHexString)
171173
.toList());
172174

173175
final BlobsBundleV1 blobsBundleV1 =

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV4Test.java

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ header, new BlockBody(List.of(blobTx), emptyList(), Optional.of(emptyList()))),
147147
final List<String> requestsWithoutRequestId =
148148
requests.stream()
149149
.sorted(Comparator.comparing(Request::getType))
150-
.map(r -> r.getData().toHexString())
150+
.map(Request::getEncodedRequest)
151+
.map(Bytes::toHexString)
151152
.toList();
152153
Optional.of(resp)
153154
.map(JsonRpcSuccessResponse.class::cast)
@@ -172,6 +173,53 @@ header, new BlockBody(List.of(blobTx), emptyList(), Optional.of(emptyList()))),
172173
verify(engineCallListener, times(1)).executionEngineCalled();
173174
}
174175

176+
@Test
177+
public void shouldExcludeEmptyRequestsInRequestsList() {
178+
179+
BlockHeader header =
180+
new BlockHeaderTestFixture().timestamp(pragueHardfork.milestone() + 1).buildHeader();
181+
PayloadIdentifier payloadIdentifier =
182+
PayloadIdentifier.forPayloadParams(
183+
Hash.ZERO,
184+
pragueHardfork.milestone(),
185+
Bytes32.random(),
186+
Address.fromHexString("0x42"),
187+
Optional.empty(),
188+
Optional.empty());
189+
190+
BlockWithReceipts block =
191+
new BlockWithReceipts(
192+
new Block(header, new BlockBody(emptyList(), emptyList(), Optional.of(emptyList()))),
193+
emptyList());
194+
final List<Request> unorderedRequests =
195+
List.of(
196+
new Request(RequestType.CONSOLIDATION, Bytes.of(1)),
197+
new Request(RequestType.DEPOSIT, Bytes.of(1)),
198+
new Request(RequestType.WITHDRAWAL, Bytes.EMPTY));
199+
PayloadWrapper payload =
200+
new PayloadWrapper(payloadIdentifier, block, Optional.of(unorderedRequests));
201+
202+
when(mergeContext.retrievePayloadById(payloadIdentifier)).thenReturn(Optional.of(payload));
203+
204+
final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V4.getMethodName(), payloadIdentifier);
205+
assertThat(resp).isInstanceOf(JsonRpcSuccessResponse.class);
206+
207+
final List<String> expectedRequests =
208+
List.of(
209+
Bytes.concatenate(Bytes.of(RequestType.DEPOSIT.getSerializedType()), Bytes.of(1))
210+
.toHexString(),
211+
Bytes.concatenate(Bytes.of(RequestType.CONSOLIDATION.getSerializedType()), Bytes.of(1))
212+
.toHexString());
213+
Optional.of(resp)
214+
.map(JsonRpcSuccessResponse.class::cast)
215+
.ifPresent(
216+
r -> {
217+
assertThat(r.getResult()).isInstanceOf(EngineGetPayloadResultV4.class);
218+
final EngineGetPayloadResultV4 res = (EngineGetPayloadResultV4) r.getResult();
219+
assertThat(res.getExecutionRequests()).isEqualTo(expectedRequests);
220+
});
221+
}
222+
175223
@Test
176224
public void shouldReturnUnsupportedFork() {
177225
final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V4.getMethodName(), mockPid);

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV4Test.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,10 @@ protected JsonRpcResponse resp(final EnginePayloadParameter payload) {
170170
final List<String> requestsWithoutRequestId =
171171
VALID_REQUESTS.stream()
172172
.sorted(Comparator.comparing(Request::getType))
173-
.map(r -> r.getData().toHexString())
173+
.map(
174+
r ->
175+
Bytes.concatenate(Bytes.of(r.getType().getSerializedType()), r.getData())
176+
.toHexString())
174177
.toList();
175178
Object[] params =
176179
maybeParentBeaconBlockRoot

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Request.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,13 @@ public RequestType getType() {
2929
public Bytes getData() {
3030
return data();
3131
}
32+
33+
/**
34+
* Gets the serialized form of the concatenated type and data.
35+
*
36+
* @return the serialized request as a byte.
37+
*/
38+
public Bytes getEncodedRequest() {
39+
return Bytes.concatenate(Bytes.of(getType().ordinal()), getData());
40+
}
3241
}

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BodyValidation.java

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,18 @@ public static Hash withdrawalsRoot(final List<Withdrawal> withdrawals) {
9292
/**
9393
* Generates the requests hash for a list of requests
9494
*
95-
* @param requests list of request
95+
* @param requests list of request (must be sorted by request type ascending)
9696
* @return the requests hash
9797
*/
9898
public static Hash requestsHash(final List<Request> requests) {
9999
List<Bytes> requestHashes = new ArrayList<>();
100-
IntStream.range(0, requests.size())
101-
.forEach(
102-
i -> {
103-
final Request request = requests.get(i);
104-
final Bytes requestBytes =
105-
Bytes.concatenate(
106-
Bytes.of(request.getType().getSerializedType()), request.getData());
107-
requestHashes.add(sha256(requestBytes));
108-
});
100+
requests.forEach(
101+
request -> {
102+
// empty requests are excluded from the hash
103+
if (!request.getData().isEmpty()) {
104+
requestHashes.add(sha256(request.getEncodedRequest()));
105+
}
106+
});
109107

110108
return Hash.wrap(sha256(Bytes.wrap(requestHashes)));
111109
}

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/GenesisStateTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ void genesisFromPrague(final DataStorageConfiguration dataStorageConfiguration)
290290
assertThat(header.getHash())
291291
.isEqualTo(
292292
Hash.fromHexString(
293-
"0x554807b22674e6d335f734485993857bbad7a9543affb0663a10c14d78135ec7"));
293+
"0x5d2d02fce02d1b7ca635ec91a4fe6f7aa36f9b3997ec4304e8c68d8f6f15d266"));
294294
assertThat(header.getGasLimit()).isEqualTo(0x2fefd8);
295295
assertThat(header.getGasUsed()).isZero();
296296
assertThat(header.getNumber()).isZero();
@@ -330,7 +330,7 @@ void genesisFromPrague(final DataStorageConfiguration dataStorageConfiguration)
330330
assertThat(header.getRequestsHash().get())
331331
.isEqualTo(
332332
Hash.fromHexString(
333-
"0x6036c41849da9c076ed79654d434017387a88fb833c2856b32e18218b3341c5f"));
333+
"0xe3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"));
334334
}
335335

336336
@Test

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/T8nExecutor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,12 @@ static T8nResult runTest(
546546
ArrayNode requests = resultObject.putArray("requests");
547547
maybeRequests
548548
.orElseGet(List::of)
549-
.forEach(request -> requests.add(request.getData().toHexString()));
549+
.forEach(
550+
request -> {
551+
if (!request.data().isEmpty()) {
552+
requests.add(request.getEncodedRequest().toHexString());
553+
}
554+
});
550555
}
551556

552557
worldState.persist(blockHeader);

0 commit comments

Comments
 (0)