Skip to content

feat(wasm-sdk): expose tiered token pricing in setPrice#3967

Draft
thepastaclaw wants to merge 5 commits into
dashpay:v3.1-devfrom
thepastaclaw:feat-js-token-tier-pricing
Draft

feat(wasm-sdk): expose tiered token pricing in setPrice#3967
thepastaclaw wants to merge 5 commits into
dashpay:v3.1-devfrom
thepastaclaw:feat-js-token-tier-pricing

Conversation

@thepastaclaw

@thepastaclaw thepastaclaw commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Tiered Token Direct-Purchase Pricing

Issue being fixed or feature implemented

The protocol supports consensus-enforced minimum direct-purchase amounts through
TokenPricingSchedule::SetPrices: the lowest tier key is the minimum purchasable
token amount. Rust SDK, FFI, and lower-level wasm-dpp2 paths can represent tiered
schedules, but the high-level JavaScript path could only call tokenSetPrice
with a flat price: bigint | null.

That meant JS users of @dashevo/evo-sdk / @dashevo/wasm-sdk could
UI-enforce "buy at least 100", but could not set the consensus-enforced tier
schedule through the normal SDK API.

What was done?

  • Extended wasm-sdk TokenSetPriceOptions with
    priceTiers?: Record<string, bigint>.
  • Mapped priceTiers to TokenPricingSchedule::SetPrices while keeping
    price mapped to SinglePrice and null mapped to disabling direct
    purchases.
  • Rejected ambiguous calls that pass both price and priceTiers.
  • Validated tier maps before building the transition:
    • at least one tier is required
    • tier amounts must be unsigned integer string keys greater than zero
    • zero-credit tier prices are allowed, matching lower SDK/protocol behavior
  • Updated evo-sdk facade tests so tokens.setPrice() covers both flat prices
    and tiered schedules forwarded unchanged to wasm-sdk.
  • Fixed the evo-sdk Mocha unit-test loader invocation to use Mocha's
    --node-option loader=ts-node/esm path with ts-node transpile-only runtime
    execution.

How Has This Been Tested?

Validation performed locally on macOS arm64:

  • cargo fmt --all -- --check
  • cargo check -p wasm-sdk --tests
  • cargo test -p wasm-sdk --lib state_transitions::token::tests::
  • cargo clippy -p wasm-sdk --lib --tests
  • yarn workspace @dashevo/wasm-sdk lint
    • Passed with one existing unrelated warning in
      packages/wasm-sdk/tests/functional/shielded.spec.ts about _e being
      unused.
  • yarn workspace @dashevo/evo-sdk lint
  • yarn workspace @dashevo/wasm-sdk build
    • Built with local Node 20.18.1 and Zig 0.13.0 wrapper for the wasm target.
    • wasm-opt was not installed locally, so wasm-pack skipped optimization
      after producing the build.
  • yarn workspace @dashevo/evo-sdk build
  • YARN_NODE_LINKER=node-modules yarn install --immutable
    • Used only to create a temporary ignored node_modules install for local
      package test execution in this worktree.
  • YARN_NODE_LINKER=node-modules yarn workspace @dashevo/evo-sdk test:unit
    • Ran through an SSH login session so ChromeHeadless can start on macOS.
    • Mocha: 212 passing.
    • Karma/ChromeHeadless: 212 completed.
  • Focused runtime forwarding check against built packages:
    • imported built @dashevo/wasm-sdk and @dashevo/evo-sdk

    • monkeypatched wasmSdk.tokenSetPrice

    • called:

      client.tokens.setPrice({
        priceTiers: { "100": 1000n, "1000": 900n },
        ...options,
      });
    • asserted evo-sdk forwards the priceTiers object unchanged

  • Pre-PR code-review gate returned ship before opening the PR.
  • Post-test-run code-review gate returned ship again after adding the
    evo-sdk test-script fix.

Breaking Changes

None. Existing price: bigint | null callers continue to work. priceTiers is
additive, and passing both fields is rejected as invalid input.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the
    corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • New Features

    • Added support for tiered direct-purchase pricing via priceTiers, alongside existing fixed price.
    • Pricing options now support explicit disabling of direct purchases when no price is provided.
  • Bug Fixes

    • Enforced mutual exclusivity between fixed pricing (price) and tiered pricing (priceTiers).
    • Added stricter validation for tier contents and detected duplicate tier keys.
  • Tests

    • Updated unit tests for the new setPrice options shape and added coverage for tier forwarding and validation rules.

thepastaclaw and others added 3 commits June 27, 2026 13:20
Add backwards-compatible `priceTiers?: Record<string, bigint>` input to
TokenSetPriceOptions so callers can configure a TokenPricingSchedule::SetPrices
schedule (per-amount tier pricing / minimum-bulk-buy), matching what lower
SDKs already support. The flat `price` field is preserved and null still
disables direct purchases; specifying both is rejected.

Validation rejects empty tier maps, a zero minimum bulk-buy amount, and
zero per-token credit prices, each with a useful error pointing callers
at the correct field. Includes focused Rust unit tests for the
validation helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a focused unit test that asserts TokensFacade.setPrice forwards a
`priceTiers` schedule unchanged to wasmSdk.tokenSetPrice, and update the
existing flat-price test to the current `price: bigint` shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lower/consensus SetPrices schedule permits zero-credit tiers for
free direct purchases at a given bulk amount, but the wasm-sdk wrapper
was rejecting them. Drop the zero-credits check while keeping the empty
map and zero-amount-key rejections.

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

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 771d9820-b9ba-4336-8c6e-9dd7bebc4ecb

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb06a9 and b9dd057.

📒 Files selected for processing (1)
  • packages/wasm-sdk/src/state_transitions/token.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/wasm-sdk/src/state_transitions/token.rs

📝 Walkthrough

Walkthrough

Extends token_set_price in the wasm SDK to accept priceTiers alongside an optional price, updates the embedded TypeScript options contract, and aligns Rust and JS tests plus the JS unit-test runner configuration.

Changes

Tiered Direct-Purchase Pricing

Layer / File(s) Summary
TokenSetPriceOptions TS interface update
packages/wasm-sdk/src/state_transitions/token.rs
Makes price optional and adds priceTiers?: Record<string, bigint> with mutual-exclusivity documentation.
Tiered pricing extraction, validation, and token_set_price wiring
packages/wasm-sdk/src/state_transitions/token.rs
Adds tier-map validation/extraction helpers and reworks token_set_price to choose between SetPrices, SinglePrice, or no schedule based on the provided options.
Rust and JS test updates
packages/wasm-sdk/src/state_transitions/token.rs, packages/js-evo-sdk/tests/unit/facades/tokens.spec.ts, packages/js-evo-sdk/package.json
Adds Rust coverage for tier validation and mode selection, updates the JS setPrice tests for the new options shape, and changes test:unit to use the ts-node ESM loader env setup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • shumkov

Poem

🐇 A hopper saw the tiers align,
with BigInt carrots in a line.
priceTiers hopped in, neat and bright,
while price took a gentler flight.
The wasm burrow hummed with glee,
as tests went bounding, bug-free.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: exposing tiered token pricing in wasm-sdk setPrice.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@thepastaclaw

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw

thepastaclaw commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator Author

✅ Review complete (commit b9dd057)

@thepastaclaw

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/wasm-sdk/src/state_transitions/token.rs`:
- Around line 2212-2237: The pricing selection in
`deserialize_token_set_price_options` / the pricing schedule builder currently
treats omitted `price` and omitted `priceTiers` the same as an explicit disable,
so require an explicit pricing mode instead. Update the validation around
`price_field_present`, `price_tiers`, and `parsed.price` so that the function
rejects requests where both pricing fields are absent, while still allowing
either `price` or `priceTiers` individually and preserving the existing
mutual-exclusion check. Ensure the error points callers to provide exactly one
of the supported pricing modes.
- Around line 2093-2102: The priceTiers parsing in token.rs currently lets
different string keys like "1" and "01" collapse to the same TokenAmount and be
silently overwritten by tiers.insert. Update the parsing logic around the
key_str.parse flow to detect when a parsed amount already exists in tiers and
return a WasmSdkError::invalid_argument instead of overwriting it. Keep the
check close to the existing priceTiers loop so the error clearly references the
duplicate parsed tier amount and the original key_str.
🪄 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

Run ID: 5025a905-b727-4052-8208-058db19dc440

📥 Commits

Reviewing files that changed from the base of the PR and between 1653b89 and 8cb06a9.

📒 Files selected for processing (3)
  • packages/js-evo-sdk/package.json
  • packages/js-evo-sdk/tests/unit/facades/tokens.spec.ts
  • packages/wasm-sdk/src/state_transitions/token.rs

Comment thread packages/wasm-sdk/src/state_transitions/token.rs Outdated
Comment thread packages/wasm-sdk/src/state_transitions/token.rs Outdated

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Code Review

Small, well-scoped wasm-sdk change adding tiered direct-purchase pricing to tokenSetPrice. Boundary handling (Reflect-based bigint extraction, mutual-exclusion check) is sound, the protocol-level schedule is reused correctly, and there are no consensus/versioning/dependency concerns. Two low-impact suggestions on the new JS boundary: a missing wasm-runtime test for the bigint parser, and a TypeScript-vs-Rust strictness mismatch for tier values.

🟡 2 suggestion(s)

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/wasm-sdk/src/state_transitions/token.rs`:
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2062-2108: No wasm/JS-runtime test covers the new priceTiers extraction path
  `extract_price_tiers` is the only piece of code that exercises the hand-rolled `Reflect::get` + `Object.entries` + `try_to_u64` path for `Record<string, bigint>`. The Rust unit tests only validate an already-constructed `BTreeMap`, and the js-evo-sdk facade spec only asserts stub forwarding. That leaves regressions in bigint→u64 conversion, the both-fields rejection, key parsing, and the SetPrices mapping uncovered by any test that actually crosses the JS/WASM boundary. Add a wasm-bindgen-test (or equivalent web-target test) that calls `tokenSetPrice` with a real JS object containing bigint tier values and asserts both the happy path and the `price`+`priceTiers` rejection.
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2100: priceTiers values are typed as bigint in TS but accepted as Number/String at runtime
  The TypeScript declaration at line 1966 says `priceTiers?: Record<string, bigint>`, but `try_to_u64` (wasm-dpp2/src/utils.rs:326) also accepts JS Number and numeric String. For plain JS callers (no TS type-checking), passing a Number > `2^53-1` will already be rounded by JS before Rust sees it, and Rust will then build a `TokenPricingSchedule::SetPrices` with a silently-rounded price. Either gate this path with an explicit `value.is_bigint()` check so the TS contract is enforced at runtime, or relax the TS type to `bigint | number | string` so it matches the implementation. Note this is the same lenient pattern used for the existing flat `price` field, so the team may decide to leave it as-is for consistency.

Comment on lines +2062 to +2108
fn extract_price_tiers(
options: &JsValue,
) -> Result<Option<BTreeMap<TokenAmount, Credits>>, WasmSdkError> {
let value = js_sys::Reflect::get(options, &JsValue::from_str("priceTiers"))
.map_err(|_| WasmSdkError::invalid_argument("failed to read 'priceTiers' from options"))?;

if value.is_undefined() || value.is_null() {
return Ok(None);
}

if !value.is_object() || js_sys::Array::is_array(&value) {
return Err(WasmSdkError::invalid_argument(
"'priceTiers' must be an object mapping token amount keys to bigint credit prices",
));
}

let obj = js_sys::Object::from(value);
let entries = js_sys::Object::entries(&obj);
let mut tiers: BTreeMap<TokenAmount, Credits> = BTreeMap::new();

for entry in entries.iter() {
let entry_arr = js_sys::Array::from(&entry);
let key = entry_arr.get(0);
let val = entry_arr.get(1);

let key_str = key.as_string().ok_or_else(|| {
WasmSdkError::invalid_argument(
"'priceTiers' keys must be strings encoding token amounts",
)
})?;

let amount: TokenAmount = key_str.parse().map_err(|err| {
WasmSdkError::invalid_argument(format!(
"Invalid token amount key '{}' in priceTiers: {}",
key_str, err
))
})?;

let credits = try_to_u64(&val, &format!("priceTiers['{}']", key_str))?;

tiers.insert(amount, credits);
}

validate_price_tiers(&tiers)?;

Ok(Some(tiers))
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🟡 Suggestion: No wasm/JS-runtime test covers the new priceTiers extraction path

extract_price_tiers is the only piece of code that exercises the hand-rolled Reflect::get + Object.entries + try_to_u64 path for Record<string, bigint>. The Rust unit tests only validate an already-constructed BTreeMap, and the js-evo-sdk facade spec only asserts stub forwarding. That leaves regressions in bigint→u64 conversion, the both-fields rejection, key parsing, and the SetPrices mapping uncovered by any test that actually crosses the JS/WASM boundary. Add a wasm-bindgen-test (or equivalent web-target test) that calls tokenSetPrice with a real JS object containing bigint tier values and asserts both the happy path and the price+priceTiers rejection.

source: ['codex', 'claude']

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Still present at b9dd057: current head adds pure-Rust helper tests, but still does not exercise the JS↔WASM priceTiers extraction path with a real JS object.

))
})?;

let credits = try_to_u64(&val, &format!("priceTiers['{}']", key_str))?;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🟡 Suggestion: priceTiers values are typed as bigint in TS but accepted as Number/String at runtime

The TypeScript declaration at line 1966 says priceTiers?: Record<string, bigint>, but try_to_u64 (wasm-dpp2/src/utils.rs:326) also accepts JS Number and numeric String. For plain JS callers (no TS type-checking), passing a Number > 2^53-1 will already be rounded by JS before Rust sees it, and Rust will then build a TokenPricingSchedule::SetPrices with a silently-rounded price. Either gate this path with an explicit value.is_bigint() check so the TS contract is enforced at runtime, or relax the TS type to bigint | number | string so it matches the implementation. Note this is the same lenient pattern used for the existing flat price field, so the team may decide to leave it as-is for consistency.

source: ['codex']

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Still present at b9dd057.

…dation

- Detect priceTiers keys that parse to the same TokenAmount (e.g. "1" vs
  "01") instead of silently overwriting via BTreeMap::insert, surfacing
  both the original and duplicate string keys in the error message.
- Require tokenSetPrice callers to specify exactly one of `price`
  (including explicit `null` for disable) or `priceTiers`, so an empty
  options object is no longer silently treated like `price: null`. The
  existing `price` + `priceTiers` rejection is preserved.
- Add focused Rust unit tests for both validation paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thepastaclaw

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Code Review

Latest delta (c348df28cb06a9) only adjusts the js-evo-sdk Mocha loader invocation in package.json; token.rs is byte-identical. Both prior priceTiers boundary suggestions still apply at head 8cb06a9 and are carried forward. Codex's new claim that omitting both price fields is a blocking bug is demoted: the same Option<u64> + #[serde(default)] shape existed pre-PR, and the code's own line 2232 comment documents the behavior as intentional, so we surface it as a suggestion to either reject or document omit-both rather than block.

🟡 3 suggestion(s)

Carried-forward prior findings

  • [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2062-2108: No wasm/JS-runtime test exercises the new priceTiers JS↔WASM pathextract_price_tiers is the only code that drives the hand-rolled Reflect::get + Object.entries + try_to_u64 extraction for Record<string, bigint>. Existing Rust unit tests only validate an already-constructed BTreeMap, and the js-evo-sdk facade spec only asserts forwarding to a stub — neither crosses the wasm-bindgen boundary. That leaves bigint→u64 conversion, the price+priceTiers mutual-exclusion check, key parsing, and the mapping to TokenPricingSchedule::SetPrices uncovered against the only place where attacker/user-controlled JS shapes (arrays, BigInts > u64::MAX, numeric strings) get coerced into a signed state transition. Add a wasm-bindgen-test (or web-target functional test alongside tests/functional/transitions/tokens.spec.ts) that calls tokenSetPrice with a real JS object containing bigint tier values and asserts both the happy path and the both-fields rejection.
  • [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2100: priceTiers values typed as bigint in TS but accepted as Number/String at runtime — The TypeScript declaration at token.rs:1966 advertises priceTiers?: Record<string, bigint>, but try_to_u64 (wasm-dpp2/src/utils.rs:326) also accepts JS Number and numeric String. A plain-JS caller without TS type-checking who passes a Number greater than Number.MAX_SAFE_INTEGER (2^53-1) will have V8 silently round the value before Rust sees it; the rounded credit price is then encoded into a TokenPricingSchedule::SetPrices the caller never intended to sign. Token credit prices routinely exceed 2^53. Either gate this call site with an explicit val.is_bigint() check so the TS contract is enforced at the boundary, or widen the TS type to bigint | number | string to match the lenient converter. The flat price field uses the same lenient pattern, so this is a conscious-consistency call rather than a hard bug — flagging so the team picks deliberately.

New findings in latest delta

  • None. The latest delta only changes packages/js-evo-sdk/package.json test-loader flags.

Additional cumulative finding

  • [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2215-2237: Omitting both price and priceTiers silently disables direct purchases — The PR loosened the TypeScript contract from price: bigint | null to price?: bigint | null, so a TS caller can now legally omit both price and priceTiers. Because parsed.price is Option<u64> with #[serde(default)], that omit-both case falls through to pricing_schedule = None, which the lower builder treats as 'no schedule' — equivalent to setting price: null to disable direct purchases. The TS doc on price says the way to disable is by passing null, so 'omit both' producing the same destructive effect is undocumented and surprising. The behavior is intentional per the line 2232 comment, but the asymmetry between docs and runtime invites accidental schedule wipes from a typoed field name. Either reject omit-both with WasmSdkError::invalid_argument("Specify either 'price' (flat or null) or 'priceTiers' (tiered)"), or update the TS doc to call out that omitting both is also a disable.
Prior finding reconciliation
  • prior-1: STILL VALID — token.rs is byte-identical between c348df2 and 8cb06a9 (delta is only js-evo-sdk/package.json). extract_price_tiers (lines 2062–2108) is unchanged and no wasm-bindgen-test or web-target test exercising the JS→WASM priceTiers path was added.
  • prior-2: STILL VALID — Line 2100 still calls try_to_u64(&val, ...) with no is_bigint() precheck. try_to_u64 in wasm-dpp2/src/utils.rs:326 still accepts BigInt, Number, and numeric String, so the TS-declared Record<string, bigint> is not enforced at the boundary.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2062-2108: No wasm/JS-runtime test exercises the new priceTiers JS↔WASM path
  `extract_price_tiers` is the only code that drives the hand-rolled `Reflect::get` + `Object.entries` + `try_to_u64` extraction for `Record<string, bigint>`. Existing Rust unit tests only validate an already-constructed `BTreeMap`, and the js-evo-sdk facade spec only asserts forwarding to a stub — neither crosses the wasm-bindgen boundary. That leaves bigint→u64 conversion, the `price`+`priceTiers` mutual-exclusion check, key parsing, and the mapping to `TokenPricingSchedule::SetPrices` uncovered against the only place where attacker/user-controlled JS shapes (arrays, BigInts > u64::MAX, numeric strings) get coerced into a signed state transition. Add a wasm-bindgen-test (or web-target functional test alongside `tests/functional/transitions/tokens.spec.ts`) that calls `tokenSetPrice` with a real JS object containing bigint tier values and asserts both the happy path and the both-fields rejection.
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2100: priceTiers values typed as bigint in TS but accepted as Number/String at runtime
  The TypeScript declaration at token.rs:1966 advertises `priceTiers?: Record<string, bigint>`, but `try_to_u64` (wasm-dpp2/src/utils.rs:326) also accepts JS `Number` and numeric `String`. A plain-JS caller without TS type-checking who passes a `Number` greater than `Number.MAX_SAFE_INTEGER` (2^53-1) will have V8 silently round the value before Rust sees it; the rounded credit price is then encoded into a `TokenPricingSchedule::SetPrices` the caller never intended to sign. Token credit prices routinely exceed 2^53. Either gate this call site with an explicit `val.is_bigint()` check so the TS contract is enforced at the boundary, or widen the TS type to `bigint | number | string` to match the lenient converter. The flat `price` field uses the same lenient pattern, so this is a conscious-consistency call rather than a hard bug — flagging so the team picks deliberately.
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2215-2237: Omitting both price and priceTiers silently disables direct purchases
  The PR loosened the TypeScript contract from `price: bigint | null` to `price?: bigint | null`, so a TS caller can now legally omit both `price` and `priceTiers`. Because `parsed.price` is `Option<u64>` with `#[serde(default)]`, that omit-both case falls through to `pricing_schedule = None`, which the lower builder treats as 'no schedule' — equivalent to setting `price: null` to disable direct purchases. The TS doc on `price` says the way to disable is by passing `null`, so 'omit both' producing the same destructive effect is undocumented and surprising. The behavior is intentional per the line 2232 comment, but the asymmetry between docs and runtime invites accidental schedule wipes from a typoed field name. Either reject omit-both with `WasmSdkError::invalid_argument("Specify either 'price' (flat or null) or 'priceTiers' (tiered)")`, or update the TS doc to call out that omitting both is also a disable.

Note: GitHub does not allow me to submit APPROVE on my own PR, so this exact-SHA review is posted as COMMENT while preserving the verified suggestions.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Code Review

Cumulative review at b9dd057. The latest delta resolves prior-3 (silent-disable footgun) by introducing validate_pricing_mode_selection and also addresses CodeRabbit's duplicate-key concern via build_price_tiers. Two prior suggestion-level findings remain valid and are carried forward: missing wasm-bindgen/JS-runtime coverage for the priceTiers extraction boundary, and the TS-vs-runtime contract mismatch where priceTiers is typed bigint but try_to_u64 silently accepts JS Number/String. No new blocking issues introduced.

🟡 2 suggestion(s)

Carried-forward findings already raised (1)

These findings were not re-posted as new inline comments because an existing review thread already covers them.

  • [SUGGESTION] (deduped existing open thread) packages/wasm-sdk/src/state_transitions/token.rs:2143-2152: priceTiers values typed as bigint in TS but accepted as Number/String at runtime — The TypeScript declaration at token.rs:1966 says priceTiers?: Record<string, bigint>, but try_to_u64 (wasm-dpp2/src/utils.rs:326) lenient-accepts JS Number and numeric String in addition to BigInt. For plain JS callers without TS type-checking, passing a Number greater than 2^53 - 1...
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/wasm-sdk/src/state_transitions/token.rs`:
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2111-2156: No wasm-bindgen/JS-runtime test exercises the priceTiers JS↔WASM boundary
  `extract_price_tiers` is the only code that crosses the JS↔WASM boundary for this new feature: it does `Reflect::get("priceTiers")`, walks `Object.entries`, parses string keys to `TokenAmount`, and routes values through `try_to_u64` (which itself accepts BigInt/Number/String). The unit tests added in this delta only validate the pure-Rust helpers (`build_price_tiers`, `validate_price_tiers`, `validate_pricing_mode_selection`) against already-parsed inputs, and the js-evo-sdk facade spec at packages/js-evo-sdk/tests/unit/facades/tokens.spec.ts:476-508 only asserts stub forwarding. Regressions in (a) BigInt→u64 conversion, (b) the both-fields rejection wired via `Reflect::get`, (c) array-as-object rejection, (d) key-string parsing, and (e) the final SetPrices vs SinglePrice mapping would not be caught by any test that actually crosses the boundary. Add a wasm-bindgen-test (or equivalent web-target test against the built artifact) that calls `tokenSetPrice` with a real JS object containing bigint tier values and asserts both the happy path and the price+priceTiers rejection.
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/token.rs:2143-2152: priceTiers values typed as bigint in TS but accepted as Number/String at runtime
  The TypeScript declaration at token.rs:1966 says `priceTiers?: Record<string, bigint>`, but `try_to_u64` (wasm-dpp2/src/utils.rs:326) lenient-accepts JS `Number` and numeric `String` in addition to `BigInt`. For plain JS callers without TS type-checking, passing a `Number` greater than `2^53 - 1` will already be rounded by V8 before Rust ever sees it, producing a `TokenPricingSchedule::SetPrices` with a silently truncated per-tier credit value at the precise boundary where precision matters most. Either enforce the TS contract at runtime by gating the value path with `val.is_bigint()` before calling `try_to_u64`, or relax the TS type to `Record<string, bigint | number | string>` so the declared contract matches behavior. Note the flat `price` field uses the same lenient pattern via serde, so the choice between strict-runtime and loose-TS should be deliberate for consistency.

@thepastaclaw

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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