-
Notifications
You must be signed in to change notification settings - Fork 44
fix(wasm-sdk): use identity contract nonce for data contract updates #2738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Data contract update transitions require identity_contract_nonce instead of identity_nonce. This aligns with the DataContractUpdateTransition struct and matches how document/token operations work. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughSwitches contract update signing to use per-contract nonces by replacing identity-wide nonce retrieval with get_identity_contract_nonce(owner_identifier, contract_identifier, ...). Introduces identity_contract_nonce and updates related error messaging and DataContractUpdateTransition construction accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant WasmSdk as WasmSdk::contract_update
participant Platform as Platform API
App->>WasmSdk: Request contract update (owner_id, contract_id, update)
note right of WasmSdk: Contract updates use per-contract nonces
WasmSdk->>Platform: get_identity_contract_nonce(owner_id, contract_id, proof=true)
Platform-->>WasmSdk: identity_contract_nonce or error
alt nonce retrieved
WasmSdk->>WasmSdk: Build DataContractUpdateTransition with per-contract nonce
WasmSdk-->>App: Signed update transition
else error
WasmSdk-->>App: Error fetching identity contract nonce
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/wasm-sdk/src/state_transitions/contracts/mod.rs (2)
245-260
: Add sanity checks: ensure updated JSON targets the same contract and ownerPrevent mismatches early by verifying the updated contract’s
$id
andownerId
match the inputs before building the transition.Apply after constructing
updated_contract
and before the version check:let updated_contract = DataContract::from_json( updates_json, true, // validate sdk.version(), ) .map_err(|e| JsValue::from_str(&format!("Failed to create updated contract from JSON: {}", e)))?; + // Sanity checks: ensure the update targets the same contract and owner + if updated_contract.id() != contract_identifier { + return Err(JsValue::from_str("Updated contract ID does not match provided contract_id")); + } + if updated_contract.owner_id() != owner_identifier { + return Err(JsValue::from_str("Updated contract ownerId does not match provided owner_id")); + } // Verify the version was incremented if updated_contract.version() <= existing_contract.version() {
262-267
: All contract-update paths uniformly use per-contract nonces with bump=trueI’ve verified across both
wasm-sdk
andrs-sdk
that every contract update transition callsget_identity_contract_nonce(..., true, None)and there are no remaining calls to the identity-wide
get_identity_nonce
within anycontract_update
or similar modules. The third parameter (true
) correctly bumps the nonce on fetch (per prior learnings), ensuring consistency with document/token operations.Bullet points of findings:
- No instances of
get_identity_nonce
in any contract-update path (e.g. instate_transitions/contracts
,tokens
,documents
inwasm-sdk
, nor inplatform/transition/*
inrs-sdk
).- All per-contract nonce fetches use
true
for bump semantics.- The bump behavior (“increment before storing”) aligns with the SDK’s caching logic documented in
sdk.rs
.Recommendations:
- Document in the SDK README (or inline docs) that callers must serialize calls per (owner, contract) to avoid nonce-bumping races, or implement a lightweight mutex in the SDK around
get_identity_contract_nonce
.- Nit: update the JS error string for clarity, for example:
- .map_err(|e| JsValue::from_str(&format!("Failed to get identity contract nonce: {}", e)))?; + .map_err(|e| JsValue::from_str(&format!("Failed to get per-contract identity nonce: {}", e)))?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/wasm-sdk/src/state_transitions/contracts/mod.rs
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/wasm-sdk/src/**/*.rs
📄 CodeRabbit inference engine (packages/wasm-sdk/CLAUDE.md)
packages/wasm-sdk/src/**/*.rs
: When implementing WASM SDK functionality, always refer to AI_REFERENCE.md first for accurate method signatures and examples.
For WASM builds, fix 'time not implemented on this platform' errors by using js_sys::Date::now().
Token functions are methods on WasmSdk, not standalone functions; avoid importing them as standalone.
The WASM SDK now fully supports where and orderBy clauses for document queries; use the specified JSON array formats and supported operators.
Files:
packages/wasm-sdk/src/state_transitions/contracts/mod.rs
🧠 Learnings (2)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2449
File: packages/rs-sdk/src/platform/transition/fungible_tokens/unfreeze.rs:148-148
Timestamp: 2025-02-14T04:31:57.245Z
Learning: The third boolean parameter in `get_identity_contract_nonce` method controls whether to bump (increment) the nonce when set to `true`.
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
PR: dashpay/platform#2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/wasm-sdk/src/state_transitions/contracts/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-and-test-wasm-sdk
- GitHub Check: build-wasm-sdk
🔇 Additional comments (1)
packages/wasm-sdk/src/state_transitions/contracts/mod.rs (1)
286-287
: Nonce correctly plumbed into DataContractUpdateTransitionPassing
identity_contract_nonce
intoDataContractUpdateTransition::new_from_data_contract
is the intended wiring for per-contract nonces. Looks good.
…2738) Co-authored-by: Claude <noreply@anthropic.com>
Issue being fixed or feature implemented
Data contract update transitions require identity_contract_nonce instead of identity_nonce. This aligns with the DataContractUpdateTransition struct and matches how document/token operations work.
What was done?
Switched to identity contract nonce for contract update
How Has This Been Tested?
Locally
Breaking Changes
🤔 Technically maybe. But not really, because it was mostly non-functional before (it only worked when your identity nonce and identity contract nonce were the same)
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Bug Fixes
Improvements