Repository navigation
feat(sdk): random token amounts in wasm-sdk and js-evo-sdk - #5751
QuantumExplorer wants to merge 24 commits into
Conversation
`tokenTransfer` and `tokenMint` take an optional `randomMinimumAmount`: the amount moved is then drawn from it up to `amount` by the block's random beacon (protocol version 15). The options pass straight to the rs-sdk builders' `with_random_minimum_amount`; rs-dpp validates the range. Refs #4682 Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Static verification of head 365959c confirms that both token operations correctly forward the optional random minimum into existing Rust SDK builders, with semantic validation remaining in DPP. The two coverage findings describe one verified, non-blocking gap: the new tests stub the WASM methods and never exercise option deserialization. Current PR checks report only successful policy/review checks; no build or test results are available, and no local builds or tests were run.
🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The diff is a small, contained addition that passes an optional random minimum amount through existing SDK builders and tests facade forwarding without changing underlying token or consensus rules. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 97% left, weekly 78% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer
🤖 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:731-737: Exercise randomMinimumAmount parsing at the WASM boundary
The added js-evo-sdk tests stub `tokenMint` and `tokenTransfer`, so they verify facade forwarding but never execute either new `Option<u64>` field's deserialization. There are no corresponding random-minimum tests under wasm-sdk. Although the current camelCase mapping and builder calls are correct, a future field-name mismatch would silently ignore the supplied minimum and produce a fixed-amount operation while these tests still passed. Add binding-level regression tests for both methods. The existing `tests/unit/action-fee-agreement-option.spec.ts` demonstrates testing rejection before network access: with otherwise-valid options, `randomMinimumAmount: '10'` should fail with `Invalid token mint options` or `Invalid token transfer options`, because primitive parsing precedes contract fetching. Also cover successful BigInt parsing and omission through an isolated parsing test or another deterministic no-network test seam. A malformed-input test verifies recognition of the field, but does not by itself verify forwarding into the builder.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Independently reviewed the complete diff at 9eaaeb1 and confirmed that both optional minima are deserialized and forwarded to the existing Rust builders without introducing binding-owned business logic; no actionable findings remain. The prior parsing-coverage suggestion is addressed by tests of both production option structs, covering presence and omission, although these do not execute JavaScript BigInt conversion. Validation was static only: current PR checks show successful policy/title checks and a skipped CodeRabbit review, but no functional build or test results.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The diff is a small, contained addition of an optional parameter passed through existing WASM builders, with boundary and facade tests rather than changes to token settlement or consensus logic. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 82% left, weekly 75% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Independently reviewed the complete two-file diff at 4c5d202 and found no actionable in-scope defects: both WASM operations parse and forward the optional minimum to the existing Rust builders without adding protocol decisions. The added parsing regression addresses the prior finding for both mint and transfer, including omission. Validation was static only; current PR checks show successful policy/title checks but no build or test results.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The diff is a small, contained addition of optional transfer and mint parameters passed through existing WASM builders, plus JavaScript facade tests, without changing validation or consensus logic. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 99% left, weekly 74% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
At head 3319cdc, the complete diff correctly exposes randomMinimumAmount for mint and transfer, parses it into the existing Rust builders, and leaves protocol validation in DPP. The added parsing regression test addresses the prior suggestion; no actionable in-scope defects remain. Validation was static only: current PR checks report policy/title success and a skipped CodeRabbit review, but no functional build or test results.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The diff is a small, contained addition of optional amount parameters passed through existing SDK builders, plus facade tests, without changing underlying validation or funds-movement rules. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 94% left, weekly 70% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort medium); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
…-js-sdk Co-Authored-By: Claude Opus 5.5 <[email protected]>
…-js-sdk Co-Authored-By: Claude Opus 5.5 <[email protected]>
…-js-sdk Co-Authored-By: Claude Opus 5.5 <[email protected]>
…-js-sdk Co-Authored-By: Claude Opus 5.5 <[email protected]>
…-js-sdk Co-Authored-By: Claude Opus 5.5 <[email protected]>
…-js-sdk Co-Authored-By: Claude Opus 5.5 <[email protected]>
Basic explanation
What this does: JavaScript apps can ask for a random token transfer or mint.
tokenTransferandtokenMintin wasm-sdk (and sosdk.tokens.transfer/sdk.tokens.mintin js-evo-sdk) take an optionalrandomMinimumAmount. With it, the amount moved is drawn fromrandomMinimumAmounttoamountby the random beacon of the block that accepts the transition (#5750);amountis what gets reserved.Before:
After:
Value: Games and prize apps in JavaScript get provably fair token payouts without trusting the app.
Risks: Low. The bindings only pass the option through to the rs-sdk builders'
with_random_minimum_amount; rs-dpp validates the range and refuses the transition before protocol version 15. Without the option, behaviour is unchanged.Stacked on #5750 (random token amounts), which is stacked on #5749.
Issue being fixed or feature implemented
Refs #4682. Exposes #5750's random token amounts to JavaScript.
What was done?
packages/wasm-sdk/src/state_transitions/token.rs:randomMinimumAmount?: bigintonTokenTransferOptionsandTokenMintOptions(with the TypeScript docs), read into the options structs and passed to the builders. No validation in the binding, per the wasm conventions.packages/js-evo-sdk/tests/unit/facades/tokens.spec.ts: the facades pass the option through unchanged.How Has This Been Tested?
cargo check -p wasm-sdkandcargo clippy -p wasm-sdk -- -D warnings: clean.Breaking Changes
None: a new optional field.
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code