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.

Sign up for GitHub

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

Sequenced pool synonym for legacy pool #6274

Merged
merged 9 commits into from
Dec 12, 2023
Merged

Sequenced pool synonym for legacy pool #6274

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a68378b
Sequenced pool synonym for legacy pool
matthew1001 Dec 11, 2023
f44e604
Class rename
matthew1001 Dec 11, 2023
bd3bf3f
Spotless fixes
matthew1001 Dec 11, 2023
17af796
Add SEQUENCED to config overview test
matthew1001 Dec 11, 2023
57a14e9
Merge branch 'main' into sequenced-pool
matthew1001 Dec 11, 2023
e566397
Update CHANGELOG.md
matthew1001 Dec 12, 2023
ed022b5
Merge branch 'main' into sequenced-pool
matthew1001 Dec 12, 2023
e90b4f1
add a fix to load correctly the storage trie in the Bonsai WorldState…
matkt Dec 12, 2023
88950b4
Merge branch 'main' into sequenced-pool
matthew1001 Dec 12, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

### Additions and Improvements
- Add error messages on authentication failures with username and password [#6212](https://github.com/hyperledger/besu/pull/6212)
- New `Sequenced` transaction pool. The pool is an evolution of the `legacy` pool and is likely to be suitable to enterprise or permissioned chains than the `layered` transaction pool. Select to use this pool with `--tx-pool=sequenced`. Supports the same options as the `legacy` pool [#6211](https://github.com/hyperledger/besu/issues/6211)
matthew1001 marked this conversation as resolved.
Show resolved Hide resolved

### Bug fixes

Expand Down
27 changes: 14 additions & 13 deletions besu/src/main/java/org/hyperledger/besu/cli/options/TransactionPoolOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.hyperledger.besu.cli.DefaultCommandValues.MANDATORY_LONG_FORMAT_HELP;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LAYERED;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LEGACY;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.SEQUENCED;

import org.hyperledger.besu.cli.converter.DurationMillisConverter;
import org.hyperledger.besu.cli.converter.FractionConverter;
Expand Down Expand Up @@ -171,10 +172,10 @@ static class Layered {

@CommandLine.ArgGroup(
validate = false,
heading = "@|bold Tx Pool Legacy Implementation Options|@%n")
private final Legacy legacyOptions = new Legacy();
heading = "@|bold Tx Pool Sequenced Implementation Options|@%n")
private final Sequenced sequencedOptions = new Sequenced();

static class Legacy {
static class Sequenced {
private static final String TX_POOL_RETENTION_HOURS = "--tx-pool-retention-hours";
private static final String TX_POOL_LIMIT_BY_ACCOUNT_PERCENTAGE =
"--tx-pool-limit-by-account-percentage";
Expand Down Expand Up @@ -272,10 +273,10 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati
config.getPendingTransactionsLayerMaxCapacityBytes();
options.layeredOptions.txPoolMaxPrioritized = config.getMaxPrioritizedTransactions();
options.layeredOptions.txPoolMaxFutureBySender = config.getMaxFutureBySender();
options.legacyOptions.txPoolLimitByAccountPercentage =
options.sequencedOptions.txPoolLimitByAccountPercentage =
config.getTxPoolLimitByAccountPercentage();
options.legacyOptions.txPoolMaxSize = config.getTxPoolMaxSize();
options.legacyOptions.pendingTxRetentionPeriod = config.getPendingTxRetentionPeriod();
options.sequencedOptions.txPoolMaxSize = config.getTxPoolMaxSize();
options.sequencedOptions.pendingTxRetentionPeriod = config.getPendingTxRetentionPeriod();
options.unstableOptions.txMessageKeepAliveSeconds =
config.getUnstable().getTxMessageKeepAliveSeconds();
options.unstableOptions.eth65TrxAnnouncedBufferingPeriod =
Expand All @@ -295,14 +296,14 @@ public void validate(
final CommandLine commandLine, final GenesisConfigOptions genesisConfigOptions) {
CommandLineUtils.failIfOptionDoesntMeetRequirement(
commandLine,
"Could not use legacy transaction pool options with layered implementation",
"Could not use legacy or sequenced transaction pool options with layered implementation",
!txPoolImplementation.equals(LAYERED),
CommandLineUtils.getCLIOptionNames(Legacy.class));
CommandLineUtils.getCLIOptionNames(Sequenced.class));

CommandLineUtils.failIfOptionDoesntMeetRequirement(
commandLine,
"Could not use layered transaction pool options with legacy implementation",
!txPoolImplementation.equals(LEGACY),
"Could not use layered transaction pool options with legacy or sequenced implementation",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Could not use layered transaction pool options with legacy or sequenced implementation",
"Could not use layered transaction pool options with legacy (sequenced) implementation",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it's difficult balancing the fact that we don't want to make the sequenced pool sound like it's the "legacy" option, even though in reality they are the same implementation. At some point the legacy option will be removed entirely so we'll update these messages to just talk about sequenced in the future. Personally I'd prefer to leave it as "legacy or sequenced" for now, but happy to discuss further and we can always raise another PR to re-word if we need to?

!txPoolImplementation.equals(LEGACY) && !txPoolImplementation.equals(SEQUENCED),
CommandLineUtils.getCLIOptionNames(Layered.class));

CommandLineUtils.failIfOptionDoesntMeetRequirement(
Expand All @@ -327,9 +328,9 @@ public TransactionPoolConfiguration toDomainObject() {
.pendingTransactionsLayerMaxCapacityBytes(layeredOptions.txPoolLayerMaxCapacity)
.maxPrioritizedTransactions(layeredOptions.txPoolMaxPrioritized)
.maxFutureBySender(layeredOptions.txPoolMaxFutureBySender)
.txPoolLimitByAccountPercentage(legacyOptions.txPoolLimitByAccountPercentage)
.txPoolMaxSize(legacyOptions.txPoolMaxSize)
.pendingTxRetentionPeriod(legacyOptions.pendingTxRetentionPeriod)
.txPoolLimitByAccountPercentage(sequencedOptions.txPoolLimitByAccountPercentage)
.txPoolMaxSize(sequencedOptions.txPoolMaxSize)
.pendingTxRetentionPeriod(sequencedOptions.pendingTxRetentionPeriod)
.unstable(
ImmutableTransactionPoolConfiguration.Unstable.builder()
.txMessageKeepAliveSeconds(unstableOptions.txMessageKeepAliveSeconds)
Expand Down
8 changes: 8 additions & 0 deletions besu/src/test/java/org/hyperledger/besu/cli/ConfigurationOverviewBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LAYERED;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LEGACY;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.SEQUENCED;
import static org.mockito.Mockito.mock;

import org.hyperledger.besu.evm.internal.EvmConfiguration;
Expand Down Expand Up @@ -180,6 +181,13 @@ void setTxPoolImplementationLegacy() {
assertThat(legacyTxPoolSelected).contains("Using LEGACY transaction pool implementation");
}

@Test
void setTxPoolImplementationSequenced() {
builder.setTxPoolImplementation(SEQUENCED);
final String sequencedTxPoolSelected = builder.build();
assertThat(sequencedTxPoolSelected).contains("Using SEQUENCED transaction pool implementation");
}

@Test
void setWorldStateUpdateModeDefault() {
builder.setWorldStateUpdateMode(EvmConfiguration.DEFAULT.worldUpdaterMode());
Expand Down
35 changes: 31 additions & 4 deletions besu/src/test/java/org/hyperledger/besu/cli/options/TransactionPoolOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LAYERED;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LEGACY;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.SEQUENCED;

import org.hyperledger.besu.cli.converter.DurationMillisConverter;
import org.hyperledger.besu.datatypes.Address;
Expand Down Expand Up @@ -63,7 +64,7 @@ public void strictTxReplayProtection_default() {
}

@Test
public void pendingTransactionRetentionPeriod() {
public void pendingTransactionRetentionPeriodLegacy() {
final int pendingTxRetentionHours = 999;
internalTestSuccess(
config ->
Expand All @@ -73,6 +74,17 @@ public void pendingTransactionRetentionPeriod() {
"--tx-pool=legacy");
}

@Test
public void pendingTransactionRetentionPeriodSequenced() {
final int pendingTxRetentionHours = 999;
internalTestSuccess(
config ->
assertThat(config.getPendingTxRetentionPeriod()).isEqualTo(pendingTxRetentionHours),
"--tx-pool-retention-hours",
String.valueOf(pendingTxRetentionHours),
"--tx-pool=sequenced");
}

@Test
public void disableLocalsDefault() {
internalTestSuccess(config -> assertThat(config.getNoLocalPriority()).isFalse());
Expand Down Expand Up @@ -193,29 +205,44 @@ public void selectLegacyImplementationByArg() {
"--tx-pool=legacy");
}

@Test
public void selectSequencedImplementationByArg() {
internalTestSuccess(
config -> assertThat(config.getTxPoolImplementation()).isEqualTo(SEQUENCED),
"--tx-pool=sequenced");
}

@Test
public void failIfLegacyOptionsWhenLayeredSelectedByDefault() {
internalTestFailure(
"Could not use legacy transaction pool options with layered implementation",
"Could not use legacy or sequenced transaction pool options with layered implementation",
"--tx-pool-max-size=1000");
}

@Test
public void failIfLegacyOptionsWhenLayeredSelectedByArg() {
internalTestFailure(
"Could not use legacy transaction pool options with layered implementation",
"Could not use legacy or sequenced transaction pool options with layered implementation",
"--tx-pool=layered",
"--tx-pool-max-size=1000");
}

@Test
public void failIfLayeredOptionsWhenLegacySelectedByArg() {
internalTestFailure(
"Could not use layered transaction pool options with legacy implementation",
"Could not use layered transaction pool options with legacy or sequenced implementation",
"--tx-pool=legacy",
"--tx-pool-max-prioritized=1000");
}

@Test
public void failIfLayeredOptionsWhenSequencedSelectedByArg() {
internalTestFailure(
"Could not use layered transaction pool options with legacy or sequenced implementation",
"--tx-pool=sequenced",
"--tx-pool-max-prioritized=1000");
}

@Test
public void byDefaultNoPrioritySenders() {
internalTestSuccess(config -> assertThat(config.getPrioritySenders()).isEmpty());
Expand Down
2 changes: 1 addition & 1 deletion besu/src/test/resources/everything_config.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ tx-pool-save-file="txpool.dump"
tx-pool-layer-max-capacity=12345678
tx-pool-max-prioritized=9876
tx-pool-max-future-by-sender=321
## Legacy
## Legacy/Sequenced
tx-pool-retention-hours=999
tx-pool-max-size=1234
tx-pool-limit-by-account-percentage=0.017
Expand Down
5 changes: 3 additions & 2 deletions ...ain/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ default int getTxMessageKeepAliveSeconds() {
}

enum Implementation {
LEGACY,
LAYERED;
LEGACY, // Remove in future version
LAYERED,
SEQUENCED; // Synonym for LEGACY
}

String DEFAULT_SAVE_FILE_NAME = "txpool.dump";
Expand Down
Loading