fix(wasm-sdk): label getTokenContractInfo parameter as tokenId, not contractId#3851
Conversation
…ctId getTokenContractInfo and getTokenContractInfoWithProofInfo labeled their parameter dataContractId, but the query is keyed by token ID. A token ID is a distinct value derived from a contract ID and position, so passing a contract ID matched no record and silently resolved to undefined. Rename the parameter to tokenId, correct the error messages, and add doc comments pointing callers to calculateTokenIdFromContract. The functional test passed a contract ID with no assertion, so it silently exercised the bug. It now derives a token ID and asserts the returned contract info. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The contractInfo and contractInfoWithProof facade methods labeled their parameter contractId, but the underlying query is keyed by token ID, so passing a contract ID silently resolved to undefined. Rename the parameter to tokenId with JSDoc pointing callers to TokensFacade.calculateId, and update the unit tests to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughToken contract info query methods in the WASM SDK are refactored to accept and parse token IDs instead of data contract IDs, with parameter names updated throughout the JS SDK facade and corresponding unit and functional tests adjusted accordingly. ChangesToken Contract Info Query Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
✅ Review complete (commit 8c28488) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3851 +/- ##
=========================================
Coverage 87.15% 87.15%
=========================================
Files 2641 2641
Lines 327793 327793
=========================================
Hits 285701 285701
Misses 42092 42092
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Small, well-scoped labeling fix that renames the public WASM/JS parameter from contract/dataContract ID to tokenId on getTokenContractInfo and its proof variant. Underlying query and FFI behavior are unchanged. One valid documentation nitpick: the new JSDoc example calls the static token-ID helper as an instance method.
💬 1 nitpick(s)
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Verified prior-1 against current head: the JSDoc example at packages/wasm-sdk/src/queries/token.rs:476 now correctly uses the static form WasmSdk.calculateTokenIdFromContract(...), matching how wasm-bindgen exposes the helper (it has no &self receiver). Both agents and CodeRabbit converge on no new in-scope findings for the 8c28488 delta (rustfmt + doc nit fix on top of the parameter renames). PR is a clean, well-scoped fix.
Issue being fixed or feature implemented
getTokenContractInfo/getTokenContractInfoWithProofInfo(wasm-sdk) and thetokens.contractInfo/tokens.contractInfoWithProoffacade (js-evo-sdk) labeled their single parameterdataContractId/contractId. The query is actually keyed by token ID, which is a distinct value derived from a data contract ID and the token's position (token_id = sha256d("dash_token" || contract_id || position)).Because the Drive query is a single-key GroveDB lookup keyed by
token_id, passing a contract ID — as the parameter name invited — matched no record and silently resolved toundefinedrather than erroring. Developers following the parameter name would conclude the token had no contract info.The Rust SDK query type (
TokenContractInfoQuery { token_id }), the gRPC proto field, and the Drive layer were already correctly namedtoken_id; the mislabeling existed only at the WASM/JS surface.What was done?
tokenIdongetTokenContractInfoandgetTokenContractInfoWithProofInfo, corrected the error messages to "Invalid token ID", and added doc comments that state the argument is a token ID and point tocalculateTokenIdFromContractfor deriving one.tokenIdon thetokens.contractInfo/tokens.contractInfoWithProoffacade methods with matching JSDoc.calculateTokenIdFromContractand asserts the returnedcontractId/tokenContractPosition.The generated
dist/*.d.tsregenerate from the wasm_bindgenjs_nameand TS source on build, so they were not hand-edited.How Has This Been Tested?
cargo build -p wasm-sdk— compiles clean.yarn workspace @dashevo/wasm-sdk test:functional— the rewrittengetTokenContractInfo()test now asserts the result, so it fails loudly if a contract ID (silentNone) is ever passed again.yarn workspace @dashevo/evo-sdk test:unit— facade pass-through tests.Breaking Changes
None at runtime. wasm_bindgen generates positional JS arguments, so renaming the parameter does not change the call signature for existing callers — it only corrects the TypeScript parameter label and generated docs. The old
dataContractId/contractIdlabel was actively misleading and is the reason for this change.Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit