Skip to content

Commit

Permalink
Plugin Api - Add evaluateTransactionPostProcessing to TransactionSele…
Browse files Browse the repository at this point in the history
…ctor interface (hyperledger#5988)

Signed-off-by: Gabriel-Trintinalia <[email protected]>
  • Loading branch information
Gabriel-Trintinalia authored and NickSneo committed Nov 12, 2023
1 parent ab7633f commit fe17410
Show file tree
Hide file tree
Showing 7 changed files with 224 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import java.util.Optional;
import java.util.concurrent.CancellationException;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -119,8 +118,7 @@ public BlockTransactionSelector(
transactionPool);
transactionSelectors = createTransactionSelectors(blockSelectionContext);
externalTransactionSelectors =
createExternalTransactionSelectors(
transactionSelectorFactory.map(List::of).orElseGet(List::of));
transactionSelectorFactory.map(TransactionSelectorFactory::create).orElse(List.of());
}

/**
Expand Down Expand Up @@ -304,8 +302,15 @@ private TransactionSelectionResult evaluateTransactionPostProcessing(
}
}

// TODO: External selectors are not used here because TransactionProcessingResult is not
// exposed to the Plugin API yet.
// Process the transaction through external selectors
for (var selector : externalTransactionSelectors) {
TransactionSelectionResult result =
selector.evaluateTransactionPostProcessing(pendingTransaction, processingResult);
// If the transaction is not selected by any external selector, return the result
if (!result.equals(TransactionSelectionResult.SELECTED)) {
return result;
}
}

// If the transaction is selected by all selectors, return SELECTED
return TransactionSelectionResult.SELECTED;
Expand All @@ -319,11 +324,4 @@ private List<AbstractTransactionSelector> createTransactionSelectors(
new BlobPriceTransactionSelector(context),
new ProcessingResultTransactionSelector(context));
}

private List<TransactionSelector> createExternalTransactionSelectors(
final List<TransactionSelectorFactory> transactionSelectorFactory) {
return transactionSelectorFactory.stream()
.map(TransactionSelectorFactory::create)
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.PendingTransaction;
import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.GasLimitCalculator;
Expand Down Expand Up @@ -66,6 +67,7 @@
import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
import org.hyperledger.besu.plugin.data.TransactionSelectionResult;
import org.hyperledger.besu.plugin.services.MetricsSystem;
import org.hyperledger.besu.plugin.services.txselection.TransactionSelector;
import org.hyperledger.besu.plugin.services.txselection.TransactionSelectorFactory;
import org.hyperledger.besu.services.kvstore.InMemoryKeyValueStorage;

Expand Down Expand Up @@ -538,7 +540,7 @@ public void shouldDiscardTransactionsThatFailValidation() {
}

@Test
public void transactionSelectionPluginShouldWork() {
public void transactionSelectionPluginShouldWork_PreProcessing() {
final ProcessableBlockHeader blockHeader = createBlock(300_000);

final Transaction selected = createTransaction(0, Wei.of(10), 21_000);
Expand All @@ -552,13 +554,26 @@ public void transactionSelectionPluginShouldWork() {

final TransactionSelectorFactory transactionSelectorFactory =
() ->
pendingTx -> {
if (pendingTx.getTransaction().equals(notSelectedTransient))
return TransactionSelectionResult.invalidTransient("transient");
if (pendingTx.getTransaction().equals(notSelectedInvalid))
return TransactionSelectionResult.invalid("invalid");
return TransactionSelectionResult.SELECTED;
};
List.of(
new TransactionSelector() {
@Override
public TransactionSelectionResult evaluateTransactionPreProcessing(
final PendingTransaction pendingTransaction) {
if (pendingTransaction.getTransaction().equals(notSelectedTransient))
return TransactionSelectionResult.invalidTransient("transient");
if (pendingTransaction.getTransaction().equals(notSelectedInvalid))
return TransactionSelectionResult.invalid("invalid");
return TransactionSelectionResult.SELECTED;
}

@Override
public TransactionSelectionResult evaluateTransactionPostProcessing(
final PendingTransaction pendingTransaction,
final org.hyperledger.besu.plugin.data.TransactionProcessingResult
processingResult) {
return TransactionSelectionResult.SELECTED;
}
});

final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
Expand Down Expand Up @@ -586,6 +601,67 @@ public void transactionSelectionPluginShouldWork() {
entry(notSelectedInvalid, TransactionSelectionResult.invalid("invalid")));
}

@Test
public void transactionSelectionPluginShouldWork_PostProcessing() {
final ProcessableBlockHeader blockHeader = createBlock(300_000);

long maxGasUsedByTransaction = 21_000;

final Transaction selected = createTransaction(0, Wei.of(10), 21_000);
ensureTransactionIsValid(selected, maxGasUsedByTransaction, 0);

// Add + 1 to gasUsedByTransaction so it will fail in the post processing selection
final Transaction notSelected = createTransaction(1, Wei.of(10), 30_000);
ensureTransactionIsValid(notSelected, maxGasUsedByTransaction + 1, 0);

final Transaction selected3 = createTransaction(3, Wei.of(10), 21_000);
ensureTransactionIsValid(selected3, maxGasUsedByTransaction, 0);

final TransactionSelectorFactory transactionSelectorFactory =
() ->
List.of(
new TransactionSelector() {
@Override
public TransactionSelectionResult evaluateTransactionPreProcessing(
final PendingTransaction pendingTransaction) {
return TransactionSelectionResult.SELECTED;
}

@Override
public TransactionSelectionResult evaluateTransactionPostProcessing(
final PendingTransaction pendingTransaction,
final org.hyperledger.besu.plugin.data.TransactionProcessingResult
processingResult) {
// the transaction with max gas +1 should fail
if (processingResult.getEstimateGasUsedByTransaction()
> maxGasUsedByTransaction) {
return TransactionSelectionResult.invalidTransient("Invalid");
}
return TransactionSelectionResult.SELECTED;
}
});

final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelectorWithTxSelPlugin(
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT,
transactionSelectorFactory);

transactionPool.addRemoteTransactions(List.of(selected, notSelected, selected3));

final TransactionSelectionResults transactionSelectionResults =
selector.buildTransactionListForBlock();

assertThat(transactionSelectionResults.getSelectedTransactions()).contains(selected, selected3);
assertThat(transactionSelectionResults.getNotSelectedTransactions())
.containsOnly(entry(notSelected, TransactionSelectionResult.invalidTransient("Invalid")));
}

@Test
public void transactionWithIncorrectNonceRemainsInPoolAndNotSelected() {
final ProcessableBlockHeader blockHeader = createBlock(5_000_000);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@

import org.apache.tuweni.bytes.Bytes;

public class TransactionProcessingResult {
public class TransactionProcessingResult
implements org.hyperledger.besu.plugin.data.TransactionProcessingResult {

/** The status of the transaction after being processed. */
public enum Status {
Expand Down Expand Up @@ -113,6 +114,7 @@ public TransactionProcessingResult(
*
* @return the logs produced by the transaction
*/
@Override
public List<Log> getLogs() {
return logs;
}
Expand All @@ -124,6 +126,7 @@ public List<Log> getLogs() {
*
* @return the gas remaining after the transaction was processed
*/
@Override
public long getGasRemaining() {
return gasRemaining;
}
Expand All @@ -134,6 +137,7 @@ public long getGasRemaining() {
*
* @return the estimate gas used
*/
@Override
public long getEstimateGasUsedByTransaction() {
return estimateGasUsedByTransaction;
}
Expand All @@ -147,28 +151,41 @@ public Status getStatus() {
return status;
}

@Override
public Bytes getOutput() {
return output;
}

/**
* Returns whether or not the transaction was invalid.
* Returns whether the transaction was invalid.
*
* @return {@code true} if the transaction was invalid; otherwise {@code false}
*/
@Override
public boolean isInvalid() {
return getStatus() == Status.INVALID;
}

/**
* Returns whether or not the transaction was successfully processed.
* Returns whether the transaction was successfully processed.
*
* @return {@code true} if the transaction was successfully processed; otherwise {@code false}
*/
@Override
public boolean isSuccessful() {
return getStatus() == Status.SUCCESSFUL;
}

/**
* Returns whether the transaction failed.
*
* @return {@code true} if the transaction failed; otherwise {@code false}
*/
@Override
public boolean isFailed() {
return getStatus() == Status.FAILED;
}

/**
* Returns the transaction validation result.
*
Expand All @@ -183,6 +200,7 @@ public ValidationResult<TransactionInvalidReason> getValidationResult() {
*
* @return the revert reason.
*/
@Override
public Optional<Bytes> getRevertReason() {
return revertReason;
}
Expand Down
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 = 'gfZY0boUMYJoAHwou3eEhcz7A/xFvJKnjMUONZ6hY3I='
knownHash = 'IPqcFdM1uy+ZDbcvzsKxMIrzhP9VoaSeanhBOLtbhfE='
}
check.dependsOn('checkAPIChanges')

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* 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.plugin.data;

import org.hyperledger.besu.evm.log.Log;

import java.util.List;
import java.util.Optional;

import org.apache.tuweni.bytes.Bytes;

/**
* This interface represents the result of processing a transaction. It provides methods to access
* various details about the transaction processing result such as logs, gas remaining, output, and
* status.
*/
public interface TransactionProcessingResult {

/**
* Return the logs produced by the transaction.
*
* <p>This is only valid when {@code TransactionProcessor#isSuccessful} returns {@code true}.
*
* @return the logs produced by the transaction
*/
List<Log> getLogs();

/**
* Returns the gas remaining after the transaction was processed.
*
* <p>This is only valid when {@code TransactionProcessor#isSuccessful} returns {@code true}.
*
* @return the gas remaining after the transaction was processed
*/
long getGasRemaining();

/**
* Returns the estimate gas used by the transaction, the difference between the transactions gas
* limit and the remaining gas
*
* @return the estimate gas used
*/
long getEstimateGasUsedByTransaction();

/**
* Returns the output.
*
* @return the output.
*/
Bytes getOutput();

/**
* Returns whether the transaction was invalid.
*
* @return {@code true} if the transaction was invalid; otherwise {@code false}
*/
boolean isInvalid();

/**
* Returns whether the transaction was successfully processed.
*
* @return {@code true} if the transaction was successfully processed; otherwise {@code false}
*/
boolean isSuccessful();

/**
* Returns whether the transaction failed.
*
* @return {@code true} if the transaction failed; otherwise {@code false}
*/
boolean isFailed();

/**
* Returns the reason why a transaction was reverted (if applicable).
*
* @return the revert reason.
*/
Optional<Bytes> getRevertReason();
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.hyperledger.besu.datatypes.PendingTransaction;
import org.hyperledger.besu.plugin.Unstable;
import org.hyperledger.besu.plugin.data.TransactionProcessingResult;
import org.hyperledger.besu.plugin.data.TransactionSelectionResult;

/** Interface for the transaction selector */
Expand All @@ -31,4 +32,15 @@ public interface TransactionSelector {
*/
TransactionSelectionResult evaluateTransactionPreProcessing(
PendingTransaction pendingTransaction);

/**
* Method called to decide whether a processed transaction is added to a block. The result can
* also indicate that no further transactions can be added to the block.
*
* @param pendingTransaction candidate transaction
* @param processingResult the transaction processing result
* @return TransactionSelectionResult that indicates whether to include the transaction
*/
TransactionSelectionResult evaluateTransactionPostProcessing(
PendingTransaction pendingTransaction, TransactionProcessingResult processingResult);
}
Loading

0 comments on commit fe17410

Please sign in to comment.