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 4 commits intodashpay:v3.1-devfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a two‑phase Prepare+Execute API to the WASM SDK for document state transitions by introducing prepare_document_create, prepare_document_replace, and prepare_document_delete which build, sign, and return signed StateTransition objects without broadcasting 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. |
|
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.
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 |
|
I think in my usage, the timeout that bites us is the |
This isn't very consistent with what I've seen in using the sdk in yappr; is the default configuration setup to retry? |
|
Re: your questions about retry behavior: Does Default config: What's actually retried: The retry loop in But — if the So if you're hitting timeouts in yappr where the wait never completes even with retries, it might be that the timeout per-attempt is too short, or the retries are exhausted. The This is exactly why I think |
|
Re: your questions about retry/timeout behavior: Does PutSettingsInput affect wait_for_response? Are retries enabled by default? The real gap (which this PR was trying to address) is that even with retries configured, the retry happens at the DAPI request level — if the broadcast succeeds but |
thepastaclaw
force-pushed
the
feat/sdk-prepare-document-apis
branch
from
67793d7 to
a4ad291
Compare
Claw please back this up with code references @shumkov if this is the case, this is a big problem for clients like yappr, and may have been the cause of many of our double posting issues. We also had some issues where the sdk itself would just timeout / fail, and then user would click post again, and then it turns out that the first one worked. IDK how this case was happening if sdk had retries as shown here... (claw maybe look if yappr needs to manually configure in sdk options this retrying? or maybe we are setting some other option and that is clobbering it or something? don't look in latest yappr, look a bit further back in history before our hacky fix for double posts) |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/wasm-sdk/src/state_transitions/document.rs (3)
465-465: Register newprepare*methods inapi-definitions.jsonand regenerate docs.The three new
#[wasm_bindgen]exports (prepareDocumentCreate,prepareDocumentReplace,prepareDocumentDelete) are now part of the public JS/TS API surface but won't appear in the generateddocs.htmlorAI_REFERENCE.mdunless they're added toapi-definitions.jsonand docs are regenerated withpython3 generate_docs.py. Based on learnings, both files are auto-generated from that JSON config and manual edits would be overwritten.Based on learnings: "In
packages/wasm-sdk/, theAI_REFERENCE.mdfile is auto-generated fromapi-definitions.json. Any documentation fixes should be made inapi-definitions.jsonrather than directly inAI_REFERENCE.md", and "WASM SDK (packages/wasm-sdk) WebAssembly bindings must be built with./build.shand documentation kept in sync usingpython3 generate_docs.py".🤖 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 465, Add the three new wasm exports to the SDK docs config: update api-definitions.json to register prepareDocumentCreate, prepareDocumentReplace, and prepareDocumentDelete (the #[wasm_bindgen(js_name = "...")] exports in document.rs) with appropriate signatures and descriptions, then regenerate the WebAssembly bindings/docs by running ./build.sh and python3 generate_docs.py so docs.html and AI_REFERENCE.md reflect the new public API surface.
466-522: Consider extracting shared option-parsing logic to reduce duplication.
prepare_document_create(lines 466–522) duplicates ~30 lines of option extraction that are also present indocument_create(lines 107–164). The only meaningful difference between them is the final call —build_document_create_or_replace_transitionvsput_to_platform_and_wait_for_response. Extracting the shared preamble into a private helper struct or a typedDocumentCreateArgswould halve the maintenance surface.This duplication also exists between
prepare_document_replaceanddocument_replace.(Already noted in PR discussion as a known follow-up — flagging here for 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 - 522, prepare_document_create duplicates the same options-parsing preamble found in document_create (and similarly for prepare_document_replace/document_replace); extract that shared logic into a small private helper (e.g., a DocumentCreateArgs or parse_document_create_args function) that returns a struct containing the parsed Document/DocumentWasm, contract_id, document_type_name, entropy_array ([u8;32]), IdentityPublicKey (from IdentityPublicKeyWasm), IdentitySignerWasm, and optional PutSettingsInput, reusing existing helpers like DocumentWasm::try_from_options, IdentityPublicKeyWasm::try_from_options, IdentitySignerWasm::try_from_options, get_or_fetch_contract, get_document_type and try_from_options_optional; then refactor prepare_document_create to call the new helper and pass its fields into build_document_create_or_replace_transition, and refactor document_create to call the same helper and pass its fields into put_to_platform_and_wait_for_response so both flows share the parsed arguments.
1147-1161: Fallback random-entropy path is unreachable from all current callers.When
document_state_transition_entropyisNoneand the document is in create mode (revision == Noneorrevision == INITIAL_REVISION), the code falls into a random-entropy branch that generates a newStdRngand rewrites the document ID. This path is currently dead:
prepare_document_createalways suppliesSome(entropy_array)(validated and required).prepare_document_replacealways suppliesNoneentropy but the revision guard ensures the replace branch is taken.This is not a bug—the helper preserves the flexibility of the original
put_to_platformlogic. However, ifStdRng::from_entropy()has platform-specific behaviour in the WASM target (i.e.,getrandomwith thejsfeature), it's worth a quick sanity check in case a future caller accidentally hits this path.🤖 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 1147 - 1161, The fallback branch that generates entropy with StdRng::from_entropy() (used in the document_state_transition_entropy None path and Document::generate_document_id_v0) is effectively unreachable today but may hit WASM where StdRng::from_entropy() behavior differs; update the fallback to be explicit: either (A) mark it unreachable/assert (so callers must supply entropy) or (B) replace StdRng::from_entropy() with a platform-safe entropy source (e.g., OsRng/getrandom) guarded by cfg(target_arch = "wasm32") and return a clear error if secure entropy is unavailable; adjust callers or document this in prepare_document_create / prepare_document_replace and keep references to Document::generate_document_id_v0, document_state_transition_entropy, StdRng::from_entropy, prepare_document_create, prepare_document_replace, and put_to_platform to locate the code.
🤖 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`:
- Line 465: Add the three new wasm exports to the SDK docs config: update
api-definitions.json to register prepareDocumentCreate, prepareDocumentReplace,
and prepareDocumentDelete (the #[wasm_bindgen(js_name = "...")] exports in
document.rs) with appropriate signatures and descriptions, then regenerate the
WebAssembly bindings/docs by running ./build.sh and python3 generate_docs.py so
docs.html and AI_REFERENCE.md reflect the new public API surface.
- Around line 466-522: prepare_document_create duplicates the same
options-parsing preamble found in document_create (and similarly for
prepare_document_replace/document_replace); extract that shared logic into a
small private helper (e.g., a DocumentCreateArgs or parse_document_create_args
function) that returns a struct containing the parsed Document/DocumentWasm,
contract_id, document_type_name, entropy_array ([u8;32]), IdentityPublicKey
(from IdentityPublicKeyWasm), IdentitySignerWasm, and optional PutSettingsInput,
reusing existing helpers like DocumentWasm::try_from_options,
IdentityPublicKeyWasm::try_from_options, IdentitySignerWasm::try_from_options,
get_or_fetch_contract, get_document_type and try_from_options_optional; then
refactor prepare_document_create to call the new helper and pass its fields into
build_document_create_or_replace_transition, and refactor document_create to
call the same helper and pass its fields into
put_to_platform_and_wait_for_response so both flows share the parsed arguments.
- Around line 1147-1161: The fallback branch that generates entropy with
StdRng::from_entropy() (used in the document_state_transition_entropy None path
and Document::generate_document_id_v0) is effectively unreachable today but may
hit WASM where StdRng::from_entropy() behavior differs; update the fallback to
be explicit: either (A) mark it unreachable/assert (so callers must supply
entropy) or (B) replace StdRng::from_entropy() with a platform-safe entropy
source (e.g., OsRng/getrandom) guarded by cfg(target_arch = "wasm32") and return
a clear error if secure entropy is unavailable; adjust callers or document this
in prepare_document_create / prepare_document_replace and keep references to
Document::generate_document_id_v0, document_state_transition_entropy,
StdRng::from_entropy, prepare_document_create, prepare_document_replace, and
put_to_platform to locate the code.
…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
Addresses CodeRabbit nitpick - more idiomatic Rust pattern that avoids calling revision() twice and the unnecessary unwrap().
…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>
Clippy 1.92 treats unnecessary_map_or as a warning, which CI promotes to error via -D warnings.
thepastaclaw
force-pushed
the
feat/sdk-prepare-document-apis
branch
from
64ee784 to
a868b10
Compare
3 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