feat(wasm-sdk): add prepare_* APIs for idempotent document state transitions#3091
feat(wasm-sdk): add prepare_* APIs for idempotent document state transitions#3091thepastaclaw wants to merge 3 commits intodashpay:v3.1-devfrom
Conversation
…sitions Add prepare variants for document create, replace, and delete operations that build and sign a StateTransition without broadcasting. This enables idempotent retry patterns where callers can cache the signed ST bytes and rebroadcast on timeout instead of creating duplicates with new nonces. New methods: - prepareDocumentCreate() — build, sign, return ST - prepareDocumentReplace() — build, sign, return ST - prepareDocumentDelete() — build, sign, return ST These pair with the existing broadcastStateTransition() and waitForResponse() methods already exposed in broadcast.rs. Closes dashpay#3090
📝 WalkthroughWalkthroughAdds a two‑phase Prepare + Execute API for document state transitions in the WASM SDK by introducing three prepare methods that build, sign, and return signed StateTransition objects without broadcasting, allowing applications to cache and rebroadcast identical signed transitions for idempotent retries. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant SDK as WASM SDK
participant Signer as Signer
participant Platform as Platform
App->>SDK: prepare_document_create(document, identityKey, signer, options)
SDK->>SDK: build_document_create_or_replace_transition(document, entropy?, ...)
SDK->>Signer: sign(stateTransition)
Signer-->>SDK: signed StateTransition
SDK-->>App: return signed StateTransition
App->>SDK: broadcastStateTransition(signedST)
SDK->>Platform: submit signed ST
Platform-->>SDK: accept/confirm
SDK-->>App: broadcast acknowledgment
App->>SDK: waitForResponse(signedST)
SDK->>Platform: query ST status
Platform-->>SDK: confirmed/result
SDK-->>App: final result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/wasm-sdk/src/state_transitions/document.rs (2)
465-521: Consider extracting shared option-parsing logic to reduce duplication.
prepare_document_create(lines 469–506) duplicates nearly all of the extraction logic fromdocument_create(lines 111–147): document, entropy, identity key, signer, contract fetch, document type, and settings. The same pattern applies toprepare_document_replacevsdocument_replace, andprepare_document_deletevsdocument_delete.A private helper (e.g.,
extract_create_options(options) → (Document, [u8;32], IdentityPublicKey, Signer, DataContract, DocumentType, Option<PutSettings>)) for each operation variant would let both the all-in-one and prepare methods share the parsing/validation code, reducing the surface area for divergence bugs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-sdk/src/state_transitions/document.rs` around lines 465 - 521, Multiple methods duplicate option parsing/validation (prepare_document_create vs document_create and similar prepare_/document_ pairs); extract the repeated logic into a private helper (e.g., extract_document_create_options) that performs DocumentWasm::try_from_options + Document conversion, entropy validation and conversion to [u8;32], IdentityPublicKeyWasm::try_from_options -> IdentityPublicKey, IdentitySignerWasm::try_from_options (or signer wrapper), calls self.get_or_fetch_contract(contract_id).await, resolves document_type via get_document_type, and parses settings via try_from_options_optional; have both prepare_document_create and document_create call this helper and return the tuple (Document, [u8;32], IdentityPublicKey, IdentitySignerWasm/Signer, DataContract, DocumentType, Option<PutSettingsInput/Into>) to eliminate duplication and keep behavior identical.
1121-1122: Minor: prefermap_oroveris_some()+unwrap()for the revision check.The double-call to
document.revision()with anunwrap()is safe (guarded byis_some()), but a more idiomatic pattern avoids the rawunwrap():♻️ Suggested simplification
- let transition = if document.revision().is_some() - && document.revision().unwrap() != INITIAL_REVISION - { + let transition = if document + .revision() + .map_or(false, |rev| rev != INITIAL_REVISION) + {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-sdk/src/state_transitions/document.rs` around lines 1121 - 1122, Replace the is_some() + unwrap() pattern when building the transition with a single map_or call on document.revision(): compute the boolean condition as document.revision().map_or(false, |r| r != INITIAL_REVISION) (or equivalent map_or_else) and use that in the if that assigns transition so you no longer call revision() twice or unwrap; update the branch that currently reads the revision check to use this mapped result (refer to document.revision() and INITIAL_REVISION in the transition assignment).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/wasm-sdk/src/state_transitions/document.rs`:
- Around line 465-521: Multiple methods duplicate option parsing/validation
(prepare_document_create vs document_create and similar prepare_/document_
pairs); extract the repeated logic into a private helper (e.g.,
extract_document_create_options) that performs DocumentWasm::try_from_options +
Document conversion, entropy validation and conversion to [u8;32],
IdentityPublicKeyWasm::try_from_options -> IdentityPublicKey,
IdentitySignerWasm::try_from_options (or signer wrapper), calls
self.get_or_fetch_contract(contract_id).await, resolves document_type via
get_document_type, and parses settings via try_from_options_optional; have both
prepare_document_create and document_create call this helper and return the
tuple (Document, [u8;32], IdentityPublicKey, IdentitySignerWasm/Signer,
DataContract, DocumentType, Option<PutSettingsInput/Into>) to eliminate
duplication and keep behavior identical.
- Around line 1121-1122: Replace the is_some() + unwrap() pattern when building
the transition with a single map_or call on document.revision(): compute the
boolean condition as document.revision().map_or(false, |r| r !=
INITIAL_REVISION) (or equivalent map_or_else) and use that in the if that
assigns transition so you no longer call revision() twice or unwrap; update the
branch that currently reads the revision check to use this mapped result (refer
to document.revision() and INITIAL_REVISION in the transition assignment).
shumkov
left a comment
shumkov
left a comment
There was a problem hiding this comment.
please provide a snippet of code which doesn't work
|
@shumkov — here's the code snippet showing what doesn't work: The Problem// Current API: documentCreate() is atomic (nonce bump + sign + broadcast + wait)
try {
await sdk.documents.create({ document, identityKey, signer });
} catch (err) {
if (isTimeoutError(err)) {
// The ST was broadcast, but waitForResponse timed out (504 from DAPI gateway).
// Did it land on Platform? We don't know.
//
// Our only option is to retry — but documentCreate() will:
// 1. Fetch a NEW nonce (old nonce + 1)
// 2. Build a NEW StateTransition with different bytes
// 3. Sign and broadcast this NEW ST
//
// If the first ST DID land, we now have TWO documents (double post).
// There is no way to rebroadcast the original ST because
// documentCreate() never exposes it to the caller.
await sdk.documents.create({ document, identityKey, signer }); // DUPLICATE
}
}This is the exact bug PastaPastaPasta/yappr#260 hit in production — DAPI gateway 504s caused double-posting. The workaround was ~200 lines of manual ST construction: // What the app had to do: manually build the full ST chain
const createTransition = new DocumentCreateTransition(document, nonce + 1n, null, null);
const batched = new BatchedTransition(createTransition.toDocumentTransition());
const batchTransition = BatchTransition.fromBatchedTransitions([batched], ownerId, 0);
const st = batchTransition.toStateTransition();
st.setIdentityContractNonce(nonce + 1n);
st.sign(PrivateKey.fromWIF(wif), identityKey);
// Cache bytes before broadcasting for safe retry
const stBytes = st.toBytes();
localStorage.setItem(cacheKey, base64Encode(stBytes));
await sdk.wasm.broadcastStateTransition(st);
await sdk.wasm.waitForResponse(st);
// On timeout: reload cached bytes, rebroadcast SAME ST (idempotent)
const cached = StateTransition.fromBytes(localStorage.getItem(cacheKey));
await sdk.wasm.broadcastStateTransition(cached);With the const st = await sdk.prepareDocumentCreate({ document, identityKey, signer });
const stBytes = st.toBytes(); // cache for retry safety
await sdk.broadcastStateTransition(st);
await sdk.waitForResponse(st);
// On timeout: StateTransition.fromBytes(stBytes) → rebroadcast |
|
@shumkov Here's the concrete problem — the high-level // Current API — double-post on timeout
try {
const result = await sdk.documentCreate({
document, identityKey, signer,
settings: { timeoutMs: 30_000 }
});
} catch (e) {
// If this was a 504 timeout, did the ST land on Platform?
// We don't know, and we can't rebroadcast the same ST.
//
// Retrying calls documentCreate() again, which:
// 1. Fetches a NEW nonce (bumped)
// 2. Builds a NEW StateTransition
// 3. Signs and broadcasts it
// → If the first one DID land, we now have a DUPLICATE document.
const result2 = await sdk.documentCreate({
document, identityKey, signer,
settings: { timeoutMs: 30_000 }
});
}This actively causes double-posting in production (see yappr#260 for the ~200-line manual workaround). With // New two-phase API — idempotent retry
const st = await sdk.prepareDocumentCreate({
document, identityKey, signer
});
const stBytes = st.toBytes(); // cache for retry safety
try {
await sdk.broadcastStateTransition(st);
await sdk.waitForResponse(st);
} catch (e) {
if (isTimeout(e)) {
// Rebroadcast the IDENTICAL ST — same nonce, same signature
// Platform will either accept it (first didn't land) or
// reject as duplicate (first succeeded). No double-post possible.
const cached = StateTransition.fromBytes(stBytes);
await sdk.broadcastStateTransition(cached);
await sdk.waitForResponse(cached);
}
}The key difference: 🤖 This was generated by an automated review bot. |
Addresses CodeRabbit nitpick - more idiomatic Rust pattern that avoids calling revision() twice and the unnecessary unwrap().
|
Addressed the For the option-parsing duplication extraction: I agree there's shared logic between the 🤖 This was generated by an automated review bot. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/wasm-sdk/src/state_transitions/document.rs (2)
1108-1108: Move the inlineusedeclaration to module-level importsPlacing
useinside a function body is valid Rust but non-idiomatic and inconsistent with the rest of the file where all trait imports are at the top.♻️ Proposed fix
-async fn build_document_create_or_replace_transition(...) ... { - use dash_sdk::dpp::data_contract::document_type::accessors::DocumentTypeV0Getters; - - let new_identity_contract_nonce = ...Move to the module-level import block (near the other
use dash_sdk::dpp::data_contract::...imports):+use dash_sdk::dpp::data_contract::document_type::accessors::DocumentTypeV0Getters;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-sdk/src/state_transitions/document.rs` at line 1108, The inline use statement for the trait DocumentTypeV0Getters should be moved from inside the function body to the module-level import block alongside the other dash_sdk::dpp::data_contract::... imports; update the top-of-file use imports to include use dash_sdk::dpp::data_contract::document_type::accessors::DocumentTypeV0Getters and remove the inline declaration so trait methods resolve consistently and match the file's import style.
465-521: Consider extracting shared option-parsing logic to eliminate duplication across prepare/non-prepare variants
prepare_document_create(lines 470–506) anddocument_create(lines 111–147) contain identical blocks for: document extraction, entropy validation, identity-key extraction, signer extraction, contract fetch, document-type lookup, and settings extraction. Same duplication exists betweendocument_replace/prepare_document_replaceanddocument_delete/prepare_document_delete. Extracting these into small helpers (e.g.parse_document_create_opts,parse_delete_document_spec) would make the diverging parts (broadcast vs. return ST) obvious and reduce maintenance surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-sdk/src/state_transitions/document.rs` around lines 465 - 521, Extract the duplicated option-parsing and validation logic into small helper functions and call them from both prepare and non-prepare variants; e.g., add a helper parse_document_create_opts that accepts PrepareDocumentCreateOptionsJs (or the shared options type) and returns the parsed Document (or DocumentWasm), a 32-byte entropy array, IdentityPublicKey, IdentitySignerWasm (or signer), the fetched DataContract, the DocumentType, and optional PutSettingsInput/converted settings; then replace the duplicated blocks in prepare_document_create and document_create to call parse_document_create_opts and use its results (similarly introduce parse_document_replace_opts and parse_document_delete_opts and use them from prepare_document_replace/document_replace and prepare_document_delete/document_delete) so the prepare* functions only differ by building/returning the state transition while non-prepare variants handle broadcasting.
🤖 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/wasm-sdk/src/state_transitions/document.rs`:
- Around line 564-607: prepare_document_replace currently delegates to
build_document_create_or_replace_transition which will treat a Document with
revision == None or INITIAL_REVISION as a create (mutating ID/entropy); add an
explicit guard in prepare_document_replace that reads the Document's revision
(from Document or DocumentWasm) and returns a WasmSdkError (or appropriate error
variant) if revision is None or equals INITIAL_REVISION, so only documents with
a non‑initial revision are allowed to proceed to
build_document_create_or_replace_transition; reference prepare_document_replace,
Document/DocumentWasm.revision(), INITIAL_REVISION, and
build_document_create_or_replace_transition when adding the check and error
return.
---
Nitpick comments:
In `@packages/wasm-sdk/src/state_transitions/document.rs`:
- Line 1108: The inline use statement for the trait DocumentTypeV0Getters should
be moved from inside the function body to the module-level import block
alongside the other dash_sdk::dpp::data_contract::... imports; update the
top-of-file use imports to include use
dash_sdk::dpp::data_contract::document_type::accessors::DocumentTypeV0Getters
and remove the inline declaration so trait methods resolve consistently and
match the file's import style.
- Around line 465-521: Extract the duplicated option-parsing and validation
logic into small helper functions and call them from both prepare and
non-prepare variants; e.g., add a helper parse_document_create_opts that accepts
PrepareDocumentCreateOptionsJs (or the shared options type) and returns the
parsed Document (or DocumentWasm), a 32-byte entropy array, IdentityPublicKey,
IdentitySignerWasm (or signer), the fetched DataContract, the DocumentType, and
optional PutSettingsInput/converted settings; then replace the duplicated blocks
in prepare_document_create and document_create to call
parse_document_create_opts and use its results (similarly introduce
parse_document_replace_opts and parse_document_delete_opts and use them from
prepare_document_replace/document_replace and
prepare_document_delete/document_delete) so the prepare* functions only differ
by building/returning the state transition while non-prepare variants handle
broadcasting.
…eplace Add a guard in prepare_document_replace to reject documents with no revision or INITIAL_REVISION, which would otherwise silently produce a create transition instead of a replace. Also move the inline DocumentTypeV0Getters import to module-level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/wasm-sdk/src/state_transitions/document.rs (1)
466-523: Substantial duplication betweenprepare_document_create/prepare_document_deleteand their non-prepare counterpartsThe option-extraction preamble (document, entropy, identity_key, signer, contract_id, document_type_name, data_contract, document_type, settings) is ~40 lines repeated verbatim in each prepare/non-prepare pair. Similarly
prepare_document_deleteanddocument_deleteshare identical document-field extraction and builder construction.A small private helper that returns the extracted options as a struct (or tuple) would eliminate the duplication and reduce the maintenance surface. The PR comments note this as a follow-up, but it's worth tracking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-sdk/src/state_transitions/document.rs` around lines 466 - 523, The prepare/document functions duplicate option extraction and validation; create a small private helper (e.g., extract_document_options or DocumentOptions) that, given &self and the PrepareDocumentCreateOptionsJs (or generic options), performs: DocumentWasm::try_from_options -> Document, extract and validate entropy into a [u8;32], IdentityPublicKeyWasm::try_from_options -> IdentityPublicKey, IdentitySignerWasm::try_from_options -> signer, derive contract_id and document_type_name, fetch data_contract via self.get_or_fetch_contract(contract_id).await, resolve document_type via get_document_type(&data_contract, &document_type_name)?, and parse optional settings; return a struct with fields (document, document_type, entropy_array, identity_key, signer, data_contract, settings) and update prepare_document_create (and prepare_document_delete/document_create/document_delete) to call this helper and use its returned values when building the state transition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/wasm-sdk/src/state_transitions/document.rs`:
- Around line 466-523: The prepare/document functions duplicate option
extraction and validation; create a small private helper (e.g.,
extract_document_options or DocumentOptions) that, given &self and the
PrepareDocumentCreateOptionsJs (or generic options), performs:
DocumentWasm::try_from_options -> Document, extract and validate entropy into a
[u8;32], IdentityPublicKeyWasm::try_from_options -> IdentityPublicKey,
IdentitySignerWasm::try_from_options -> signer, derive contract_id and
document_type_name, fetch data_contract via
self.get_or_fetch_contract(contract_id).await, resolve document_type via
get_document_type(&data_contract, &document_type_name)?, and parse optional
settings; return a struct with fields (document, document_type, entropy_array,
identity_key, signer, data_contract, settings) and update
prepare_document_create (and
prepare_document_delete/document_create/document_delete) to call this helper and
use its returned values when building the state transition.
|
@thepastaclaw @PastaPastaPasta it's already retring on timeouts. you can configure retries and timeouts passing settings platform/packages/wasm-sdk/src/settings.rs Line 192 in 3f49766 |
|
Here's the problem scenario: // Current API — atomic, no way to retry
try {
await sdk.documentCreate(document, identity, documentType, entropy);
} catch (e) {
if (e.message.includes("timeout")) {
// Cannot retry! Calling documentCreate again would:
// 1. Fetch a new nonce (incremented from the first attempt)
// 2. Build a NEW state transition with different nonce
// 3. The original ST might still be in mempool — now we have a conflict
//
// We also cannot rebroadcast the original ST because
// we never had access to it.
}
}
// With prepare_* API — caller controls retry
const signedST = await sdk.prepareDocumentCreate(document, identity, documentType, entropy);
const txid = signedST.transactionId();
try {
await sdk.broadcastAndWait(signedST);
} catch (e) {
if (e.message.includes("timeout")) {
// Safe to rebroadcast the SAME signed ST
await sdk.broadcastAndWait(signedST);
// Or check if the original went through:
// await sdk.waitForStateTransitionResult(txid);
}
}The core issue is that |
|
Ah, I see — the SDK already has built-in retry logic with configurable The remaining question would be whether there are cases where the caller needs the signed ST for reasons beyond retry (e.g., storing the txid for tracking before broadcast, or broadcasting through a different channel). But if the existing retry mechanism covers the practical use cases thephez was hitting, this PR may not be needed. @PastaPastaPasta @thephez — does the existing retry config in |
|
@shumkov — I looked at the retry mechanism you linked ( But it doesn't cover the case this PR addresses: application-level failure recovery. Consider this scenario:
The SDK's built-in retry can't help here because the retry loop is inside the same With
This is a standard two-phase pattern (prepare + commit) that any app dealing with unreliable networks needs. The SDK's retry covers the happy path; I'm keeping this PR open — it solves a real problem that |
2 participants
Issue
Closes #3090
Problem
The high-level document APIs (
documentCreate,documentReplace,documentDelete) in the WASM SDK atomically bundle nonce management, ST construction, signing, broadcasting, and waiting. On timeout, callers cannot rebroadcast the same signed ST — retrying creates a duplicate with a new nonce.Solution
Implements Option A (Two-Phase API) from the issue: add
prepare_*variants for each document operation that return a signedStateTransitionwithout broadcasting:prepareDocumentCreate()— build, sign, return STprepareDocumentReplace()— build, sign, return STprepareDocumentDelete()— build, sign, return STThese pair with the already-existing
broadcastStateTransition()andwaitForResponse()methods inbroadcast.rs.Usage Pattern
This gives applications full control over retry and caching strategy while leveraging Platform's built-in duplicate ST rejection.
Changes
packages/wasm-sdk/src/state_transitions/document.rs:prepareDocumentCreate()— builds and signs a create ST without broadcastingprepareDocumentReplace()— builds and signs a replace ST without broadcastingprepareDocumentDelete()— builds and signs a delete ST without broadcastingbuild_document_create_or_replace_transition()helper that replicates the ST construction logic fromPutDocument::put_to_platform(nonce fetch, transition creation, signing) without the broadcast stepTesting
The existing document operation tests validate the build/sign/broadcast pipeline. The prepare variants reuse the same construction logic, stopping before broadcast. The
broadcastStateTransitionandwaitForResponsemethods are already tested inbroadcast.rs. Manual testing with the yappr application (which prompted this issue) confirms the two-phase pattern works correctly.Summary by CodeRabbit