Repository navigation
fix(platform-wallet): keep contact request holds consistent after an unconfirmed send - #5908
QuantumExplorer wants to merge 3 commits into
Conversation
…unconfirmed send A public contact request whose broadcast may have been delivered is held until a proved read settles it. While it is held or being sent: - Its signed bytes are broadcast again only where a public request to the recipient would still go out by what the wallet tracks: not once a private request holds the pair or their requests came or went privately, not while the sender's read of the private store is rewound, not while private payments may be unrestored, and never from a private send. A public send does it only after its own store read succeeded. - One public send holds a pair at a time: a second to the same recipient is refused with PublicContactRequestUnconfirmed, with nothing signed. - A send that finds its unconfirmed request on Platform records it and returns it as the request sent, instead of building another past it. Also: - The lost-writes flag is read again after the flush, in note_read_back_once_durable and settled_requests_durable. - register_contact_account, register_external_contact_account and the accept path's adopted request act only on the handle's wallet generation. - A private request paid for whose read-back fails on the handle is reported pending. The README, the error's documentation and the FFI code registry describe what code 76 now also covers, when held bytes are broadcast again, and that a request which never lands and whose nonce is never used stays held for the life of the process. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 36 minutes. View limit details
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. |
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
|
🔍 Review in progress — actively reviewing now (commit 974c9da) · triage: critical |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The PR improves duplicate public-request handling, but several lifecycle and durability races still undermine its stated guarantees. In particular, a recorded request can be followed by a second send, stale generations can mutate or persist state into replacements, held broadcasts are not serialized with private-state changes, and the lost-write check is not atomic with journal discharge. Wallet tests passed; the Swift build remains queued.
🔴 5 blocking | 🟡 3 suggestion(s)
3 finding(s) not shown inline (the lines are not part of this PR's diff)
🔴 Blocking: Do not retry with a new reference after recording the settled request
packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:1286-1315
record_sent_contact_request persists the discovered request and releases the unconfirmed hold before registering the receiving account. If register_contact_account then fails, settle_unconfirmed_public_request_logged converts that error to None, so the public send continues to claim the pair and prepare another request. Because the first request is already tracked, preparation advances to a new account-reference version and signs/broadcasts a second request, charging the user again. Return or adopt the recorded request independently of receiving-account registration, and retry that registration separately.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, rust-quality, security-auditor)
🔴 Blocking: Fence external-account completion callbacks to the wallet generation
packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:4470-4536
note_external_account_registered and store_contact_account_label reacquire the wallet by wallet_id only. They do not verify self.generation, even though they are called after external-account registration returns and may run after the original wallet was removed and recreated. A stale drain or accept operation can therefore stamp external_account_reference, clear payment_channel_broken, or persist a label into the replacement generation. In particular, stamping the replacement's live marker can suppress the rebuild that should restore its actual xpub. Thread the generation through these completion helpers and validate it under the same mutation lock before changing memory or persistence.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, rust-quality, security-auditor)
🟡 Suggestion: Apply generation fencing to the external-account fast path
packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs:485-505
The early AlreadyExisted check looks up the wallet by ID alone, unlike the later insertion path. A stale handle can therefore return AlreadyExisted against a same-ID replacement and cause callers such as the deferred-registration drain to clear the queued operation without reaching the generation check. This can suppress a needed rebuild when the replacement's existing account is stale or was built from a different request payload. Make the fast-path lookup generation-aware as well.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, rust-quality, security-auditor)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate wallet change that alters paid contact-request sending, signing, broadcasting, retry, and persistence behavior, directly affecting funds movement and signature handling in contact_requests.rs and private_contact_requests.rs. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.
In `packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:1286-1315: Do not retry with a new reference after recording the settled request
`record_sent_contact_request` persists the discovered request and releases the unconfirmed hold before registering the receiving account. If `register_contact_account` then fails, `settle_unconfirmed_public_request_logged` converts that error to `None`, so the public send continues to claim the pair and prepare another request. Because the first request is already tracked, preparation advances to a new account-reference version and signs/broadcasts a second request, charging the user again. Return or adopt the recorded request independently of receiving-account registration, and retry that registration separately.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:4470-4536: Fence external-account completion callbacks to the wallet generation
`note_external_account_registered` and `store_contact_account_label` reacquire the wallet by `wallet_id` only. They do not verify `self.generation`, even though they are called after external-account registration returns and may run after the original wallet was removed and recreated. A stale drain or accept operation can therefore stamp `external_account_reference`, clear `payment_channel_broken`, or persist a label into the replacement generation. In particular, stamping the replacement's live marker can suppress the rebuild that should restore its actual xpub. Thread the generation through these completion helpers and validate it under the same mutation lock before changing memory or persistence.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:1473-1481: Serialize held-request eligibility with rebroadcast
`may_broadcast_held_public_request` checks the generation and private-pair state under a read lock, returns a boolean, and releases the lock before `broadcast_tracking_delivery` runs. A concurrent private-store sync can ingest a private request in that interval, causing the held public bytes to be broadcast after the pair became private. Teardown can also replace the wallet after the check and before submission. The lifecycle gate must cover both the liveness check and dispatch, and private-state ingestion must share a per-pair reservation or equivalent gate so the privacy decision cannot be invalidated before the broadcast.
In `packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs:218-279: Hold the generation lifecycle gate across contact-account persistence
`register_contact_account` and the external-account path acquire the wallet-manager lock and perform `persister.store` without first acquiring `self.generation.payment_guard()`. The generation check therefore only protects the in-memory lookup. Teardown acquires the generation's exclusive gate before waiting for the manager lock, so registration can persist an account for the old generation while teardown is waiting; a same-ID replacement can then load that stale account and address-pool state. Acquire the generation guard before the manager lock and retain it through the persistence and in-memory mutations, or use an equivalent generation-bound operation guard for both registration paths.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs:485-505: Apply generation fencing to the external-account fast path
The early `AlreadyExisted` check looks up the wallet by ID alone, unlike the later insertion path. A stale handle can therefore return `AlreadyExisted` against a same-ID replacement and cause callers such as the deferred-registration drain to clear the queued operation without reaching the generation check. This can suppress a needed rebuild when the replacement's existing account is stale or was built from a different request payload. Make the fast-path lookup generation-aware as well.
In `packages/rs-platform-wallet/src/wallet/persister.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/persister.rs:337-348: Linearize durability certification with flush failure
The post-flush `buffered_writes_lost()` load is not atomic with the subsequent `note_read_back_private_entries(pending)`. Another concurrent flush can fail permanently after line 340 and set the latch before line 348 records the commitments, so read-back is certified as durable even though the buffer was discarded. `PrivatePaymentRestore::settled_requests_durable` has the same check/flush/check gap before its caller discharges journal records. A restart can then lose the local request record and release the hold that prevents a repeated private payment. Use a shared durability/flush lock or an epoch protocol that covers the flush result, certification, and journal discharge as one linearized operation.
In `packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:1796-1817: Do not map every post-payment read error to pending
After payment, the read-back uses `unwrap_or_else` to map every `with_managed` error to `None` and then `PrivateContactRequestPending`. Today the closure is infallible and the expected errors are stale-handle or removed-identity cases, but a future fallible closure or persistence-related error would be silently reported as pending instead of surfacing the real failure. Match the intended stale-handle and identity-removal variants explicitly and propagate unrelated errors.
In `packages/rs-platform-wallet/src/wallet/platform_wallet.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/platform_wallet.rs:3136-3152: Add a direct test for settled-request durability certification
`settled_requests_durable` gained the same post-flush lost-write check as `note_read_back_once_durable`, but the PR only tests the persister-side helper. The restore path has a different caller and discharges journal records based on this result, so a regression that removes the second read or changes its flush branch would not be caught. Add a race test covering a permanent flush failure overtaking the restore path's own flush before settled records are discharged.
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them. |
github-actions
Bot
added
waiting-self-review
…holds - A request recorded as sent is returned whether or not its receiving account could be registered: a registration that fails is logged and left to the next sync. Its error used to send a retry on to sign a second request past the one just recorded. - The repeat broadcast of a held public request takes the wallet generation's lifecycle gate before its check and keeps it through the dispatch, so nothing goes out for a wallet that is being removed. - note_external_account_registered, store_contact_account_label and the external registration's early already-exists check act only on the handle's wallet generation. - Durability is certified in one place, WalletPersister::flush_durably. A flush is counted as in progress from before the backend is called until its outcome is recorded, and a flush is trusted only when, after it, none is in progress and none has failed for good. A flush that had failed in the backend but not yet recorded it could otherwise be missed. A read-back noted while another flush is in progress is kept for a later call. Tests cover each, and the restoration path's use of the durability check. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The PR fixes several previously identified wallet-generation, duplicate-send, and durability paths, and the available Rust workspace and Swift CI checks pass; wallet-specific Rust tests were skipped. Two blocking races remain in the changed contact-request and persistence coordination: private state can become tracked between the held-request eligibility check and rebroadcast, and a stale settlement can overwrite a newer outgoing request; a separate direct backend flush also bypasses the new durability certification state.
🔴 3 blocking
1 finding(s) not shown inline (the lines are not part of this PR's diff)
🔴 Blocking: Linearize durability certification with flush failure
packages/rs-platform-wallet/src/manager/mod.rs:839-847
WalletPersister::flush_durably tracks in-flight flushes and fatal loss only in state owned by that WalletPersister handle. However, PlatformWalletManager::settle_published_backfill still calls the shared backend directly through self.persister.flush(wallet_id). A fatal direct flush can drop the backend buffer without incrementing this handle's flushes_in_flight or setting its buffered_writes_lost latch; a later flush_durably can then successfully flush an empty buffer and certify records that were lost. Move the durability epoch/loss state to the shared backend or manager, or route every flush for a wallet through the same shared durability coordinator.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality, security-auditor)
Still open from earlier rounds (1)
- 🔴 Blocking: Serialize held-request eligibility with rebroadcast —
packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:1493-1500(thread)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate wallet change that alters paid contact-request signing, retry, persistence, and concurrent lifecycle behavior, directly affecting funds movement and key-handling paths. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.
In `packages/rs-platform-wallet/src/manager/mod.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/mod.rs:839-847: Linearize durability certification with flush failure
`WalletPersister::flush_durably` tracks in-flight flushes and fatal loss only in state owned by that `WalletPersister` handle. However, `PlatformWalletManager::settle_published_backfill` still calls the shared backend directly through `self.persister.flush(wallet_id)`. A fatal direct flush can drop the backend buffer without incrementing this handle's `flushes_in_flight` or setting its `buffered_writes_lost` latch; a later `flush_durably` can then successfully flush an empty buffer and certify records that were lost. Move the durability epoch/loss state to the shared backend or manager, or route every flush for a wallet through the same shared durability coordinator.
In `packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:1515-1525: Guard settled-request recording against a newer attempt
Settlement snapshots `held` before performing its SDK and document reads, but it does not revalidate that attempt before recording a found request. Two settlement calls can both find request A; the first records A and releases the hold, then a new send records newer request B. The second stale settlement subsequently enters this branch and calls `record_sent_contact_request` for A. `add_sent_contact_request` treats the differing reference as a rotation and overwrites B in persisted and in-memory contact state, so later reference rotation can reuse or derive from stale state. Revalidate the held attempt and current outgoing request under the same manager critical section before applying the settlement result, or serialize settlements per pair.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:1493-1500: Serialize held-request eligibility with rebroadcast
(carried; thread: https://github.com/dashpay/platform/pull/5908#discussion_r4562023470)
Out-of-scope follow-up suggestions (1)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Coordinate first public send with private-store ingestion — The same tracked-state window exists for a fresh public send: after
read_private_store_before_decidingreturns, private-store ingestion can record the pair beforeclaim_public_pairand signing. This is the pre-existing counterpart of the held-retry race and is explicitly left unchanged by this PR, but it can publish a relationship that became private during the send.- Follow-up: Open a follow-up issue or PR to coordinate private-store ingestion with public contact-request claiming and signing.
…sh of a wallet id together - A settle of an unconfirmed public request records the request it found on Platform only while its own hold still stands, checked in the lock it records in. Its reads run with no lock held, so another settle of the same hold can record the request and release the hold first, and a request sent after that is the one tracked: the later settle used to record the older request over it. It now records nothing, and answers the request only when that is still the one tracked. - The flushes of the records under a wallet id are counted in one ledger per id, kept with the manager's per-wallet state: how many are in progress and how many failed for good. A wallet's persister, the persister of a wallet removed or created under the same id, and the manager's own flush of a replaced backfill record all go through it. The count used to live in each wallet's persister, so a flush made anywhere else could drop the buffer without that wallet knowing, and a later flush of its own would vouch for records that were gone. A wallet made after such a failure starts clean, as it tracks what it read from disk. Tests cover each: a settle parked between its reads and its recording, the manager's flush, and a removed wallet's flush against its successor. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
Your move: thepastaclaw left review threads unresolved; resolve them. |
Basic explanation
What this does: When the wallet sends a public DashPay contact request and cannot tell whether it reached the network, it keeps the request "held" and may send the same signed bytes again later. This PR tightens what happens while a request is held or being sent. The wallet no longer sends those bytes again once it knows the two identities exchange requests privately. Only one public request to a recipient can be in progress at a time. A retry that finds the first request already on Platform returns that request instead of signing a second one. It also closes three smaller gaps in the same code.
Value: A pair the wallet knows to be private is not published by a background retry. A user who retries after an unclear outcome does not pay for a second request. A host gets "pending" instead of an unknown error for a private request that was paid for.
Risks: Low. Wallet code only, no consensus change. Two behaviours a host can notice: a second public send to a recipient while one is in progress is now refused with code 76 where it used to sign a second request, and a held request is broadcast again in fewer situations, so in those it stays held until a proved read settles it or the process ends. Nothing here was run against a live network.
Issue being fixed or feature implemented
Follow-up to #5810, #5812 and #5813, from a review of their last pushes. The outcome handling this touches is still interim until Platform can prove that a transition executed or cannot execute (#4270).
PublicContactRequestUnconfirmed(FFI 76), nothing signedWalletNotFound(unknown error over FFI)PrivateContactRequestPending(FFI 71)The retry case, step by step:
The concurrent case:
What was done?
network/contact_requests.rssettle_unconfirmed_public_requesttakesbroadcast_againfrom its caller and returns the request when it recorded one. A public send passes whether its own read of the private store succeeded, the public sync passestrue, a private send passesfalse.may_broadcast_held_public_request: before the held bytes go out again it checks, under the wallet lock and on the handle's wallet generation, that the hold is still the same attempt, that private payments are restored, that the sender's read of the private store is not rewound, and that no private request holds the pair (private_pair_refusal, now shared withclaim_public_pair).send_public_contact_requestreturns the request its settle step just recorded.record_settled_public_request: a settle records the request it found, and releases the hold, only while its own attempt still holds the pair, checked in the lock it records in. When the hold is gone it records nothing, and answers the request only if that is still the one tracked.record_sent_contact_requestlogs a receiving account registration that fails and still returns the recorded request; the sync registers the account later.note_external_account_registeredandstore_contact_account_labelact only on the handle's wallet generation.claim_public_pairrefuses a second claim on a pair.current_wallet_info_mut.state/managed_identity: one public claim per recipient (claim_public_contact_requestreturnsNonewhen one is held);DashPayState::private_store_read_rewound.network/contacts.rs:register_contact_accountandregister_external_contact_accountlook their wallet up through the generation-fenced helpers, the external registration's early already-exists check included (current_wallet_info).network/private_contact_requests.rs: after the payment, a failed read-back on the handle reports the request pending.persister.rs,platform_wallet.rs:note_read_back_once_durableandsettled_requests_durableboth go through the newWalletPersister::flush_durably. A flush is counted as in progress from before the backend is called until its outcome is recorded, andflush_durablytrusts its own flush only when, afterwards, none is in progress and none has failed for good. A notice made while another flush is in progress is kept for a later call.changeset/flush_ledger.rs(new),changeset/core_bridge.rs,manager/mod.rs: the count of flushes in progress and the count of flushes that failed for good live in aFlushLedgerper wallet id, kept in the manager'sDurableCursorStatebeside the per-wallet cursors, as a backend buffers a wallet's records under its id whoever stored them.PlatformWallet::newgives the wallet's persister the ledger of its id, and the manager's own flush of a replaced backfill record (settle_published_backfill) goes through the same one. A persister remembers how many failures the ledger held when it was made, so a wallet loaded or created after a failure starts clean.README.md,ERROR_CODE_REGISTRY.md,error.rs: what 76 now also covers, when held bytes are broadcast again, and that a request which never lands and whose nonce is never used stays held for the life of the process.Left as it is, on purpose:
DashPayView::public_contact_request_unconfirmedstill reports only a request held as unconfirmed, not one being sent. Both example apps poll it during their own sends, so widening it would show their "being checked" text during a normal send; that belongs with an app change.record_sent_contact_request, whose callers hold it.How Has This Been Tested?
New tests, each checked to fail with its fix taken out:
should_broadcast_an_unconfirmed_public_request_again_only_while_its_pair_is_clearshould_refuse_a_second_public_send_while_one_is_being_sentshould_return_the_unconfirmed_request_a_retry_finds_on_platformshould_register_no_contact_account_on_a_replacement_from_a_stale_handleshould_report_a_paid_private_request_pending_once_its_wallet_is_replacedshould_note_no_read_back_when_a_flush_fails_for_good_during_its_ownshould_note_no_read_back_while_another_flush_has_not_returnedshould_keep_a_settled_record_when_a_flush_fails_for_good_during_the_restorationshould_return_the_recorded_request_when_its_receiving_account_is_not_registeredshould_not_broadcast_a_held_request_again_while_its_wallet_is_being_removedshould_complete_no_external_account_on_a_replacement_from_a_stale_handleshould_record_nothing_from_a_settle_whose_hold_was_settled_meanwhileshould_count_its_backfill_flush_in_the_wallet_ids_flush_ledgershould_count_a_removed_wallets_flush_against_its_successorRun locally:
cargo test -p platform-wallet --all-features --lib -- wallet::identity:: wallet::persister:: wallet::platform_wallet:: manager:: changeset::: 1070 passed.cargo fmt --all -- --check: clean.cargo clippy --package platform-wallet --package platform-wallet-ffi --all-targets --all-features --locked -- --no-deps -D warnings: clean.cargo check --package platform-wallet --all-targets --locked(without the shielded feature): compiles. Its two dead-code warnings are inasset_lock, which this PR does not touch.Not run locally: the rest of the wallet suite and the Swift and Kotlin builds, which CI runs, and anything against a live network.
Breaking Changes
None. No consensus or protocol version change.
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
974c9da/self-reviewedrs-platform-wallet-ffi(packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md) — HashEngineering or ZocoLini or llbartekll or romchornyirs-platform-wallet(packages/rs-platform-wallet/README.md,packages/rs-platform-wallet/src/changeset/core_bridge.rs,packages/rs-platform-wallet/src/changeset/flush_ledger.rsand 11 more) — HashEngineering or ZocoLini or llbartekll or romchornyiWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.