fix(sdk): default to nonce 0 for first-time identity/contract interactions#3170
Conversation
…tions When Platform returns `None` for an identity or identity-contract nonce, it means the identity has never interacted with that contract (or Platform at all). Previously this was treated as an error (`IdentityNonceNotFound`), which broke first-time document creation for freshly registered identities. Now the SDK defaults to nonce 0 in that case, allowing first document creations to succeed without special-casing by callers. The worst case for a stale DAPI node returning None incorrectly is a retriable nonce mismatch, which is strictly better than an unconditional failure. Closes #3169 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe internal cache and SDK docs were changed so platform-missing identity and identity-contract nonces default to 0 (first-time interaction) instead of erroring; debug logs were added and three tests cover bump/no-bump first-interaction scenarios. Public function signatures were unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant SDK as SDK/InternalCache
participant Platform as Platform
Client->>SDK: request get_identity_nonce(identity_id, bump_first)
SDK->>SDK: check internal cache (stale/missing?)
alt cache has valid nonce
SDK-->>Client: return cached nonce (maybe bumped)
else cache missing/stale
SDK->>Platform: fetch nonce for identity (may be None)
alt Platform returns a nonce
SDK-->>Client: return fetched nonce (maybe bumped)
else Platform returns None
SDK->>SDK: log debug "platform returned no nonce"
SDK-->>Client: treat as 0 (and bump to 1 if bump_first)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
✅ gRPC Query Coverage Report |
GroveDB absence proofs cannot distinguish "identity missing", "contract missing", and "no nonce yet" — all three produce None. Drive's own query handler already applies unwrap_or_default() for the non-proven path, so defaulting to 0 in the SDK is consistent. Document this limitation inline so future readers understand the trade-off. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…istence Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates nonce handling so first-time identity and identity–contract interactions default to nonce 0 when Platform returns None, avoiding IdentityNonceNotFound and unblocking initial document submissions.
Changes:
- Default
Nonenonces from Platform to0(with debug logging) for identity and identity–contract nonce fetches - Update SDK doc comments to describe the new default-to-zero behavior
- Add tests covering
None → 0 → bump to 1for both nonce paths
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| packages/rs-sdk/src/sdk.rs | Updates public method doc comments to reflect default-to-zero nonce behavior. |
| packages/rs-sdk/src/internal_cache/mod.rs | Implements None → 0 fallback in cache fetch closures and adds regression tests for first interactions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…lse test - Align get_identity_contract_nonce doc with get_identity_nonce pattern - Rename tests to *_then_bumps_to_one to match actual assertions - Add first_identity_nonce_defaults_to_zero_without_bump test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "58f595a9367ff31f0614f01dabf997c8fd186fe4a0634cd05dfed532568aaeb6"
)Xcode manual integration:
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/rs-sdk/src/internal_cache/mod.rs (2)
184-201: Minor log message inconsistency withget_identity_nonce.The identity nonce handler (line 153) logs "unknown identity, node missing data, or first interaction" while this identity-contract handler only mentions "first interaction". For consistency and completeness, consider aligning the messages since both cases can have the same root causes.
🔧 Suggested alignment
tracing::debug!( identity_id = %identity_id, contract_id = %contract_id, "Platform returned no nonce for identity-contract pair; \ - defaulting to 0 (first interaction)" + defaulting to 0 (unknown identity, node missing data, or first interaction)" );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/internal_cache/mod.rs` around lines 184 - 201, Update the debug message in the identity-contract nonce fallback (the unwrap_or_else in the IdentityContractNonceFetcher::fetch_with_settings call used to compute `nonce`) to match the wording used by `get_identity_nonce` — include "unknown identity, node missing data, or first interaction" instead of only "first interaction" so both handlers log the same possible root causes; locate the unwrap_or_else closure where `tracing::debug!` is called for `identity_id` and `contract_id` and replace the message text accordingly.
684-753: Good test coverage for the first-time interaction fix.The three tests comprehensively cover the main scenarios for issue
#3169. For completeness, consider adding a test forfirst_identity_contract_nonce_defaults_to_zero_without_bumpto match the identity nonce coverage pattern, though this is optional since the underlyingget_or_fetch_noncelogic is already tested.🧪 Optional: Add test for identity-contract without bump
#[tokio::test] async fn first_identity_contract_nonce_defaults_to_zero_without_bump() { use drive_proof_verifier::types::IdentityContractNonceFetcher; let mut sdk = crate::Sdk::new_mock(); let identity_id = Identifier::default(); let contract_id = Identifier::from([1u8; 32]); let settings = PutSettings::default(); // Platform returns None (identity has never interacted with contract). sdk.mock() .expect_fetch::<IdentityContractNonceFetcher, _>((identity_id, contract_id), None) .await .expect("set mock expectation"); // bump_first=false: should return the raw default of 0. let nonce = sdk .get_identity_contract_nonce(identity_id, contract_id, false, Some(settings)) .await .unwrap(); assert_eq!(nonce, 0, "first identity-contract nonce without bump should be 0"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/internal_cache/mod.rs` around lines 684 - 753, Add a complementary test for the identity-contract case that mirrors the existing first_identity_nonce_defaults_to_zero_without_bump test: create a new #[tokio::test] named first_identity_contract_nonce_defaults_to_zero_without_bump, set up a mock Sdk, an identity_id and contract_id, expect_fetch::<IdentityContractNonceFetcher, _>((identity_id, contract_id), None), call get_identity_contract_nonce(identity_id, contract_id, false, Some(settings)).await.unwrap(), and assert_eq!(nonce, 0) to verify bump_first=false returns 0.
🤖 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/rs-sdk/src/internal_cache/mod.rs`:
- Around line 184-201: Update the debug message in the identity-contract nonce
fallback (the unwrap_or_else in the
IdentityContractNonceFetcher::fetch_with_settings call used to compute `nonce`)
to match the wording used by `get_identity_nonce` — include "unknown identity,
node missing data, or first interaction" instead of only "first interaction" so
both handlers log the same possible root causes; locate the unwrap_or_else
closure where `tracing::debug!` is called for `identity_id` and `contract_id`
and replace the message text accordingly.
- Around line 684-753: Add a complementary test for the identity-contract case
that mirrors the existing first_identity_nonce_defaults_to_zero_without_bump
test: create a new #[tokio::test] named
first_identity_contract_nonce_defaults_to_zero_without_bump, set up a mock Sdk,
an identity_id and contract_id, expect_fetch::<IdentityContractNonceFetcher,
_>((identity_id, contract_id), None), call
get_identity_contract_nonce(identity_id, contract_id, false,
Some(settings)).await.unwrap(), and assert_eq!(nonce, 0) to verify
bump_first=false returns 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99dbbcb2-143d-4300-a19c-5f24d33e36ed
📒 Files selected for processing (1)
packages/rs-sdk/src/internal_cache/mod.rs
Issue being fixed or feature implemented
When an identity interacts with a contract for the first time, Platform returns
Nonefor the nonce. The SDK previously surfaced this asError::IdentityNonceNotFound, causingdocument_create(and other state transitions) to fail on first use.Closes #3169
What was done?
NonceCache::get_identity_nonce/get_identity_contract_nonce(internal_cache/mod.rs): ChangedNoneresponse from Platform to default to nonce0(viaunwrap_or_else) instead of returningError::IdentityNonceNotFound. This matches Drive's ownunwrap_or_default()behavior. Downgraded log level fromwarntodebug.Sdk::get_identity_nonce/get_identity_contract_nonce(sdk.rs): Replaced verbose doc comments with concise descriptions clarifying that a missing nonce is treated as0before applying the optional bump.NonceCache::get_or_fetch_nonce(internal_cache/mod.rs): Simplified error doc — removedIdentityNonceNotFoundreference since it's no longer returned.None → 0defaulting:first_identity_nonce_defaults_to_zero_then_bumps_to_one—bump_first=truefirst_identity_contract_nonce_defaults_to_zero_then_bumps_to_one—bump_first=truefirst_identity_nonce_defaults_to_zero_without_bump—bump_first=false, directly verifies the0defaultHow Has This Been Tested?
cargo test -p dash-sdk --lib -- first_identity_nonce first_identity_contract_nonce3 new unit tests in
packages/rs-sdk/src/internal_cache/mod.rsusing mock SDK withexpect_fetchreturningNoneto simulate first-time interactions.Breaking Changes
None. The
Error::IdentityNonceNotFoundvariant still exists but is no longer returned by the nonce cache. Callers that previously caught this error will simply stop seeing it — no API signature changes.Checklist:
For repository code-owners and collaborators only
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Bug Fixes
Tests
Documentation