Skip to content

Commit

Permalink
test coverage and fix for subtle re-org bug prior to proposals
Browse files Browse the repository at this point in the history
Signed-off-by: Justin Florentine <justin+github@florentine.us>
  • Loading branch information
jflo committed Oct 4, 2023
1 parent a01919a commit 5a6af50
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,18 @@ public void test() throws IOException {
testsContext.mapper.readValue(testCaseFileURI.toURL(), JsonRpcTestCase.class);

final String rpcMethod = String.valueOf(testCase.getRequest().get("method"));

OkHttpClient client = testsContext.httpClient;
if (System.getenv("BESU_DEBUG_CHILD_PROCESS_PORT") != null) {
// if running in debug mode, set a longer timeout
client =
testsContext
.httpClient
.newBuilder()
.readTimeout(900, java.util.concurrent.TimeUnit.SECONDS)
.build();
}
final Call testRequest =
testsContext.httpClient.newCall(
client.newCall(
new Request.Builder()
.url(getRpcUrl(rpcMethod))
.post(RequestBody.create(testCase.getRequest().toString(), MEDIA_TYPE_JSON))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public class ExecutionEngineCancunBlockBulidingAcceptanceTest extends AbstractJs

private static JsonRpcTestsContext testsContext;

public ExecutionEngineCancunBlockBulidingAcceptanceTest(final String ignored, final URI testCaseFileURI) {
public ExecutionEngineCancunBlockBulidingAcceptanceTest(
final String ignored, final URI testCaseFileURI) {
super(ignored, testsContext, testCaseFileURI);
}

Expand Down Expand Up @@ -77,6 +78,8 @@ protected void evaluateResponse(
final ObjectNode blobsBundle = (ObjectNode) result.get("blobsBundle");
assertThat(execPayload.get("transactions").getNodeType()).isEqualTo(JsonNodeType.ARRAY);
final ArrayNode transactions = (ArrayNode) execPayload.get("transactions");
// actually, you need to decode the transactions and count how many unique
// versioned hashes are referenced amongst them.
assertThat(blobsBundle.get("commitments").getNodeType()).isEqualTo(JsonNodeType.ARRAY);
final ArrayNode commitments = (ArrayNode) blobsBundle.get("commitments");
assertThat(blobsBundle.get("blobs").getNodeType()).isEqualTo(JsonNodeType.ARRAY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@
import com.fasterxml.jackson.annotation.JsonGetter;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import org.apache.tuweni.bytes.Bytes;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@JsonPropertyOrder({"commitments", "proofs", "blobs"})
public class BlobsBundleV1 {

private static final Logger LOG = LoggerFactory.getLogger(BlobsBundleV1.class);
private final List<String> commitments;

private final List<String> proofs;
Expand Down Expand Up @@ -67,6 +70,14 @@ public BlobsBundleV1(final List<Transaction> transactions) {
.map(Blob::getData)
.map(Bytes::toString)
.collect(Collectors.toList());

LOG.debug(
"BlobsBundleV1: totalTxs: {}, blobTxs: {}, commitments: {}, proofs: {}, blobs: {}",
transactions.size(),
blobsWithCommitments.size(),
commitments.size(),
proofs.size(),
blobs.size());
}

public BlobsBundleV1(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ protected BlockHeader createBlockHeader(
.baseFeePerGas(Wei.ONE)
.timestamp(super.experimentalHardfork.milestone())
.excessBlobGas(BlobGas.ZERO)
.blobGasUsed(100L)
.blobGasUsed(0L)
.buildHeader();

BlockHeader mockHeader =
Expand All @@ -185,7 +185,7 @@ protected BlockHeader createBlockHeader(
.timestamp(parentBlockHeader.getTimestamp() + 1)
.withdrawalsRoot(maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null))
.excessBlobGas(BlobGas.ZERO)
.blobGasUsed(100L)
.blobGasUsed(0L)
.depositsRoot(maybeDeposits.map(BodyValidation::depositsRoot).orElse(null))
.parentBeaconBlockRoot(
maybeParentBeaconBlockRoot.isPresent() ? maybeParentBeaconBlockRoot : null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ private static Stream<Arguments> provideOpaqueBytesNoBlobsWithCommitments() {
createArgument(
"0x03f89d850120b996ed81f1843b9aca00847735940e8307a12094000000000000000000000000000000000010101001855f495f4955c0847735940ee1a001d552e24560ec2f168be1d4a6385df61c70afe4288f00a3ad172da1a6f2b4f280a0b6690786e5fe79df67dcb60e8a9e8555142c3c96ffd5097c838717f0a7f64129a0112f01ed0cd3b86495f01736fbbc1b793f71565223aa26f093471a4d8605d198"),
createArgument(
"0x03f897850120b996ed80840bebc200843b9aca078303345094c8d369b164361a8961286cfbab3bc10f962185a88080c08411e1a300e1a0011df88a2971c8a7ac494a7ba37ec1acaa1fc1edeeb38c839b5d1693d47b69b080a032f122f06e5802224db4c8a58fd22c75173a713f63f89936f811c144b9e40129a043a2a872cbfa5727007adf6a48febe5f190d2e4cd5ed6122823fb6ff47ecda32"));
"0x03f897850120b996ed80840bebc200843b9aca078303345094c8d369b164361a8961286cfbab3bc10f962185a88080c08411e1a300e1a0011df88a2971c8a7ac494a7ba37ec1acaa1fc1edeeb38c839b5d1693d47b69b080a032f122f06e5802224db4c8a58fd22c75173a713f63f89936f811c144b9e40129a043a2a872cbfa5727007adf6a48febe5f190d2e4cd5ed6122823fb6ff47ecda32"),
createArgument(
"0x03f8928501a1f0ff4313843b9aca00843b9aca0082520894e7249813d8ccf6fa95a2203f46a64166073d58878080c001e1a00134a7258134a61a4f36f876480b75a12ec5c9fd5bcf8a27c42f78ffd6149eec01a0da6b8722b5df41d2458fc4486c85e1ac936e8437f2c4001bcde73b7352b4c830a017412017e67474a9d75edf392d7ced91a2bf11358215150b69b62cb8e0d01871"));
}

private static Stream<Arguments> provideOpaqueBytesForNetwork() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,10 @@ && strictReplayProtectionShouldBeEnforcedLocally(chainHeadBlockHeader)
return ValidationResultAndAccount.invalid(
TransactionInvalidReason.INVALID_TRANSACTION_FORMAT,
"EIP-1559 transaction are not allowed yet");
} else if (transaction.getType().equals(TransactionType.BLOB)
&& transaction.getBlobsWithCommitments().isEmpty()) {
return ValidationResultAndAccount.invalid(
TransactionInvalidReason.INVALID_BLOBS, "Blob transaction must have at least one blob");
}

// Call the transaction validator plugin if one is available
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
import org.hyperledger.besu.ethereum.core.BlobTestFixture;
import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockBody;
import org.hyperledger.besu.ethereum.core.BlockHeader;
Expand Down Expand Up @@ -144,6 +145,7 @@ public abstract class AbstractTransactionPoolTest {
protected PendingTransactions transactions;
protected final Transaction transaction0 = createTransaction(0);
protected final Transaction transaction1 = createTransaction(1);
protected final Transaction transactionBlob = createBlobTransaction(0);

protected final Transaction transactionOtherSender = createTransaction(1, KEY_PAIR2);
private ExecutionContextTestFixture executionContext;
Expand Down Expand Up @@ -454,6 +456,40 @@ public void shouldNotReAddTransactionsThatAreInBothForksWhenReorgHappens() {
assertTransactionPending(transaction1);
}

@Test
public void shouldNotReAddBlobTxsWhenReorgHappens() {
givenTransactionIsValid(transaction0);
givenTransactionIsValid(transaction1);
givenTransactionIsValid(transactionBlob);

addAndAssertRemoteTransactionValid(transaction0);
addAndAssertRemoteTransactionValid(transaction1);
addAndAssertRemoteTransactionInvalid(transactionBlob);

final BlockHeader commonParent = getHeaderForCurrentChainHead();
final Block originalFork1 = appendBlock(Difficulty.of(1000), commonParent, transaction0);
final Block originalFork2 =
appendBlock(Difficulty.of(10), originalFork1.getHeader(), transaction1);
final Block originalFork3 =
appendBlock(Difficulty.of(1), originalFork2.getHeader(), transactionBlob);
assertTransactionNotPending(transaction0);
assertTransactionNotPending(transaction1);
assertTransactionNotPending(transactionBlob);

final Block reorgFork1 = appendBlock(Difficulty.ONE, commonParent);
verifyChainHeadIs(originalFork3);

final Block reorgFork2 = appendBlock(Difficulty.of(2000), reorgFork1.getHeader());
verifyChainHeadIs(reorgFork2);

final Block reorgFork3 = appendBlock(Difficulty.of(3000), reorgFork2.getHeader());
verifyChainHeadIs(reorgFork3);

assertTransactionNotPending(transactionBlob);
assertTransactionPending(transaction0);
assertTransactionPending(transaction1);
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void addLocalTransaction_strictReplayProtectionOn_txWithChainId_chainIdIsConfigured(
Expand Down Expand Up @@ -1189,6 +1225,18 @@ protected Transaction createFrontierTransaction(final int transactionNumber, fin
.createTransaction(KEY_PAIR1);
}

protected Transaction createBlobTransaction(final int nonce) {
return new TransactionTestFixture()
.nonce(nonce)
.gasLimit(blockGasLimit)
.gasPrice(null)
.maxFeePerGas(Optional.of(Wei.of(5000L)))
.maxPriorityFeePerGas(Optional.of(Wei.of(1000L)))
.type(TransactionType.BLOB)
.blobsWithCommitments(Optional.of(new BlobTestFixture().createBlobsWithCommitments(1)))
.createTransaction(KEY_PAIR1);
}

protected int add1559TxAndGetPendingTxsCount(
final Wei genesisBaseFee,
final Wei minGasPrice,
Expand Down

0 comments on commit 5a6af50

Please sign in to comment.