refactor: unordered transactions ante handling #24573
Conversation
Ironbird - launch a networkTo use Ironbird, you can use the following commands:
Custom Load Test ConfigurationYou can provide a custom load test configuration using the `--load-test-config=` flag:Use |
| @@ -258,6 +293,11 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul | |||
|
|
|||
| utx, ok := tx.(sdk.TxWithUnordered) | |||
| isUnordered := ok && utx.GetUnordered() | |||
| unorderedEnabled := svd.ak.IsUnorderedTransactionsEnabled() | |||
There was a problem hiding this comment.
should we just use ak and not have the unorderedTxManagerfield at all?
There was a problem hiding this comment.
yeah i can do that instead
x/auth/ante/sigverify.go
Outdated
| maxTxTimeoutDuration time.Duration | ||
| unorderedTxGasCost uint64 | ||
| unorderedTxManager UnorderedNonceManager |
There was a problem hiding this comment.
The only wonky thing is that we have these options that are not really signing related on the signing decorator
There was a problem hiding this comment.
i feel like an option controlling how far ahead a tx's nonce value is before its considered invalid is related enough to make sense here.
the gas cost does feel odd, but one could also consider its related to the cost of verifying a more complex tx type (unordered)
x/auth/module.go
Outdated
| start := telemetry.Now() | ||
| defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyPreBlocker) |
There was a problem hiding this comment.
we should have this outside of the if
📝 WalkthroughWalkthroughThe changes introduce a new mechanism for enabling and configuring unordered transactions in the Cosmos SDK's authentication module. The Changes
Sequence Diagram(s)sequenceDiagram
participant App as SimApp
participant Keeper as AccountKeeper
participant Ante as AnteHandler
participant SigVer as SigVerificationDecorator
App->>Keeper: NewAccountKeeper(..., WithUnorderedTransactions(true))
App->>Ante: NewAnteHandler(HandlerOptions{SigVerifyOptions: [...]})
Ante->>SigVer: NewSigVerificationDecorator(AccountKeeper, SignModeHandler, ...SigVerifyOptions)
Note over SigVer: SigVerificationDecorator configured for unordered txs
User->>App: BroadcastTx (unordered, with timeout)
App->>Ante: AnteHandler(ctx, tx, ...)
Ante->>SigVer: AnteHandle(ctx, tx, ...)
SigVer->>Keeper: IsUnorderedTransactionsEnabled()
alt Unordered tx enabled
SigVer->>SigVer: verifyUnorderedNonce(ctx, tx)
SigVer->>Keeper: TryAddUnorderedNonce(ctx, sender, timestamp)
SigVer->>Ante: Continue to next decorator
else Unordered tx disabled
SigVer-->>Ante: Return error (unordered txs not enabled)
end
Ante-->>App: AnteHandler result
sequenceDiagram
participant Module as AppModule
participant Keeper as AccountKeeper
Module->>Keeper: NewAccountKeeper(..., WithUnorderedTransactions(flag))
Module->>Module: PreBlock(ctx)
Module->>Keeper: IsUnorderedTransactionsEnabled()
alt Unordered tx enabled
Module->>Keeper: RemoveExpiredUnorderedNonces(ctx)
end
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
x/auth/ante/sigverify.go (1)
460-465:extractSignersBytesduplicates logic already available above
sigTx.GetSigners()is executed earlier inAnteHandle; we could pass that slice down rather than re-reading from the tx, avoiding another type-assert and keeping signer ordering consistent.Minor, but keeps the call graph tight.
🧹 Nitpick comments (10)
UPGRADE_GUIDE.md (2)
409-422: Clear upgrade instructions for enabling unordered transactions.The instructions now clearly explain how to enable unordered transactions through the AccountKeeper configuration with the new WithUnorderedTransactions option.
Consider using spaces instead of tabs in the code block for consistent formatting across Markdown files.
- app.AccountKeeper = authkeeper.NewAccountKeeper( - appCodec, - runtime.NewKVStoreService(keys[authtypes.StoreKey]), - authtypes.ProtoBaseAccount, - maccPerms, - authcodec.NewBech32Codec(sdk.Bech32MainPrefix), - sdk.Bech32MainPrefix, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - authkeeper.WithUnorderedTransactions(true), // new option! - ) + app.AccountKeeper = authkeeper.NewAccountKeeper( + appCodec, + runtime.NewKVStoreService(keys[authtypes.StoreKey]), + authtypes.ProtoBaseAccount, + maccPerms, + authcodec.NewBech32Codec(sdk.Bech32MainPrefix), + sdk.Bech32MainPrefix, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + authkeeper.WithUnorderedTransactions(true), // new option! + )🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
412-412: Hard tabs
Column: 1(MD010, no-hard-tabs)
413-413: Hard tabs
Column: 1(MD010, no-hard-tabs)
414-414: Hard tabs
Column: 1(MD010, no-hard-tabs)
415-415: Hard tabs
Column: 1(MD010, no-hard-tabs)
416-416: Hard tabs
Column: 1(MD010, no-hard-tabs)
417-417: Hard tabs
Column: 1(MD010, no-hard-tabs)
418-418: Hard tabs
Column: 1(MD010, no-hard-tabs)
419-419: Hard tabs
Column: 1(MD010, no-hard-tabs)
420-420: Hard tabs
Column: 1(MD010, no-hard-tabs)
421-421: Hard tabs
Column: 1(MD010, no-hard-tabs)
437-441: Updated ante handler setup example.This example clearly shows how to pass the SigVerifyOptions to the NewSigVerificationDecorator.
Replace tabs with spaces for consistent formatting in the code block.
- // ... snip - ante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler, options.SigVerifyOptions...), // supply new options + // ... snip + ante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler, options.SigVerifyOptions...), // supply new options🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
439-439: Hard tabs
Column: 1(MD010, no-hard-tabs)
UPGRADING.md (2)
30-43: Clear upgrade instructions for enabling unordered transactions.The instructions explain how to enable unordered transactions using the new WithUnorderedTransactions option.
Consider using spaces instead of tabs in the code block for consistent formatting across Markdown files.
- app.AccountKeeper = authkeeper.NewAccountKeeper( - appCodec, - runtime.NewKVStoreService(keys[authtypes.StoreKey]), - authtypes.ProtoBaseAccount, - maccPerms, - authcodec.NewBech32Codec(sdk.Bech32MainPrefix), - sdk.Bech32MainPrefix, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - authkeeper.WithUnorderedTransactions(true), // new option! - ) + app.AccountKeeper = authkeeper.NewAccountKeeper( + appCodec, + runtime.NewKVStoreService(keys[authtypes.StoreKey]), + authtypes.ProtoBaseAccount, + maccPerms, + authcodec.NewBech32Codec(sdk.Bech32MainPrefix), + sdk.Bech32MainPrefix, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + authkeeper.WithUnorderedTransactions(true), // new option! + )🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
33-33: Hard tabs
Column: 1(MD010, no-hard-tabs)
34-34: Hard tabs
Column: 1(MD010, no-hard-tabs)
35-35: Hard tabs
Column: 1(MD010, no-hard-tabs)
36-36: Hard tabs
Column: 1(MD010, no-hard-tabs)
37-37: Hard tabs
Column: 1(MD010, no-hard-tabs)
38-38: Hard tabs
Column: 1(MD010, no-hard-tabs)
39-39: Hard tabs
Column: 1(MD010, no-hard-tabs)
40-40: Hard tabs
Column: 1(MD010, no-hard-tabs)
41-41: Hard tabs
Column: 1(MD010, no-hard-tabs)
42-42: Hard tabs
Column: 1(MD010, no-hard-tabs)
59-62: Updated ante handler setup example.This example clearly shows how to pass the SigVerifyOptions to the NewSigVerificationDecorator.
Replace tab with spaces for consistent formatting in the "snip" line.
- // ... snip ... + // ... snip ...x/auth/ante/sigverify_test.go (2)
258-289: Comprehensive test helper for generating transactions.The genTestTx function is well-implemented with good error handling and flexibility for creating different types of test transactions. It will help ensure robust test coverage for the new unordered transaction functionality.
Remove the redundant error check at line 286, as the same error is already checked at line 284:
- require.NoError(t, err)
291-319: Useful helper for multi-signed unordered transactions.The genMultiSignedUnorderedTx function provides a clean way to generate test transactions with multiple signers, which is important for comprehensive test coverage of complex scenarios.
Remove the redundant error check at line 316, as the same error is already checked at line 314:
- require.NoError(t, err)x/auth/ante/unordered_test.go (1)
72-83: DuplicateTxConfigcreation – extract a helper
txConfigOptsand theauthtx.NewTxConfigWithOptionscall are repeated twice insidereset. This is only test code, but the duplication makes the function longer and harder to tweak (e.g. when adding a new sign-mode).Consider extracting a small helper:
func buildTxConfig(ir codectypes.InterfaceRegistry, queryFn banktypes.MetadataQueryFn, modes []signing.SignMode) client.TxConfig { … }Call it once for
suite.clientCtxand once for the ante-specific config.x/auth/ante/testutil_test.go (2)
82-85: Option order makes the call brittle – move the new flag to a struct option
keeper.NewAccountKeepernow receiveskeeper.WithUnorderedTransactions(enableUnorderedTxs)as the last positional argument. Any future change in the constructor’s signature will silently break this line.Prefer functional-options consistently:
- sdk.Bech32MainPrefix, types.NewModuleAddress("gov").String(), keeper.WithUnorderedTransactions(enableUnorderedTxs), + sdk.Bech32MainPrefix, + types.NewModuleAddress("gov").String(), + keeper.WithUnorderedTransactions(enableUnorderedTxs),Even better, switch the boolean
enableUnorderedTxscoming intosetupSuiteto an explicit[]keeper.AccountKeeperOptionso callers cannot swap booleans by accident.
230-237: SettingTimeoutTimestampon ordered txs may mask bugs
createTxunconditionally callsSetTimeoutTimestamp(unorderedTimeout).
Whenunordered == false, the SDK currently ignores the field, but future validation might reject non-zero timestamps on ordered txs.- suite.txBuilder.SetTimeoutTimestamp(unorderedTimeout) + if unordered { + suite.txBuilder.SetTimeoutTimestamp(unorderedTimeout) + }A tiny guard keeps the builder’s behaviour crystal-clear.
x/auth/ante/sigverify.go (1)
447-454: Partial state write before duplicate-nonce failure – tighten the flow
TryAddUnorderedNonceis called inside a loop.
On the first signer that duplicates a nonce, an error is returned; earlier successful insertions for previous signers are discarded only because the entire ante-handler wraps a cached store.While this is safe today, making the operation explicitly atomic is clearer and future-proof:
- Run a read-only pass to detect duplicates for all signers.
- If no duplicates, run a second pass that records the nonce and charges gas.
This avoids relying on implicit cache semantics and makes reasoning about state changes easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
x/auth/ante/testutil/expected_keepers_mocks.gois excluded by!**/*_mocks.go
📒 Files selected for processing (16)
UPGRADE_GUIDE.md(1 hunks)UPGRADING.md(1 hunks)simapp/ante.go(1 hunks)simapp/app.go(3 hunks)simapp/app_di.go(1 hunks)x/auth/ante/ante.go(2 hunks)x/auth/ante/ante_test.go(1 hunks)x/auth/ante/expected_keepers.go(1 hunks)x/auth/ante/sigverify.go(6 hunks)x/auth/ante/sigverify_test.go(3 hunks)x/auth/ante/testutil_test.go(5 hunks)x/auth/ante/unordered.go(0 hunks)x/auth/ante/unordered_test.go(1 hunks)x/auth/keeper/keeper.go(4 hunks)x/auth/keeper/unordered_tx_test.go(2 hunks)x/auth/module.go(3 hunks)
💤 Files with no reviewable changes (1)
- x/auth/ante/unordered.go
🧰 Additional context used
🧬 Code Graph Analysis (5)
simapp/ante.go (3)
x/auth/ante/sigverify.go (1)
NewSigVerificationDecorator(260-273)x/auth/ante/expected_keepers.go (1)
AccountKeeper(15-24)x/auth/keeper/keeper.go (1)
AccountKeeper(88-111)
x/auth/ante/ante_test.go (1)
x/auth/ante/testutil_test.go (1)
SetupTestSuiteWithUnordered(123-126)
x/auth/keeper/keeper.go (1)
x/auth/ante/expected_keepers.go (1)
AccountKeeper(15-24)
x/auth/keeper/unordered_tx_test.go (1)
x/auth/keeper/keeper.go (1)
WithUnorderedTransactions(115-119)
x/auth/module.go (2)
types/context.go (1)
UnwrapSDKContext(392-397)x/auth/keeper/keeper.go (3)
InitOption(113-113)WithUnorderedTransactions(115-119)NewAccountKeeper(129-163)
🪛 markdownlint-cli2 (0.17.2)
UPGRADING.md
33-33: Hard tabs
Column: 1
(MD010, no-hard-tabs)
34-34: Hard tabs
Column: 1
(MD010, no-hard-tabs)
35-35: Hard tabs
Column: 1
(MD010, no-hard-tabs)
36-36: Hard tabs
Column: 1
(MD010, no-hard-tabs)
37-37: Hard tabs
Column: 1
(MD010, no-hard-tabs)
38-38: Hard tabs
Column: 1
(MD010, no-hard-tabs)
39-39: Hard tabs
Column: 1
(MD010, no-hard-tabs)
40-40: Hard tabs
Column: 1
(MD010, no-hard-tabs)
41-41: Hard tabs
Column: 1
(MD010, no-hard-tabs)
42-42: Hard tabs
Column: 1
(MD010, no-hard-tabs)
UPGRADE_GUIDE.md
412-412: Hard tabs
Column: 1
(MD010, no-hard-tabs)
413-413: Hard tabs
Column: 1
(MD010, no-hard-tabs)
414-414: Hard tabs
Column: 1
(MD010, no-hard-tabs)
415-415: Hard tabs
Column: 1
(MD010, no-hard-tabs)
416-416: Hard tabs
Column: 1
(MD010, no-hard-tabs)
417-417: Hard tabs
Column: 1
(MD010, no-hard-tabs)
418-418: Hard tabs
Column: 1
(MD010, no-hard-tabs)
419-419: Hard tabs
Column: 1
(MD010, no-hard-tabs)
420-420: Hard tabs
Column: 1
(MD010, no-hard-tabs)
421-421: Hard tabs
Column: 1
(MD010, no-hard-tabs)
439-439: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: test-system-legacy
- GitHub Check: test-system
- GitHub Check: tests (03)
- GitHub Check: tests (02)
- GitHub Check: tests (01)
- GitHub Check: tests (00)
- GitHub Check: test-sim-nondeterminism
- GitHub Check: test-e2e
- GitHub Check: test-integration
- GitHub Check: build (arm64)
- GitHub Check: build (amd64)
- GitHub Check: Analyze
- GitHub Check: golangci-lint
- GitHub Check: Gosec
- GitHub Check: Summary
🔇 Additional comments (28)
x/auth/keeper/unordered_tx_test.go (2)
38-38: Enables unordered transactions consistently with the refactored approach.The addition of
keeper.WithUnorderedTransactions(true)duringAccountKeeperinitialization aligns with the broader refactoring effort where unordered transaction support is now enabled via keeper options rather than through ante handler options.
316-316: Same initialization pattern applied consistently.This second instance of
keeper.WithUnorderedTransactions(true)ensures consistent testing environment setup, properly enabling unordered transactions for the second test case as well.simapp/app_di.go (1)
277-283: Simplified HandlerOptions structure.The refactored code removes the
UnorderedNonceManagerfield fromHandlerOptions, which aligns with the architectural change where unordered transaction handling is now controlled at the keeper level rather than through dedicated ante handlers.x/auth/ante/ante_test.go (1)
1453-1453: Updated test setup to use new unordered transactions configuration.Test now initializes using
SetupTestSuiteWithUnorderedwith an explicit parameter to enable unordered transactions, consistent with the new approach where unordered transaction support is explicitly configured rather than implicitly assumed.simapp/ante.go (1)
46-46: Updated signature verification decorator with options support.The
NewSigVerificationDecoratornow receives configuration options throughoptions.SigVerifyOptions, allowing for fine-grained control of unordered transaction behavior such as gas costs and timeout durations. This is a cleaner approach than having a separate unordered transaction decorator.x/auth/ante/expected_keepers.go (1)
21-21: Interface expansion aligns with unordered transactions refactoringThe addition of
IsUnorderedTransactionsEnabled()to theAccountKeeperinterface is part of consolidating unordered transaction functionality directly into theAccountKeeper. This method will be used to check whether unordered transaction support is enabled.x/auth/keeper/keeper.go (5)
96-96: New field enables unordered transaction supportAdding the
enableUnorderedTxsboolean field to theAccountKeeperstruct provides a way to track whether unordered transactions are enabled for this keeper instance.
113-119: Good use of functional options patternThe functional options pattern implementation is clean and provides a flexible way to configure the
AccountKeeperduring initialization. This approach maintains backward compatibility while allowing new features to be added.
131-131: Updated constructor signature supports initialization optionsThe
NewAccountKeeperfunction now accepts variadicInitOptionparameters, allowing flexible configuration such as enabling unordered transactions.
159-161: Options are applied after keeper constructionThe implementation correctly applies all initialization options after the keeper has been constructed, ensuring that the keeper object is fully initialized before customization.
165-167: Feature flag accessor implementationThe
IsUnorderedTransactionsEnabledmethod provides a clean way to check whether unordered transactions are enabled. This method implements the interface method added toAccountKeeperinx/auth/ante/expected_keepers.go.simapp/app.go (3)
8-10: Import reorderingThe imports for "io" and "maps" have been reordered but remain included.
310-310: Enabling unordered transactions in SimAppThe
NewSimAppfunction now enables unordered transactions by default for theAccountKeeper. This aligns with the new initialization pattern introduced in the refactoring.
702-712: Simplified ante handler configurationThe ante handler configuration has been updated to use the new
SigVerifyOptionsapproach for configuring unordered transaction behavior. This replaces the previous approach that used separate decorators and managers. The options include:
WithUnorderedTxGasCost- Sets the additional gas cost for unordered transactionsWithMaxTxTimeoutDuration- Sets the maximum allowed timeout for unordered transactionsThis change centralizes unordered transaction configuration and simplifies the ante handler setup.
x/auth/module.go (3)
224-224: Feature flag for unordered transactionsAdding the
EnableUnorderedTransactionsboolean field toModuleInputsallows this feature to be toggled at the module level via app configuration.
105-110: Conditionally clean up expired unordered noncesThe
PreBlockmethod now only attempts to remove expired unordered nonces if unordered transactions are enabled. This is an efficient approach that avoids unnecessary processing when the feature is disabled.
254-258: Initialize with unordered transactions optionThe
ProvideModulefunction now creates an options slice that includes the unordered transactions setting. This ensures that theAccountKeeperis properly configured based on the module inputs.x/auth/ante/ante.go (2)
23-26: Good documentation addition for SigVerifyOptions.The addition of well-documented SigVerifyOptions field in the HandlerOptions struct is a good practice. The comments clearly explain the purpose of the field and how it can be used to modify signature verification behavior.
56-56: Well-structured refactoring of the ante handler configuration.This change consolidates the unordered transaction handling into the signature verification decorator by passing the SigVerifyOptions, which simplifies the codebase by removing the previous dedicated decorator for unordered transactions.
UPGRADE_GUIDE.md (2)
424-426: Clear explanation of default behavior and configuration options.This explanation is important for users who want to understand the default behavior and how to customize it.
429-433: Well-structured SigVerifyOptions example.The example shows how to configure the options with default constants, making it easy for users to understand and modify.
UPGRADING.md (2)
45-46: Good explanation of default values and configuration options.This section clearly explains the default behavior and how to customize it through SigVerifyOptions.
50-54: Well-formatted configuration example.The example shows how to configure the options with default constants, making it easy for users to understand and modify.
x/auth/ante/sigverify_test.go (4)
251-256: Well-structured options struct for test transaction generation.The genTxOptions struct provides a clean way to configure test transactions with various parameters including unordered flags and timestamps, which will be useful for testing the new unordered transaction functionality.
392-403: Important test case for unordered transactions with disabled feature.This test ensures that unordered transactions are properly rejected when the feature is disabled, which is an important edge case to test.
405-406: Good test suite setup for unordered transactions.Using a specialized test suite with unordered transactions enabled ensures that the tests reflect the expected behavior in that configuration.
468-468: Updated test case description to reflect actual behavior.The description now clearly indicates that unordered transactions should not increment the sequence number, which aligns with the expected behavior.
x/auth/ante/sigverify.go (1)
413-421: Method name mismatch – possible compilation error
unorderedTx.GetTimeoutTimeStamp()uses “TimeStamp” (capital S).
Earlier, the builder method isSetTimeoutTimestamp. The SDK interfaces useGetTimeoutTimestamp()(lowercase s) as well.Please confirm the interface:
timeoutTimestamp := unorderedTx.GetTimeoutTimestamp() // typical SDK spellingIf the current code doesn’t compile, CI will catch it, but fixing it here avoids churn.
…h://github.com/cosmos/cosmos-sdk into technicallyty/SDK-314/unordered-tx-ante-refactor
x/auth/ante/unordered_test.go
Outdated
| var priv1, priv2, priv3 cryptotypes.PrivKey | ||
| var antehandler sdk.AnteHandler | ||
| var defaultSignMode signing.SignMode | ||
| var accs []sdk.AccountI | ||
| var msgs []sdk.Msg |
x/auth/ante/unordered_test.go
Outdated
| enabledSignModes := []signing.SignMode{signing.SignMode_SIGN_MODE_DIRECT, signing.SignMode_SIGN_MODE_TEXTUAL, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON} | ||
| // Since TEXTUAL is not enabled by default, we create a custom TxConfig | ||
| // here which includes it. | ||
| txConfigOpts := authtx.ConfigOptions{ | ||
| TextualCoinMetadataQueryFn: txmodule.NewGRPCCoinMetadataQueryFn(suite.clientCtx), | ||
| EnabledSignModes: enabledSignModes, | ||
| } |
There was a problem hiding this comment.
removed some stuff slim down test copying
x/auth/module.go
Outdated
| @@ -218,6 +220,8 @@ type ModuleInputs struct { | |||
|
|
|||
| // LegacySubspace is used solely for migration of x/params managed parameters | |||
| LegacySubspace exported.Subspace `optional:"true"` | |||
|
|
|||
| EnableUnorderedTransactions bool `optional:"true"` | |||
There was a problem hiding this comment.
How can we verify that this works properly?
There was a problem hiding this comment.
the legacy system test should fail if this doesn't work, since this is opt-in
| @@ -108,6 +110,14 @@ type AccountKeeper struct { | |||
| UnorderedNonces collections.KeySet[collections.Pair[int64, []byte]] | |||
| } | |||
|
|
|||
| type InitOption func(*AccountKeeper) | |||
|
|
|||
| func WithUnorderedTransactions(enable bool) InitOption { | |||
There was a problem hiding this comment.
can we godoc this and what the side effects are
| @@ -93,6 +93,8 @@ type AccountKeeper struct { | |||
| permAddrs map[string]types.PermissionsForAddress | |||
| bech32Prefix string | |||
|
|
|||
| enableUnorderedTxs bool | |||
There was a problem hiding this comment.
can we doc what this does and how it is used
|
good to merge once @aaronc approves as well |
UPGRADE_GUIDE.md
Outdated
| ante.NewIncrementSequenceDecorator(options.AccountKeeper), | ||
| // NEW !! NEW !! NEW !! | ||
| ante.NewUnorderedTxDecorator(options.UnorderedNonceManager, options.UnorderedTxOptions...) | ||
| // ... snip |
There was a problem hiding this comment.
Maybe say "other decorators" instead of snip?
UPGRADING.md
Outdated
| ante.NewIncrementSequenceDecorator(options.AccountKeeper), | ||
| // NEW !! NEW !! NEW !! | ||
| ante.NewUnorderedTxDecorator(options.UnorderedNonceManager, options.UnorderedTxOptions...) | ||
| // ... snip ... |
x/auth/ante/sigverify.go
Outdated
| type SigVerificationDecoratorOption func(svd *SigVerificationDecorator) | ||
|
|
||
| // WithMaxTxTimeoutDuration sets the maximum TTL a transaction can define for unordered transactions. | ||
| func WithMaxTxTimeoutDuration(duration time.Duration) SigVerificationDecoratorOption { |
There was a problem hiding this comment.
I know it's a bit vebose, but I think we should be clearer that this only applies to unordered tx's. We don't enforce this max duration if the tx isn't unordered right?
| func WithMaxTxTimeoutDuration(duration time.Duration) SigVerificationDecoratorOption { | |
| func WithMaxUnorderedTimeoutDuration(duration time.Duration) SigVerificationDecoratorOption { |
| @@ -108,6 +112,17 @@ type AccountKeeper struct { | |||
| UnorderedNonces collections.KeySet[collections.Pair[int64, []byte]] | |||
| } | |||
|
|
|||
| type InitOption func(*AccountKeeper) | |||
There was a problem hiding this comment.
| type InitOption func(*AccountKeeper) | |
| type InitOption interface { | |
| IsManyPerContainerType() | |
| apply(*AccountKeeper) | |
| } | |
| type initOption func(*AccountKeeper) | |
| func (i initOption) apply(ak *AccountKeeper) { i(ak) } | |
| func (initOption) IsManyPerContainerType() {} |
There was a problem hiding this comment.
What does making this change give us given that they way @technicallyty has it works? This seems more confusing to me
There was a problem hiding this comment.
ManyPerContainer allows the options to be supplied from potentially many sources in the dependency graph. They don't have to all be supplied at once as an array. If we don't care about that it doesn't matter, but it follows the same pattern as BaseAppOption, CustomGetSigner, HandlerRoute where options might be collected from multiple sources.
There was a problem hiding this comment.
i think here we really only want one source (injecting in your app.go) right?
There was a problem hiding this comment.
supplying the options in separate places feels like it would be a rare case, and potentially messy for developers to do. i think it is ok as is
x/auth/module.go
Outdated
| @@ -218,6 +220,8 @@ type ModuleInputs struct { | |||
|
|
|||
| // LegacySubspace is used solely for migration of x/params managed parameters | |||
| LegacySubspace exported.Subspace `optional:"true"` | |||
|
|
|||
| KeeperOptions []keeper.InitOption `optional:"true"` | |||
There was a problem hiding this comment.
| KeeperOptions []keeper.InitOption `optional:"true"` | |
| KeeperOptions []keeper.InitOption |
There was a problem hiding this comment.
why do we not need this tag?
There was a problem hiding this comment.
ManyPerContainer dependencies are always optional
| return func(ak *AccountKeeper) { | ||
| ak.enableUnorderedTxs = enable | ||
| } |
There was a problem hiding this comment.
| return func(ak *AccountKeeper) { | |
| ak.enableUnorderedTxs = enable | |
| } | |
| return initOption(func(ak *AccountKeeper) { | |
| ak.enableUnorderedTxs = enable | |
| }) |
simapp/app_di.go
Outdated
| @@ -139,7 +139,7 @@ func NewSimApp( | |||
| // with the prefix defined in the auth module configuration. | |||
| // | |||
| // func() address.Codec { return <- custom address codec type -> } | |||
|
|
|||
| []authkeeper.InitOption{authkeeper.WithUnorderedTransactions(true)}, | |||
There was a problem hiding this comment.
| []authkeeper.InitOption{authkeeper.WithUnorderedTransactions(true)}, | |
| authkeeper.WithUnorderedTransactions(true), |
Description
changes how unordered transactions ante handling works
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues. Your PR will not be merged unless you satisfy
all of these items.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Refactor