fix: Ethereum Transaction validation optimisations

hedera-node/hapi-utils/src/main/java/com/hedera/node/app/hapi/utils/ethereum/EthTxData.java

@@ -78,16 +78,29 @@ public record EthTxData(
     public static EthTxData populateEthTxData(final byte[] data) {
         try {
             final var decoder = RLPDecoder.RLP_STRICT.sequenceIterator(data);
-            final var rlpItem = decoder.next();
-            if (rlpItem.isList()) {
-                return populateLegacyEthTxData(rlpItem, data);
+            final var firstItem = decoder.next();
+
+            // A legacy transaction is a bare RLP list, so it is its own envelope.
+            if (firstItem.isList()) {
+                return consumesAllOf(firstItem, data) ? populateLegacyEthTxData(firstItem, data) : null;
+            }
+
+            // A typed transaction (EIP-2718) is a one-byte type tag followed by its payload list, so there the
+            // envelope is the second item. Unsupported types fall through to `null` below; decoding their
+            // payload first is wasted work on an already-rejected input, but it keeps the check in one place,
+            // and a malformed payload only raises `IllegalArgumentException`, which this method already maps
+            // to `null`.
+            final var type = asByte(firstItem);
+            final var envelope = decoder.next();
+            if (!consumesAllOf(envelope, data)) {
+                return null;
             }
 
-            return switch (asByte(rlpItem)) {
-                case 1 -> populateEip2390EthTxData(decoder.next(), data);
-                case 2 -> populateEip1559EthTxData(decoder.next(), data);
+            return switch (type) {
+                case 1 -> populateEip2390EthTxData(envelope, data);
+                case 2 -> populateEip1559EthTxData(envelope, data);
                 case 3 -> null; // We don't currently support Cancun "blob" transactions
-                case 4 -> populateEip7702EthTxData(decoder.next(), data);
+                case 4 -> populateEip7702EthTxData(envelope, data);
                 default -> null;
             };
 
@@ -601,6 +614,27 @@ public List<CodeDelegation> extractCodeDelegations() throws IllegalArgumentExcep
         }
     }
 
+    /// Returns whether the given envelope item ends exactly at the end of {@code data}, i.e. whether the RLP
+    /// encoding consumed the whole input as EIP-2718 requires and as `ethereum_data` is specified ("the
+    /// complete transaction data").
+    ///
+    /// `RLPDecoder.sequenceIterator` is a *sequence* reader, so anything past the envelope is simply left
+    /// unread rather than reported: `tx || extra` would otherwise parse as `tx`, yielding identical fields but
+    /// a different `keccak256(rawTx)` — the value externalized as a record's `ethereum_hash`. Requiring full
+    /// consumption keeps that hash a function of the transaction rather than of how its bytes were framed.
+    ///
+    /// This governs only bytes *outside* the envelope. It is unrelated to trailing bytes *inside* ABI-encoded
+    /// `callData`, which are part of the signed payload and which HIP-1342 deliberately permits; neither rule
+    /// generalizes to the other layer.
+    ///
+    /// A positional comparison is preferred over `decoder.hasNext()`, which reaches the same two outcomes only
+    /// by way of exception control flow: it returns `true` for a well-formed trailing item but throws
+    /// headlong's `ShortInputException` for a malformed one, relying on that being an
+    /// `IllegalArgumentException` for {@link #populateEthTxData} to map it to `null`.
+    private static boolean consumesAllOf(@NonNull final RLPItem envelope, @NonNull final byte[] data) {
+        return envelope.endIndex == data.length;
+    }
+
     /**
      * Encodes the transaction data into a EthTxData according to legacy RLP format.
      *

hedera-node/hapi-utils/src/test/java/com/hedera/node/app/hapi/utils/ethereum/EthTxDataTest.java

@@ -19,6 +19,7 @@
 import com.esaulpaugh.headlong.rlp.RLPList;
 import com.esaulpaugh.headlong.util.Integers;
 import com.google.protobuf.ByteString;
+import com.hedera.node.app.hapi.utils.MiscCryptoUtils;
 import com.hedera.node.app.hapi.utils.ethereum.EthTxData.EthTransactionType;
 import java.math.BigInteger;
 import java.util.Arrays;
@@ -380,6 +381,67 @@ void whiteBoxDecodingErrors() {
 
         // poorly wrapped typed transaction
         assertNull(EthTxData.populateEthTxData(RLPEncoder.sequence(new byte[] {2}, oneByte, oneByte)));
+
+        // Trailing bytes after a complete envelope - the mirror of "Trimmed End Bytes" above
+        assertNull(EthTxData.populateEthTxData(withSuffix(HexFormat.of().parseHex(RAW_TX_TYPE_0), new byte[] {0})));
+    }
+
+    /// Returns `data || suffix`.
+    private static byte[] withSuffix(final byte[] data, final byte[] suffix) {
+        final var suffixed = Arrays.copyOf(data, data.length + suffix.length);
+        System.arraycopy(suffix, 0, suffixed, data.length, suffix.length);
+        return suffixed;
+    }
+
+    /// Bytes past the end of the RLP envelope must be rejected outright, never left unread.
+    ///
+    /// EIP-2718 requires the envelope to consume its whole input, and `ethereum_data` is specified as the
+    /// complete transaction data. Since a record's `ethereum_hash` is `keccak256` of exactly those bytes,
+    /// tolerating a longer framing would make the recorded hash depend on the framing rather than on the
+    /// transaction: the fields, the signer and the operation would all be unchanged.
+    ///
+    /// `00`, `01`, `80` and `c0` are each well-formed RLP items on their own, so those are the trailers that
+    /// parse cleanly and would slip past a weaker check. `ff` and `deadbeef` are malformed, covering the case
+    /// where the trailer itself provokes a decoder error.
+    @ParameterizedTest(name = "suffix=0x{0}")
+    @ValueSource(strings = {"00", "01", "80", "c0", "ff", "deadbeef", "ffffffffffffffffffffffff"})
+    void rejectsTrailingBytesAfterEnvelope(final String suffixHex) {
+        final var suffix = HexFormat.of().parseHex(suffixHex);
+        for (final var canonicalHex : List.of(
+                RAW_TX_TYPE_0,
+                RAW_TX_TYPE_0_WITH_CHAIN_ID_11155111,
+                RAW_TX_TYPE_1,
+                RAW_TX_TYPE_2,
+                EIP155_DEMO,
+                EIP155_UNPROTECTED)) {
+            final var canonical = HexFormat.of().parseHex(canonicalHex);
+            assertNotNull(EthTxData.populateEthTxData(canonical), () -> canonicalHex + " must still parse");
+            assertNull(
+                    EthTxData.populateEthTxData(withSuffix(canonical, suffix)),
+                    () -> canonicalHex + " with trailing 0x" + suffixHex + " must be rejected");
+        }
+    }
+
+    /// A transaction's `ethereum_hash` must be fixed by the signed envelope alone.
+    @Test
+    void ethereumHashIsKeccakOfTheCanonicalEnvelope() {
+        for (final var canonicalHex :
+                List.of(RAW_TX_TYPE_0, RAW_TX_TYPE_1, RAW_TX_TYPE_2, EIP155_DEMO, EIP155_UNPROTECTED)) {
+            final var canonical = HexFormat.of().parseHex(canonicalHex);
+            final var parsed = requireNonNull(EthTxData.populateEthTxData(canonical));
+            assertArrayEquals(MiscCryptoUtils.keccak256DigestOf(canonical), parsed.getEthereumHash(), canonicalHex);
+        }
+    }
+
+    /// Guards the boundary from the accepting side: a programmatically built type-2 envelope parses, and the
+    /// same bytes plus one trailing zero do not. The other fixtures are hard-coded hex, so this is the only
+    /// case that proves the check tracks the encoded length rather than a fixture's known size.
+    @Test
+    void acceptsBuiltTypedEnvelopeButRejectsItWithOneTrailingByte() {
+        final var canonical = RLPEncoder.sequence(new byte[] {2}, Arrays.asList(normalRlpData()));
+
+        assertNotNull(EthTxData.populateEthTxData(canonical));
+        assertNull(EthTxData.populateEthTxData(Arrays.copyOf(canonical, canonical.length + 1)));
     }
 
     byte[][] normalRlpData() {

hedera-node/hapi-utils/src/test/java/com/hedera/node/app/hapi/utils/ethereum/EthTxDataType4TransactionTest.java

@@ -307,6 +307,43 @@ void populateEip7702EthTxDataReturnsNullWhenWrongNumberOfItemsInList() {
         assertNull(tx);
     }
 
+    /// EIP-2718 requires the envelope to consume its whole input. Bytes past its end would leave every field
+    /// intact while changing `keccak256(rawTx)` - the value externalized as a record's `ethereum_hash` - so
+    /// they must be rejected rather than left unread. See `EthTxDataTest.rejectsTrailingBytesAfterEnvelope`
+    /// for the legacy, type-1 and type-2 equivalents.
+    @Test
+    void populateEip7702EthTxDataReturnsNullWhenTrailingBytesFollowEnvelope() {
+        final byte[] authorizationList = rlpList(
+                rlpBytes(new byte[] {0x01}),
+                rlpBytes(repeat((byte) 0x11, 20)),
+                rlpUInt(5),
+                rlpUInt(1),
+                rlpBytes(repeat((byte) 0x22, 32)),
+                rlpBytes(repeat((byte) 0x33, 32)));
+        final byte[] canonical = buildType4Raw(
+                fillBytes(2, 0x01),
+                1,
+                fillBytes(3, 0x02),
+                fillBytes(3, 0x03),
+                100,
+                fillBytes(20, 0x04),
+                0L,
+                new byte[] {},
+                new Object[] {},
+                authorizationList,
+                27,
+                fillBytes(32, 0x05),
+                fillBytes(32, 0x06));
+
+        assertNotNull(EthTxData.populateEthTxData(canonical));
+
+        for (final byte[] suffix : new byte[][] {{0x00}, {(byte) 0x80}, {(byte) 0xc0}, {(byte) 0xff}}) {
+            final var suffixed = Arrays.copyOf(canonical, canonical.length + suffix.length);
+            System.arraycopy(suffix, 0, suffixed, canonical.length, suffix.length);
+            assertNull(EthTxData.populateEthTxData(suffixed), "trailing byte must be rejected");
+        }
+    }
+
     private static byte[] rlpBytes(byte[] bytes) {
         if (bytes.length == 1 && (bytes[0] & 0xFF) < 0x80) {
             return new byte[] {bytes[0]};