feat(sdk): 0g x402 js sdk#2
Conversation
📝 WalkthroughWalkthroughAdds a new JavaScript SDK package Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer
participant GitHub as "GitHub (Tag Push)"
participant Actions as "GitHub Actions"
participant Bun as "Bun (setup/cache)"
participant Builder as "tsup / TypeScript"
participant NPM as "npm Registry"
Developer->>GitHub: push tag `sdk@0.1.0`
GitHub->>Actions: trigger release-sdk workflow
Actions->>Bun: install/setup Bun, restore cache
Actions->>Builder: `bun install --frozen-lockfile`, `bun run typecheck`, `bun run build`
Builder-->>Actions: produce `dist/*`
Actions->>NPM: authenticate (NODE_AUTH_TOKEN) and `npm publish` from `packages/sdk/js`
NPM-->>Actions: publish result
Actions-->>Developer: workflow status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
.github/workflows/release-sdk.yml (3)
46-48: Same pinning suggestion applies to the publish job.For consistency and reproducibility, use the same pinned Bun version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-sdk.yml around lines 46 - 48, The publish job currently uses the setup action with bun-version: latest (uses: oven-sh/setup-bun@v2 and bun-version: latest); update that block to pin the same specific Bun version used elsewhere in the workflow by replacing bun-version: latest with the exact version string (the same value used in the other job) so the publish job and the rest of the workflow use an identical, reproducible Bun release.
38-56: Build is duplicated in the publish job.The
validatejob already runsbun run build, butpublishrebuilds from scratch. Consider using GitHub Actions artifacts to share the build output between jobs, reducing CI time and ensuring the exact validated artifacts are published.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-sdk.yml around lines 38 - 56, The publish job currently re-runs the same build step that validate already ran; instead modify the workflow so validate produces and uploads the built artifacts (from packages/sdk/js build output) using upload-artifact, and change the publish job to remove the duplicate "bun run build" step and add a download-artifact step to retrieve the exact validated artifacts before publishing; reference the job names "validate" and "publish" and the build step "bun run build" and the working directory "packages/sdk/js" when making these edits.
15-17: Consider pinning the Bun version for reproducible builds.Using
bun-version: latestmay cause unexpected behavior if Bun releases a breaking change. Consider pinning to a specific version (e.g., matchingpackageManagerin rootpackage.json).Suggested change
- uses: oven-sh/setup-bun@v2 with: - bun-version: latest + bun-version: "1.3.12"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-sdk.yml around lines 15 - 17, The workflow uses "uses: oven-sh/setup-bun@v2" with bun-version: latest which makes releases unreproducible; change the bun-version input to a pinned semantic version (or a workflow variable) that matches the project's declared package manager version (e.g., the "packageManager" value in root package.json) so builds use a known Bun release; update the bun-version field in that setup-bun step to the specific version string (or reference an env/workflow input) and document the chosen version in the workflow comments.packages/sdk/js/src/x402/server.ts (2)
6-6: Consider renamingdecimaltodecimalsfor ERC-20 convention.The standard ERC-20 function is
decimals()(plural). Usingdecimalswould align with common Web3 conventions and reduce cognitive friction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/src/x402/server.ts` at line 6, Rename the `decimal` property to `decimals` across the codebase: update the type/interface and any occurrences where `decimal` is read, written, serialized, or passed (e.g., in the server.ts definition and uses of that symbol), adjust any tests or consumers to reference `decimals`, and ensure any JSON (de)serialization and external API contracts are updated to use the new `decimals` key (or add a temporary mapping layer if you must preserve backward compatibility).
34-35: ClarifyincludeEip712Domainlogic with a comment.The condition
!assetInfo.assetTransferMethod || assetInfo.supportsEip2612may be confusing to future readers. A brief comment explaining when EIP-712 domain fields are needed would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/src/x402/server.ts` around lines 34 - 35, Add a brief inline comment above the includeEip712Domain assignment explaining the condition: state that EIP‑712 domain fields should be included when there is no explicit assetTransferMethod (so the code must rely on signed domain-based approvals) or when the asset explicitly supports EIP‑2612 (permit-based approvals), e.g., clarifying that the boolean covers cases where signed permit/domain data is required for transfers; reference includeEip712Domain, assetInfo.assetTransferMethod, and assetInfo.supportsEip2612 in the comment.packages/sdk/js/tsconfig.json (1)
9-9: Consider ifdomlibraries are needed.The SDK appears server-focused (x402/server export), but
libincludesdomanddom.iterable. If no DOM APIs are used, removing these would slightly reduce type pollution and clarify the package's server-side intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/tsconfig.json` at line 9, The tsconfig "lib" entry currently includes "dom" and "dom.iterable", which pollutes server-side types; inspect codepaths exported from the package (notably any server-focused exports like the x402/server API) for usage of DOM APIs (window, Document, HTMLElement, fetch if only using node's global, etc.), and if none are used remove "dom" and "dom.iterable" from the "lib" array in tsconfig.json so only server-relevant libs (e.g., "es2022") remain; if some files legitimately use DOM types, instead narrow the lib by moving DOM-using code to a DOM-specific package or add per-file/type-only declarations rather than keeping DOM libs project-wide.packages/sdk/js/package.json (1)
34-39: Consider adding root-levelmainandtypesfields for compatibility.While the
exportsfield is the modern approach, some older bundlers and tools may not fully support subpath exports. If broader compatibility is needed, consider adding fallback fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/package.json` around lines 34 - 39, Add root-level fallback "main" and "types" fields to package.json so older bundlers can import the package; set "main": "./dist/x402/server.js" and "types": "./dist/x402/server.d.ts" to mirror the existing "./x402/server" export entry, keeping the current "exports" field intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk/js/src/x402/server.ts`:
- Around line 30-32: The token amount calculation uses floating-point
multiplication which risks precision loss; replace the Math.floor(amount * 10 **
assetInfo.decimal).toString() approach in the tokenAmount computation by
converting the incoming amount and scale to an exact integer operation (e.g.,
parse the string amount and use BigInt or a bignumber library) so you compute
amount * 10^decimal without float rounding, then floor/convert to string; update
the logic around the tokenAmount variable and any callers that pass amount to
ensure amount is handled as a string or BigInt and use assetInfo.decimal when
building the integer multiplier.
---
Nitpick comments:
In @.github/workflows/release-sdk.yml:
- Around line 46-48: The publish job currently uses the setup action with
bun-version: latest (uses: oven-sh/setup-bun@v2 and bun-version: latest); update
that block to pin the same specific Bun version used elsewhere in the workflow
by replacing bun-version: latest with the exact version string (the same value
used in the other job) so the publish job and the rest of the workflow use an
identical, reproducible Bun release.
- Around line 38-56: The publish job currently re-runs the same build step that
validate already ran; instead modify the workflow so validate produces and
uploads the built artifacts (from packages/sdk/js build output) using
upload-artifact, and change the publish job to remove the duplicate "bun run
build" step and add a download-artifact step to retrieve the exact validated
artifacts before publishing; reference the job names "validate" and "publish"
and the build step "bun run build" and the working directory "packages/sdk/js"
when making these edits.
- Around line 15-17: The workflow uses "uses: oven-sh/setup-bun@v2" with
bun-version: latest which makes releases unreproducible; change the bun-version
input to a pinned semantic version (or a workflow variable) that matches the
project's declared package manager version (e.g., the "packageManager" value in
root package.json) so builds use a known Bun release; update the bun-version
field in that setup-bun step to the specific version string (or reference an
env/workflow input) and document the chosen version in the workflow comments.
In `@packages/sdk/js/package.json`:
- Around line 34-39: Add root-level fallback "main" and "types" fields to
package.json so older bundlers can import the package; set "main":
"./dist/x402/server.js" and "types": "./dist/x402/server.d.ts" to mirror the
existing "./x402/server" export entry, keeping the current "exports" field
intact.
In `@packages/sdk/js/src/x402/server.ts`:
- Line 6: Rename the `decimal` property to `decimals` across the codebase:
update the type/interface and any occurrences where `decimal` is read, written,
serialized, or passed (e.g., in the server.ts definition and uses of that
symbol), adjust any tests or consumers to reference `decimals`, and ensure any
JSON (de)serialization and external API contracts are updated to use the new
`decimals` key (or add a temporary mapping layer if you must preserve backward
compatibility).
- Around line 34-35: Add a brief inline comment above the includeEip712Domain
assignment explaining the condition: state that EIP‑712 domain fields should be
included when there is no explicit assetTransferMethod (so the code must rely on
signed domain-based approvals) or when the asset explicitly supports EIP‑2612
(permit-based approvals), e.g., clarifying that the boolean covers cases where
signed permit/domain data is required for transfers; reference
includeEip712Domain, assetInfo.assetTransferMethod, and
assetInfo.supportsEip2612 in the comment.
In `@packages/sdk/js/tsconfig.json`:
- Line 9: The tsconfig "lib" entry currently includes "dom" and "dom.iterable",
which pollutes server-side types; inspect codepaths exported from the package
(notably any server-focused exports like the x402/server API) for usage of DOM
APIs (window, Document, HTMLElement, fetch if only using node's global, etc.),
and if none are used remove "dom" and "dom.iterable" from the "lib" array in
tsconfig.json so only server-relevant libs (e.g., "es2022") remain; if some
files legitimately use DOM types, instead narrow the lib by moving DOM-using
code to a DOM-specific package or add per-file/type-only declarations rather
than keeping DOM libs project-wide.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 99c0f804-9610-461d-8861-347828ac653f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/release-sdk.ymlpackage.jsonpackages/sdk/js/biome.jsonpackages/sdk/js/package.jsonpackages/sdk/js/reset.d.tspackages/sdk/js/src/x402/server.tspackages/sdk/js/tsconfig.json
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/sdk/js/src/x402/server.ts (2)
40-46:⚠️ Potential issue | 🟠 MajorMajor:
amount: numberat the parser boundary can lose exact decimal intent before conversion.Line 45 (
amount.toString()) cannot recover precision already lost innumber. If@x402/evm/exact/serversupports string amounts, prefer string input for money parsing.What is the TypeScript signature of registerMoneyParser in `@x402/evm/exact/server`, and does it support receiving the amount as a string for exact decimal handling?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/src/x402/server.ts` around lines 40 - 46, The parser currently takes amount: number which can lose decimal precision when converted via amount.toString(); change the parser callback passed to registerMoneyParser to accept amount as string (or string | number) and pass that string directly into convertToTokenAmount instead of calling toString(); if registerMoneyParser's TypeScript signature in `@x402/evm/exact/server` doesn't accept string amounts, update its type to allow string (or string | number) so exact decimals are preserved, and adjust the implementation to use the string code path when available; relevant symbols: registerMoneyParser, convertToTokenAmount, DEFAULT_STABLECOINS.
22-33:⚠️ Potential issue | 🔴 CriticalCritical:
convertToTokenAmountstill uses float parsing, which can corrupt token amounts.Line 26 converts to
numberfirst, so precision/scientific notation issues can still produce wrong or non-digit outputs for financial values.Suggested fix (string-first deterministic conversion)
export const convertToTokenAmount = ( decimalAmount: string, decimals: number ) => { - const amount = parseFloat(decimalAmount); - if (Number.isNaN(amount)) { - throw new Error(`Invalid amount: ${decimalAmount}`); - } - const [intPart, decPart = ""] = String(amount).split("."); + if (!Number.isInteger(decimals) || decimals < 0) { + throw new Error(`Invalid decimals: ${decimals}`); + } + const normalized = decimalAmount.trim(); + if (!/^\d+(\.\d+)?$/.test(normalized)) { + throw new Error(`Invalid amount: ${decimalAmount}`); + } + const [intPart, decPart = ""] = normalized.split("."); const paddedDec = decPart.padEnd(decimals, "0").slice(0, decimals); - const tokenAmount = (intPart + paddedDec).replace(/^0+/, "") || "0"; + const tokenAmount = `${intPart}${paddedDec}`.replace(/^0+/, "") || "0"; return tokenAmount; };In JavaScript, what do parseFloat("1.2abc"), parseFloat("Infinity"), and String(1e-7) return, and why is parseFloat unsuitable for exact financial decimal-to-integer conversion?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/src/x402/server.ts` around lines 22 - 33, convertToTokenAmount currently calls parseFloat which can corrupt precise token values and mishandle scientific notation or stray characters; instead treat decimalAmount as a string: trim it, validate it with a strict decimal regex (e.g. allow optional integer part and optional fractional part but no exponent/letters), reject inputs like "Infinity" or "1.2abc", then split on the literal '.' from the original string to get intPart and decPart, pad or slice decPart to the target decimals, concatenate intPart+paddedDec, strip leading zeros (return "0" if empty) and throw on invalid input; update the convertToTokenAmount function to use this string-first deterministic flow and reference convertToTokenAmount in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/sdk/js/src/x402/server.ts`:
- Around line 40-46: The parser currently takes amount: number which can lose
decimal precision when converted via amount.toString(); change the parser
callback passed to registerMoneyParser to accept amount as string (or string |
number) and pass that string directly into convertToTokenAmount instead of
calling toString(); if registerMoneyParser's TypeScript signature in
`@x402/evm/exact/server` doesn't accept string amounts, update its type to allow
string (or string | number) so exact decimals are preserved, and adjust the
implementation to use the string code path when available; relevant symbols:
registerMoneyParser, convertToTokenAmount, DEFAULT_STABLECOINS.
- Around line 22-33: convertToTokenAmount currently calls parseFloat which can
corrupt precise token values and mishandle scientific notation or stray
characters; instead treat decimalAmount as a string: trim it, validate it with a
strict decimal regex (e.g. allow optional integer part and optional fractional
part but no exponent/letters), reject inputs like "Infinity" or "1.2abc", then
split on the literal '.' from the original string to get intPart and decPart,
pad or slice decPart to the target decimals, concatenate intPart+paddedDec,
strip leading zeros (return "0" if empty) and throw on invalid input; update the
convertToTokenAmount function to use this string-first deterministic flow and
reference convertToTokenAmount in your change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 40cae0ca-254e-4661-b045-0fe66b8d08a0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
packages/sdk/js/package.jsonpackages/sdk/js/src/x402/server.tspackages/sdk/js/tsconfig.jsonpackages/sdk/js/tsup.config.ts
✅ Files skipped from review due to trivial changes (3)
- packages/sdk/js/tsconfig.json
- packages/sdk/js/tsup.config.ts
- packages/sdk/js/package.json
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/sdk/js/src/x402/server.ts (1)
43-49:⚠️ Potential issue | 🟠 MajorHandle numeric
amountvalues that stringify to scientific notation.On Line 48,
amount.toString()can yield values like1e-7/1e+21, which then fail the validator inconvertToTokenAmount(Line 30) and throw for otherwise valid numeric inputs.💡 Proposed fix
+const numberToPlainDecimalString = (value: number): string => { + if (!Number.isFinite(value) || value < 0) { + throw new Error(`Invalid amount: ${value}`); + } + + const raw = value.toString(); + if (!/[eE]/.test(raw)) return raw; + + const [mantissa, expRaw] = raw.toLowerCase().split("e"); + const exp = Number(expRaw); + const [whole, frac = ""] = mantissa.split("."); + const digits = `${whole}${frac}`.replace(/^0+/, "") || "0"; + const decimalIndex = whole.length + exp; + + if (decimalIndex <= 0) return `0.${"0".repeat(-decimalIndex)}${digits}`; + if (decimalIndex >= digits.length) + return `${digits}${"0".repeat(decimalIndex - digits.length)}`; + return `${digits.slice(0, decimalIndex)}.${digits.slice(decimalIndex)}`; +}; + export class ExactEvmScheme extends X402ExactEvmScheme { constructor() { super(); @@ const tokenAmount = convertToTokenAmount( - amount.toString(), + numberToPlainDecimalString(amount), assetInfo.decimal );#!/bin/bash # Verify JavaScript scientific-notation behavior against current regex expectations. node - <<'NODE' const re = /^\d+(\.\d+)?$/; for (const v of [1e-7, 1e21, 0.000001, 12.34]) { const s = v.toString(); console.log(`${v} -> "${s}" | regexMatch=${re.test(s)}`); } NODE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/src/x402/server.ts` around lines 43 - 49, The parser currently passes amount.toString() into convertToTokenAmount which fails for numbers rendered in scientific notation; update the registerMoneyParser callback to first convert numeric amounts to a non-exponential decimal string before calling convertToTokenAmount (use the asset precision from assetInfo.decimal), e.g., by using a BigNumber/decimal library or a utility that formats the Number into a fixed-point string with enough decimals and trims trailing zeros, then pass that string to convertToTokenAmount; modify the callback around registerMoneyParser and ensure convertToTokenAmount receives a plain decimal string for all numeric inputs.
🧹 Nitpick comments (1)
packages/sdk/js/src/x402/server.ts (1)
13-20: Consider freezing exported default coin metadata.
DEFAULT_STABLECOINSis part of the public API and currently mutable at runtime. Freezing helps prevent accidental mutations affecting parser behavior.🔒 Proposed hardening
-export const DEFAULT_STABLECOINS: DefaultStablecoins = { - "eip155:16661": { +export const DEFAULT_STABLECOINS: Readonly<DefaultStablecoins> = Object.freeze({ + "eip155:16661": Object.freeze({ address: "0x1f3aa82227281ca364bfb3d253b0f1af1da6473e", name: "Bridged USDC", decimal: 6, version: "2", - }, -}; + }), +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/js/src/x402/server.ts` around lines 13 - 20, DEFAULT_STABLECOINS is exported mutable global state; make it immutable by freezing the exported value so runtime code cannot alter it. Create the object as a local const (typed as DefaultStablecoins), then freeze the top-level map with Object.freeze and also freeze each nested coin metadata object (e.g., the entry for "eip155:16661") to achieve an effective deep freeze before exporting DEFAULT_STABLECOINS; reference the symbols DEFAULT_STABLECOINS and type DefaultStablecoins when locating where to apply the freezes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/sdk/js/src/x402/server.ts`:
- Around line 43-49: The parser currently passes amount.toString() into
convertToTokenAmount which fails for numbers rendered in scientific notation;
update the registerMoneyParser callback to first convert numeric amounts to a
non-exponential decimal string before calling convertToTokenAmount (use the
asset precision from assetInfo.decimal), e.g., by using a BigNumber/decimal
library or a utility that formats the Number into a fixed-point string with
enough decimals and trims trailing zeros, then pass that string to
convertToTokenAmount; modify the callback around registerMoneyParser and ensure
convertToTokenAmount receives a plain decimal string for all numeric inputs.
---
Nitpick comments:
In `@packages/sdk/js/src/x402/server.ts`:
- Around line 13-20: DEFAULT_STABLECOINS is exported mutable global state; make
it immutable by freezing the exported value so runtime code cannot alter it.
Create the object as a local const (typed as DefaultStablecoins), then freeze
the top-level map with Object.freeze and also freeze each nested coin metadata
object (e.g., the entry for "eip155:16661") to achieve an effective deep freeze
before exporting DEFAULT_STABLECOINS; reference the symbols DEFAULT_STABLECOINS
and type DefaultStablecoins when locating where to apply the freezes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df19ca03-5986-4253-8661-504013c3ace8
📒 Files selected for processing (1)
packages/sdk/js/src/x402/server.ts
Summary by CodeRabbit
New Features
Chores