Skip to content
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.

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

[4844] Add encodingContext to TransactionEncoder and TransactionDecoder #5820

Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
feaab57
Add decode type to TransactionDecoder
Gabriel-Trintinalia Aug 29, 2023
1499768
Merge branch 'main' into add-decode-type-to-transaction-decoder
Gabriel-Trintinalia Aug 29, 2023
318bc16
Refactoring TransactionDecoder
Gabriel-Trintinalia Aug 31, 2023
59665a7
Merge main
Gabriel-Trintinalia Aug 31, 2023
9f06b73
Rename class
Gabriel-Trintinalia Aug 31, 2023
f31a783
Fix spotless
Gabriel-Trintinalia Aug 31, 2023
7b3f751
Change names
Gabriel-Trintinalia Aug 31, 2023
b20f6d4
Fix spotless
Gabriel-Trintinalia Aug 31, 2023
ed726a4
Invert methods order
Gabriel-Trintinalia Sep 1, 2023
ede2408
Add comments
Gabriel-Trintinalia Sep 1, 2023
a1402c8
Code simplification
Gabriel-Trintinalia Sep 1, 2023
792e735
Code simplification
Gabriel-Trintinalia Sep 1, 2023
3fdc9a9
Code simplification
Gabriel-Trintinalia Sep 1, 2023
63124a1
Code simplification
Gabriel-Trintinalia Sep 1, 2023
8594933
Code simplification
Gabriel-Trintinalia Sep 2, 2023
b453cf7
fix tests
Gabriel-Trintinalia Sep 3, 2023
a877c99
Add javadoc
Gabriel-Trintinalia Sep 4, 2023
b678893
Add javadoc
Gabriel-Trintinalia Sep 4, 2023
8ee0c89
Refactoring
Gabriel-Trintinalia Sep 4, 2023
1f1fc12
Merge branch 'add-decode-type-to-transaction-decoder' of https://gith…
Gabriel-Trintinalia Sep 4, 2023
88ba4bf
Merge branch 'transaction-encoding-refactor' into add-decode-type-to-…
Gabriel-Trintinalia Sep 4, 2023
25a78ed
Merge branch 'main' into add-decode-type-to-transaction-decoder
Gabriel-Trintinalia Sep 4, 2023
0d62138
Use Transaction encoder instead of writeTo
Gabriel-Trintinalia Sep 4, 2023
ea4651d
Merge branch 'add-decode-type-to-transaction-decoder' of https://gith…
Gabriel-Trintinalia Sep 4, 2023
c99cb7c
Merge branch 'main' into add-decode-type-to-transaction-decoder
Gabriel-Trintinalia Sep 5, 2023
80a10b8
Move enter and leave list to inner method as pr suggestion
Gabriel-Trintinalia Sep 8, 2023
30eca31
Size calculation should use opaque bytes instead of rlp
Gabriel-Trintinalia Sep 8, 2023
987a4a5
PR suggestions
Gabriel-Trintinalia Sep 8, 2023
b57c906
Fix spotless
Gabriel-Trintinalia Sep 8, 2023
2084eff
Merge branch 'main' into add-decode-type-to-transaction-decoder
Gabriel-Trintinalia Sep 8, 2023
386112e
Merge branch 'main' into add-decode-type-to-transaction-decoder
macfarla Sep 12, 2023
ef7fc75
Merge branch 'main' into add-decode-type-to-transaction-decoder
macfarla Sep 13, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.hyperledger.besu.ethereum.core.Difficulty;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.Withdrawal;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionDecoder;
import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
import org.hyperledger.besu.ethereum.mainnet.BodyValidation;
Expand Down Expand Up @@ -170,7 +171,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
transactions =
blockParam.getTransactions().stream()
.map(Bytes::fromHexString)
.map(TransactionDecoder::decodeOpaqueBytes)
.map(in -> TransactionDecoder.decodeOpaqueBytes(in, EncodingContext.BLOCK_BODY))
.collect(Collectors.toList());
} catch (final RLPException | IllegalArgumentException e) {
return respondWithInvalid(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockValueCalculator;
import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;

import java.util.ArrayList;
Expand Down Expand Up @@ -95,7 +96,9 @@ public BlockResult transactionComplete(final Block block) {
public EngineGetPayloadResultV1 payloadTransactionCompleteV1(final Block block) {
final List<String> txs =
Gabriel-Trintinalia marked this conversation as resolved.
Show resolved Hide resolved
block.getBody().getTransactions().stream()
.map(TransactionEncoder::encodeOpaqueBytes)
.map(
transaction ->
TransactionEncoder.encodeOpaqueBytes(transaction, EncodingContext.BLOCK_BODY))
.map(Bytes::toHexString)
.collect(Collectors.toList());

Expand All @@ -106,7 +109,9 @@ public EngineGetPayloadResultV2 payloadTransactionCompleteV2(
final BlockWithReceipts blockWithReceipts) {
final List<String> txs =
blockWithReceipts.getBlock().getBody().getTransactions().stream()
.map(TransactionEncoder::encodeOpaqueBytes)
.map(
transaction ->
TransactionEncoder.encodeOpaqueBytes(transaction, EncodingContext.BLOCK_BODY))
.map(Bytes::toHexString)
.collect(Collectors.toList());

Expand All @@ -132,7 +137,9 @@ public EngineGetPayloadResultV3 payloadTransactionCompleteV3(
final BlockWithReceipts blockWithReceipts) {
final List<String> txs =
blockWithReceipts.getBlock().getBody().getTransactions().stream()
.map(TransactionEncoder::encodeOpaqueBytes)
.map(
transaction ->
TransactionEncoder.encodeOpaqueBytes(transaction, EncodingContext.BLOCK_BODY))
.map(Bytes::toHexString)
.collect(Collectors.toList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameter;
import org.hyperledger.besu.ethereum.core.BlockBody;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;

import java.util.Collections;
Expand Down Expand Up @@ -52,7 +53,9 @@ public static class PayloadBody {
public PayloadBody(final BlockBody blockBody) {
this.transactions =
blockBody.getTransactions().stream()
.map(TransactionEncoder::encodeOpaqueBytes)
.map(
transaction ->
TransactionEncoder.encodeOpaqueBytes(transaction, EncodingContext.BLOCK_BODY))
.map(Bytes::toHexString)
.collect(Collectors.toList());
this.withdrawals =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.hyperledger.besu.datatypes.VersionedHash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;

import java.util.List;
Expand Down Expand Up @@ -99,7 +100,9 @@ public TransactionPendingResult(final Transaction transaction) {
this.input = transaction.getPayload().toString();
this.nonce = Quantity.create(transaction.getNonce());
this.publicKey = transaction.getPublicKey().orElse(null);
this.raw = TransactionEncoder.encodeOpaqueBytes(transaction).toString();
this.raw =
TransactionEncoder.encodeOpaqueBytes(transaction, EncodingContext.POOLED_TRANSACTION)
.toString();
this.to = transaction.getTo().map(Address::toHexString).orElse(null);
this.type =
transactionType.equals(TransactionType.FRONTIER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcRequestException;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionDecoder;
import org.hyperledger.besu.ethereum.rlp.RLPException;

Expand All @@ -27,7 +28,7 @@ public static Transaction decodeRawTransaction(final String rawTransaction)
throws InvalidJsonRpcRequestException {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we rename this method or get rid of it? It does not reflect which format to decode now that we have the two different kinds ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any suggestions for names? It is used to handle the serialization in the RPC apis

try {
Bytes txnBytes = Bytes.fromHexString(rawTransaction);
return TransactionDecoder.decodeOpaqueBytes(txnBytes);
return TransactionDecoder.decodeOpaqueBytes(txnBytes, EncodingContext.POOLED_TRANSACTION);
} catch (final IllegalArgumentException e) {
throw new InvalidJsonRpcRequestException("Invalid raw transaction hex", e);
} catch (final RLPException r) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import org.hyperledger.besu.crypto.KeyPair;
import org.hyperledger.besu.crypto.SignatureAlgorithm;
import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.BlobGas;
import org.hyperledger.besu.datatypes.BlobsWithCommitments;
import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.BlockProcessingOutputs;
import org.hyperledger.besu.ethereum.BlockProcessingResult;
Expand All @@ -35,18 +41,26 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EnginePayloadStatusResult;
import org.hyperledger.besu.ethereum.core.BlobTestFixture;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
import org.hyperledger.besu.ethereum.core.Deposit;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.TransactionTestFixture;
import org.hyperledger.besu.ethereum.core.Withdrawal;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;
import org.hyperledger.besu.ethereum.mainnet.BodyValidation;
import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
import org.hyperledger.besu.evm.gascalculator.CancunGasCalculator;

import java.math.BigInteger;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;

import com.google.common.base.Suppliers;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -56,6 +70,9 @@

@ExtendWith(MockitoExtension.class)
public class EngineNewPayloadV3Test extends EngineNewPayloadV2Test {
private static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
Suppliers.memoize(SignatureAlgorithmFactory::getInstance);
protected static final KeyPair senderKeys = SIGNATURE_ALGORITHM.get().generateKeyPair();

public EngineNewPayloadV3Test() {}

Expand Down Expand Up @@ -194,4 +211,42 @@ public void shouldValidateExcessBlobGasCorrectly() {
assertThat(jsonRpcError.getData()).isEqualTo("Missing blob gas fields");
verify(engineCallListener, times(1)).executionEngineCalled();
}

@Test
public void shouldRejectTransactionsWithFullBlob() {
Gabriel-Trintinalia marked this conversation as resolved.
Show resolved Hide resolved

Bytes transactionWithBlobsBytes =
TransactionEncoder.encodeOpaqueBytes(
createTransactionWithBlobs(), EncodingContext.POOLED_TRANSACTION);

List<String> transactions = List.of(transactionWithBlobsBytes.toString());

BlockHeader mockHeader =
setupValidPayload(
new BlockProcessingResult(Optional.of(new BlockProcessingOutputs(null, List.of()))),
Optional.empty(),
Optional.empty());
var resp = resp(mockEnginePayload(mockHeader, transactions));

EnginePayloadStatusResult res = fromSuccessResp(resp);
assertThat(res.getStatusAsString()).isEqualTo(INVALID.name());
assertThat(res.getError()).isEqualTo("Failed to decode transactions from block parameter");
verify(engineCallListener, times(1)).executionEngineCalled();
}

private Transaction createTransactionWithBlobs() {
BlobTestFixture blobTestFixture = new BlobTestFixture();
BlobsWithCommitments bwc = blobTestFixture.createBlobsWithCommitments(1);

return new TransactionTestFixture()
.to(Optional.of(Address.fromHexString("0xDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF")))
.type(TransactionType.BLOB)
.chainId(Optional.of(BigInteger.ONE))
.maxFeePerGas(Optional.of(Wei.of(15)))
.maxFeePerBlobGas(Optional.of(Wei.of(128)))
.maxPriorityFeePerGas(Optional.of(Wei.of(1)))
.blobsWithCommitments(Optional.of(bwc))
.versionedHashes(Optional.of(bwc.getVersionedHashes()))
.createTransaction(senderKeys);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;
import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;

Expand Down Expand Up @@ -59,7 +60,7 @@ public class DomainObjectDecodeUtilsTest {
@Test
public void testAccessListRLPSerDes() {
final BytesValueRLPOutput encoded = new BytesValueRLPOutput();
TransactionEncoder.encodeForWire(accessListTxn, encoded);
TransactionEncoder.encodeRLP(accessListTxn, encoded, EncodingContext.POOLED_TRANSACTION);
Transaction decoded =
DomainObjectDecodeUtils.decodeRawTransaction(encoded.encoded().toHexString());
Assertions.assertThat(decoded.getAccessList().isPresent()).isTrue();
Expand All @@ -68,7 +69,8 @@ public void testAccessListRLPSerDes() {

@Test
public void testAccessList2718OpaqueSerDes() {
final Bytes encoded = TransactionEncoder.encodeOpaqueBytes(accessListTxn);
final Bytes encoded =
TransactionEncoder.encodeOpaqueBytes(accessListTxn, EncodingContext.POOLED_TRANSACTION);
Transaction decoded = DomainObjectDecodeUtils.decodeRawTransaction(encoded.toString());
Assertions.assertThat(decoded.getAccessList().isPresent()).isTrue();
Assertions.assertThat(decoded.getAccessList().map(List::size).get()).isEqualTo(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.datatypes.VersionedHash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.encoding.AccessListTransactionEncoder;
import org.hyperledger.besu.ethereum.core.encoding.BlobTransactionEncoder;
import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
import org.hyperledger.besu.ethereum.core.encoding.TransactionDecoder;
import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;
import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
Expand Down Expand Up @@ -126,7 +128,7 @@ public static Transaction readFrom(final Bytes rlpBytes) {
}

public static Transaction readFrom(final RLPInput rlpInput) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can only decode one of the two encoding formats now. Should this be renamed? Should it decode both formats? Should we remove it and only rely on the TransactionDecoder?

return TransactionDecoder.decodeForWire(rlpInput);
return TransactionDecoder.decodeRLP(rlpInput, EncodingContext.BLOCK_BODY);
}

/**
Expand Down Expand Up @@ -613,7 +615,7 @@ private Bytes32 getOrComputeSenderRecoveryHash() {
* @param out the output to write the transaction to
*/
public void writeTo(final RLPOutput out) {
Copy link
Contributor

Choose a reason for hiding this comment

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

s.a.

TransactionEncoder.encodeForWire(this, out);
TransactionEncoder.encodeRLP(this, out, EncodingContext.BLOCK_BODY);
}

@Override
Expand Down Expand Up @@ -676,17 +678,19 @@ public int getSize() {
}

private void memoizeHashAndSize() {
final Bytes bytes = TransactionEncoder.encodeOpaqueBytes(this);
final Bytes bytes = TransactionEncoder.encodeOpaqueBytes(this, EncodingContext.BLOCK_BODY);
hash = Hash.hash(bytes);

if (transactionType.supportsBlob()) {
if (getBlobsWithCommitments().isPresent()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure that we always want the size including the blobs when they are available? Or should we have a size and a size method that takes the encoding context as an argument?
It looks like size is only used in the TransactionAnnouncement. Should we rename it to announcementSize?

size = bytes.size();
final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
TransactionEncoder.encodeRLP(this, rlpOutput, EncodingContext.POOLED_TRANSACTION);
size = rlpOutput.encodedSize();
return;
}
}
final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
TransactionEncoder.encodeForWire(transactionType, bytes, rlpOutput);
TransactionEncoder.encodeRLP(transactionType, bytes, rlpOutput);
size = rlpOutput.encodedSize();
}

Expand Down Expand Up @@ -968,7 +972,7 @@ private static void eip1559PreimageFields(
rlpOutput.writeBytes(to.map(Bytes::copy).orElse(Bytes.EMPTY));
rlpOutput.writeUInt256Scalar(value);
rlpOutput.writeBytes(payload);
TransactionEncoder.writeAccessList(rlpOutput, accessList);
AccessListTransactionEncoder.writeAccessList(rlpOutput, accessList);
}

private static Bytes blobPreimage(
Expand Down Expand Up @@ -1019,7 +1023,7 @@ private static Bytes accessListPreimage(
RLP.encode(
rlpOutput -> {
rlpOutput.startList();
TransactionEncoder.encodeAccessListInner(
AccessListTransactionEncoder.encodeAccessListInner(
chainId, nonce, gasPrice, gasLimit, to, value, payload, accessList, rlpOutput);
rlpOutput.endList();
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright Hyperledger Besu Contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.ethereum.core.encoding;

import org.hyperledger.besu.crypto.SignatureAlgorithm;
import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
import org.hyperledger.besu.datatypes.AccessListEntry;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.rlp.RLPInput;

import java.math.BigInteger;
import java.util.function.Supplier;

import com.google.common.base.Suppliers;

class AccessListTransactionDecoder {
private static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
Suppliers.memoize(SignatureAlgorithmFactory::getInstance);

public static Transaction decode(final RLPInput rlpInput) {
rlpInput.enterList();
final Transaction.Builder preSignatureTransactionBuilder =
Transaction.builder()
.type(TransactionType.ACCESS_LIST)
.chainId(BigInteger.valueOf(rlpInput.readLongScalar()))
.nonce(rlpInput.readLongScalar())
.gasPrice(Wei.of(rlpInput.readUInt256Scalar()))
.gasLimit(rlpInput.readLongScalar())
.to(
rlpInput.readBytes(
addressBytes -> addressBytes.size() == 0 ? null : Address.wrap(addressBytes)))
.value(Wei.of(rlpInput.readUInt256Scalar()))
.payload(rlpInput.readBytes())
.accessList(
rlpInput.readList(
accessListEntryRLPInput -> {
Gabriel-Trintinalia marked this conversation as resolved.
Show resolved Hide resolved
accessListEntryRLPInput.enterList();
final AccessListEntry accessListEntry =
new AccessListEntry(
Address.wrap(accessListEntryRLPInput.readBytes()),
accessListEntryRLPInput.readList(RLPInput::readBytes32));
accessListEntryRLPInput.leaveList();
return accessListEntry;
}));
final byte recId = (byte) rlpInput.readIntScalar();
final Transaction transaction =
preSignatureTransactionBuilder
.signature(
SIGNATURE_ALGORITHM
.get()
.createSignature(
rlpInput.readUInt256Scalar().toUnsignedBigInteger(),
rlpInput.readUInt256Scalar().toUnsignedBigInteger(),
recId))
.build();
rlpInput.leaveList();
return transaction;
}
}
Loading
Loading