refactor: PoC for unordered txs per new spec #24010

technicallyty · 2025-03-14T21:55:57Z

Description

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.

I have...

included the correct type prefix in the PR title, you can find examples of the prefixes below:
confirmed ! in the type prefix if API or client breaking change
targeted the correct branch (see PR Targeting)
provided a link to the relevant issue or specification
reviewed "Files changed" and left comments if necessary
included the necessary unit and integration tests
added a changelog entry to CHANGELOG.md
updated the relevant documentation or specification, including comments for documenting Go code
confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.

I have...

confirmed the correct type prefix in the PR title
confirmed all author checklist items have been addressed
reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

…refactor

github-actions · 2025-03-19T20:31:40Z

    ⚠️ **govulncheck found vulnerabilities:**


    >mkdir -p /home/runner/work/cosmos-sdk/cosmos-sdk/build/

GOBIN=/home/runner/work/cosmos-sdk/cosmos-sdk/build go install golang.org/x/vuln/cmd/govulncheck@latest
go: downloading golang.org/x/vuln v1.1.4
go: downloading golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7
go: downloading golang.org/x/mod v0.22.0
go: downloading golang.org/x/tools v0.29.0
/home/runner/work/cosmos-sdk/cosmos-sdk/build/govulncheck ./...
=== Symbol Results ===

Vulnerability #1: GO-2025-3443
CometBFT allows a malicious peer to stall the network by disseminating
seemingly valid block parts in github.com/cometbft/cometbft
More info: https://pkg.go.dev/vuln/GO-2025-3443
Module: github.com/cometbft/cometbft
Found in: github.com/cometbft/[email protected]
Fixed in: github.com/cometbft/[email protected]
Example traces found:
#1: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#2: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#3: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto
#4: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto

Your code is affected by 1 vulnerability from 1 module.
This scan also found 0 vulnerabilities in packages you import and 2
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.
make: *** [Makefile:155: vulncheck] Error 3

github-actions · 2025-03-19T20:35:38Z

    ⚠️ **govulncheck found vulnerabilities:**


    >mkdir -p /home/runner/work/cosmos-sdk/cosmos-sdk/build/

GOBIN=/home/runner/work/cosmos-sdk/cosmos-sdk/build go install golang.org/x/vuln/cmd/govulncheck@latest
go: downloading golang.org/x/vuln v1.1.4
go: downloading golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7
go: downloading golang.org/x/mod v0.22.0
go: downloading golang.org/x/tools v0.29.0
/home/runner/work/cosmos-sdk/cosmos-sdk/build/govulncheck ./...
=== Symbol Results ===

Vulnerability #1: GO-2025-3443
CometBFT allows a malicious peer to stall the network by disseminating
seemingly valid block parts in github.com/cometbft/cometbft
More info: https://pkg.go.dev/vuln/GO-2025-3443
Module: github.com/cometbft/cometbft
Found in: github.com/cometbft/[email protected]
Fixed in: github.com/cometbft/[email protected]
Example traces found:
#1: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#2: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#3: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto
#4: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto

Your code is affected by 1 vulnerability from 1 module.
This scan also found 0 vulnerabilities in packages you import and 2
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.
make: *** [Makefile:155: vulncheck] Error 3

aaronc · 2025-03-19T21:18:47Z

x/auth/ante/expected_keepers.go

@@ -19,6 +20,13 @@ type AccountKeeper interface {
 	AddressCodec() address.Codec
 }

+// UnorderedSequenceManager defines the contract needed for UnorderedSequence management.
+type UnorderedSequenceManager interface {


How about UnorderedNonce? It's not really a sequence

makes sense, updated to nonce

aaronc · 2025-03-19T21:20:36Z

x/auth/ante/unordered.go

-const DefaultSha256GasCost = 25
+const (
+	// MaxTimeoutDuration defines the maximum TTL a transaction can define.
+	MaxTimeoutDuration = 10 * time.Minute


I think this should be configurable in the ante handler options with a default

ok, added the ability to set options on the ante. let me know if that works

aaronc · 2025-03-19T21:23:46Z

x/auth/keeper/keeper.go

+
+// ContainsUnorderedSequence reports whether the sender has used this timestamp already.
+func (ak AccountKeeper) ContainsUnorderedSequence(ctx sdk.Context, sender []byte, timestamp time.Time) (bool, error) {
+	return ak.UnorderedSequences.Has(ctx, collections.Join(uint64(timestamp.UnixNano()), sender))


Why do we need to cast to uint64, can we just use int64 as the collections type?

Also FWIW UnixNano will cause this code to break after 2262 😜

oops, switched to int64 keys

github-actions · 2025-03-19T22:19:13Z

    ⚠️ **govulncheck found vulnerabilities:**


    >mkdir -p /home/runner/work/cosmos-sdk/cosmos-sdk/build/

GOBIN=/home/runner/work/cosmos-sdk/cosmos-sdk/build go install golang.org/x/vuln/cmd/govulncheck@latest
go: downloading golang.org/x/vuln v1.1.4
go: downloading golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7
go: downloading golang.org/x/mod v0.22.0
go: downloading golang.org/x/tools v0.29.0
/home/runner/work/cosmos-sdk/cosmos-sdk/build/govulncheck ./...
=== Symbol Results ===

Vulnerability #1: GO-2025-3443
CometBFT allows a malicious peer to stall the network by disseminating
seemingly valid block parts in github.com/cometbft/cometbft
More info: https://pkg.go.dev/vuln/GO-2025-3443
Module: github.com/cometbft/cometbft
Found in: github.com/cometbft/[email protected]
Fixed in: github.com/cometbft/[email protected]
Example traces found:
#1: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#2: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#3: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto
#4: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto

Your code is affected by 1 vulnerability from 1 module.
This scan also found 0 vulnerabilities in packages you import and 2
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.
make: *** [Makefile:155: vulncheck] Error 3

github-actions · 2025-03-19T23:14:35Z

    ⚠️ **govulncheck found vulnerabilities:**


    >mkdir -p /home/runner/work/cosmos-sdk/cosmos-sdk/build/

GOBIN=/home/runner/work/cosmos-sdk/cosmos-sdk/build go install golang.org/x/vuln/cmd/govulncheck@latest
go: downloading golang.org/x/vuln v1.1.4
go: downloading golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7
go: downloading golang.org/x/mod v0.22.0
go: downloading golang.org/x/tools v0.29.0
/home/runner/work/cosmos-sdk/cosmos-sdk/build/govulncheck ./...
=== Symbol Results ===

Vulnerability #1: GO-2025-3443
CometBFT allows a malicious peer to stall the network by disseminating
seemingly valid block parts in github.com/cometbft/cometbft
More info: https://pkg.go.dev/vuln/GO-2025-3443
Module: github.com/cometbft/cometbft
Found in: github.com/cometbft/[email protected]
Fixed in: github.com/cometbft/[email protected]
Example traces found:
#1: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#2: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#3: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto
#4: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto

Your code is affected by 1 vulnerability from 1 module.
This scan also found 0 vulnerabilities in packages you import and 2
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.
make: *** [Makefile:155: vulncheck] Error 3

github-actions · 2025-03-19T23:39:24Z

    ⚠️ **govulncheck found vulnerabilities:**


    >mkdir -p /home/runner/work/cosmos-sdk/cosmos-sdk/build/

GOBIN=/home/runner/work/cosmos-sdk/cosmos-sdk/build go install golang.org/x/vuln/cmd/govulncheck@latest
go: downloading golang.org/x/vuln v1.1.4
go: downloading golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7
go: downloading golang.org/x/mod v0.22.0
go: downloading golang.org/x/tools v0.29.0
/home/runner/work/cosmos-sdk/cosmos-sdk/build/govulncheck ./...
=== Symbol Results ===

Vulnerability #1: GO-2025-3443
CometBFT allows a malicious peer to stall the network by disseminating
seemingly valid block parts in github.com/cometbft/cometbft
More info: https://pkg.go.dev/vuln/GO-2025-3443
Module: github.com/cometbft/cometbft
Found in: github.com/cometbft/[email protected]
Fixed in: github.com/cometbft/[email protected]
Example traces found:
#1: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#2: testutil/network/util.go:74:24: network.startInProcess calls service.BaseService.Start, which eventually calls types.Part.ValidateBasic
#3: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto
#4: client/rpc/block.go:56:36: rpc.QueryBlocks calls local.Local.BlockSearch, which eventually calls types.PartFromProto

Your code is affected by 1 vulnerability from 1 module.
This scan also found 0 vulnerabilities in packages you import and 2
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.
make: *** [Makefile:155: vulncheck] Error 3

aljo242 · 2025-03-20T00:05:20Z

simapp/app.go

@@ -536,6 +532,7 @@ func NewSimApp(
 	// NOTE: upgrade module is required to be prioritized
 	app.ModuleManager.SetOrderPreBlockers(
 		upgradetypes.ModuleName,
+		authtypes.ModuleName,


note to make sure we have this added in the upgrade guide for adding utx

opened #24078

x/auth/ante/unordered.go

aljo242 · 2025-03-20T14:52:05Z

x/auth/keeper/keeper.go

+// -------------------------------------
+
+// ContainsUnorderedNonce reports whether the sender has used this timestamp already.
+func (ak AccountKeeper) ContainsUnorderedNonce(ctx sdk.Context, sender []byte, timestamp time.Time) (bool, error) {


to me it is more clear if the timestamp is called timeout because thats what they always are right?

done in comments, renames, gas

x/auth/module.go

aljo242

Approving - I left one small comment and looks like we just have a formatting lint

x/auth/ante/unordered.go

…refactor

…hub.com/cosmos/cosmos-sdk into technicallyty/23976/unordered-tx-refactor

aaronc

The API looks good but left one comment. I did not get a chance to thoroughly review the tests yet. I'd like to do that before ACKing but otherwise ACK

aaronc · 2025-03-21T20:09:13Z

x/auth/ante/expected_keepers.go

+type UnorderedNonceManager interface {
+	RemoveExpiredUnorderedNonces(ctx sdk.Context) error
+	AddUnorderedNonce(ctx sdk.Context, sender []byte, timestamp time.Time) error
+	ContainsUnorderedNonce(ctx sdk.Context, sender []byte, timestamp time.Time) (bool, error)


Probably feasible to collapse this into a single TryAdd method - that's a bit safer, but this is okay. TryAdd is safer because Add should actually fail if the entry exists and we want to avoid calling Has twice

made adjustment here: TryAdd

makes the ante code simpler 👍

aaronc

I think we should also check that if a sequence is provided by any signer and unordered is also true, then this is an error. What do you think?

aaronc · 2025-03-21T21:49:55Z

x/auth/keeper/unordered_tx_test.go

+	reset := func() {
+		mockStoreKey := storetypes.NewKVStoreKey("test")
+		storeService := runtime.NewKVStoreService(mockStoreKey)
+		ctx = testutil.DefaultContextWithDB(t, mockStoreKey, storetypes.NewTransientStoreKey("transient_test")).Ctx


Why TransientStoreKey? It should be a regular one

the 2nd key is intended to be a transient key. this is the function signature:

DefaultContextWithDB(tb testing.TB, key, tkey storetypes.StoreKey)

technicallyty · 2025-03-21T22:14:53Z

I think we should also check that if a sequence is provided by any signer and unordered is also true, then this is an error. What do you think?

hmm.. this sounds sane, but this would require some careful consideration of where sequences are set/signed and changing this based on the unordered in txbody. if we really want to do this, should probably be another PR

aaronc · 2025-03-21T22:55:59Z

I think we should also check that if a sequence is provided by any signer and unordered is also true, then this is an error. What do you think?

hmm.. this sounds sane, but this would require some careful consideration of where sequences are set/signed and changing this based on the unordered in txbody. if we really want to do this, should probably be another PR

Separate PR is fine, but conceptually does it make sense that if someone sets a sequence plus unordered that should be interpreted as user error? Basically unclear intent

technicallyty added 5 commits March 12, 2025 13:28

WIP unordered refactor

007e9f1

implement multiple signers handling

d0c7237

use collections..

e0cef44

make UTXs more behind the scenes...

fd79847

set ttl max

304bfd4

github-actions bot added C:x/auth Type: ADR labels Mar 14, 2025

ok

d8aa37e

technicallyty mentioned this pull request Mar 17, 2025

docs: adr-070 unordered transactions refactor #23974

Merged

12 tasks

technicallyty added 4 commits March 17, 2025 16:30

tests

9375652

make simapp deal with the optional unordered thing

4bd3769

fix

f5ca415

changes from adr

e163124

technicallyty marked this pull request as ready for review March 18, 2025 22:02

technicallyty requested a review from a team as a code owner March 18, 2025 22:02

technicallyty added 3 commits March 19, 2025 09:54

use GetSigners for addresses

bfc6c46

move everyting to keeper, rename stuff

164ce70

Merge branch 'release/v0.53.x' into technicallyty/23976/unordered-tx-…

6fd8b98

…refactor

github-actions bot removed the Type: ADR label Mar 19, 2025

revert no longer needed testutil change

a2949e7

aaronc reviewed Mar 19, 2025

View reviewed changes

technicallyty added 5 commits March 19, 2025 14:41

use int64 not uint64

87ae5c6

unordered sequences -> unordered nonces rename

6512d5a

optional timeout set

6f2a710

add some docs

5adc7c2

test

4202b27

fix test util

0bebc3b

linter

afc389f

aljo242 reviewed Mar 20, 2025

View reviewed changes

technicallyty added 2 commits March 20, 2025 09:41

comments, renames, gas

0ca4fca

lingering unordered sequence verbiage

7816fe5

This comment was marked as duplicate.

Sign in to view

options

c8805af

This comment was marked as resolved.

Sign in to view

This comment was marked as spam.

Sign in to view

aljo242 approved these changes Mar 20, 2025

View reviewed changes

x/auth/ante/unordered.go Outdated Show resolved Hide resolved

technicallyty added 10 commits March 20, 2025 12:10

linter

1f52d1f

changes

5acf257

Merge branch 'release/v0.53.x' into technicallyty/23976/unordered-tx-…

070e3c9

…refactor

Merge branch 'release/v0.53.x' into technicallyty/23976/unordered-tx-…

5387be1

…refactor

Merge branch 'technicallyty/23976/unordered-tx-refactor' of ssh://git…

2deed33

…hub.com/cosmos/cosmos-sdk into technicallyty/23976/unordered-tx-refactor

fix lint

450e2dc

lint

a93d895

liter

ec0157e

lint order

9a5a588

lint

2db0155

technicallyty requested a review from aaronc March 21, 2025 20:03

aaronc reviewed Mar 21, 2025

View reviewed changes

technicallyty added 2 commits March 21, 2025 14:00

TryAdd

a4a40c2

structure

f38e629

aaronc approved these changes Mar 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: PoC for unordered txs per new spec #24010

refactor: PoC for unordered txs per new spec #24010

technicallyty commented Mar 14, 2025 •

edited

Loading

github-actions bot commented Mar 19, 2025

github-actions bot commented Mar 19, 2025

aaronc Mar 19, 2025

technicallyty Mar 19, 2025

aaronc Mar 19, 2025

technicallyty Mar 19, 2025

aaronc Mar 19, 2025

technicallyty Mar 19, 2025

github-actions bot commented Mar 19, 2025

github-actions bot commented Mar 19, 2025

github-actions bot commented Mar 19, 2025

aljo242 Mar 20, 2025

technicallyty Mar 20, 2025

aljo242 Mar 20, 2025

technicallyty Mar 20, 2025

This comment was marked as duplicate.

This comment was marked as resolved.

This comment was marked as spam.

aljo242 left a comment

aaronc left a comment

aaronc Mar 21, 2025

technicallyty Mar 21, 2025

aaronc left a comment

aaronc Mar 21, 2025

technicallyty Mar 21, 2025

technicallyty commented Mar 21, 2025

aaronc commented Mar 21, 2025

refactor: PoC for unordered txs per new spec #24010

Are you sure you want to change the base?

refactor: PoC for unordered txs per new spec #24010

Conversation

technicallyty commented Mar 14, 2025 • edited Loading

Description

Author Checklist

Reviewers Checklist

github-actions bot commented Mar 19, 2025

github-actions bot commented Mar 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 19, 2025

github-actions bot commented Mar 19, 2025

github-actions bot commented Mar 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

This comment was marked as resolved.

This comment was marked as spam.

aljo242 left a comment

Choose a reason for hiding this comment

aaronc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

technicallyty commented Mar 21, 2025

aaronc commented Mar 21, 2025

technicallyty commented Mar 14, 2025 •

edited

Loading