Skip to content

fix(storage): keep token amounts exact instead of truncating to uint64 - #2221

Open
AkramBitar wants to merge 2 commits into
mainfrom
fix-token-amount-uint64-truncation-2021
Open

fix(storage): keep token amounts exact instead of truncating to uint64#2221
AkramBitar wants to merge 2 commits into
mainfrom
fix-token-amount-uint64-truncation-2021

Conversation

@AkramBitar

@AkramBitar AkramBitar commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #2021

The problem

Every token has an amount. We store it twice: once as text (the source of truth) and once
as a number, so the database can add amounts up.

The text version can hold very large amounts. The number version was declared as a type
that stops at 20 digits. When a token's amount was bigger than that, the conversion did
not fail or warn — it just kept the leftover part and stored a completely different
number. The two copies then disagreed, and nothing said so.

Nobody notices today because no query uses the number column yet. As soon as one does —
for example to pick which tokens to spend — a huge token could look tiny and the wrong
tokens would be chosen.

The fix

  • Store the amount as a big number that has no size limit, so the value written matches
    the source of truth exactly.
  • If an amount is missing, or too large even for the database column, the write is now
    rejected with a clear error instead of quietly saving something wrong.
  • Added tests: one saves an amount larger than the old limit and reads it back unchanged,
    one checks the bad cases are refused.

Note for reviewers

No shipped driver can produce such an amount today — fabtoken rejects a precision above 64
bits (fabtoken/v1/setup/setup.go:256), zkatdlog supports 16/32/64
(zkatdlog/nogh/v1/setup/setup.go:42) — so this is a prerequisite for #2020, not a live
incident.

Also: only Postgres keeps such amounts exactly. SQLite gives a NUMERIC column NUMERIC
affinity and converts an integer wider than int64 to REAL, so it cannot store them exactly
whatever the Go type is — after this change that errors on read instead of silently
corrupting. That needs a per-driver column type plus a migration, and it matters for #2020's
index and ORDER BY on amount; happy to open a separate issue if you'd like it tracked.

Checks: make checks and make lint clean; storage and token service tests pass,
including the PostgreSQL suite.

@AkramBitar AkramBitar added this to the Q3/26 milestone Aug 13, 2026
@AkramBitar AkramBitar added bug Something isn't working db storage labels Aug 13, 2026
@AkramBitar AkramBitar self-assigned this Aug 13, 2026
TokenRecord.Amount was uint64 while the backing column is
amount NUMERIC(78, 0), sized for a 256-bit value. AppendToken narrowed the
quantity with big.Int.Uint64(), which returns the low 64 bits with no error
for anything larger, so for a token whose quantity exceeds 2^64-1 the amount
column silently received a wrapped value while the authoritative hex quantity
column stayed correct.

Widen the field to *big.Int and insert its exact decimal representation,
following the pattern already used for movement and transaction records.
StoreToken now refuses a nil amount (the column is NOT NULL) and one wider
than maxAmountBits, so a value the column cannot hold surfaces as an error
instead of a row whose two amount columns disagree. No consumer relied on the
uint64 type; the only call sites were the test fixtures updated here.

Note that only Postgres can hold such a value: SQLite gives a NUMERIC column
NUMERIC affinity and converts an integer literal too large for int64 to REAL.
With this change that fails loudly on read rather than corrupting silently.
The exact round-trip case is therefore wired into the Postgres suite, while
the write-time validation case runs on every backend.

Fixes #2021

Signed-off-by: AkramBitar <akram@il.ibm.com>
@AkramBitar
AkramBitar force-pushed the fix-token-amount-uint64-truncation-2021 branch from 18556d1 to b63f3fa Compare August 13, 2026 15:00
@AkramBitar
AkramBitar requested review from Effi-S, SaidAltury-ibm and adecaro and removed request for Effi-S August 13, 2026 15:18
The amount width check was duplicated across StoreToken, AddTransaction and
AddMovement, and the test asserted on a substring of its message. Extract
validateAmount and return ErrAmountMissing / ErrAmountOutOfRange from the
driver package, following the ErrTokenDoesNotExist convention, so callers and
tests can match with errors.Is instead of on message text.

Signed-off-by: AkramBitar <akram@il.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working db storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TokenRecord.Amount is uint64 but the column is NUMERIC(78,0), silently truncating token amounts on write

1 participant