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

Add transaction selector based on min priority fee parameter #6083

2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
- Accept `input` and `data` field for the payload of transaction-related RPC methods [#6094](https://github.com/hyperledger/besu/pull/6094)
- Add APIs to set and get the min gas price a transaction must pay for being selected during block creation [#6097](https://github.com/hyperledger/besu/pull/6097)
- TraceService: return results for transactions in block [#6086](https://github.com/hyperledger/besu/pull/6086)
- New option `--min-priority-fee` that sets the minimum priority fee a transaction must meet to be selected for a block. [#6080](https://github.com/hyperledger/besu/pull/6080) [#6083](https://github.com/hyperledger/besu/pull/6083)
- Implement new `miner_setMinPriorityFee` and `miner_getMinPriorityFee` RPC methods [#6080](https://github.com/hyperledger/besu/pull/6080)
Copy link
Contributor

Choose a reason for hiding this comment

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

are these really related to mining?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they are related to block creation. I think it is the same case of this: #6027 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

the rename from miner to block creation is on the todo list, but it is not trivial and should be done with care, it will introduce a lot of breaking changes and need for external tools to update.
I suggest that for the moment we keep it as is, and we consider miner a synonym of block creation


### Bug fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.AllAcceptingTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.BlobPriceTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.BlockSizeTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.MinPriorityFeePerGasTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.PriceTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.ProcessingResultTransactionSelector;
import org.hyperledger.besu.ethereum.chain.Blockchain;
Expand Down Expand Up @@ -132,6 +133,7 @@ private List<AbstractTransactionSelector> createTransactionSelectors(
new BlockSizeTransactionSelector(context),
new PriceTransactionSelector(context),
new BlobPriceTransactionSelector(context),
new MinPriorityFeePerGasTransactionSelector(context),
new ProcessingResultTransactionSelector(context));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* 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.blockcreation.txselection.selectors;

import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.blockcreation.txselection.BlockSelectionContext;
import org.hyperledger.besu.ethereum.blockcreation.txselection.TransactionSelectionResults;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction;
import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
import org.hyperledger.besu.plugin.data.TransactionSelectionResult;

/** This class is responsible for selecting transactions based on the minimum priority fee. */
public class MinPriorityFeePerGasTransactionSelector extends AbstractTransactionSelector {

/**
* Constructor for MinPriorityFeeSelector.
*
* @param context The context of block selection.
*/
public MinPriorityFeePerGasTransactionSelector(final BlockSelectionContext context) {
super(context);
}

/**
* Evaluates a transaction before processing.
*
* @param pendingTransaction The transaction to be evaluated.
* @param transactionSelectionResults The results of other transaction evaluations in the same
* block.
* @return TransactionSelectionResult. If the priority fee is below the minimum, it returns an
* invalid transient result. Otherwise, it returns a selected result.
*/
@Override
public TransactionSelectionResult evaluateTransactionPreProcessing(
final PendingTransaction pendingTransaction,
final TransactionSelectionResults transactionSelectionResults) {
if (isPriorityFeePriceBelowMinimum(pendingTransaction)) {
return TransactionSelectionResult.PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN;
}
return TransactionSelectionResult.SELECTED;
}

/**
* Checks if the priority fee price is below the minimum.
*
* @param pendingTransaction The transaction to check.
* @return boolean. Returns true if the minimum priority fee price is below the minimum, false
* otherwise.
*/
private boolean isPriorityFeePriceBelowMinimum(final PendingTransaction pendingTransaction) {
// Priority txs are exempt from this check
if (pendingTransaction.hasPriority()) {
return false;
}
Wei priorityFeePerGas =
pendingTransaction
.getTransaction()
.getEffectivePriorityFeePerGas(context.processableBlockHeader().getBaseFee());
return priorityFeePerGas.lessThan(context.miningParameters().getMinPriorityFeePerGas());
}

/**
* No evaluation is performed post-processing.
*
* @param pendingTransaction The processed transaction.
* @param processingResult The result of the transaction processing.
* @return Always returns SELECTED.
*/
@Override
public TransactionSelectionResult evaluateTransactionPostProcessing(
final PendingTransaction pendingTransaction,
final TransactionSelectionResults blockTransactionResults,
final TransactionProcessingResult processingResult) {
return TransactionSelectionResult.SELECTED;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,34 @@ public void decreaseOfMinGasPriceAtRuntimeIncludeTxThatWasPreviouslyNotSelected(
assertThat(results2.getNotSelectedTransactions()).isEmpty();
}

@Test
public void shouldNotSelectTransactionsWithPriorityFeeLessThanConfig() {
ProcessableBlockHeader blockHeader = createBlock(5_000_000, Wei.ONE);
miningParameters.setMinPriorityFeePerGas(Wei.of(7));
final Transaction tx1 = createTransaction(1, Wei.of(8), 100_000);
ensureTransactionIsValid(tx1);
// transaction tx2 should not be selected
final Transaction tx2 = createTransaction(2, Wei.of(7), 100_000);
Gabriel-Trintinalia marked this conversation as resolved.
Show resolved Hide resolved
ensureTransactionIsValid(tx2);
transactionPool.addRemoteTransactions(List.of(tx1, tx2));

final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor,
blockHeader,
Wei.ZERO,
AddressHelpers.ofValue(1),
Wei.ZERO,
MIN_OCCUPANCY_100_PERCENT);

final TransactionSelectionResults results = selector.buildTransactionListForBlock();

assertThat(results.getSelectedTransactions()).containsOnly(tx1);
assertThat(results.getNotSelectedTransactions())
.containsOnly(
entry(tx2, TransactionSelectionResult.PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN));
}

protected BlockTransactionSelector createBlockSelector(
final MainnetTransactionProcessor transactionProcessor,
final ProcessableBlockHeader blockHeader,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,47 @@ public void transactionFromSameSenderWithMixedTypes() {
.containsExactly(txFrontier1, txLondon1, txFrontier2, txLondon2);
assertThat(results.getNotSelectedTransactions()).isEmpty();
}

@Test
@Override
public void shouldNotSelectTransactionsWithPriorityFeeLessThanConfig() {
ProcessableBlockHeader blockHeader = createBlock(5_000_000, Wei.ONE);
miningParameters.setMinPriorityFeePerGas(Wei.of(7));

final Transaction tx1 = createEIP1559Transaction(1, Wei.of(8), Wei.of(8), 100_000);
ensureTransactionIsValid(tx1);

// transaction tx2 should not be selected
final Transaction tx2 = createEIP1559Transaction(2, Wei.of(7), Wei.of(7), 100_000);
ensureTransactionIsValid(tx2);

// transaction tx3 should be selected
final Transaction tx3 = createEIP1559Transaction(3, Wei.of(8), Wei.of(8), 100_000);
ensureTransactionIsValid(tx3);

// transaction tx4 should not be selected
final Transaction tx4 = createEIP1559Transaction(4, Wei.of(8), Wei.of(6), 100_000);
ensureTransactionIsValid(tx4);

transactionPool.addRemoteTransactions(List.of(tx1, tx2, tx3, tx4));

assertThat(transactionPool.getPendingTransactions().size()).isEqualTo(4);

final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor,
blockHeader,
Wei.ZERO,
AddressHelpers.ofValue(1),
Wei.ZERO,
MIN_OCCUPANCY_100_PERCENT);

final TransactionSelectionResults results = selector.buildTransactionListForBlock();

assertThat(results.getSelectedTransactions()).containsOnly(tx1, tx3);
assertThat(results.getNotSelectedTransactions())
.containsOnly(
entry(tx2, TransactionSelectionResult.PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN),
entry(tx4, TransactionSelectionResult.PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* 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.blockcreation;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.blockcreation.txselection.BlockSelectionContext;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.AbstractTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.MinPriorityFeePerGasTransactionSelector;
import org.hyperledger.besu.ethereum.core.MiningParameters;
import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction;
import org.hyperledger.besu.plugin.data.TransactionSelectionResult;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class MinPriorityFeePerGasTransactionSelectorTest {
private AbstractTransactionSelector transactionSelector;

private final int minPriorityFeeParameter = 7;

@BeforeEach
public void initialize() {
MiningParameters miningParameters =
MiningParameters.newDefault().setMinPriorityFeePerGas(Wei.of(minPriorityFeeParameter));
BlockSelectionContext context =
new BlockSelectionContext(
miningParameters,
null,
null,
mock(ProcessableBlockHeader.class),
null,
null,
null,
null);
transactionSelector = new MinPriorityFeePerGasTransactionSelector(context);
}

@Test
public void shouldNotSelectWhen_PriorityFeePerGas_IsLessThan_MinPriorityFeePerGas() {
Copy link
Contributor

Choose a reason for hiding this comment

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

helpful test names!

var transaction = mockTransactionWithPriorityFee(minPriorityFeeParameter - 1);
assertSelection(transaction, TransactionSelectionResult.PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN);
}

@Test
public void shouldSelectWhen_PriorityFeePerGas_IsEqual_MinPriorityFeePerGas() {
var transaction = mockTransactionWithPriorityFee(minPriorityFeeParameter);
assertSelection(transaction, TransactionSelectionResult.SELECTED);
}

@Test
public void shouldSelectWhen_PriorityFeePerGas_IsGreaterThan_MinPriorityFeePerGas() {
var transaction = mockTransactionWithPriorityFee(minPriorityFeeParameter + 1);
assertSelection(transaction, TransactionSelectionResult.SELECTED);
}

@Test
public void shouldSelectWhenPrioritySender() {
var prioritySenderTransaction = mockTransactionWithPriorityFee(minPriorityFeeParameter - 1);
assertSelection(
prioritySenderTransaction,
TransactionSelectionResult.PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN);
when(prioritySenderTransaction.hasPriority()).thenReturn(true);
assertSelection(prioritySenderTransaction, TransactionSelectionResult.SELECTED);
}

private void assertSelection(
Gabriel-Trintinalia marked this conversation as resolved.
Show resolved Hide resolved
final PendingTransaction transaction, final TransactionSelectionResult expectedResult) {
var actualResult = transactionSelector.evaluateTransactionPreProcessing(transaction, null);
assertThat(actualResult).isEqualTo(expectedResult);
}

private PendingTransaction mockTransactionWithPriorityFee(final int priorityFeePerGas) {
PendingTransaction mockTransaction = mock(PendingTransaction.class);
Transaction transaction = mock(Transaction.class);
when(mockTransaction.getTransaction()).thenReturn(transaction);
when(transaction.getEffectivePriorityFeePerGas(any())).thenReturn(Wei.of(priorityFeePerGas));
return mockTransaction;
}
}
2 changes: 1 addition & 1 deletion plugin-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Calculated : ${currentHash}
tasks.register('checkAPIChanges', FileStateChecker) {
description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
files = sourceSets.main.allJava.files
knownHash = 'ZXBvp7wuHQ8j4Gty2zg/gKdzgrOXSpehYukMuH98W/Y='
knownHash = 'kyCYfllc1IcisRZIYuLxhC+0+POCzcMQPhE8F8mx1Ns='
}
check.dependsOn('checkAPIChanges')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,19 @@ public String toString() {
public static final TransactionSelectionResult CURRENT_TX_PRICE_BELOW_MIN =
TransactionSelectionResult.invalidTransient("CURRENT_TX_PRICE_BELOW_MIN");
/**
* The transaction has not been selected since its data price is below the current network data
* The transaction has not been selected since its blob price is below the current network blob
* price, but the selection should continue.
*/
public static final TransactionSelectionResult BLOB_PRICE_BELOW_CURRENT_MIN =
TransactionSelectionResult.invalidTransient("BLOB_PRICE_BELOW_CURRENT_MIN");

/**
* The transaction has not been selected since its priority fee is below the configured min
* priority fee per gas, but the selection should continue.
*/
public static final TransactionSelectionResult PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN =
TransactionSelectionResult.invalidTransient("PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN");

private final Status status;
private final Optional<String> maybeInvalidReason;

Expand Down
Loading