Skip to content

fix(mcp_impl): validate JSON amounts as exact integers before big.Int conversion#143

Open
gfyrag wants to merge 1 commit into
test/mcp-float-truncation-bugfrom
fix/mcp-validate-integer-amounts
Open

fix(mcp_impl): validate JSON amounts as exact integers before big.Int conversion#143
gfyrag wants to merge 1 commit into
test/mcp-float-truncation-bugfrom
fix/mcp-validate-integer-amounts

Conversation

@gfyrag
Copy link
Copy Markdown
Contributor

@gfyrag gfyrag commented Jun 3, 2026

Summary

Validates JSON amounts before converting to *big.Int in
internal/mcp_impl/handlers.go::parseBalancesJson. Stacked on #142
(the failing regression test); merging this flips the test green.

JSON numeric values reach the handler as float64 (Go's default). The
previous code used big.NewFloat(amount).Int(new(big.Int)), which:

  • silently truncated fractional parts (100.9100)
  • silently rounded values past ±(2^53 - 1) (already lost in JSON
    decoding before reaching the handler)

The fix introduces a single guard before the conversion:

if amount < -maxExactJSONInt || amount > maxExactJSONInt || amount != float64(int64(amount)) {
    return error
}
iBalances[account][asset] = big.NewInt(int64(amount))

where maxExactJSONInt = 9_007_199_254_740_991 (2^53 - 1). The error
message names the offending account/asset so callers can pinpoint the
bad balance.

Stack

PR What it does Target
#142 Failing regression tests main
this PR Adds the integer-validation guard #142 branch

Test plan

🤖 Generated with Claude Code

… conversion

`parseBalancesJson` reads each balance amount as `float64` (Go's JSON
default for `any`) and previously fed it straight into
`big.NewFloat(amount).Int(new(big.Int))`, which silently truncated
fractional parts and rounded values past float64's exact-integer range
(±(2^53 - 1)). For a financial DSL, both are silent data corruption.

Add a guard before the conversion:

  - Reject magnitudes outside [-(2^53-1), 2^53-1] — beyond this range
    float64 cannot uniquely represent every integer, so the value the
    caller meant has already been lost in JSON decoding.
  - Reject fractional values (`amount != float64(int64(amount))`).
  - Use `big.NewInt(int64(amount))` directly since the validation has
    proven the float represents an exact int64.

The error message names the offending account/asset so the caller
knows which balance to fix.

This flips the failing regression tests in the stacked parent PR from
red to green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 90d98fc6-dc5c-49ed-9bc0-e1449d77f1a9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/mcp-validate-integer-amounts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (test/mcp-float-truncation-bug@ec9933f). Learn more about missing BASE report.

Files with missing lines Patch % Lines
internal/mcp_impl/handlers.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                        @@
##             test/mcp-float-truncation-bug     #143   +/-   ##
================================================================
  Coverage                                 ?   67.21%           
================================================================
  Files                                    ?       47           
  Lines                                    ?     5069           
  Branches                                 ?        0           
================================================================
  Hits                                     ?     3407           
  Misses                                   ?     1462           
  Partials                                 ?      200           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant