Skip to content

fix(M2): Decimal-based amount parsing for /utxo/transfer (precision + overflow)#2814

Merged
Scottcjn merged 1 commit intomainfrom
fix/M2-decimal-amount-parsing
Apr 30, 2026
Merged

fix(M2): Decimal-based amount parsing for /utxo/transfer (precision + overflow)#2814
Scottcjn merged 1 commit intomainfrom
fix/M2-decimal-amount-parsing

Conversation

@Scottcjn
Copy link
Copy Markdown
Owner

Closes BossChaos audit finding M2 (Bounty #2867).

/utxo/transfer parsed amounts via float() which has two bugs:

  1. Precision loss: float(0.29) * 100_000_000 = 28999999.999...int() truncates to 28999999. User sent 0.29 RTC, system credited 0.28999999 RTC.
  2. OverflowError: float('1e308') = infint(inf * UNIT) raises 500 instead of clean 400.

Replaced with _parse_rtc_amount() helper using Decimal:

  • Decimal(str(0.29)) → 29000000 nrtc exactly
  • Bounds check (<= 2^53 RTC) catches overflow before int conversion
  • Rejects Infinity, NaN, negative cleanly
  • Returns 400 with error message instead of 500

Verified all 8 edge cases pass.

+55/-2 in node/utxo_endpoints.py. Audit credit @BossChaos #2744 M2 (paid 25 RTC). Fix-bounty: 25 RTC Medium-tier.

…nd overflowed (closes #2867 M2)

The /utxo/transfer endpoint parsed amounts via float(), which has two bugs:
1. Precision loss: float(0.29) * 100_000_000 = 28999999.999... → int() = 28999999.
   User sent 0.29 RTC, system credited 0.28999999 RTC (1 nrtc lost per tx).
2. OverflowError on large input: float('1e308') = inf → int(inf * UNIT) raises.
   Crashes the request handler instead of returning a clean 400.

Replaced with _parse_rtc_amount() helper using Decimal:
- Decimal(str(0.29)) is exact: 29000000 nrtc as expected
- Bounds check (<= 2^53 RTC) catches overflow before int() conversion
- Rejects Infinity, NaN, negative
- Returns clean 400 with error message instead of 500

Verified all 8 edge cases pass including the headline 0.29 precision bug.

Audit credit: BossChaos #2744 M2 (paid 25 RTC for the find)
Bounty: #2867 fix-bounty Medium-tier
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/M PR: 51-200 lines labels Apr 30, 2026
@Scottcjn Scottcjn merged commit 6b6995f into main Apr 30, 2026
11 of 12 checks passed
@Scottcjn Scottcjn deleted the fix/M2-decimal-amount-parsing branch April 30, 2026 21:44
@github-actions
Copy link
Copy Markdown
Contributor

✅ BCOS v2 Scan Results

Metric Value
Trust Score 60/100
Certificate ID BCOS-2c1826fa
Tier L1 (met)

BCOS Badge

What does this mean?

The BCOS (Beacon Certified Open Source) engine scans for:

  • SPDX license header compliance
  • Known CVE vulnerabilities (OSV database)
  • Static analysis findings (Semgrep)
  • SBOM completeness
  • Dependency freshness
  • Test infrastructure evidence
  • Review attestation tier

Full report | What is BCOS?


BCOS v2 Engine - Free & Open Source (MIT) - Elyan Labs

Scottcjn added a commit that referenced this pull request May 1, 2026
… C1 PoC, RC_P2P_SECRET) (#2859)

Five tests on main were broken by yesterday's audit fixes (#2812-#2816 + #2800):

1. test_mempool_add_manage_tx_undefined (#2812 follow-up)
   - Was asserting the BUG exists (manage_tx undefined). After fix, manage_tx
     IS defined. Updated to assert FIX is in place + smoke-test no NameError.

2. test_pncounter_max_merge_inflation
   - Imports rustchain_p2p_gossip which raises SystemExit if RC_P2P_SECRET
     not set. CI workflow didn't set it. Added RC_P2P_SECRET to ci.yml env.

3. test_bounty_lifecycle_workflow (#2800 follow-up)
   - haoyousun60-create's #2800 added admin auth on /api/bounties/<id>/claim.
     Test was sending request without X-Admin-Key. Added the header.

4. test_utxo_transfer_rejects_duplicate_nonce (#2814 M2 follow-up)
5. test_utxo_transfer_failed_attempt_does_not_burn_nonce (#2814 M2 follow-up)
   - M2 fix made amount_rtc / fee_rtc Decimal types internally for precision.
     Decimal isn't JSON-serializable, so signed-payload construction
     (json.dumps) and response jsonify both broke.
   - Cast to float for the signed payload (preserves byte-identical signature
     bytes vs what wallets compute) and for the response jsonify.
   - Decimal arithmetic still happens internally for the int(amount * UNIT)
     conversion, so the precision-loss + overflow guards from M2 are intact.

All 6 tests pass locally with the env vars set.

Co-authored-by: Scottcjn <scottbphone12@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/M PR: 51-200 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants