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 block limit #2

Merged
merged 4 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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 @@ -11,6 +11,8 @@

public class LineaOptions implements CLIOptions<LineaParameters> {
private static final String TRANSACTION_MAX_CALLDATA_SIZE = "--Xtransaction-max-calldata-size";
private static final String BLOCK_MAX_CALLDATA_SIZE = "--Xblock-max-calldata-size";
;

public static LineaOptions create() {
return new LineaOptions();
Expand All @@ -24,20 +26,34 @@ public static LineaOptions create() {
"If specified, overrides the max size in bytes allowed in the transaction calldata field, specified by the current hard fork")
private Integer transactionMaxCalldataSize;

@CommandLine.Option(
hidden = true,
names = {BLOCK_MAX_CALLDATA_SIZE},
paramLabel = "<INTEGER>",
description =
"If specified, overrides the max size in bytes allowed in the transaction calldata field, specified by the current hard fork")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adapt the description to block max calldata size

private Integer blockMaxCalldataSize;

public Optional<Integer> getTransactionMaxCalldataSize() {
return Optional.ofNullable(transactionMaxCalldataSize);
}

public Optional<Integer> getBlockMaxCalldataSize() {
return Optional.ofNullable(blockMaxCalldataSize);
}

@Override
public LineaParameters toDomainObject() {
return new LineaParameters(transactionMaxCalldataSize);
return new LineaParameters(transactionMaxCalldataSize, blockMaxCalldataSize);
}

@Override
public List<String> getCLIOptions() {
final List<String> cliOptions = new ArrayList<>(1);
final List<String> cliOptions = new ArrayList<>(2);
getTransactionMaxCalldataSize()
.ifPresent(size -> cliOptions.add(TRANSACTION_MAX_CALLDATA_SIZE + "=" + size));
getBlockMaxCalldataSize()
.ifPresent(size -> cliOptions.add(BLOCK_MAX_CALLDATA_SIZE + "=" + size));
return cliOptions;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.TransactionReceipt;
import org.hyperledger.besu.ethereum.linea.LineaParameters;
import org.hyperledger.besu.ethereum.mainnet.PoWHasher;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
Expand Down Expand Up @@ -115,7 +116,8 @@ public class EthGetTransactionReceiptTest {
null,
Optional.empty(),
null,
true);
true,
LineaParameters.DEFAULT);
private final ProtocolSpec statusTransactionTypeSpec =
new ProtocolSpec(
"status",
Expand Down Expand Up @@ -144,7 +146,8 @@ public class EthGetTransactionReceiptTest {
null,
Optional.empty(),
null,
true);
true,
LineaParameters.DEFAULT);

@SuppressWarnings("unchecked")
private final ProtocolSchedule protocolSchedule = mock(ProtocolSchedule.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,8 @@ private BlockTransactionSelector.TransactionSelectionResults selectTransactions(
dataGasPrice,
protocolSpec.getFeeMarket(),
protocolSpec.getGasCalculator(),
protocolSpec.getGasLimitCalculator());
protocolSpec.getGasLimitCalculator(),
protocolSpec.getLineaParameters());

if (transactions.isPresent()) {
return selector.evaluateTransactions(transactions.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.hyperledger.besu.ethereum.core.TransactionReceipt;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionSelectionResult;
import org.hyperledger.besu.ethereum.linea.LineaParameters;
import org.hyperledger.besu.ethereum.mainnet.AbstractBlockProcessor;
import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionProcessor;
import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionValidator;
Expand All @@ -45,7 +46,9 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CancellationException;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -75,6 +78,12 @@
* not cleared between executions of buildTransactionListForBlock().
*/
public class BlockTransactionSelector {

private final int blockMaxCalldataSize;
private final Function<Transaction, Optional<TransactionSelectionResult>>
lineaMaxCalldataSizeEnforcer;
private int blockCalldataSum;

public static class TransactionValidationResult {
private final Transaction transaction;
private final ValidationResult<TransactionInvalidReason> validationResult;
Expand Down Expand Up @@ -240,7 +249,8 @@ public BlockTransactionSelector(
final Wei dataGasPrice,
final FeeMarket feeMarket,
final GasCalculator gasCalculator,
final GasLimitCalculator gasLimitCalculator) {
final GasLimitCalculator gasLimitCalculator,
final LineaParameters lineaParameters) {
this.transactionProcessor = transactionProcessor;
this.blockchain = blockchain;
this.worldState = worldState;
Expand All @@ -255,6 +265,13 @@ public BlockTransactionSelector(
this.feeMarket = feeMarket;
this.gasCalculator = gasCalculator;
this.gasLimitCalculator = gasLimitCalculator;
if (lineaParameters.maybeBlockCalldataMaxSize().isPresent()) {
blockMaxCalldataSize = lineaParameters.maybeBlockCalldataMaxSize().getAsInt();
lineaMaxCalldataSizeEnforcer = (t) -> maxCalldataEnforcer(t);
} else {
blockMaxCalldataSize = 0;
lineaMaxCalldataSizeEnforcer = (t) -> Optional.empty();
}
}

/*
Expand Down Expand Up @@ -304,6 +321,12 @@ private TransactionSelectionResult evaluateTransaction(
throw new CancellationException("Cancelled during transaction selection.");
}

final Optional<TransactionSelectionResult> maybeResult =
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems this could simplified to be a Predicate

lineaMaxCalldataSizeEnforcer.apply(transaction);
if (maybeResult.isPresent()) {
return maybeResult.get();
}

if (transactionTooLargeForBlock(transaction)) {
LOG.atTrace()
.setMessage("Transaction {} too large to select for block creation")
Expand Down Expand Up @@ -534,4 +557,13 @@ private boolean blockOccupancyAboveThreshold() {
occupancyRatio);
return occupancyRatio >= minBlockOccupancyRatio;
}

private Optional<TransactionSelectionResult> maxCalldataEnforcer(final Transaction transaction) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems this could simplified to just return a boolean

this.blockCalldataSum += transaction.getPayload().size();
if (blockCalldataSum > blockMaxCalldataSize) {
return Optional.of(TransactionSelectionResult.COMPLETE_OPERATION);
} else {
return Optional.empty();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
import org.hyperledger.besu.ethereum.eth.transactions.sorter.BaseFeePendingTransactionsSorter;
import org.hyperledger.besu.ethereum.linea.LineaParameters;
import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionProcessor;
import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionValidator;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
Expand Down Expand Up @@ -264,7 +265,8 @@ public void useSingleGasSpaceForAllTransactions() {
Wei.ZERO,
FeeMarket.london(0L),
new LondonGasCalculator(),
GasLimitCalculator.constant());
GasLimitCalculator.constant(),
LineaParameters.DEFAULT);

// this should fill up all the block space
final Transaction fillingLegacyTx =
Expand Down Expand Up @@ -469,7 +471,8 @@ protected BlockTransactionSelector createBlockSelector(
dataGasPrice,
getFeeMarket(),
new LondonGasCalculator(),
GasLimitCalculator.constant());
GasLimitCalculator.constant(),
LineaParameters.DEFAULT);
return selector;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,26 @@
import java.util.OptionalInt;

public class LineaParameters {
public static final LineaParameters DEFAULT = new LineaParameters(null);
public static final LineaParameters DEFAULT = new LineaParameters(null, null);

private final OptionalInt transactionCalldataMaxSize;
private final OptionalInt blockCalldataMaxSize;

public LineaParameters(final Integer transactionCalldataMaxSize) {
public LineaParameters(
final Integer transactionCalldataMaxSize, final Integer blockCalldataMaxSize) {
this.transactionCalldataMaxSize =
isNull(transactionCalldataMaxSize)
? OptionalInt.empty()
: OptionalInt.of(transactionCalldataMaxSize);
this.blockCalldataMaxSize =
isNull(blockCalldataMaxSize) ? OptionalInt.empty() : OptionalInt.of(blockCalldataMaxSize);
}

public OptionalInt maybeTransactionCalldataMaxSize() {
return transactionCalldataMaxSize;
}

public OptionalInt maybeBlockCalldataMaxSize() {
return blockCalldataMaxSize;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Set;

public class LineaProtocolSpecs {
public static final int DEFAULT_BLOCK_MAX_CALLDATA_SIZE = 1000000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be private

private static final int LINEA_MAX_TX_CALLDATA_SIZE =
10000; // fake value replace with the actual one when known

Expand Down Expand Up @@ -52,6 +53,7 @@ static ProtocolSpecBuilder lineaDefinition(
TransactionType.EIP1559),
quorumCompatibilityMode,
txCalldataMaxSize))
.lineaParameters(lineaParameters)
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should not pass lineaParameters here, but we can abstract and pass only calldata limits, also because lineaParameters only contain the configuration overrides, and not the value specified in the hard fork definition

.name("Linea");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.hyperledger.besu.ethereum.chain.BadBlockManager;
import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
import org.hyperledger.besu.ethereum.core.BlockImporter;
import org.hyperledger.besu.ethereum.linea.LineaParameters;
import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
import org.hyperledger.besu.ethereum.privacy.PrivateTransactionProcessor;
import org.hyperledger.besu.evm.EVM;
Expand Down Expand Up @@ -81,6 +82,8 @@ public class ProtocolSpec {
private final DepositsValidator depositsValidator;

private final boolean isPoS;
private final LineaParameters lineaParameters;

/**
* Creates a new protocol specification instance.
*
Expand Down Expand Up @@ -111,6 +114,7 @@ public class ProtocolSpec {
* @param withdrawalsProcessor the Withdrawals processor to use
* @param depositsValidator the withdrawals validator to use
* @param isPoS indicates whether the current spec is PoS
* @param lineaParameters linea parameters
*/
public ProtocolSpec(
final String name,
Expand Down Expand Up @@ -139,7 +143,8 @@ public ProtocolSpec(
final WithdrawalsValidator withdrawalsValidator,
final Optional<WithdrawalsProcessor> withdrawalsProcessor,
final DepositsValidator depositsValidator,
final boolean isPoS) {
final boolean isPoS,
final LineaParameters lineaParameters) {
this.name = name;
this.evm = evm;
this.transactionValidator = transactionValidator;
Expand Down Expand Up @@ -167,6 +172,7 @@ public ProtocolSpec(
this.withdrawalsProcessor = withdrawalsProcessor;
this.depositsValidator = depositsValidator;
this.isPoS = isPoS;
this.lineaParameters = lineaParameters;
}

/**
Expand Down Expand Up @@ -387,4 +393,8 @@ public DepositsValidator getDepositsValidator() {
public boolean isPoS() {
return isPoS;
}

public LineaParameters getLineaParameters() {
return lineaParameters;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

as described before, we can replace this with blockMaxCalldataSize

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.hyperledger.besu.ethereum.core.BlockImporter;
import org.hyperledger.besu.ethereum.core.GoQuorumPrivacyParameters;
import org.hyperledger.besu.ethereum.core.PrivacyParameters;
import org.hyperledger.besu.ethereum.linea.LineaParameters;
import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
import org.hyperledger.besu.ethereum.mainnet.precompiles.privacy.FlexiblePrivacyPrecompiledContract;
import org.hyperledger.besu.ethereum.mainnet.precompiles.privacy.PrivacyPluginPrecompiledContract;
Expand Down Expand Up @@ -82,6 +83,7 @@ public class ProtocolSpecBuilder {
private BadBlockManager badBlockManager;
private PoWHasher powHasher = PoWHasher.ETHASH_LIGHT;
private boolean isPoS = false;
private LineaParameters lineaParameters = LineaParameters.DEFAULT;

public ProtocolSpecBuilder gasCalculator(final Supplier<GasCalculator> gasCalculatorBuilder) {
this.gasCalculatorBuilder = gasCalculatorBuilder;
Expand Down Expand Up @@ -271,6 +273,11 @@ public ProtocolSpecBuilder isPoS(final boolean isPoS) {
return this;
}

public ProtocolSpecBuilder lineaParameters(final LineaParameters lineaParameters) {
this.lineaParameters = lineaParameters;
return this;
}

public ProtocolSpec build(final HeaderBasedProtocolSchedule protocolSchedule) {
checkNotNull(gasCalculatorBuilder, "Missing gasCalculator");
checkNotNull(gasLimitCalculator, "Missing gasLimitCalculator");
Expand Down Expand Up @@ -379,7 +386,8 @@ public ProtocolSpec build(final HeaderBasedProtocolSchedule protocolSchedule) {
withdrawalsValidator,
Optional.ofNullable(withdrawalsProcessor),
depositsValidator,
isPoS);
isPoS,
lineaParameters);
}

private PrivateTransactionProcessor createPrivateTransactionProcessor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockImporter;
import org.hyperledger.besu.ethereum.core.TransactionFilter;
import org.hyperledger.besu.ethereum.linea.LineaParameters;
import org.hyperledger.besu.ethereum.mainnet.BlockProcessor;
import org.hyperledger.besu.ethereum.mainnet.MainnetBlockImporter;
import org.hyperledger.besu.ethereum.mainnet.MainnetBlockProcessor;
Expand Down Expand Up @@ -86,7 +87,8 @@ public ProtocolSpec getByBlockNumber(final long number) {
original.getWithdrawalsValidator(),
original.getWithdrawalsProcessor(),
original.getDepositsValidator(),
original.isPoS());
original.isPoS(),
LineaParameters.DEFAULT);
}

@Override
Expand Down