Skip to content

fix: reject --oid values above 2^53-1 on perp cancel - #497

Merged
kome12 merged 1 commit into
nansen-ai:mainfrom
crazywriter1:fix/perp-cancel-oid-safe-integer-v2
Aug 26, 2026
Merged

fix: reject --oid values above 2^53-1 on perp cancel#497
kome12 merged 1 commit into
nansen-ai:mainfrom
crazywriter1:fix/perp-cancel-oid-safe-integer-v2

Conversation

@crazywriter1

Copy link
Copy Markdown
Contributor

Summary

  • perp cancel --oid parsed via parseInt which silently rounds uint64 IDs above 2^53-1, potentially cancelling the wrong order
  • Add Number.isSafeInteger guard — same logic the response path already uses to withhold unsafe oids from output
  • Add regression tests: unsafe oid rejected, MAX_SAFE_INTEGER accepted

Test plan

  • npx vitest run src/__tests__/perp.test.js -t cancel
  • --oid 9007199254740992 (2^53) → error: "exceeds safe integer precision"
  • --oid 9007199254740991 (2^53-1) → passes validation

Large Hyperliquid uint64 order IDs are silently rounded by JS Number,
which could cancel the wrong order. Refuse at input for the same reason
the response path withholds unsafe oids.
@nansen-pr-reviewer

Copy link
Copy Markdown

pr-reviewer Summary for #223a9d5

No issues found

The code review completed successfully with no findings.

Review effort: 1/5 (Trivial)

Summary

This PR correctly fixes a real trading-safety bug: parseInt on a uint64 order ID above 2^53-1 silently produces the wrong value, which could cancel the wrong order. The Number.isSafeInteger guard is the right tool here, is consistent with how the response path already handles unsafe oids, and is placed at exactly the right point in the call chain — before the value reaches buildCancelAction. The regression tests are well-structured and cover both the rejection boundary and the acceptance boundary. The changeset is present, correctly typed as patch, and targets the right package. src/schema.json requires no update since the --oid option already exists and its type/description haven't changed. No findings.


Token usage: 828 input, 2,151 output, 163,103 cache read, 17,597 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

@nansen-pr-reviewer nansen-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a minimal effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected

If you have any concerns, please request a manual review.

@crazywriter1

Copy link
Copy Markdown
Contributor Author

Friendly ping. This is ready for review when you have a moment. Happy to address any feedback. @kome12

@kome12
kome12 merged commit 1c3a37b into nansen-ai:main Aug 26, 2026
9 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 26, 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