From 5b4a36f73517d2a50dd7aa3462804f6869eabeea Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 28 Nov 2024 16:48:51 +0000 Subject: [PATCH] [Engine-API] Exclude empty requests in requests list --- .../electra/ConsolidationRequest.java | 2 + .../versions/electra/DepositRequest.java | 2 + .../electra/ExecutionRequestsDataCodec.java | 90 +++++++++++-------- .../versions/electra/WithdrawalRequest.java | 2 + .../ExecutionRequestsDataCodecTest.java | 57 +++++------- 5 files changed, 83 insertions(+), 70 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ConsolidationRequest.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ConsolidationRequest.java index 89002b50754..3327b990f65 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ConsolidationRequest.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ConsolidationRequest.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.spec.datastructures.execution.versions.electra; +import org.apache.tuweni.bytes.Bytes; import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.infrastructure.bytes.Bytes20; import tech.pegasys.teku.infrastructure.ssz.collections.SszByteVector; @@ -25,6 +26,7 @@ public class ConsolidationRequest extends Container3 { public static final byte REQUEST_TYPE = 0x2; + public static final Bytes REQUEST_TYPE_PREFIX = Bytes.of(REQUEST_TYPE); protected ConsolidationRequest( final ConsolidationRequestSchema schema, diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/DepositRequest.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/DepositRequest.java index fd0f5219ddc..72f504cbed5 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/DepositRequest.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/DepositRequest.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.spec.datastructures.execution.versions.electra; +import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.bls.BLSSignature; @@ -30,6 +31,7 @@ public class DepositRequest DepositRequest, SszPublicKey, SszBytes32, SszUInt64, SszSignature, SszUInt64> { public static final byte REQUEST_TYPE = 0x0; + public static final Bytes REQUEST_TYPE_PREFIX = Bytes.of(REQUEST_TYPE); DepositRequest( final DepositRequestSchema schema, diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodec.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodec.java index e3127504af6..82a78cacff2 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodec.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodec.java @@ -13,9 +13,9 @@ package tech.pegasys.teku.spec.datastructures.execution.versions.electra; +import java.util.ArrayList; import java.util.List; import org.apache.tuweni.bytes.Bytes; -import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.spec.datastructures.execution.ExecutionRequestsBuilder; /* @@ -23,8 +23,6 @@ */ public class ExecutionRequestsDataCodec { - private static final int EXPECTED_REQUEST_DATA_ELEMENTS = 3; - private final ExecutionRequestsSchema executionRequestsSchema; public ExecutionRequestsDataCodec(final ExecutionRequestsSchema executionRequestsSchema) { @@ -34,44 +32,47 @@ public ExecutionRequestsDataCodec(final ExecutionRequestsSchema executionRequest /** * Decodes the execution requests received from the EL. * - * @param executionRequestData list of encoded execution requests from the EL + * @param executionRequests list of encoded execution requests from the EL * @return an ExecutionRequests object with the requests */ - public ExecutionRequests decode(final List executionRequestData) { - if (executionRequestData.size() != EXPECTED_REQUEST_DATA_ELEMENTS) { - throw new IllegalArgumentException( - "Invalid number of execution request data elements: expected " - + EXPECTED_REQUEST_DATA_ELEMENTS - + ", received " - + executionRequestData.size()); - } - + public ExecutionRequests decode(final List executionRequests) { final ExecutionRequestsBuilder executionRequestsBuilder = new ExecutionRequestsBuilderElectra(executionRequestsSchema); - for (int index = 0; index < executionRequestData.size(); index++) { - // The request type is implicitly defined as the index of the element in executionRequestData - switch ((byte) index) { + byte previousRequestType = -1; + for (final Bytes request : executionRequests) { + if (request.isEmpty()) { + throw new IllegalArgumentException("Execution request data must not be empty"); + } + final byte requestType = request.get(0); + if (requestType <= previousRequestType) { + throw new IllegalArgumentException( + "Execution requests are not in strictly ascending order"); + } + final Bytes requestData = request.slice(1); + switch (requestType) { case DepositRequest.REQUEST_TYPE -> executionRequestsBuilder.deposits( executionRequestsSchema .getDepositRequestsSchema() - .sszDeserialize(executionRequestData.get(index)) + .sszDeserialize(requestData) .asList()); case WithdrawalRequest.REQUEST_TYPE -> executionRequestsBuilder.withdrawals( executionRequestsSchema .getWithdrawalRequestsSchema() - .sszDeserialize(executionRequestData.get(index)) + .sszDeserialize(requestData) .asList()); case ConsolidationRequest.REQUEST_TYPE -> executionRequestsBuilder.consolidations( executionRequestsSchema .getConsolidationRequestsSchema() - .sszDeserialize(executionRequestData.get(index)) + .sszDeserialize(requestData) .asList()); - default -> throw new IllegalArgumentException("Invalid execution request type: " + index); + default -> + throw new IllegalArgumentException("Invalid execution request type: " + requestType); } + previousRequestType = requestType; } return executionRequestsBuilder.build(); @@ -84,22 +85,37 @@ public ExecutionRequests decode(final List executionRequestData) { * @return list of encoded execution requests */ public List encode(final ExecutionRequests executionRequests) { - final SszList depositRequestsSszList = - executionRequestsSchema - .getDepositRequestsSchema() - .createFromElements(executionRequests.getDeposits()); - final SszList withdrawalRequestsSszList = - executionRequestsSchema - .getWithdrawalRequestsSchema() - .createFromElements(executionRequests.getWithdrawals()); - final SszList consolidationRequestsSszList = - executionRequestsSchema - .getConsolidationRequestsSchema() - .createFromElements(executionRequests.getConsolidations()); - - return List.of( - depositRequestsSszList.sszSerialize(), - withdrawalRequestsSszList.sszSerialize(), - consolidationRequestsSszList.sszSerialize()); + final List executionRequestsData = new ArrayList<>(); + final List deposits = executionRequests.getDeposits(); + if (!deposits.isEmpty()) { + final Bytes depositRequestsData = + executionRequestsSchema + .getDepositRequestsSchema() + .createFromElements(deposits) + .sszSerialize(); + executionRequestsData.add( + Bytes.concatenate(DepositRequest.REQUEST_TYPE_PREFIX, depositRequestsData)); + } + final List withdrawals = executionRequests.getWithdrawals(); + if (!withdrawals.isEmpty()) { + final Bytes withdrawalsRequestsData = + executionRequestsSchema + .getWithdrawalRequestsSchema() + .createFromElements(withdrawals) + .sszSerialize(); + executionRequestsData.add( + Bytes.concatenate(WithdrawalRequest.REQUEST_TYPE_PREFIX, withdrawalsRequestsData)); + } + final List consolidations = executionRequests.getConsolidations(); + if (!consolidations.isEmpty()) { + final Bytes consolidationRequestsData = + executionRequestsSchema + .getConsolidationRequestsSchema() + .createFromElements(consolidations) + .sszSerialize(); + executionRequestsData.add( + Bytes.concatenate(ConsolidationRequest.REQUEST_TYPE_PREFIX, consolidationRequestsData)); + } + return executionRequestsData; } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/WithdrawalRequest.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/WithdrawalRequest.java index dfd8d6bed1a..335ad7d9b2c 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/WithdrawalRequest.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/WithdrawalRequest.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.spec.datastructures.execution.versions.electra; +import org.apache.tuweni.bytes.Bytes; import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.infrastructure.bytes.Bytes20; import tech.pegasys.teku.infrastructure.ssz.collections.SszByteVector; @@ -27,6 +28,7 @@ public class WithdrawalRequest extends Container3 { public static final byte REQUEST_TYPE = 0x1; + public static final Bytes REQUEST_TYPE_PREFIX = Bytes.of(REQUEST_TYPE); protected WithdrawalRequest( final WithdrawalRequestSchema schema, diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodecTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodecTest.java index fe90e70770e..70b5e0e0aba 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodecTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/datastructures/execution/versions/electra/ExecutionRequestsDataCodecTest.java @@ -70,10 +70,8 @@ public void decodeExecutionRequestData() { } @Test - public void decodeExecutionRequestDataWithAllRequestTypesEmpty() { - final List executionRequestsData = List.of(Bytes.EMPTY, Bytes.EMPTY, Bytes.EMPTY); - - final ExecutionRequests executionRequests = codec.decode(executionRequestsData); + public void decodeExecutionRequestsDataWithNoRequests() { + final ExecutionRequests executionRequests = codec.decode(List.of()); assertThat(executionRequests.getDeposits()).isEmpty(); assertThat(executionRequests.getWithdrawals()).isEmpty(); @@ -81,9 +79,9 @@ public void decodeExecutionRequestDataWithAllRequestTypesEmpty() { } @Test - public void decodeExecutionRequestDataWithOneRequestTypeEmpty() { + public void decodeExecutionRequestsDataWithOneRequestMissing() { final List executionRequestsData = - List.of(depositRequestListEncoded, Bytes.EMPTY, consolidationRequestsListEncoded); + List.of(depositRequestListEncoded, consolidationRequestsListEncoded); final ExecutionRequests executionRequests = codec.decode(executionRequestsData); @@ -93,46 +91,39 @@ public void decodeExecutionRequestDataWithOneRequestTypeEmpty() { } @Test - public void decodeExecutionRequestDataWithMoreElementsThanExpected() { + public void decodeExecutionRequestsDataWithInvalidRequestType() { final List invalidExecutionRequestsData = - List.of( - depositRequestListEncoded, - withdrawalRequestsListEncoded, - consolidationRequestsListEncoded, - Bytes.random(10)); + List.of(depositRequestListEncoded, withdrawalRequestsListEncoded, Bytes.of(9)); assertThatThrownBy(() -> codec.decode(invalidExecutionRequestsData)) - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid execution request type: 9"); } @Test - public void decodeExecutionRequestDataWithLessThanExpectedElements() { + public void decodeExecutionRequestDataWithRequestsNotOrderedInAscendingOrder() { final List invalidExecutionRequestsData = - List.of(depositRequestListEncoded, withdrawalRequestsListEncoded); + List.of( + depositRequestListEncoded, + consolidationRequestsListEncoded, + withdrawalRequestsListEncoded); assertThatThrownBy(() -> codec.decode(invalidExecutionRequestsData)) - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Execution requests are not in strictly ascending order"); } @Test - public void decodeExecutionRequestDataWithMoreThanExpectedElements() { + public void decodeExecutionRequestDataWithRepeatedRequestsOfSameType() { final List invalidExecutionRequestsData = List.of( depositRequestListEncoded, - withdrawalRequestsListEncoded, consolidationRequestsListEncoded, - depositRequestListEncoded); - - assertThatThrownBy(() -> codec.decode(invalidExecutionRequestsData)) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void decodeExecutionRequestDataWithZeroElements() { - final List invalidExecutionRequestsData = List.of(); + consolidationRequestsListEncoded); assertThatThrownBy(() -> codec.decode(invalidExecutionRequestsData)) - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Execution requests are not in strictly ascending order"); } @Test @@ -165,7 +156,7 @@ public void encodeWithWithOneEmptyRequestList() { final List encodedRequests = codec.encode(executionRequests); assertThat(encodedRequests) - .containsExactly(depositRequestListEncoded, Bytes.EMPTY, consolidationRequestsListEncoded); + .containsExactly(depositRequestListEncoded, consolidationRequestsListEncoded); } @Test @@ -179,14 +170,14 @@ public void encodeWithAllEmptyRequestLists() { final List encodedRequests = codec.encode(executionRequests); - assertThat(encodedRequests).containsExactly(Bytes.EMPTY, Bytes.EMPTY, Bytes.EMPTY); + assertThat(encodedRequests).isEmpty(); } // Examples taken from // https://github.com/ethereum/execution-apis/blob/main/src/engine/openrpc/methods/payload.yaml private final Bytes depositRequestListEncoded = Bytes.fromHexString( - "0x96a96086cff07df17668f35f7418ef8798079167e3f4f9b72ecde17b28226137cf454ab1dd20ef5d924786ab3483c2f9003f5102dabe0a27b1746098d1dc17a5d3fbd478759fea9287e4e419b3c3cef20100000000000000b1acdb2c4d3df3f1b8d3bfd33421660df358d84d78d16c4603551935f4b67643373e7eb63dcb16ec359be0ec41fee33b03a16e80745f2374ff1d3c352508ac5d857c6476d3c3bcf7e6ca37427c9209f17be3af5264c0e2132b3dd1156c28b4e9f000000000000000a5c85a60ba2905c215f6a12872e62b1ee037051364244043a5f639aa81b04a204c55e7cc851f29c7c183be253ea1510b001db70c485b6264692f26b8aeaab5b0c384180df8e2184a21a808a3ec8e86ca01000000000000009561731785b48cf1886412234531e4940064584463e96ac63a1a154320227e333fb51addc4a89b7e0d3f862d7c1fd4ea03bd8eb3d8806f1e7daf591cbbbb92b0beb74d13c01617f22c5026b4f9f9f294a8a7c32db895de3b01bee0132c9209e1f100000000000000"); + "0x0096a96086cff07df17668f35f7418ef8798079167e3f4f9b72ecde17b28226137cf454ab1dd20ef5d924786ab3483c2f9003f5102dabe0a27b1746098d1dc17a5d3fbd478759fea9287e4e419b3c3cef20100000000000000b1acdb2c4d3df3f1b8d3bfd33421660df358d84d78d16c4603551935f4b67643373e7eb63dcb16ec359be0ec41fee33b03a16e80745f2374ff1d3c352508ac5d857c6476d3c3bcf7e6ca37427c9209f17be3af5264c0e2132b3dd1156c28b4e9f000000000000000a5c85a60ba2905c215f6a12872e62b1ee037051364244043a5f639aa81b04a204c55e7cc851f29c7c183be253ea1510b001db70c485b6264692f26b8aeaab5b0c384180df8e2184a21a808a3ec8e86ca01000000000000009561731785b48cf1886412234531e4940064584463e96ac63a1a154320227e333fb51addc4a89b7e0d3f862d7c1fd4ea03bd8eb3d8806f1e7daf591cbbbb92b0beb74d13c01617f22c5026b4f9f9f294a8a7c32db895de3b01bee0132c9209e1f100000000000000"); private final DepositRequest depositRequest1 = new DepositRequest( @@ -218,7 +209,7 @@ public void encodeWithAllEmptyRequestLists() { private final Bytes withdrawalRequestsListEncoded = Bytes.fromHexString( - "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b85103a5617937691dfeeb89b86a80d5dc9e3c9d3a1a0e7ce311e26e0bb732eabaa47ffa288f0d54de28209a62a7d29d0000000000000000000000000000000000000000000000000000010f698daeed734da114470da559bd4b4c7259e1f7952555241dcbc90cf194a2ef676fc6005f3672fada2a3645edb297a75530100000000000000"); + "0x01a94f5374fce5edbc8e2a8697c15331677e6ebf0b85103a5617937691dfeeb89b86a80d5dc9e3c9d3a1a0e7ce311e26e0bb732eabaa47ffa288f0d54de28209a62a7d29d0000000000000000000000000000000000000000000000000000010f698daeed734da114470da559bd4b4c7259e1f7952555241dcbc90cf194a2ef676fc6005f3672fada2a3645edb297a75530100000000000000"); private final WithdrawalRequest withdrawalRequest1 = new WithdrawalRequest( @@ -240,7 +231,7 @@ public void encodeWithAllEmptyRequestLists() { private final Bytes consolidationRequestsListEncoded = Bytes.fromHexString( - "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b85103a5617937691dfeeb89b86a80d5dc9e3c9d3a1a0e7ce311e26e0bb732eabaa47ffa288f0d54de28209a62a7d29d098daeed734da114470da559bd4b4c7259e1f7952555241dcbc90cf194a2ef676fc6005f3672fada2a3645edb297a7553"); + "0x02a94f5374fce5edbc8e2a8697c15331677e6ebf0b85103a5617937691dfeeb89b86a80d5dc9e3c9d3a1a0e7ce311e26e0bb732eabaa47ffa288f0d54de28209a62a7d29d098daeed734da114470da559bd4b4c7259e1f7952555241dcbc90cf194a2ef676fc6005f3672fada2a3645edb297a7553"); private final ConsolidationRequest consolidationRequest1 = new ConsolidationRequest(