From 3fd964406577f329598856bf4bc0b371a00689ff Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 6 Aug 2026 15:23:02 -0700 Subject: [PATCH 1/2] fix(platform-wallet): settle already-consumed locks and survive an ambiguous resume broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways a completed or in-flight asset-lock top-up could never finish, both observed on Android testnet: **Platform's "already completely used" verdict was dropped.** When a transition is rejected with IdentityAssetLockTransactionOutPointAlreadyConsumedError, the credits it would have bought already landed — an earlier attempt succeeded and the client never learned. Nothing recorded that locally: consume_asset_lock was only ever called on the success path, so the lock stayed in the resumable set and every recovery pass re-submitted it for the same deterministic rejection. Clients that block new funding while a lock is unresolved could not buy credits at all until they special-cased the error string. Both funded flows now classify that rejection (typed, via the consensus error rather than its message), mark the lock Consumed, and return the same AssetLockAlreadyConsumed a resume of a consumed lock already raises — so callers need one terminal case, not a string match. **A Built-status resume aborted on an ambiguous re-broadcast.** The Built arm propagated every broadcast error, including MaybeSent. For a lock stuck at Built whose transaction WAS broadcast (the app died between the send and the status advance), MaybeSent is the expected answer on every retry — DAPI classifies all failures that way — so the resume failed, the lock stayed Built, and the next pass repeated it. The top-up never completed. Only a definite Rejected now stops the resume; MaybeSent advances to Broadcast and proceeds to the proof wait, matching what the Broadcast arm already does with the identical signal and keeping a genuinely un-broadcast tx resumable at Built. Tests: two regression tests covering the ambiguous and definite branches (status transition asserted, not just the error). cargo test -p platform-wallet --lib asset_lock:: — 26 passed. cargo clippy -p platform-wallet --lib --tests — no new warnings (the 3 reported are present on the unmodified base). Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet/src/error.rs | 41 +++++ .../src/wallet/asset_lock/sync/recovery.rs | 142 +++++++++++++++++- .../wallet/identity/network/registration.rs | 64 +++++++- 3 files changed, 241 insertions(+), 6 deletions(-) diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 9d6ce8a0a4e..503ca22b707 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -514,6 +514,47 @@ pub fn is_instant_lock_proof_invalid(error: &dash_sdk::Error) -> bool { ) } +/// Extract the outpoint from Platform's rejection of an asset lock whose +/// credit output has already been spent by an earlier transition +/// (`IdentityAssetLockTransactionOutPointAlreadyConsumedError`, "Asset lock +/// transaction {txid} output {n} already completely used"). +/// +/// The rejection is **deterministic and terminal**: the credits it would have +/// bought already landed, so every retry receives the same answer. Callers +/// treat it as the success-shaped outcome it really is — mark the lock +/// [`Consumed`](crate::wallet::asset_lock::tracked::AssetLockStatus::Consumed) +/// and stop resuming it — rather than as a failure to retry. +/// +/// Returns the outpoint Platform named, which identifies the local tracked +/// lock without trusting the caller's bookkeeping. +/// +/// Companion to [`is_instant_lock_proof_invalid`]: both classify a Platform +/// rejection of an asset-lock proof, but that one is retryable via a CL +/// upgrade while this one can never succeed. +pub fn asset_lock_already_consumed_out_point( + error: &dash_sdk::Error, +) -> Option { + use dpp::consensus::basic::BasicError; + use dpp::consensus::ConsensusError; + + let consensus_error = match error { + dash_sdk::Error::StateTransitionBroadcastError(broadcast_err) => { + broadcast_err.cause.as_ref() + } + dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()), + _ => None, + }; + match consensus_error { + Some(ConsensusError::BasicError( + BasicError::IdentityAssetLockTransactionOutPointAlreadyConsumedError(e), + )) => Some(dashcore::OutPoint { + txid: e.transaction_id(), + vout: e.output_index() as u32, + }), + _ => None, + } +} + /// Check whether a platform-wallet error represents a *Core-side* /// InstantSend lock timeout (the asset-lock manager waited the full /// timeout for an IS-lock proof and never observed one). diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index c0e81a90dbc..5d190086ccd 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -4,7 +4,7 @@ //! resolving status from wallet info, resuming interrupted locks, //! and re-deriving private keys. -use crate::broadcaster::TransactionBroadcaster; +use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use std::time::Duration; use dashcore::Address as DashAddress; @@ -252,7 +252,31 @@ impl AssetLockManager { let proof = match status { AssetLockStatus::Built => { // Re-broadcast and wait for proof. - self.broadcaster.broadcast(&tx).await?; + // + // Only a DEFINITE rejection stops the resume. `MaybeSent` + // means the outcome is unknown — and for a lock stuck at + // `Built` that is the expected answer when the app died + // between a successful broadcast and this status advance: + // the tx is in a mempool (or mined) and every re-broadcast + // reports the same ambiguity. Failing on it left the lock at + // `Built` forever, so each recovery pass repeated the same + // broadcast and the same abort, and the top-up never + // completed. Advancing to `Broadcast` and waiting matches + // what the `Broadcast` arm below already does with the + // identical signal. + match self.broadcaster.broadcast(&tx).await { + Ok(_) => {} + Err(BroadcastError::MaybeSent { reason }) => { + tracing::warn!( + outpoint = %out_point, + reason = %reason, + "resume_asset_lock: re-broadcast of a Built lock returned an \ + unknown outcome (the network may already hold this tx); \ + advancing to Broadcast and waiting for proof" + ); + } + Err(rejected) => return Err(rejected.into()), + } let cs = self .advance_asset_lock_status(out_point, AssetLockStatus::Broadcast, None) .await?; @@ -464,7 +488,9 @@ mod tests { ClientStartState, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, }; use crate::error::PlatformWalletError; - use crate::test_support::{funded_wallet_manager, AlwaysRejectedBroadcaster}; + use crate::test_support::{ + funded_wallet_manager, AlwaysMaybeSentBroadcaster, AlwaysRejectedBroadcaster, + }; use crate::wallet::asset_lock::manager::AssetLockManager; use crate::wallet::asset_lock::tracked::{AssetLockStatus, TrackedAssetLock}; use crate::wallet::core::WalletGeneration; @@ -647,6 +673,116 @@ mod tests { ); } + /// Builds a tracked `Built`-status lock on a funded wallet and resumes it + /// through `broadcaster`, returning the resume error and the lock's status + /// afterwards. Shared by the two ambiguity/rejection cases below. + async fn resume_built_lock_with( + broadcaster: Arc, + ) -> (PlatformWalletError, AssetLockStatus) { + let (wallet_manager, wallet_id, _balance, signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + let sdk = Arc::new( + dash_sdk::SdkBuilder::new_mock() + .with_network(Network::Testnet) + .build() + .expect("mock sdk"), + ); + let manager = AssetLockManager::new( + sdk, + Arc::clone(&wallet_manager), + wallet_id, + Arc::new(Notify::new()), + broadcaster, + WalletPersister::new(wallet_id, Arc::new(RecordingPersistence::default())), + ); + let (transaction, _path) = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::IdentityTopUp, + 4, + &signer, + ) + .await + .expect("build asset lock"); + let out_point = OutPoint::new(transaction.txid(), 0); + { + let mut wm = wallet_manager.write().await; + wm.get_wallet_info_mut(&wallet_id) + .expect("wallet must remain registered") + .tracked_asset_locks + .insert( + out_point, + TrackedAssetLock { + out_point, + transaction, + account_index: 0, + funding_type: AssetLockFundingType::IdentityTopUp, + identity_index: 4, + amount: 1_000_000, + status: AssetLockStatus::Built, + proof: None, + }, + ); + } + + let error = manager + .resume_asset_lock(&out_point, Some(Duration::from_millis(10))) + .await + .expect_err("no proof event should arrive in either case"); + let status = wallet_manager + .read() + .await + .get_wallet_info(&wallet_id) + .expect("wallet") + .tracked_asset_locks + .get(&out_point) + .expect("lock stays tracked") + .status + .clone(); + (error, status) + } + + /// An AMBIGUOUS re-broadcast must not end the resume. A lock sitting at + /// `Built` whose transaction was in fact already broadcast (app killed + /// between the send and the status advance) draws `MaybeSent` on every + /// retry, so failing on it pinned the lock at `Built` forever and the + /// top-up could never complete. It must advance to `Broadcast` and go on + /// to wait for the proof — here, until the 10ms test timeout. + #[tokio::test] + async fn built_resume_survives_an_ambiguous_rebroadcast_and_advances() { + let (error, status) = resume_built_lock_with(Arc::new(AlwaysMaybeSentBroadcaster)).await; + + assert!( + matches!(error, PlatformWalletError::FinalityTimeout(_)), + "resume must reach the proof wait, not fail on the broadcast: {error:?}" + ); + assert_eq!( + status, + AssetLockStatus::Broadcast, + "an ambiguous re-broadcast must still advance the lock, or every \ + later pass repeats the same broadcast and the same failure" + ); + } + + /// A DEFINITE rejection is the opposite case and must keep failing the + /// resume: nothing is on the network, so no proof can ever arrive, and + /// the lock stays at `Built` for a later retry to re-send. + #[tokio::test] + async fn built_resume_still_fails_on_a_definite_rejection() { + let (error, status) = resume_built_lock_with(Arc::new(AlwaysRejectedBroadcaster)).await; + + assert!( + matches!(error, PlatformWalletError::TransactionBroadcast(_)), + "a definite rejection must surface as a broadcast failure: {error:?}" + ); + assert_eq!( + status, + AssetLockStatus::Built, + "a tx that never entered the network must stay resumable at Built" + ); + } + /// A lazily-created `IdentityTopUp` funding account must survive a /// restart. Its persisted registration round (account xpub + pool /// snapshot) is the ONLY record the load path can rebuild the account diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs b/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs index e0924008335..5bad34cf7a5 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs @@ -65,7 +65,9 @@ use dash_sdk::platform::transition::put_identity::PutIdentity; use dash_sdk::platform::transition::put_settings::PutSettings; use dash_sdk::platform::transition::top_up_identity::TopUpIdentity; -use crate::error::{is_instant_lock_proof_invalid, PlatformWalletError}; +use crate::error::{ + asset_lock_already_consumed_out_point, is_instant_lock_proof_invalid, PlatformWalletError, +}; use crate::wallet::asset_lock::orchestration::{ out_point_from_proof, submit_with_cl_height_retry, FundingResolution, ResolvedFunding, }; @@ -261,7 +263,15 @@ impl IdentityWallet { .await .map_err(PlatformWalletError::Sdk)? } - Err(e) => return Err(PlatformWalletError::Sdk(e)), + // See the matching arm in `top_up_identity_with_funding`: a + // credit output Platform already spent is terminal, and this is + // the only place that can record it locally on the failure path. + Err(e) => { + if let Some(out_point) = asset_lock_already_consumed_out_point(&e) { + return Err(self.settle_already_consumed_lock(out_point).await); + } + return Err(PlatformWalletError::Sdk(e)); + } }; // Step 4 (best-effort): bookkeeping — add to local @@ -492,7 +502,18 @@ impl IdentityWallet { .await .map_err(PlatformWalletError::Sdk)? } - Err(e) => return Err(PlatformWalletError::Sdk(e)), + // Platform says this credit output was already spent — the + // top-up it would have paid for landed earlier. Record that + // locally (nothing else ever does: the success path is the only + // other caller of `consume_asset_lock`) so the lock leaves the + // resumable set instead of being retried against the same + // deterministic rejection forever. + Err(e) => { + if let Some(out_point) = asset_lock_already_consumed_out_point(&e) { + return Err(self.settle_already_consumed_lock(out_point).await); + } + return Err(PlatformWalletError::Sdk(e)); + } }; // Step 4 (best-effort): persist the new balance + clean up the @@ -550,6 +571,43 @@ impl IdentityWallet { } } +impl IdentityWallet { + /// Record Platform's "already completely used" verdict for `out_point` + /// locally and return the typed error describing it. + /// + /// The credits this lock paid for exist on chain — an earlier attempt + /// succeeded and the client never learned. Marking it + /// [`Consumed`](crate::wallet::asset_lock::tracked::AssetLockStatus::Consumed) + /// takes it out of the resumable set, which is what stops a recovery + /// worker retrying it on every pass (and, for clients that block new + /// funding while a lock is unresolved, unblocks the next purchase). + /// + /// Returns [`AssetLockAlreadyConsumed`](PlatformWalletError::AssetLockAlreadyConsumed) + /// — the same typed error a resume of an already-consumed lock raises, so + /// callers need one terminal case, not a Platform error-string match. + /// A bookkeeping failure here can only be `WalletNotFound`; it is logged + /// rather than returned, because the verdict itself is what the caller + /// must act on. + async fn settle_already_consumed_lock( + &self, + out_point: dashcore::OutPoint, + ) -> PlatformWalletError { + tracing::info!( + outpoint = %out_point, + "Platform rejected the asset lock as already completely used — its \ + credits landed on an earlier attempt; marking the lock consumed" + ); + if let Err(e) = self.asset_locks.consume_asset_lock(&out_point).await { + tracing::warn!( + outpoint = %out_point, + error = %e, + "consume_asset_lock failed after Platform's already-used rejection" + ); + } + PlatformWalletError::AssetLockAlreadyConsumed(out_point) + } +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- From 319c4c9666c741fc655589b0a6e54914d7803c80 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 16:55:27 +0700 Subject: [PATCH 2/2] fix(platform-wallet): bind the already-used verdict to the submitted outpoint and classify every submit arm Review follow-ups to the already-consumed settlement: - The settlement moves onto AssetLockManager as settle_reported_consumed and now settles ONLY when Platform's verdict names the outpoint this submission actually carried. The verdict is an unauthenticated error relayed by one DAPI node; binding it stops a fabricated answer from tombstoning an unrelated lock (which wiped its proof and stranded the burned funds). - The IS->CL fallback resubmits in registration, top-up, and the platform-address funding flow previously mapped rejections straight to a generic Sdk error, so a consumed verdict on the second attempt left the lock resumable and the endless-retry loop reachable. All six rejection arms now route through the shared classification. - Tests: classifier extraction from both wire shapes plus unrelated/causeless passthrough (error.rs), and a manager-level test proving an unbound or unrelated rejection leaves the lock untouched while the bound verdict tombstones it. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet/src/error.rs | 70 ++++++++++- .../src/wallet/asset_lock/sync/recovery.rs | 116 ++++++++++++++++++ .../src/wallet/asset_lock/sync/tracking.rs | 70 ++++++++++- .../wallet/identity/network/registration.rs | 92 ++++++-------- .../fund_from_asset_lock.rs | 25 +++- 5 files changed, 311 insertions(+), 62 deletions(-) diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 503ca22b707..e65126db809 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -525,8 +525,11 @@ pub fn is_instant_lock_proof_invalid(error: &dash_sdk::Error) -> bool { /// [`Consumed`](crate::wallet::asset_lock::tracked::AssetLockStatus::Consumed) /// and stop resuming it — rather than as a failure to retry. /// -/// Returns the outpoint Platform named, which identifies the local tracked -/// lock without trusting the caller's bookkeeping. +/// Returns the outpoint Platform named. The verdict is an unauthenticated +/// error relayed by a single DAPI node — callers must bind it to the +/// outpoint they actually submitted before settling anything +/// ([`AssetLockManager::settle_reported_consumed`](crate::wallet::asset_lock::manager::AssetLockManager::settle_reported_consumed) +/// does), or a fabricated verdict could tombstone an unrelated lock. /// /// Companion to [`is_instant_lock_proof_invalid`]: both classify a Platform /// rejection of an asset-lock proof, but that one is retryable via a CL @@ -947,4 +950,67 @@ mod address_nonce_tests { assert_eq!(got.provided_nonce(), 9); assert_eq!(got.expected_nonce(), 10); } + + /// Platform's "already completely used" rejection naming `out_point`, + /// as the raw consensus error both wire shapes carry. + fn already_consumed_cause(out_point: dashcore::OutPoint) -> dpp::consensus::ConsensusError { + use dpp::consensus::basic::identity::IdentityAssetLockTransactionOutPointAlreadyConsumedError; + use dpp::consensus::basic::BasicError; + use dpp::consensus::ConsensusError; + ConsensusError::BasicError( + BasicError::IdentityAssetLockTransactionOutPointAlreadyConsumedError( + IdentityAssetLockTransactionOutPointAlreadyConsumedError::new( + out_point.txid, + out_point.vout as usize, + ), + ), + ) + } + + fn already_consumed_test_out_point() -> dashcore::OutPoint { + use dashcore::hashes::Hash as _; + dashcore::OutPoint { + txid: dashcore::Txid::from_byte_array([7u8; 32]), + vout: 3, + } + } + + #[test] + fn extracts_already_consumed_out_point_from_both_shapes() { + let out_point = already_consumed_test_out_point(); + let protocol = dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new( + already_consumed_cause(out_point), + ))); + // The gRPC broadcast/wait rejection shape; the classifier reads only + // the cause, so the code and message are inert. + let broadcast = + dash_sdk::Error::StateTransitionBroadcastError(StateTransitionBroadcastError { + code: 10504, + message: "asset lock already completely used".to_string(), + cause: Some(already_consumed_cause(out_point)), + }); + for err in [protocol, broadcast] { + assert_eq!(asset_lock_already_consumed_out_point(&err), Some(out_point)); + } + } + + #[test] + fn already_consumed_classifier_ignores_unrelated_and_causeless_errors() { + // A plainly unrelated SDK error. + assert!( + asset_lock_already_consumed_out_point(&dash_sdk::Error::Generic("boom".to_string())) + .is_none() + ); + // A different consensus rejection (address-nonce) must not read as a + // consumed verdict. + assert!(asset_lock_already_consumed_out_point(&protocol_shape(1, 2)).is_none()); + // The DAPI wait-timeout shape: no consensus cause, no verdict. + let causeless = + dash_sdk::Error::StateTransitionBroadcastError(StateTransitionBroadcastError { + code: 0, + message: "timeout".to_string(), + cause: None, + }); + assert!(asset_lock_already_consumed_out_point(&causeless).is_none()); + } } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index 5d190086ccd..91f02f827ad 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -925,4 +925,120 @@ mod tests { "re-derived credit-output path must match the build-time path" ); } + + /// Platform's "already completely used" verdict is an unauthenticated + /// error relayed by one DAPI node: `settle_reported_consumed` must + /// tombstone the tracked lock only when the verdict names the outpoint + /// this very submission carried, and pass every other rejection through + /// untouched — a fabricated verdict naming a different outpoint must + /// not strand an unrelated lock. + #[tokio::test] + async fn already_used_verdict_settles_only_the_submitted_outpoint() { + fn already_consumed_error(out_point: OutPoint) -> dash_sdk::Error { + use dpp::consensus::basic::identity::IdentityAssetLockTransactionOutPointAlreadyConsumedError; + use dpp::consensus::basic::BasicError; + use dpp::consensus::ConsensusError; + dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new( + ConsensusError::BasicError( + BasicError::IdentityAssetLockTransactionOutPointAlreadyConsumedError( + IdentityAssetLockTransactionOutPointAlreadyConsumedError::new( + out_point.txid, + out_point.vout as usize, + ), + ), + ), + ))) + } + + let (wallet_manager, wallet_id, _balance, signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + let sdk = Arc::new( + dash_sdk::SdkBuilder::new_mock() + .with_network(Network::Testnet) + .build() + .expect("mock sdk"), + ); + let manager = AssetLockManager::new( + sdk, + Arc::clone(&wallet_manager), + wallet_id, + Arc::new(Notify::new()), + Arc::new(RecordingBroadcaster::default()), + WalletPersister::new(wallet_id, Arc::new(RecordingPersistence::default())), + ); + let (transaction, _path) = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::IdentityTopUp, + 4, + &signer, + ) + .await + .expect("build asset lock"); + let out_point = OutPoint::new(transaction.txid(), 0); + { + let mut wm = wallet_manager.write().await; + wm.get_wallet_info_mut(&wallet_id) + .expect("wallet must remain registered") + .tracked_asset_locks + .insert( + out_point, + TrackedAssetLock { + out_point, + transaction, + account_index: 0, + funding_type: AssetLockFundingType::IdentityTopUp, + identity_index: 4, + amount: 1_000_000, + status: AssetLockStatus::Built, + proof: None, + }, + ); + } + let lock_status = || async { + wallet_manager + .read() + .await + .get_wallet_info(&wallet_id) + .expect("wallet") + .tracked_asset_locks + .get(&out_point) + .expect("lock stays tracked") + .status + .clone() + }; + + // A verdict naming a DIFFERENT outpoint passes through untouched. + let foreign = OutPoint::new(Txid::from_byte_array([9u8; 32]), 0); + let unbound = manager + .settle_reported_consumed(already_consumed_error(foreign), &out_point) + .await; + assert!( + matches!(unbound, PlatformWalletError::Sdk(_)), + "an unbound verdict must surface as a plain SDK error: {unbound:?}" + ); + assert_eq!( + lock_status().await, + AssetLockStatus::Built, + "an unbound verdict must not settle the local lock" + ); + + // A rejection that is no consumed-verdict at all passes through too. + let unrelated = manager + .settle_reported_consumed(dash_sdk::Error::Generic("boom".to_string()), &out_point) + .await; + assert!(matches!(unrelated, PlatformWalletError::Sdk(_))); + assert_eq!(lock_status().await, AssetLockStatus::Built); + + // The bound verdict settles: typed terminal error + Consumed tombstone. + let bound = manager + .settle_reported_consumed(already_consumed_error(out_point), &out_point) + .await; + assert!(matches!( + bound, + PlatformWalletError::AssetLockAlreadyConsumed(actual) if actual == out_point + )); + assert_eq!(lock_status().await, AssetLockStatus::Consumed); + } } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs index d6051217077..901509c81f9 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs @@ -121,10 +121,12 @@ impl AssetLockManager { /// **Why queue internally** (unlike `track_asset_lock` / /// `advance_asset_lock_status`, which return a changeset and let /// the caller queue it): `queue_asset_lock_changeset` is - /// `pub(super)` to the `asset_lock` module, so the only callers - /// of `consume_asset_lock` — the identity registration and - /// top-up flows in `wallet/identity/network/registration.rs` — - /// can't queue the changeset themselves. The other mutators are + /// `pub(super)` to the `asset_lock` module, so the callers of + /// `consume_asset_lock` outside it — the identity registration + /// and top-up success paths in + /// `wallet/identity/network/registration.rs` (the failure paths + /// go through [`settle_reported_consumed`](Self::settle_reported_consumed)) + /// — can't queue the changeset themselves. The other mutators are /// only called from inside `asset_lock/build.rs`, which IS in /// the module and queues at the call site. Queueing here closes /// the gap without widening the `pub(super)` visibility. @@ -167,6 +169,66 @@ impl AssetLockManager { Ok(cs) } + /// Classify a Platform rejection of a funded submission that carried + /// `submitted`'s proof, settling the tracked lock when — and only + /// when — Platform's "already completely used" verdict names that + /// same outpoint. + /// + /// The verdict arrives as an unauthenticated consensus error relayed + /// by a single DAPI node, so it is trusted no further than the lock + /// whose proof this very submission carried: a verdict naming any + /// *other* outpoint is passed through as a plain + /// [`Sdk`](PlatformWalletError::Sdk) error instead of tombstoning an + /// unrelated lock (which would wipe its proof and strand real burned + /// funds on a fabricated answer). + /// + /// On a bound verdict the credits the lock paid for already landed — + /// an earlier attempt succeeded and the client never learned. Marking + /// the lock [`Consumed`](AssetLockStatus::Consumed) takes it out of + /// the resumable set, which is what stops a recovery worker retrying + /// it against the same deterministic rejection on every pass (and, + /// for clients that block new funding while a lock is unresolved, + /// unblocks the next purchase). Returns + /// [`AssetLockAlreadyConsumed`](PlatformWalletError::AssetLockAlreadyConsumed) + /// — the same typed error a resume of a consumed lock raises, so + /// callers need one terminal case, not a Platform error-string match. + /// A bookkeeping failure here can only be `WalletNotFound`; it is + /// logged rather than returned, because the verdict itself is what + /// the caller must act on. + pub(crate) async fn settle_reported_consumed( + &self, + error: dash_sdk::Error, + submitted: &OutPoint, + ) -> PlatformWalletError { + match crate::error::asset_lock_already_consumed_out_point(&error) { + Some(reported) if reported == *submitted => { + tracing::info!( + outpoint = %reported, + "Platform rejected the asset lock as already completely used — its \ + credits landed on an earlier attempt; marking the lock consumed" + ); + if let Err(e) = self.consume_asset_lock(&reported).await { + tracing::warn!( + outpoint = %reported, + error = %e, + "consume_asset_lock failed after Platform's already-used rejection" + ); + } + PlatformWalletError::AssetLockAlreadyConsumed(reported) + } + Some(reported) => { + tracing::warn!( + reported = %reported, + submitted = %submitted, + "Platform's already-used verdict names a different outpoint than the \ + one submitted; not settling any local lock" + ); + PlatformWalletError::Sdk(error) + } + None => PlatformWalletError::Sdk(error), + } + } + /// Advance the status of a tracked asset lock and optionally attach the proof. /// /// Returns an [`AssetLockChangeSet`] carrying a full snapshot of the diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs b/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs index 5bad34cf7a5..bd19be65b7f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/registration.rs @@ -65,9 +65,7 @@ use dash_sdk::platform::transition::put_identity::PutIdentity; use dash_sdk::platform::transition::put_settings::PutSettings; use dash_sdk::platform::transition::top_up_identity::TopUpIdentity; -use crate::error::{ - asset_lock_already_consumed_out_point, is_instant_lock_proof_invalid, PlatformWalletError, -}; +use crate::error::{is_instant_lock_proof_invalid, PlatformWalletError}; use crate::wallet::asset_lock::orchestration::{ out_point_from_proof, submit_with_cl_height_retry, FundingResolution, ResolvedFunding, }; @@ -250,7 +248,13 @@ impl IdentityWallet { .asset_locks .upgrade_to_chain_lock_proof(&out_point, None) .await?; - submit_with_cl_height_retry(settings, |s| { + // The CL resubmit can draw the same "already completely + // used" verdict as the first attempt (e.g. the IS submit + // was ambiguous but actually committed), so its rejection + // is classified identically — a bare `Sdk` error here + // would leave the lock resumable and reachable by the + // endless-retry loop the settlement exists to end. + match submit_with_cl_height_retry(settings, |s| { placeholder.put_to_platform_and_wait_for_response_with_signer( &self.sdk, chain_proof.clone(), @@ -261,16 +265,24 @@ impl IdentityWallet { ) }) .await - .map_err(PlatformWalletError::Sdk)? + { + Ok(identity) => identity, + Err(e) => { + return Err(self + .asset_locks + .settle_reported_consumed(e, &proof_out_point) + .await) + } + } } // See the matching arm in `top_up_identity_with_funding`: a // credit output Platform already spent is terminal, and this is // the only place that can record it locally on the failure path. Err(e) => { - if let Some(out_point) = asset_lock_already_consumed_out_point(&e) { - return Err(self.settle_already_consumed_lock(out_point).await); - } - return Err(PlatformWalletError::Sdk(e)); + return Err(self + .asset_locks + .settle_reported_consumed(e, &proof_out_point) + .await) } }; @@ -490,7 +502,10 @@ impl IdentityWallet { .asset_locks .upgrade_to_chain_lock_proof(&out_point, None) .await?; - submit_with_cl_height_retry(settings, |s| { + // Classified like the first attempt: the CL resubmit can + // draw the same terminal "already completely used" verdict + // (see the registration flow's matching arm). + match submit_with_cl_height_retry(settings, |s| { identity.top_up_identity_with_signer( &self.sdk, chain_proof.clone(), @@ -500,19 +515,27 @@ impl IdentityWallet { ) }) .await - .map_err(PlatformWalletError::Sdk)? + { + Ok(balance) => balance, + Err(e) => { + return Err(self + .asset_locks + .settle_reported_consumed(e, &proof_out_point) + .await) + } + } } // Platform says this credit output was already spent — the // top-up it would have paid for landed earlier. Record that // locally (nothing else ever does: the success path is the only - // other caller of `consume_asset_lock`) so the lock leaves the + // other consumer of the tracked lock) so the lock leaves the // resumable set instead of being retried against the same // deterministic rejection forever. Err(e) => { - if let Some(out_point) = asset_lock_already_consumed_out_point(&e) { - return Err(self.settle_already_consumed_lock(out_point).await); - } - return Err(PlatformWalletError::Sdk(e)); + return Err(self + .asset_locks + .settle_reported_consumed(e, &proof_out_point) + .await) } }; @@ -571,43 +594,6 @@ impl IdentityWallet { } } -impl IdentityWallet { - /// Record Platform's "already completely used" verdict for `out_point` - /// locally and return the typed error describing it. - /// - /// The credits this lock paid for exist on chain — an earlier attempt - /// succeeded and the client never learned. Marking it - /// [`Consumed`](crate::wallet::asset_lock::tracked::AssetLockStatus::Consumed) - /// takes it out of the resumable set, which is what stops a recovery - /// worker retrying it on every pass (and, for clients that block new - /// funding while a lock is unresolved, unblocks the next purchase). - /// - /// Returns [`AssetLockAlreadyConsumed`](PlatformWalletError::AssetLockAlreadyConsumed) - /// — the same typed error a resume of an already-consumed lock raises, so - /// callers need one terminal case, not a Platform error-string match. - /// A bookkeeping failure here can only be `WalletNotFound`; it is logged - /// rather than returned, because the verdict itself is what the caller - /// must act on. - async fn settle_already_consumed_lock( - &self, - out_point: dashcore::OutPoint, - ) -> PlatformWalletError { - tracing::info!( - outpoint = %out_point, - "Platform rejected the asset lock as already completely used — its \ - credits landed on an earlier attempt; marking the lock consumed" - ); - if let Err(e) = self.asset_locks.consume_asset_lock(&out_point).await { - tracing::warn!( - outpoint = %out_point, - error = %e, - "consume_asset_lock failed after Platform's already-used rejection" - ); - } - PlatformWalletError::AssetLockAlreadyConsumed(out_point) - } -} - // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs index 2858c63fa6d..674f67b0f5d 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs @@ -247,7 +247,10 @@ impl PlatformAddressWallet { ) .await?; self.asset_locks.queue_asset_lock_changeset(cs); - submit_with_cl_height_retry(settings, |s| { + // Classified like the first attempt: the CL resubmit can + // draw the same terminal "already completely used" verdict + // (see `register_identity_with_funding`'s matching arm). + match submit_with_cl_height_retry(settings, |s| { addresses.top_up_with_signers( &self.sdk, chain_proof.clone(), @@ -259,9 +262,25 @@ impl PlatformAddressWallet { ) }) .await - .map_err(PlatformWalletError::Sdk)? + { + Ok(infos) => infos, + Err(e) => { + return Err(self + .asset_locks + .settle_reported_consumed(e, &proof_out_point) + .await) + } + } + } + // A credit output Platform already spent is terminal for this + // flow too — settle the tracked lock (outpoint-bound) instead + // of leaving it resumable against a deterministic rejection. + Err(e) => { + return Err(self + .asset_locks + .settle_reported_consumed(e, &proof_out_point) + .await) } - Err(e) => return Err(PlatformWalletError::Sdk(e)), }; // Step 4: bookkeeping + cleanup. Write the proof-attested