Skip to content
/ besu Public
forked from hyperledger/besu

Commit

Permalink
Blob transaction replacement rule (hyperledger#6874)
Browse files Browse the repository at this point in the history
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: amsmota <antonio.mota@citi.com>
  • Loading branch information
fab-10 authored and amsmota committed Apr 16, 2024
1 parent 9feb430 commit f204715
Show file tree
Hide file tree
Showing 18 changed files with 221 additions and 84 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
- Experimental Snap Sync Server [#6640](https://github.com/hyperledger/besu/pull/6640)
- Upgrade Reference Tests to 13.2 [#6854](https://github.com/hyperledger/besu/pull/6854)
- Update Web3j dependencies [#6811](https://github.com/hyperledger/besu/pull/6811)
- Add `tx-pool-blob-price-bump` option to configure the price bump percentage required to replace blob transactions (by default 100%) [#6874](https://github.com/hyperledger/besu/pull/6874)

### Bug fixes
- Fix txpool dump/restore race condition [#6665](https://github.com/hyperledger/besu/pull/6665)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
private static final String TX_POOL_ENABLE_SAVE_RESTORE = "--tx-pool-enable-save-restore";
private static final String TX_POOL_SAVE_FILE = "--tx-pool-save-file";
private static final String TX_POOL_PRICE_BUMP = "--tx-pool-price-bump";
private static final String TX_POOL_BLOB_PRICE_BUMP = "--tx-pool-blob-price-bump";
private static final String RPC_TX_FEECAP = "--rpc-tx-feecap";
private static final String STRICT_TX_REPLAY_PROTECTION_ENABLED_FLAG =
"--strict-tx-replay-protection-enabled";
Expand Down Expand Up @@ -102,6 +103,15 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
arity = "1")
private Percentage priceBump = TransactionPoolConfiguration.DEFAULT_PRICE_BUMP;

@CommandLine.Option(
names = {TX_POOL_BLOB_PRICE_BUMP},
paramLabel = "<Percentage>",
converter = PercentageConverter.class,
description =
"Blob price bump percentage to replace an already existing transaction blob tx (default: ${DEFAULT-VALUE})",
arity = "1")
private Percentage blobPriceBump = TransactionPoolConfiguration.DEFAULT_BLOB_PRICE_BUMP;

@CommandLine.Option(
names = {RPC_TX_FEECAP},
description =
Expand Down Expand Up @@ -277,6 +287,7 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati
options.saveRestoreEnabled = config.getEnableSaveRestore();
options.noLocalPriority = config.getNoLocalPriority();
options.priceBump = config.getPriceBump();
options.blobPriceBump = config.getBlobPriceBump();
options.txFeeCap = config.getTxFeeCap();
options.saveFile = config.getSaveFile();
options.strictTxReplayProtectionEnabled = config.getStrictTransactionReplayProtectionEnabled();
Expand Down Expand Up @@ -334,6 +345,7 @@ public TransactionPoolConfiguration toDomainObject() {
.enableSaveRestore(saveRestoreEnabled)
.noLocalPriority(noLocalPriority)
.priceBump(priceBump)
.blobPriceBump(blobPriceBump)
.txFeeCap(txFeeCap)
.saveFile(saveFile)
.strictTransactionReplayProtectionEnabled(strictTxReplayProtectionEnabled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,31 @@ public void invalidPriceBumpShouldFail() {
"101");
}

@Test
public void blobPriceBump() {
final Percentage blobPriceBump = Percentage.fromInt(50);
internalTestSuccess(
config -> assertThat(config.getBlobPriceBump()).isEqualTo(blobPriceBump),
"--tx-pool-blob-price-bump",
blobPriceBump.toString());
}

@Test
public void invalidBlobPriceBumpShouldFail() {
internalTestFailure(
"Invalid value: 101, should be a number between 0 and 100 inclusive",
"--tx-pool-blob-price-bump",
"101");
}

@Test
public void defaultBlobPriceBump() {
internalTestSuccess(
config ->
assertThat(config.getBlobPriceBump())
.isEqualTo(TransactionPoolConfiguration.DEFAULT_BLOB_PRICE_BUMP));
}

@Test
public void txFeeCap() {
final Wei txFeeCap = Wei.fromEth(2);
Expand Down
1 change: 1 addition & 0 deletions besu/src/test/resources/everything_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ privacy-flexible-groups-enabled=false
# Transaction Pool
tx-pool="layered"
tx-pool-price-bump=13
tx-pool-blob-price-bump=100
rpc-tx-feecap=2000000000000000000
strict-tx-replay-protection-enabled=true
tx-pool-no-local-priority=false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ public abstract class AbstractIsolationTests {
ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(100).build();

protected final TransactionPoolReplacementHandler transactionReplacementHandler =
new TransactionPoolReplacementHandler(poolConfiguration.getPriceBump());
new TransactionPoolReplacementHandler(
poolConfiguration.getPriceBump(), poolConfiguration.getBlobPriceBump());

protected final BiFunction<PendingTransaction, PendingTransaction, Boolean>
transactionReplacementTester =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ enum Implementation {
int DEFAULT_TX_RETENTION_HOURS = 13;
boolean DEFAULT_STRICT_TX_REPLAY_PROTECTION_ENABLED = false;
Percentage DEFAULT_PRICE_BUMP = Percentage.fromInt(10);
Percentage DEFAULT_BLOB_PRICE_BUMP = Percentage.fromInt(100);
Wei DEFAULT_RPC_TX_FEE_CAP = Wei.fromEth(1);
boolean DEFAULT_NO_LOCAL_PRIORITY = false;
boolean DEFAULT_ENABLE_SAVE_RESTORE = false;
Expand Down Expand Up @@ -102,6 +103,11 @@ default Percentage getPriceBump() {
return DEFAULT_PRICE_BUMP;
}

@Value.Default
default Percentage getBlobPriceBump() {
return DEFAULT_BLOB_PRICE_BUMP;
}

@Value.Default
default Wei getTxFeeCap() {
return DEFAULT_RPC_TX_FEE_CAP;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,9 @@ private static PendingTransactions createLayeredPendingTransactions(
final MiningParameters miningParameters) {

final TransactionPoolReplacementHandler transactionReplacementHandler =
new TransactionPoolReplacementHandler(transactionPoolConfiguration.getPriceBump());
new TransactionPoolReplacementHandler(
transactionPoolConfiguration.getPriceBump(),
transactionPoolConfiguration.getBlobPriceBump());

final BiFunction<PendingTransaction, PendingTransaction, Boolean> transactionReplacementTester =
(t1, t2) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@
public class TransactionPoolReplacementHandler {
private final List<TransactionPoolReplacementRule> rules;

public TransactionPoolReplacementHandler(final Percentage priceBump) {
public TransactionPoolReplacementHandler(
final Percentage priceBump, final Percentage blobPriceBump) {
this(
asList(
new TransactionReplacementByGasPriceRule(priceBump),
new TransactionReplacementByFeeMarketRule(priceBump)));
new TransactionReplacementByFeeMarketRule(priceBump, blobPriceBump)));
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ public class TransactionReplacementByFeeMarketRule implements TransactionPoolRep
private static final TransactionPriceCalculator EIP1559_CALCULATOR =
TransactionPriceCalculator.eip1559();
private final Percentage priceBump;
private final Percentage blobPriceBump;

public TransactionReplacementByFeeMarketRule(final Percentage priceBump) {
public TransactionReplacementByFeeMarketRule(
final Percentage priceBump, final Percentage blobPriceBump) {
this.priceBump = priceBump;
this.blobPriceBump = blobPriceBump;
}

@Override
Expand All @@ -39,6 +42,16 @@ public boolean shouldReplace(
final PendingTransaction newPendingTransaction,
final Optional<Wei> maybeBaseFee) {

return validExecutionPriceReplacement(
existingPendingTransaction, newPendingTransaction, maybeBaseFee)
&& validBlobPriceReplacement(existingPendingTransaction, newPendingTransaction);
}

private boolean validExecutionPriceReplacement(
final PendingTransaction existingPendingTransaction,
final PendingTransaction newPendingTransaction,
final Optional<Wei> maybeBaseFee) {

// bail early if basefee is absent or neither transaction supports 1559 fee market
if (maybeBaseFee.isEmpty()
|| !(isNotGasPriced(existingPendingTransaction) || isNotGasPriced(newPendingTransaction))) {
Expand All @@ -65,6 +78,37 @@ public boolean shouldReplace(
return false;
}

private boolean validBlobPriceReplacement(
final PendingTransaction existingPendingTransaction,
final PendingTransaction newPendingTransaction) {

final var existingType = existingPendingTransaction.getTransaction().getType();
final var newType = newPendingTransaction.getTransaction().getType();

if (existingType.supportsBlob() || newType.supportsBlob()) {
if (existingType.supportsBlob() && newType.supportsBlob()) {
final Wei replacementThreshold =
existingPendingTransaction
.getTransaction()
.getMaxFeePerBlobGas()
.orElseThrow()
.multiply(100 + blobPriceBump.getValue())
.divide(100);
return newPendingTransaction
.getTransaction()
.getMaxFeePerBlobGas()
.orElseThrow()
.compareTo(replacementThreshold)
>= 0;
}
// blob tx can only replace and be replaced by blob tx
return false;
}

// in case no blob tx, then we are fine
return true;
}

private Wei priceOf(final Transaction transaction, final Optional<Wei> maybeBaseFee) {
final TransactionPriceCalculator transactionPriceCalculator =
transaction.getType().supports1559FeeMarket() ? EIP1559_CALCULATOR : FRONTIER_CALCULATOR;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ public AbstractPendingTransactionsSorter(
this.clock = clock;
this.chainHeadHeaderSupplier = chainHeadHeaderSupplier;
this.transactionReplacementHandler =
new TransactionPoolReplacementHandler(poolConfig.getPriceBump());
new TransactionPoolReplacementHandler(
poolConfig.getPriceBump(), poolConfig.getBlobPriceBump());
final LabelledMetric<Counter> transactionAddedCounter =
metricsSystem.createLabelledCounter(
BesuMetricCategory.TRANSACTION_POOL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ private TransactionPool createTransactionPool(
final TransactionPoolConfiguration poolConfig = configBuilder.build();

final TransactionPoolReplacementHandler transactionReplacementHandler =
new TransactionPoolReplacementHandler(poolConfig.getPriceBump());
new TransactionPoolReplacementHandler(
poolConfig.getPriceBump(), poolConfig.getBlobPriceBump());

final BiFunction<PendingTransaction, PendingTransaction, Boolean> transactionReplacementTester =
(t1, t2) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import static org.mockito.Mockito.when;

import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.datatypes.VersionedHash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.Transaction;

import java.math.BigInteger;
import java.util.List;

public class AbstractTransactionReplacementTest {
protected static PendingTransaction frontierTx(final long price) {
Expand Down Expand Up @@ -50,4 +52,20 @@ protected static PendingTransaction eip1559Tx(
when(pendingTransaction.getTransaction()).thenReturn(transaction);
return pendingTransaction;
}

protected static PendingTransaction blobTx(
final long maxPriorityFeePerGas, final long maxFeePerGas, final long maxFeePerBlobGas) {
final PendingTransaction pendingTransaction = mock(PendingTransaction.class);
final Transaction transaction =
Transaction.builder()
.chainId(BigInteger.ZERO)
.type(TransactionType.BLOB)
.maxPriorityFeePerGas(Wei.of(maxPriorityFeePerGas))
.maxFeePerGas(Wei.of(maxFeePerGas))
.maxFeePerBlobGas(Wei.of(maxFeePerBlobGas))
.versionedHashes(List.of(VersionedHash.DEFAULT_VERSIONED_HASH))
.build();
when(pendingTransaction.getTransaction()).thenReturn(transaction);
return pendingTransaction;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,52 +34,60 @@ public static Collection<Object[]> data() {
new Object[][] {

// basefee absent
{frontierTx(5L), frontierTx(6L), empty(), 0, false},
{frontierTx(5L), frontierTx(5L), empty(), 0, false},
{frontierTx(5L), frontierTx(4L), empty(), 0, false},
{frontierTx(100L), frontierTx(105L), empty(), 10, false},
{frontierTx(100L), frontierTx(110L), empty(), 10, false},
{frontierTx(100L), frontierTx(111L), empty(), 10, false},
{frontierTx(5L), frontierTx(6L), empty(), 0, 100, false},
{frontierTx(5L), frontierTx(5L), empty(), 0, 100, false},
{frontierTx(5L), frontierTx(4L), empty(), 0, 100, false},
{frontierTx(100L), frontierTx(105L), empty(), 10, 100, false},
{frontierTx(100L), frontierTx(110L), empty(), 10, 100, false},
{frontierTx(100L), frontierTx(111L), empty(), 10, 100, false},
// basefee present
{frontierTx(5L), frontierTx(6L), Optional.of(Wei.of(3L)), 0, false},
{frontierTx(5L), frontierTx(5L), Optional.of(Wei.of(3L)), 0, false},
{frontierTx(5L), frontierTx(4L), Optional.of(Wei.of(3L)), 0, false},
{frontierTx(100L), frontierTx(105L), Optional.of(Wei.of(3L)), 10, false},
{frontierTx(100L), frontierTx(110L), Optional.of(Wei.of(3L)), 10, false},
{frontierTx(100L), frontierTx(111L), Optional.of(Wei.of(3L)), 10, false},
{frontierTx(5L), frontierTx(6L), Optional.of(Wei.of(3L)), 0, 100, false},
{frontierTx(5L), frontierTx(5L), Optional.of(Wei.of(3L)), 0, 100, false},
{frontierTx(5L), frontierTx(4L), Optional.of(Wei.of(3L)), 0, 100, false},
{frontierTx(100L), frontierTx(105L), Optional.of(Wei.of(3L)), 10, 100, false},
{frontierTx(100L), frontierTx(110L), Optional.of(Wei.of(3L)), 10, 100, false},
{frontierTx(100L), frontierTx(111L), Optional.of(Wei.of(3L)), 10, 100, false},
// eip1559 replacing frontier
{frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(Wei.of(1L)), 0, false},
{frontierTx(5L), eip1559Tx(3L, 5L), Optional.of(Wei.of(3L)), 0, true},
{frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(Wei.of(3L)), 0, true},
{frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(Wei.of(1L)), 0, 100, false},
{frontierTx(5L), eip1559Tx(3L, 5L), Optional.of(Wei.of(3L)), 0, 100, true},
{frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(Wei.of(3L)), 0, 100, true},
// frontier replacing 1559
{eip1559Tx(3L, 8L), frontierTx(7L), Optional.of(Wei.of(4L)), 0, true},
{eip1559Tx(3L, 8L), frontierTx(7L), Optional.of(Wei.of(5L)), 0, false},
{eip1559Tx(3L, 8L), frontierTx(8L), Optional.of(Wei.of(4L)), 0, true},
{eip1559Tx(3L, 8L), frontierTx(7L), Optional.of(Wei.of(4L)), 0, 100, true},
{eip1559Tx(3L, 8L), frontierTx(7L), Optional.of(Wei.of(5L)), 0, 100, false},
{eip1559Tx(3L, 8L), frontierTx(8L), Optional.of(Wei.of(4L)), 0, 100, true},
// eip1559 replacing eip1559
{eip1559Tx(3L, 6L), eip1559Tx(3L, 6L), Optional.of(Wei.of(3L)), 0, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(3L)), 0, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(4L)), 0, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(5L)), 0, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(6L)), 0, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(7L)), 0, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(8L)), 0, true},
{eip1559Tx(10L, 200L), eip1559Tx(10L, 200L), Optional.of(Wei.of(90L)), 10, false},
{eip1559Tx(10L, 200L), eip1559Tx(15L, 200L), Optional.of(Wei.of(90L)), 10, false},
{eip1559Tx(10L, 200L), eip1559Tx(20L, 200L), Optional.of(Wei.of(90L)), 10, true},
{eip1559Tx(10L, 200L), eip1559Tx(20L, 200L), Optional.of(Wei.of(200L)), 10, true},
{eip1559Tx(10L, 200L), eip1559Tx(10L, 220L), Optional.of(Wei.of(220L)), 10, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 6L), Optional.of(Wei.of(3L)), 0, 100, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(3L)), 0, 100, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(4L)), 0, 100, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(5L)), 0, 100, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(6L)), 0, 100, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(7L)), 0, 100, true},
{eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(8L)), 0, 100, true},
{eip1559Tx(10L, 200L), eip1559Tx(10L, 200L), Optional.of(Wei.of(90L)), 10, 100, false},
{eip1559Tx(10L, 200L), eip1559Tx(15L, 200L), Optional.of(Wei.of(90L)), 10, 100, false},
{eip1559Tx(10L, 200L), eip1559Tx(20L, 200L), Optional.of(Wei.of(90L)), 10, 100, true},
{eip1559Tx(10L, 200L), eip1559Tx(20L, 200L), Optional.of(Wei.of(200L)), 10, 100, true},
{eip1559Tx(10L, 200L), eip1559Tx(10L, 220L), Optional.of(Wei.of(220L)), 10, 100, true},
// pathological, priority fee > max fee
{eip1559Tx(8L, 6L), eip1559Tx(3L, 6L), Optional.of(Wei.of(2L)), 0, false},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(3L)), 0, true},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(4L)), 0, true},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 6L), Optional.of(Wei.of(2L)), 0, 100, false},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(3L)), 0, 100, true},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.of(Wei.of(4L)), 0, 100, true},
// pathological, eip1559 without basefee
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.empty(), 0, false},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.empty(), 0, false},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.empty(), 0, 100, false},
{eip1559Tx(8L, 6L), eip1559Tx(3L, 7L), Optional.empty(), 0, 100, false},
// zero base fee market
{frontierTx(0L), frontierTx(0L), Optional.of(Wei.ZERO), 0, false},
{eip1559Tx(0L, 0L), frontierTx(0L), Optional.of(Wei.ZERO), 0, true},
{frontierTx(0L), eip1559Tx(0L, 0L), Optional.of(Wei.ZERO), 0, true},
{eip1559Tx(0L, 0L), eip1559Tx(0L, 0L), Optional.of(Wei.ZERO), 0, true},
{frontierTx(0L), frontierTx(0L), Optional.of(Wei.ZERO), 0, 100, false},
{eip1559Tx(0L, 0L), frontierTx(0L), Optional.of(Wei.ZERO), 0, 100, true},
{frontierTx(0L), eip1559Tx(0L, 0L), Optional.of(Wei.ZERO), 0, 100, true},
{eip1559Tx(0L, 0L), eip1559Tx(0L, 0L), Optional.of(Wei.ZERO), 0, 100, true},
// blob tx
{blobTx(10L, 200L, 1L), blobTx(20L, 220L, 1L), Optional.of(Wei.of(90L)), 10, 0, true},
{blobTx(10L, 200L, 1L), blobTx(20L, 220L, 2L), Optional.of(Wei.of(90L)), 10, 100, true},
{blobTx(10L, 200L, 1L), blobTx(20L, 220L, 1L), Optional.of(Wei.of(90L)), 10, 100, false},
{blobTx(10L, 200L, 2L), blobTx(20L, 220L, 3L), Optional.of(Wei.of(90L)), 10, 100, false},
// // blob tx must be replaced by blob tx only
{blobTx(3L, 6L, 1L), eip1559Tx(3L, 6L), Optional.of(Wei.of(3L)), 0, 0, false},
{eip1559Tx(3L, 6L), blobTx(3L, 6L, 1L), Optional.of(Wei.of(3L)), 0, 0, false},
});
}

Expand All @@ -90,10 +98,12 @@ public void shouldReplace(
final PendingTransaction newTx,
final Optional<Wei> baseFee,
final int priceBump,
final int blobPriceBump,
final boolean expected) {

assertThat(
new TransactionReplacementByFeeMarketRule(Percentage.fromInt(priceBump))
new TransactionReplacementByFeeMarketRule(
Percentage.fromInt(priceBump), Percentage.fromInt(blobPriceBump))
.shouldReplace(oldTx, newTx, baseFee))
.isEqualTo(expected);
}
Expand Down
Loading

0 comments on commit f204715

Please sign in to comment.