fix(encode): validate string format before BigInt conversion#178
fix(encode): validate string format before BigInt conversion#1786figpsolseeker wants to merge 1 commit intodcccrypto:mainfrom
Conversation
The encode helpers (encU64, encI64, encU128, encI128) and encodePushOraclePrice all accepted `bigint | string` but converted strings via raw `BigInt(val)` without format validation. This throws an unhelpful `SyntaxError: Cannot convert X to a BigInt` on inputs containing whitespace, scientific notation (1e6), decimal points (1.5), hex prefixes (0x1A), or other non-decimal-integer strings. Since many instruction arg interfaces accept `bigint | string` (over 60 fields across the SDK), this is a systemic boundary issue. The fix adds a shared `safeBigInt()` helper in encode.ts that validates the string matches a strict decimal integer pattern before calling BigInt(). The regex is identical to the one already used in validation.ts for input validators, ensuring consistency. The helper produces a clear, actionable error message: encU64: string must be a decimal integer (got "1.5e6"). Example valid inputs: "0", "123", "-456" encodePushOraclePrice also had a direct `BigInt(args.priceE6)` call that bypassed the encode helpers. This is now inline-validated with the same regex pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/abi/instructions.ts (1)
567-579: Consider de-duplicating decimal-integer validation logic.Line 572-Line 577 reimplements the same string-to-bigint guard already present in
src/abi/encode.ts. Centralizing this parser would keep regex and error semantics aligned across entry points.♻️ Proposed refactor
- const price = typeof args.priceE6 === "string" - ? (() => { - if (!/^-?(0|[1-9]\d*)$/.test(args.priceE6)) { - throw new Error( - `encodePushOraclePrice: priceE6 must be a decimal integer string (got ${JSON.stringify(args.priceE6)})`, - ); - } - return BigInt(args.priceE6); - })() - : args.priceE6; + const price = typeof args.priceE6 === "string" + ? safeBigInt(args.priceE6, "encodePushOraclePrice") + : args.priceE6;// Additional change outside this segment: // src/abi/encode.ts export function safeBigInt(val: string, caller: string): bigint { ... } // src/abi/instructions.ts import list import { ..., safeBigInt } from "./encode.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/abi/instructions.ts` around lines 567 - 579, Extract the string-to-BigInt validation into a shared helper (e.g., export function safeBigInt(val: string, caller: string): bigint) in src/abi/encode.ts and import it into src/abi/instructions.ts; then replace the inline regex + BigInt block in the encodePushOraclePrice path with a call to safeBigInt(args.priceE6, "encodePushOraclePrice") (keeping the existing branch: if typeof args.priceE6 === "string" then call safeBigInt else use args.priceE6) and add safeBigInt to the import list from "./encode.js" so regex and error messages are centralized and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/abi/instructions.ts`:
- Around line 567-579: Extract the string-to-BigInt validation into a shared
helper (e.g., export function safeBigInt(val: string, caller: string): bigint)
in src/abi/encode.ts and import it into src/abi/instructions.ts; then replace
the inline regex + BigInt block in the encodePushOraclePrice path with a call to
safeBigInt(args.priceE6, "encodePushOraclePrice") (keeping the existing branch:
if typeof args.priceE6 === "string" then call safeBigInt else use args.priceE6)
and add safeBigInt to the import list from "./encode.js" so regex and error
messages are centralized and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d14626b-1478-4940-9702-00f1ffecb720
📒 Files selected for processing (2)
src/abi/encode.tssrc/abi/instructions.ts
Summary
The encode helpers (
encU64,encI64,encU128,encI128) andencodePushOraclePriceall acceptedbigint | stringbut converted strings via rawBigInt(val)without format validation. This throws an unhelpfulSyntaxError: Cannot convert X to a BigInton inputs containing whitespace, scientific notation (1e6), decimal points (1.5), hex prefixes (0x1A), or other non-decimal-integer strings.Since over 60 instruction arg fields across the SDK accept
bigint | string, this is a systemic boundary issue affecting any consumer that passes user-supplied strings.Changes
src/abi/encode.ts— AddedsafeBigInt(val, caller)helper that validates strings against/^-?(0|[1-9]\d*)$/(same regex asvalidation.ts) before callingBigInt(). Replaced the rawBigInt(val)call inencU64,encI64,encU128, andencI128.src/abi/instructions.ts:567—encodePushOraclePricehad a directBigInt(args.priceE6)that bypassed the encode helpers. Now inline-validated with the same regex.Error messages are now clear and actionable:
This is a non-breaking change — all previously-valid inputs (
bigintvalues and correctly-formatted decimal strings) produce identical results.Test plan
npm run lint(tsc --noEmit) cleannpx vitest run test/encode.test.ts— all 54 tests passvitest run: same 14 pre-existing failures asmain, zero new failuresSummary by CodeRabbit
Release Notes