Skip to content

fix: F-2026-18789 | [Dual Defense] First Fund Migration id=0 Is Unvotable - #345

Merged
0xNilesh merged 1 commit into
audit-fixesfrom
F-2026-18789
Aug 26, 2026
Merged

fix: F-2026-18789 | [Dual Defense] First Fund Migration id=0 Is Unvotable#345
0xNilesh merged 1 commit into
audit-fixesfrom
F-2026-18789

Conversation

@0xNilesh

Copy link
Copy Markdown
Member

Issue

InitiateFundMigration allocated migration ids straight off a collections.Sequence, whose first value is 0:

migrationId, err := k.NextMigrationId.Next(ctx)   // first call returns 0

while MsgVoteFundMigration.ValidateBasic treats 0 as "unset" and rejects it (x/utss/types/msg_vote_fund_migration.go:18):

if msg.MigrationId == 0 { return errors.Wrap(sdkerrors.ErrInvalidRequest, "migration_id is required") }

So the first fund migration on any chain got id=0 and was unvotable — every universal validator's vote died in ValidateBasic before the keeper was ever reached. The migration then:

  • never reached quorum, so it was never finalized,
  • never left PendingMigrations (removal only happens after a finalized vote),
  • and made InitiateFundMigration refuse every later migration for that chain ("pending migration already exists for chain ...").

The fund-migration lane for that chain was bricked with no recovery short of an upgrade. audit-fixes is a fresh-genesis branch, so this is deterministic on first use.

Fix

Allocate ids as sequence + 1 (x/utss/keeper/msg_initiate_fund_migration.go):

seq, err := k.NextMigrationId.Next(ctx)
if err != nil { ... }
migrationId := seq + 1

Stored ids now start at 1 and 0 stays reserved for "unset", so the ValidateBasic guard remains a real check rather than being loosened. This is a pure code fix — it holds regardless of genesis content or existing state, and needs no upgrade handler or state migration.

Rejected alternatives: seeding the counter from genesis (depends on genesis being right), and allowing id == 0 through ValidateBasic (removes a real validation and makes 0 ambiguous with "unset").

Genesis round-trips cleanly: ExportGenesis writes NextMigrationId.Peek(), InitGenesis restores it, and the next allocation is still strictly greater than every id already in state.

Tests

test/integration/utss/fund_migration_test.go:

  • TestInitiateFundMigration_FirstMigrationIsVotable — three independent subtests over the very first migration a fresh chain allocates (the fixture asserts a virgin sequence first, so the case is genuinely the one that used to be unreachable):
    • the id is never the reserved 0, and the record really is stored under it;
    • a MsgVoteFundMigration carrying that id passes ValidateBasic — the exact gate that bricked the lane;
    • all three validators vote through the msg server, the migration reaches COMPLETED, leaves PendingMigrations, and a second migration for the same chain is then accepted under id 2.
  • TestVoteFundMigration_ZeroMigrationIdStaysRejected — pins the other half of the contract: 0 still means "unset" and is still rejected.
  • TestFundMigrationIdsSurviveGenesisRoundTrip — export/import must not re-issue an id that is already taken.
  • Existing TestInitiateFundMigration/Successfully initiates fund migration updated from uint64(0) to uint64(1), and the "nothing left behind" check in the fee-rejection subtest changed from Get(ctx, 0) (vacuous once 0 is never used) to a walk asserting the collection is empty.

Mutation check

With the fix reverted and the tests kept, four of them fail — including on the real bug surface:

--- FAIL: TestInitiateFundMigration/Successfully_initiates_fund_migration (0.06s)
        Error:  Not equal:
                expected: 0x1
                actual  : 0x0

--- FAIL: TestInitiateFundMigration_FirstMigrationIsVotable/the_first_id_is_never_the_reserved_0 (0.06s)
        Error:  Should not be zero, but was 0
        Messages: the first migration id must never be 0: MsgVoteFundMigration.ValidateBasic rejects 0 as unset

--- FAIL: TestInitiateFundMigration_FirstMigrationIsVotable/a_vote_on_the_first_migration_passes_ValidateBasic (0.05s)
        Error:  Received unexpected error:
                x/utss/types.(*MsgVoteFundMigration).ValidateBasic
                        x/utss/types/msg_vote_fund_migration.go:19
                migration_id is required: invalid request
        Messages: a vote on the first migration must survive ValidateBasic

--- FAIL: TestInitiateFundMigration_FirstMigrationIsVotable/the_first_migration_finalizes_and_unblocks_the_chain (0.05s)
        Error:  Received unexpected error:
                x/utss/types.(*MsgVoteFundMigration).ValidateBasic
                        x/utss/types/msg_vote_fund_migration.go:19
                migration_id is required: invalid request

--- FAIL: TestFundMigrationIdsSurviveGenesisRoundTrip (0.04s)
        Error:  Not equal:
                expected: 0x1
                actual  : 0x0

Restoring the fix returns all of them to PASS.

Full suite

go test -mod=readonly -p 1 -count=1 -tags="ledger test_ledger_mock test" ./x/... ./test/integration/... — 15 ok, 0 FAIL, exit 0:

ok  github.com/pushchain/push-chain-node/x/uexecutor/keeper           1.087s
ok  github.com/pushchain/push-chain-node/x/uexecutor/types            0.628s
ok  github.com/pushchain/push-chain-node/x/uregistry/keeper           1.173s
ok  github.com/pushchain/push-chain-node/x/uregistry/migrations/v3    1.075s
ok  github.com/pushchain/push-chain-node/x/uregistry/types            0.625s
ok  github.com/pushchain/push-chain-node/x/utss/keeper                1.130s
ok  github.com/pushchain/push-chain-node/x/utss/types                 1.089s
ok  github.com/pushchain/push-chain-node/x/uvalidator/keeper          1.086s
ok  github.com/pushchain/push-chain-node/x/uvalidator/types           1.317s
ok  github.com/pushchain/push-chain-node/test/integration/ante        1.473s
ok  github.com/pushchain/push-chain-node/test/integration/uexecutor  12.372s
ok  github.com/pushchain/push-chain-node/test/integration/upgrades    2.372s
ok  github.com/pushchain/push-chain-node/test/integration/uregistry   1.738s
ok  github.com/pushchain/push-chain-node/test/integration/utss        4.730s
ok  github.com/pushchain/push-chain-node/test/integration/uvalidator  7.839s

Ids came straight off collections.Sequence, whose first value is 0, while
MsgVoteFundMigration.ValidateBasic rejects migration_id == 0 as "unset".
The first migration on a fresh chain was therefore unvotable, never left
PendingMigrations, and blocked every later migration for that chain.

Allocate as sequence + 1: stored ids start at 1 and 0 stays reserved for
"unset", so the ValidateBasic guard remains a real check.

F-2026-18789
@0xNilesh
0xNilesh merged commit d5d8ee8 into audit-fixes Aug 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant