Skip to content

fix(proof): correct atomic-unit thresholds + mirror SetUint64 to prover#15

Open
DHEBP wants to merge 2 commits into
DEROFDN:community-devfrom
DHEBP:fix/proof-arithmetic-hardening
Open

fix(proof): correct atomic-unit thresholds + mirror SetUint64 to prover#15
DHEBP wants to merge 2 commits into
DEROFDN:community-devfrom
DHEBP:fix/proof-arithmetic-hardening

Conversation

@DHEBP
Copy link
Copy Markdown

@DHEBP DHEBP commented May 31, 2026

Summary

Two related arithmetic-correctness fixes on the proof-validation path, both follow-ups to PR #14.

  • proof/proof_validation.go: the hard-cap and reasonable-amount thresholds assumed 1 DERO = 1,000,000 atomic units, but DERO uses 100,000 atomic units per DERO (matches globals/globals.go:295 and the GetInfo RPC supply conversion at rpc_dero_getinfo.go:81). The threshold constants were therefore 10× too high — the hard-cap check effectively triggered at ~220M DERO instead of the documented 22M. Reorder the const block so atomicUnitsPerDero is defined first, then rewrite each threshold as X * atomicUnitsPerDero so the conversion factor is explicit at every use site. Same correction applied to the largeThreshold local variable in DetectSuspiciousProofPatterns.
  • proof/proof_validation_test.go: four test cases referenced the buggy constants by value. After the constant correction, those amounts would correspond to 210M / 230M / 500M DERO — outside the labelled intent. Update the four affected test amounts and tighten the labels.
  • cryptography/crypto/proof_generate.go: mirror PR feat(explorer): proof validation security + modern UI refresh #14's verifier-side SetInt64(int64(amount)) → SetUint64(amount) hardening to the prover side. The previous pattern is bounded in legitimate use (transfer amounts and balances cap well below 2^63) and only causes self-DoS if it ever fires, but leaving the asymmetric pattern in place is forensic-screenshot bait once readers compare verifier and prover. Drop the misleading // this should be reduced comments — at this scale (values bounded by 2^64) the bn256 group order (~2^254) makes modular reduction a no-op.
  • Warning 4 in DetectSuspiciousProofPatterns: the hardcoded 1_000_000_000_000 literal flagged exact multiples of 10M DERO under the correct conversion. Restore the original intent — exact multiples of 1M DERO — via 1_000_000 * atomicUnitsPerDero.

Background

atomicUnitsPerDero = 100_000 is correct (confirmed against the canonical decimals constant). Where PR #14 went off-by-10× was in the threshold constants themselves:

  • 21_000_000 DERO × 100_000 atomic/DERO = 2_100_000_000_000 atomic. The constant had 21_000_000_000_00010× too high.
  • Same arithmetic error for maxReasonableAmountAtomic, currentSupplyApproxAtomic, and the local largeThreshold / Warning 4 literal.

Practical exploitability: none as a chain-attack. The primary gate that rejects fabricated impossible amounts is the cryptographic commitment match at proof/proof.go:99. The validator at proof/proof.go:54 is an early-rejection shortcut, not a defense layer. The off-by-10× error meant the shortcut let through fabricated proofs with displayed amounts up to ~220M DERO (which the crypto path rejected on the commitment-match check), instead of cutting them at 22M as the comments and error messages claimed.

But the constants disagreed with their own labels and the test labels by 10×, which produces misleading rejection messages ("amount X DERO exceeds DERO hard cap (21M)" computed from a threshold that's actually 210M). This PR aligns the math with the labels and tightens the prover/verifier symmetry.

Risk

Minimal. The validator's behavior changes only in the band of amounts between the old buggy threshold (~220M DERO atomic) and the corrected threshold (22M DERO atomic). No legitimate transfer can fall in that band (the on-chain hard cap is 21M DERO), so the only inputs affected are intentionally fabricated proof strings — which is what the validator exists to reject.

The prover-side SetUint64 change has no behavioral effect for legitimate amounts (which are far below 2^63); it makes prover signed-overflow behavior consistent with the post-PR-#14 verifier.

The Warning 4 change affects only the (currently unwired) DetectSuspiciousProofPatterns function — no user-visible effect in community-dev. Included for arithmetic consistency and to align with the matching change in DHEBP/HOLOGRAM @ a8902d9.

Test plan

  • go build ./proof/... ./cryptography/... — passes
  • go test ./proof/... — passes (all 11 existing wraparound + hard-cap test cases pass against the corrected constants and the updated 4 test amounts)
  • go vet ./proof/... — clean (pre-existing unreachable code warning in proof_innerproduct.go:309 is unrelated and untouched by this PR)

Notes

  • Sibling fix shipped on the HOLOGRAM repo (which has its own independent copy of proof_validation.go) via DHEBP/HOLOGRAM @ da6f814 and @ a8902d9.
  • Does not wire DetectSuspiciousProofPatterns into any display path — it remains unwired in community-dev. Either deleting it as dead code or wiring it into the explorer's display layer is a separate follow-up if desired.
  • Two commits to make review easier — constants/tests/prover hardening in commit 1, Warning 4 in commit 2.

DHEBP added 2 commits May 31, 2026 12:06
…over

Two related arithmetic-correctness fixes on the proof verification path,
both follow-ups to PR DEROFDN#14.

proof/proof_validation.go: the hard-cap and reasonable-amount thresholds
were computed as if 1 DERO = 1,000,000 atomic units, but DERO actually
uses 100,000 atomic units per DERO (matches globals/globals.go:295 and
the GetInfo RPC supply conversion at rpc_dero_getinfo.go:81). The constants
were therefore 10x too high — the validator's hard-cap check effectively
triggered at ~220M DERO instead of the documented 22M. Reorder the const
block so atomicUnitsPerDero is defined first, then rewrite each threshold
as X * atomicUnitsPerDero so the conversion factor is explicit at every
use site. Same fix applied to the largeThreshold local variable inside
DetectSuspiciousProofPatterns.

proof/proof_validation_test.go: four test cases referenced the buggy
constants by value (e.g., 21_000_000_000_000 // "21M DERO"). With the
corrected constants those amounts now correspond to 210M DERO atomic
units — outside the labelled intent. Update the four affected test
amounts and tighten their labels.

cryptography/crypto/proof_generate.go: mirror PR DEROFDN#14's verifier-side
SetInt64(int64(amount)) -> SetUint64(amount) hardening to the prover
side. The previous pattern is bounded in legitimate use (transfer
amounts and balances cap well below 2^63) and only causes self-DoS if
it ever fires, but the asymmetry between prover and verifier is forensic
noise once readers compare them. Drop the misleading "this should be
reduced" comments — values at this scale (bounded by 2^64) are far
smaller than the bn256 group order (~2^254), so modular reduction would
be a no-op.

No behavioral change for legitimate transfers. The validator's hard-cap
rejection now actually fires at 22M DERO as the comments claim, rather
than at 220M.
Warning 4 used a hardcoded 1 trillion atomic literal, which at the correct
100k atomic/DERO conversion flagged only multiples of 10M DERO (just 10M and
20M in the valid range). Restore the original intent of flagging exact
multiples of 1M DERO via 1_000_000 * atomicUnitsPerDero.

Note: DetectSuspiciousProofPatterns has zero callers in community-dev so
this has no user-visible effect in this tree today, but it aligns the
internal arithmetic with the corrected constants and matches the
matching change in HOLOGRAM (DHEBP/HOLOGRAM @ a8902d9).
@8lecramm 8lecramm self-assigned this May 31, 2026
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