Skip to content

Convert account Number to bigint - #1956

Merged
codingki merged 2 commits into
mainfrom
eric/acc-id
May 4, 2026
Merged

codingki merged 2 commits into
mainfrom
eric/acc-id

Conversation

@Eric-Warehime

Copy link
Copy Markdown
Contributor

Breaking change converts account number to use bigint to.

@Eric-Warehime
Eric-Warehime requested a review from codingki April 30, 2026 17:59
@greptile-apps

greptile-apps Bot commented Apr 30, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR migrates accountNumber from number to bigint across the @cosmjs/stargate, @cosmjs/amino, and @cosmjs/proto-signing packages to handle Cosmos SDK 0.53+ accounts whose GenerateID() counter can exceed Number.MAX_SAFE_INTEGER. The Uint53.fromString call in signdoc.ts is correctly replaced with Uint64.fromString, which internally uses native BigInt and preserves precision for 64-bit values.

Confidence Score: 4/5

Safe to merge — all signing paths correctly propagate bigint and no precision loss can occur.

Only P2 findings remain (test fixture inconsistency in proto-signing testutils). All core signing flows — signDirect, signAmino, getSequence — correctly handle the bigint type end-to-end.

packages/proto-signing/src/testutils.ts — three test vectors still use number literal 1 instead of 1n

Important Files Changed

Filename Overview
packages/stargate/src/accounts.ts Account.accountNumber changed to bigint; uses toBigInt() from Uint64 which handles values above Number.MAX_SAFE_INTEGER correctly
packages/amino/src/signdoc.ts Accepts bigint for accountNumber and switches from Uint53 to Uint64 for safe >53-bit conversion; correct change
packages/stargate/src/signingstargateclient.ts SignerData.accountNumber changed to bigint; signDirect and signAmino paths both correctly forward bigint to their respective makeSignDoc functions
packages/stargate/src/stargateclient.ts SequenceResponse.accountNumber changed to bigint; getSequence() propagates the bigint from Account cleanly
packages/proto-signing/src/signing.ts accountNumber parameter widened to number
packages/stargate/src/testutils.ts Test fixtures updated to bigint literals (16n, 0n) consistently
packages/cosmwasm/src/testutils.ts Test fixtures updated to bigint literals (16n, 0n) consistently
packages/stargate/src/modules/auth/queries.spec.ts Updated to use unused.accountNumber (now bigint) directly; comparison updated to use >= 0n
packages/stargate/src/stargateclient.spec.ts accountNumber comparisons updated to use bigint comparison (>= 0n) instead of toBeGreaterThanOrEqual
packages/stargate/src/accounts.spec.ts Test expectation updated to use 56n bigint literal; aligns with new Account.accountNumber type

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[StargateClient.getSequence] -->|"accountNumber: bigint"| B[SequenceResponse]
    B --> C{Signer type}
    C -->|OfflineDirectSigner| D[signDirect]
    C -->|AminoSigner| E[signAmino]
    D -->|"makeSignDoc(…, accountNumber: bigint)"| F["proto-signing\nBigInt(accountNumber) → SignDoc"]
    E -->|"makeSignDocAmino(…, accountNumber: bigint)"| G["amino\nUint64.fromString(accountNumber.toString()) → string"]
    H[accountFromAny] -->|"Uint64.toBigInt()"| I["Account.accountNumber: bigint"]
    I --> A
Loading

Comments Outside Diff (1)

  1. packages/proto-signing/src/testutils.ts, line 19 (link)

    P2 Test fixture accountNumber not updated to bigint

    The three test vectors in this file still use accountNumber: 1 (a number) while the rest of the codebase now uses bigint literals. makeSignDoc accepts number | bigint so there's no runtime error, but it's inconsistent with the pattern established across packages/stargate/src/testutils.ts and packages/cosmwasm/src/testutils.ts. Consider updating all three vectors to 1n for uniformity.

Reviews (1): Last reviewed commit: "Convert account ID to bigint" | Re-trigger Greptile

@codingki
codingki merged commit e39b5ff into main May 4, 2026
15 checks passed
@codingki
codingki deleted the eric/acc-id branch May 4, 2026 14:28
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.

2 participants