Skip to content

feat(store): arbitrary-precision amounts for EVM compatibility - #236

Open
Nexha-dev wants to merge 1 commit into
Octo-Protocol-org:mainfrom
Nexha-dev:feat/arbitrary-precision-amounts
Open

feat(store): arbitrary-precision amounts for EVM compatibility#236
Nexha-dev wants to merge 1 commit into
Octo-Protocol-org:mainfrom
Nexha-dev:feat/arbitrary-precision-amounts

Conversation

@Nexha-dev

Copy link
Copy Markdown

Summary

  • Adds octo-chain::Amount, an arbitrary-precision non-negative base-unit integer wrapping primitive_types::U256, with fallible TryFrom<i64>/to_i64()/to_u64()/TryFrom<BigDecimal> conversions — every narrowing conversion rejects out-of-range, negative, or non-integer input instead of truncating or wrapping.
  • Adds migration 0021_numeric_amounts.sql: amount_base_units NUMERIC(78,0) on transactions/withdrawals, backfilled from amount_stroops, with a NOT VALID + VALIDATE CONSTRAINT > 0 check so it never takes a long-lived lock. amount_stroops stays the column of record for at least one release (rollback path).
  • Amount's Serialize/Deserialize round-trips through JSON strings only — the deserializer has no visit_u64/visit_i64/visit_f64 overrides, so a bare JSON number is rejected outright, not silently coerced (a uint256 in a JSON number loses precision in every JS client, per AD-3).
  • Replaces format_amount's f64 round-trip (crates/api/src/routes/submit.rs) with exact integer/modulo arithmetic parameterised by a decimals count — matters once amounts exceed 2^53, which 18-decimal EVM amounts routinely do.
  • Extends the wallet-core/crypto clippy::cast_possible_truncation / cast_sign_loss lint wall to octo-chain, octo-store, octo-ingest, and octo-api.
  • Updates octo-store's record_deposit/record_withdrawal_transaction/create_withdrawal to dual-write amount_base_units via a fallible Amount::try_from conversion, and octo-ingest's amount parser docs/tests to lock in that every value it can produce fits Amount without error.
  • Updates docs/api.md / docs/openapi.yaml to document the new internal column and make clear the public API is unchanged in this PRamount_stroops still serialises as a JSON number; the string-typed cutover is a separate, versioned breaking change tracked in feat(api): Multi-chain API surface, OpenAPI, and webhooks #227.

Why primitive-types over alloy-primitives/ruint

See the module doc in crates/chain/src/amount.rs. Short version: the workspace pins rust-version = "1.84"; primitive-types has a long track record at a low MSRV with no churn that would fight that pin. alloy-primitives is the type the alloy EVM RPC client ecosystem is built on, but which EVM RPC client octo uses is explicitly out of scope here (#217) — pulling it in now would silently pre-decide that choice and drag its dependency tree into crates (like octo-store) that have nothing to do with EVM RPC. primitive_types::U256's Display/FromStr are decimal out of the box, matching the JSON-string, no-floats representation this type needs, where alloy_primitives::U256 defaults to hex. Amount's public API (string/BigDecimal/i64 conversions) does not need to change if a later issue standardizes on ruint/alloy-primitives for the EVM RPC path — only Amount's private backing type would, which is the point of the newtype.

Remaining as i64 casts in touched crates

  • crates/ingest/src/lib.rs:164 and crates/api/src/auth.rs:644: Duration::as_secs() as i64 — pre-existing, untouched by this diff, and a Unix timestamp, not an amount. Same-width u64 -> i64 is clippy::cast_possible_wrap, not cast_possible_truncation/cast_sign_loss, so it's unaffected by (and doesn't defeat the purpose of) the new lint wall.
  • crates/chain/src/amount.rs: the two as casts left (i64::MAX as u64 in to_i64's bound check, and value as u64 in TryFrom<i64>) are each preceded by an explicit range/sign guard that makes the cast lossless, and are commented explaining why; #[allow] is scoped to the single expression, not the crate.

Test plan

  • cargo test -p octo-chain -p octo-api -p octo-ingest --lib — includes the new Amount property tests (roundtrip_via_bigdecimal, roundtrip_via_json_string, full uint256 range via proptest), explicit boundary tests (0, 1, i64::MAX, i64::MAX + 1, u64::MAX, 2^256 - 1), the JSON-number-rejected regression test, and the format_amount integer-arithmetic tests.
  • cargo test -p octo-store --tests against a live Postgres — includes i64_max_stroops_round_trips_through_amount_base_units, one_eth_in_wei_round_trips_through_store_and_json_bit_identically (the case the old i64 schema could not represent), withdrawal_dual_writes_amount_base_units, and the migration idempotency/expected-version-set tests.
  • cargo clippy --workspace --all-targets -- -D warnings — clean.
  • cargo build --workspace --all-targets — clean.

Coordination

This is a breaking-API-change preparation PR, not the cutover itself: amount_stroops still serialises as a JSON number, unchanged, in this PR. Per the issue guidelines, the actual string-typed public API cutover must be versioned and coordinated with #227 before that follow-up merges.

Closes #215

i64 stroops assume 7 decimals and 64-bit range; ETH is 18 decimals and
ERC-20 amounts are uint256, so ~9.3 ETH would silently overflow.

Adds octo-chain::Amount, wrapping primitive_types::U256 (chosen over
alloy-primitives/ruint for MSRV compatibility and decimal Display/FromStr
out of the box, without pre-deciding the EVM RPC stack from Octo-Protocol-org#217 — see
the module docs in crates/chain/src/amount.rs for the full rationale).
Every narrowing conversion (TryFrom<i64>, to_i64, to_u64,
TryFrom<BigDecimal>) is fallible and rejects out-of-range, negative, or
non-integer input instead of truncating or wrapping.

Storage gains amount_base_units NUMERIC(78,0) (migration
0021_numeric_amounts.sql), dual-written alongside amount_stroops for at
least one release; the >0 check is added NOT VALID + VALIDATE
CONSTRAINT so it never takes a long-lived lock. amount_stroops stays the
column of record for now — the public API still returns it unchanged as
a JSON number; the string-typed cutover is a separate, versioned change
tracked in Octo-Protocol-org#227.

Amount's Serialize/Deserialize round-trips through JSON strings only —
the Deserialize visitor has no visit_u64/visit_i64/visit_f64 overrides,
so a bare JSON number is rejected outright rather than silently
coerced, since a uint256 in a JSON number loses precision in every
JavaScript client.

format_amount (crates/api/src/routes/submit.rs) no longer round-trips
through f64: it's now exact integer/modulo arithmetic parameterised by
a decimals count, which matters once amounts exceed 2^53 (18-decimal
EVM amounts routinely do, even for small balances).

The wallet-core/crypto clippy::cast_possible_truncation /
cast_sign_loss lint wall is extended to octo-chain, octo-store,
octo-ingest, and octo-api, since these crates now handle amount
conversions where an unchecked `as` cast is a fund-loss bug. The
remaining `as i64` casts in the diff's crates (Duration::as_secs() as
i64 for timestamps in ingest/lib.rs and api/auth.rs) are pre-existing,
untouched by this change, and are Unix timestamps rather than amounts,
so they are out of scope here and unaffected by the new lint wall
(same-width u64->i64 is cast_possible_wrap, not cast_possible_truncation
or cast_sign_loss).

Property tests round-trip the full uint256 range through both
BigDecimal and JSON string. Explicit boundary cases cover 0, 1,
i64::MAX, i64::MAX+1, u64::MAX, and 2^256-1. A regression test locks in
that a JSON number amount is rejected, not coerced. A store-level test
confirms 10^18 (1 ETH) survives a full store -> API JSON round trip
bit-identically, which the old i64/f64 schema could not represent.

BREAKING CHANGE: none yet for public API consumers — amount_stroops is
unchanged. The eventual string-typed amount field cutover across the
public API is tracked separately in Octo-Protocol-org#227 and must be versioned before
merge, per that issue's coordination requirement.

Closes Octo-Protocol-org#215
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.

feat(store): Arbitrary-precision amounts — replace i64 stroops with NUMERIC(78,0)

1 participant