diff --git a/crates/client/README.md b/crates/client/README.md index e2d0f557..f0da3b81 100644 --- a/crates/client/README.md +++ b/crates/client/README.md @@ -23,6 +23,30 @@ let client = GuardianClient::connect("http://localhost:50051") .with_signer(signer); ``` +### Abandoning a Stuck Candidate + +If an approved transaction dies client-side after guardian approval (RPC +submit failure, prover timeout, crash), its candidate delta keeps the +account locked — new proposals are answered `409 conflict_pending_delta` — +until the server's grace period and retry budget run out. Only the client +knows the transaction will never land: + +```rust +// Records an abandon intent; the guardian's worker confirms over a short +// quarantine that the tx did not land, then releases the account. +let response = client.abandon_candidate(&account_id, nonce).await?; +assert_eq!(response.state, "pending"); + +// Poll the delta for the resolution: still `candidate` -> waiting, +// `canonical` -> the tx landed after all, `discarded` with reason +// `client_abandoned` -> the account is released. +let delta = client.get_delta(&account_id, nonce).await?; +``` + +Retries are idempotent (the original request timestamp is preserved). The +server refuses with `GUARDIAN_CANDIDATE_LANDED` when the transaction +demonstrably landed. + ## Authentication The client uses Falcon Poseidon2 signatures for authenticated requests. Here is how to set it up: diff --git a/crates/client/proto/guardian.proto b/crates/client/proto/guardian.proto index dbe9c69d..2543bfc1 100644 --- a/crates/client/proto/guardian.proto +++ b/crates/client/proto/guardian.proto @@ -34,6 +34,10 @@ service Guardian { // Sign a delta proposal rpc SignDeltaProposal(SignDeltaProposalRequest) returns (SignDeltaProposalResponse); + // Abandon a pending canonicalization candidate whose transaction will + // never land (issue #319), releasing the account immediately. + rpc AbandonDeltaCandidate(AbandonDeltaCandidateRequest) returns (AbandonDeltaCandidateResponse); + // Resolve a public-key commitment to the set of account IDs whose // authorization set contains it. Authentication is by proof-of-possession // against the queried commitment, using the lookup-bound signing primitive @@ -131,6 +135,9 @@ message DeltaStatus { string canonical_at = 3; string discarded_at = 4; } + // Why a discarded delta was discarded ("client_abandoned"); empty for + // other statuses and for discards without a recorded reason. + string discard_reason = 5; } // Pending status with cosigner signatures @@ -260,3 +267,25 @@ message GetAccountByKeyCommitmentResponse { message AccountRef { string account_id = 1; } + +// Abandon-candidate request (issue #319): nonce pins the exact candidate +// so a stale request cannot release a newer one. +message AbandonDeltaCandidateRequest { + string account_id = 1; + uint64 nonce = 2; +} + +message AbandonDeltaCandidateResponse { + bool success = 1; + string message = 2; + string account_id = 3; + uint64 nonce = 4; + // "pending" while the worker still has to resolve the intent (the + // account stays locked until then); "abandoned" once the delta is + // discarded as client-abandoned and the account released. + string state = 5; + // RFC 3339 UTC timestamp of the recorded abandon request. Retries + // return the original timestamp; empty once resolved. + string abandon_requested_at = 6; + string error_code = 7; +} diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 78dfbaa1..0ada14ee 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -3,11 +3,12 @@ use crate::error::{ClientError, ClientResult}; use crate::keystore::Signer; use crate::proto::guardian_client::GuardianClient as GuardianGrpcClient; use crate::proto::{ - AuthConfig, ConfigureRequest, ConfigureResponse, GetAccountByKeyCommitmentRequest, - GetAccountByKeyCommitmentResponse, GetDeltaProposalRequest, GetDeltaProposalResponse, - GetDeltaProposalsRequest, GetDeltaProposalsResponse, GetDeltaRequest, GetDeltaResponse, - GetDeltaSinceRequest, GetDeltaSinceResponse, GetPubkeyRequest, GetStateRequest, - GetStateResponse, ProposalSignature as ProtoProposalSignature, PushDeltaProposalRequest, + AbandonDeltaCandidateRequest, AbandonDeltaCandidateResponse, AuthConfig, ConfigureRequest, + ConfigureResponse, GetAccountByKeyCommitmentRequest, GetAccountByKeyCommitmentResponse, + GetDeltaProposalRequest, GetDeltaProposalResponse, GetDeltaProposalsRequest, + GetDeltaProposalsResponse, GetDeltaRequest, GetDeltaResponse, GetDeltaSinceRequest, + GetDeltaSinceResponse, GetPubkeyRequest, GetStateRequest, GetStateResponse, + ProposalSignature as ProtoProposalSignature, PushDeltaProposalRequest, PushDeltaProposalResponse, PushDeltaRequest, PushDeltaResponse, SignDeltaProposalRequest, SignDeltaProposalResponse, }; @@ -397,6 +398,43 @@ impl GuardianClient { Ok(inner) } + + /// Request abandonment of a pending canonicalization candidate whose + /// transaction will never land on-chain (issue #319). + /// + /// The request records an abandon *intent*: the delta stays a + /// candidate — the account stays locked — until the guardian's + /// canonicalization worker confirms over the abandon quarantine that + /// the transaction did not land, then discards the delta as + /// `client_abandoned` and releases the account (typically well under + /// a minute). Poll `get_delta` for the resolution; the returned + /// `state` is `"pending"` or, when already resolved, `"abandoned"`. + /// + /// `nonce` pins the exact candidate (learned from the delta feed). + /// The server refuses with `GUARDIAN_CANDIDATE_LANDED` when the + /// transaction actually landed. Retries are idempotent and preserve + /// the original request timestamp. + pub async fn abandon_candidate( + &mut self, + account_id: &AccountId, + nonce: u64, + ) -> ClientResult { + let mut request = tonic::Request::new(AbandonDeltaCandidateRequest { + account_id: account_id.to_string(), + nonce, + }); + + self.add_auth_metadata(&mut request, account_id)?; + + let response = self.client.abandon_delta_candidate(request).await?; + let inner = response.into_inner(); + + if !inner.success { + return Err(ClientError::ServerError(inner.message.clone())); + } + + Ok(inner) + } } fn attach_auth_headers( diff --git a/crates/client/src/testing/mocks.rs b/crates/client/src/testing/mocks.rs index c2568b59..9f369246 100644 --- a/crates/client/src/testing/mocks.rs +++ b/crates/client/src/testing/mocks.rs @@ -1,11 +1,12 @@ use crate::proto::guardian_server::{Guardian, GuardianServer}; use crate::proto::{ - AccountState, ConfigureRequest, ConfigureResponse, DeltaObject as ProtoDeltaObject, - GetAccountByKeyCommitmentRequest, GetAccountByKeyCommitmentResponse, GetDeltaProposalRequest, - GetDeltaProposalResponse, GetDeltaProposalsRequest, GetDeltaProposalsResponse, GetDeltaRequest, - GetDeltaResponse, GetDeltaSinceRequest, GetDeltaSinceResponse, GetPubkeyRequest, - GetStateRequest, GetStateResponse, PushDeltaProposalRequest, PushDeltaProposalResponse, - PushDeltaRequest, PushDeltaResponse, SignDeltaProposalRequest, SignDeltaProposalResponse, + AbandonDeltaCandidateRequest, AbandonDeltaCandidateResponse, AccountState, ConfigureRequest, + ConfigureResponse, DeltaObject as ProtoDeltaObject, GetAccountByKeyCommitmentRequest, + GetAccountByKeyCommitmentResponse, GetDeltaProposalRequest, GetDeltaProposalResponse, + GetDeltaProposalsRequest, GetDeltaProposalsResponse, GetDeltaRequest, GetDeltaResponse, + GetDeltaSinceRequest, GetDeltaSinceResponse, GetPubkeyRequest, GetStateRequest, + GetStateResponse, PushDeltaProposalRequest, PushDeltaProposalResponse, PushDeltaRequest, + PushDeltaResponse, SignDeltaProposalRequest, SignDeltaProposalResponse, }; use std::net::SocketAddr; use std::sync::{Arc, Mutex as StdMutex}; @@ -26,6 +27,8 @@ pub struct MockGuardianService { get_state_response: Arc>>>, get_account_by_key_commitment_response: Arc>>>, + abandon_delta_candidate_response: + Arc>>>, } impl MockGuardianService { @@ -98,6 +101,14 @@ impl MockGuardianService { *self.get_account_by_key_commitment_response.lock().unwrap() = Some(response); self } + + pub fn with_abandon_delta_candidate( + self, + response: Result, + ) -> Self { + *self.abandon_delta_candidate_response.lock().unwrap() = Some(response); + self + } } #[tonic::async_trait] @@ -223,6 +234,31 @@ impl Guardian for MockGuardianService { response.map(Response::new) } + async fn abandon_delta_candidate( + &self, + request: Request, + ) -> Result, Status> { + let data = request.into_inner(); + let response = self + .abandon_delta_candidate_response + .lock() + .unwrap() + .take() + .unwrap_or_else(|| { + Ok(AbandonDeltaCandidateResponse { + success: true, + message: String::new(), + account_id: data.account_id, + nonce: data.nonce, + state: "pending".to_string(), + abandon_requested_at: "2026-07-14T12:00:00Z".to_string(), + error_code: String::new(), + }) + }); + + response.map(Response::new) + } + async fn push_delta( &self, _request: Request, diff --git a/crates/miden-multisig-client/README.md b/crates/miden-multisig-client/README.md index 139bcebf..cffe6619 100644 --- a/crates/miden-multisig-client/README.md +++ b/crates/miden-multisig-client/README.md @@ -78,6 +78,32 @@ client.sign_proposal(&to_sign.id).await?; client.execute_proposal(&proposal.id).await?; ``` +### Recovering From a Dead Transaction (Abandon) + +If `execute_proposal` dies after guardian approval (RPC submit failure, +prover timeout, crash), the approved candidate keeps the account locked on +GUARDIAN. Record an abandon intent and poll for the resolution: + +```rust +use miden_multisig_client::{AbandonRequestState, AbandonStatus}; + +// The nonce the proposal was pushed with (committed account nonce + 1). +let state = client.abandon_candidate(nonce).await?; +assert_eq!(state, AbandonRequestState::Pending); + +// The guardian's worker confirms over a short quarantine (typically well +// under a minute) that the transaction did not land, then releases the +// account. +loop { + match client.abandon_status(nonce).await? { + AbandonStatus::Waiting => tokio::time::sleep(Duration::from_secs(5)).await, + AbandonStatus::Abandoned => break, // account released + AbandonStatus::Landed => break, // tx landed after all + AbandonStatus::Unexpected => anyhow::bail!("unexpected candidate state"), + } +} +``` + ### Fallback to Offline (if GUARDIAN unavailable) If the GUARDIAN endpoint can’t be reached, the SDK can produce an offline proposal only for `SwitchGuardian` transactions: diff --git a/crates/miden-multisig-client/src/client/mod.rs b/crates/miden-multisig-client/src/client/mod.rs index e0854f7b..de465742 100644 --- a/crates/miden-multisig-client/src/client/mod.rs +++ b/crates/miden-multisig-client/src/client/mod.rs @@ -16,6 +16,7 @@ mod io; mod notes; mod offline; mod proposals; +pub use proposals::{AbandonRequestState, AbandonStatus}; use std::path::PathBuf; use std::sync::Arc; diff --git a/crates/miden-multisig-client/src/client/proposals.rs b/crates/miden-multisig-client/src/client/proposals.rs index a1528a7c..27f6ed90 100644 --- a/crates/miden-multisig-client/src/client/proposals.rs +++ b/crates/miden-multisig-client/src/client/proposals.rs @@ -9,6 +9,30 @@ use guardian_shared::{ProposalSignature, ToJson}; use miden_client::transaction::TransactionRequest; use super::{MultisigClient, ProposalResult}; + +/// Immediate outcome of an abandon request (issue #319). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AbandonRequestState { + /// The intent is recorded; GUARDIAN's worker resolves it after the + /// abandon quarantine. Poll [`MultisigClient::abandon_status`]. + Pending, + /// The abandon was already resolved; the account is released. + Abandoned, +} + +/// Resolution of an abandon request, as observed via the delta feed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AbandonStatus { + /// The delta is still a candidate; the quarantine is running. + Waiting, + /// The transaction landed after all; the delta canonicalized. + Landed, + /// The abandon completed; the delta is discarded as client-abandoned + /// and the account is released. + Abandoned, + /// The delta is missing or in a state no abandon flow produces. + Unexpected, +} use crate::error::{MultisigError, Result}; use crate::execution::{ SignatureAdvice, SignatureInput, build_final_transaction_request, collect_signature_advice, @@ -556,6 +580,83 @@ impl MultisigClient { Err(e) => Err(e), } } + + /// Requests abandonment of a pending canonicalization candidate whose + /// transaction will never land on-chain (issue #319). + /// + /// Call this after an approved transaction died client-side (RPC + /// submit failure, prover timeout, crash). The request records an + /// abandon *intent* on GUARDIAN; the account stays locked until the + /// guardian's canonicalization worker confirms over a short + /// quarantine (typically well under a minute) that the transaction + /// did not land, then releases the account. Poll [`Self::abandon_status`] + /// for the resolution. + /// + /// `nonce` pins the exact candidate to release; it is the nonce the + /// proposal was pushed with (committed account nonce + 1). Retries + /// are idempotent and preserve the original request timestamp. + /// + /// # Errors + /// + /// Returns an error if the candidate's transaction actually landed + /// on-chain (`GUARDIAN_CANDIDATE_LANDED` — the server will + /// canonicalize it shortly), if no candidate exists at this nonce, + /// or if GUARDIAN cannot be reached. + pub async fn abandon_candidate(&mut self, nonce: u64) -> Result { + let account_id = self.require_account()?.id(); + + let mut guardian_client = self.create_authenticated_guardian_client().await?; + let response = guardian_client + .abandon_candidate(&account_id, nonce) + .await + .map_err(|e| { + MultisigError::GuardianServer(format!("failed to abandon candidate: {}", e)) + })?; + + Ok(match response.state.as_str() { + "abandoned" => AbandonRequestState::Abandoned, + _ => AbandonRequestState::Pending, + }) + } + + /// Polls GUARDIAN for the resolution of an abandon request made with + /// [`Self::abandon_candidate`]. + pub async fn abandon_status(&mut self, nonce: u64) -> Result { + let account_id = self.require_account()?.id(); + + let mut guardian_client = self.create_authenticated_guardian_client().await?; + let response = match guardian_client.get_delta(&account_id, nonce).await { + Ok(response) => response, + // A missing delta is unexpected for an abandon in flight: the + // worker preserves an abandoned delta as discarded history. + Err(e) if e.to_string().to_lowercase().contains("not found") => { + return Ok(AbandonStatus::Unexpected); + } + Err(e) => { + return Err(MultisigError::GuardianServer(format!( + "failed to poll abandon status: {}", + e + ))); + } + }; + + let Some(delta) = response.delta else { + return Ok(AbandonStatus::Unexpected); + }; + let Some(status) = delta.status else { + return Ok(AbandonStatus::Unexpected); + }; + + use guardian_client::delta_status::Status as ProtoStatus; + Ok(match status.status { + Some(ProtoStatus::CandidateAt(_)) => AbandonStatus::Waiting, + Some(ProtoStatus::CanonicalAt(_)) => AbandonStatus::Landed, + Some(ProtoStatus::DiscardedAt(_)) if status.discard_reason == "client_abandoned" => { + AbandonStatus::Abandoned + } + _ => AbandonStatus::Unexpected, + }) + } } #[cfg(test)] diff --git a/crates/miden-multisig-client/src/lib.rs b/crates/miden-multisig-client/src/lib.rs index 06449077..dc916773 100644 --- a/crates/miden-multisig-client/src/lib.rs +++ b/crates/miden-multisig-client/src/lib.rs @@ -54,6 +54,7 @@ pub(crate) type MidenSdkClient = Client; // Main client pub use builder::MultisigClientBuilder; +pub use client::{AbandonRequestState, AbandonStatus}; pub use client::{ ConsumableNote, MultisigClient, NoteFilter, ProposalResult, RecoveredAccount, StateVerificationResult, diff --git a/crates/server/proto/guardian.proto b/crates/server/proto/guardian.proto index d61507f0..d0f373ad 100644 --- a/crates/server/proto/guardian.proto +++ b/crates/server/proto/guardian.proto @@ -34,6 +34,10 @@ service Guardian { // Sign a delta proposal rpc SignDeltaProposal(SignDeltaProposalRequest) returns (SignDeltaProposalResponse); + // Abandon a pending canonicalization candidate whose transaction will + // never land (issue #319), releasing the account immediately. + rpc AbandonDeltaCandidate(AbandonDeltaCandidateRequest) returns (AbandonDeltaCandidateResponse); + // Resolve a public-key commitment to the set of account IDs whose // authorization set contains it. Authentication is by proof-of-possession // against the queried commitment, using the lookup-bound signing primitive @@ -164,6 +168,9 @@ message DeltaStatus { string canonical_at = 3; string discarded_at = 4; } + // Why a discarded delta was discarded ("client_abandoned"); empty for + // other statuses and for discards without a recorded reason. + string discard_reason = 5; } // Pending status with cosigner signatures @@ -298,3 +305,25 @@ message GetAccountByKeyCommitmentResponse { message AccountRef { string account_id = 1; } + +// Abandon-candidate request (issue #319): nonce pins the exact candidate +// so a stale request cannot release a newer one. +message AbandonDeltaCandidateRequest { + string account_id = 1; + uint64 nonce = 2; +} + +message AbandonDeltaCandidateResponse { + bool success = 1; + string message = 2; + string account_id = 3; + uint64 nonce = 4; + // "pending" while the worker still has to resolve the intent (the + // account stays locked until then); "abandoned" once the delta is + // discarded as client-abandoned and the account released. + string state = 5; + // RFC 3339 UTC timestamp of the recorded abandon request. Retries + // return the original timestamp; empty once resolved. + string abandon_requested_at = 6; + string error_code = 7; +} diff --git a/crates/server/src/api/grpc.rs b/crates/server/src/api/grpc.rs index 8f9b57e6..e8d84770 100644 --- a/crates/server/src/api/grpc.rs +++ b/crates/server/src/api/grpc.rs @@ -321,6 +321,44 @@ impl Guardian for GuardianService { } } + /// Request abandonment of a pending canonicalization candidate + /// (issue #319): records the intent; the worker resolves it after the + /// abandon quarantine. Mirror of HTTP `POST /delta/candidate/abandon`. + async fn abandon_delta_candidate( + &self, + request: Request, + ) -> Result, Status> { + let credentials = authenticated_request(&request)?; + let data = request.into_inner(); + + let params = services::AbandonCandidateParams { + account_id: data.account_id.clone(), + nonce: data.nonce, + credentials, + }; + + match services::abandon_candidate(&self.app_state, params).await { + Ok(response) => Ok(Response::new(AbandonDeltaCandidateResponse { + success: true, + message: "Abandon intent accepted".to_string(), + account_id: response.account_id, + nonce: response.nonce, + state: response.state.as_str().to_string(), + abandon_requested_at: response.abandon_requested_at.unwrap_or_default(), + error_code: String::new(), + })), + Err(e) => Ok(Response::new(AbandonDeltaCandidateResponse { + success: false, + message: e.to_string(), + account_id: data.account_id, + nonce: data.nonce, + state: String::new(), + abandon_requested_at: String::new(), + error_code: e.code().to_string(), + })), + } + } + /// Resolve a public-key commitment to the set of account IDs that /// authorize it. Mirror of HTTP `GET /state/lookup`. async fn get_account_by_key_commitment( @@ -371,7 +409,7 @@ fn delta_to_proto(delta: &DeltaObject) -> guardian::DeltaObject { crate::delta_object::DeltaStatus::Canonical { timestamp } => { (Some(timestamp.clone()), Some(timestamp.clone()), None) } - crate::delta_object::DeltaStatus::Discarded { timestamp } => { + crate::delta_object::DeltaStatus::Discarded { timestamp, .. } => { (None, None, Some(timestamp.clone())) } }; @@ -400,23 +438,34 @@ fn delta_to_proto(delta: &DeltaObject) -> guardian::DeltaObject { cosigner_sigs: proto_cosigner_sigs, }, )), + discard_reason: String::new(), }) } crate::delta_object::DeltaStatus::Candidate { timestamp, .. } => Some(DeltaStatusGrpc { status: Some(guardian::delta_status::Status::CandidateAt( timestamp.clone(), )), + discard_reason: String::new(), }), crate::delta_object::DeltaStatus::Canonical { timestamp } => Some(DeltaStatusGrpc { status: Some(guardian::delta_status::Status::CanonicalAt( timestamp.clone(), )), + discard_reason: String::new(), }), - crate::delta_object::DeltaStatus::Discarded { timestamp } => Some(DeltaStatusGrpc { - status: Some(guardian::delta_status::Status::DiscardedAt( - timestamp.clone(), - )), - }), + crate::delta_object::DeltaStatus::Discarded { timestamp, reason } => { + Some(DeltaStatusGrpc { + status: Some(guardian::delta_status::Status::DiscardedAt( + timestamp.clone(), + )), + discard_reason: match reason { + Some(crate::delta_object::DiscardReason::ClientAbandoned) => { + "client_abandoned".to_string() + } + None => String::new(), + }, + }) + } }; guardian::DeltaObject { @@ -695,6 +744,116 @@ mod tests { assert_eq!(inner.delta.unwrap().nonce, 1); } + fn abandon_grpc_fixtures( + storage: &MockStorageBackend, + network: &MockNetworkClient, + metadata: &MockMetadataStore, + account_id: &str, + signer: &TestSigner, + landed: bool, + ) { + let account_json: serde_json::Value = serde_json::from_str(fixtures::ACCOUNT_JSON).unwrap(); + let delta_fixture: serde_json::Value = + serde_json::from_str(fixtures::DELTA_1_JSON).unwrap(); + let candidate = crate::delta_object::DeltaObject { + account_id: account_id.to_string(), + nonce: 1, + prev_commitment: "0x123".to_string(), + new_commitment: Some("0x456".to_string()), + delta_payload: delta_fixture["delta_payload"].clone(), + ack_sig: String::new(), + ack_pubkey: String::new(), + ack_scheme: String::new(), + status: DeltaStatus::candidate("2024-11-14T12:00:00Z".to_string()), + metadata: None, + }; + + let _ = metadata.clone().with_get(Ok(Some(create_account_metadata( + account_id.to_string(), + vec![signer.commitment_hex.clone()], + )))); + let _ = storage + .clone() + .with_pull_delta(Ok(candidate)) + .with_pull_state(Ok(create_state_object( + account_id.to_string(), + "0x123".to_string(), + account_json, + ))); + let verify = if landed { + Ok(crate::network::StateVerification::Match) + } else { + Ok(crate::network::StateVerification::Mismatch { + on_chain: "0x123".to_string(), + }) + }; + let _ = network + .clone() + .with_apply_delta(Ok((serde_json::json!({"new": true}), "0x456".to_string()))) + .with_verify_commitment(verify); + } + + #[tokio::test] + async fn test_grpc_abandon_delta_candidate_success() { + let (state, storage, network, metadata) = create_test_state(); + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b".to_string(); + let signer = TestSigner::new(); + abandon_grpc_fixtures(&storage, &network, &metadata, &account_id, &signer, false); + let service = create_service(state); + + let request = create_request_with_auth( + AbandonDeltaCandidateRequest { + account_id: account_id.clone(), + nonce: 1, + }, + &signer, + &account_id, + ); + let response = service + .abandon_delta_candidate(request) + .await + .expect("gRPC call should succeed") + .into_inner(); + + assert!(response.success, "expected success: {}", response.message); + assert_eq!(response.account_id, account_id); + assert_eq!(response.nonce, 1); + assert_eq!(response.state, "pending"); + assert!(!response.abandon_requested_at.is_empty()); + assert!(response.error_code.is_empty()); + // Intent only: nothing is deleted at request time. + assert!(storage.get_delete_delta_calls().is_empty()); + let _ = account_id; + } + + #[tokio::test] + async fn test_grpc_abandon_delta_candidate_landed_error_code() { + let (state, storage, network, metadata) = create_test_state(); + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b".to_string(); + let signer = TestSigner::new(); + abandon_grpc_fixtures(&storage, &network, &metadata, &account_id, &signer, true); + let service = create_service(state); + + let request = create_request_with_auth( + AbandonDeltaCandidateRequest { + account_id: account_id.clone(), + nonce: 1, + }, + &signer, + &account_id, + ); + let response = service + .abandon_delta_candidate(request) + .await + .expect("gRPC transport should not error") + .into_inner(); + + assert!(!response.success); + assert_eq!(response.error_code, "GUARDIAN_CANDIDATE_LANDED"); + assert!(response.state.is_empty()); + assert!(storage.get_delete_delta_calls().is_empty()); + } + #[tokio::test] async fn test_grpc_push_delta_proposal_missing_tx_summary() { let (state, storage, _network, metadata) = create_test_state(); diff --git a/crates/server/src/api/http.rs b/crates/server/src/api/http.rs index 73f363ea..809d91bf 100644 --- a/crates/server/src/api/http.rs +++ b/crates/server/src/api/http.rs @@ -3,9 +3,9 @@ use crate::error::GuardianError; use crate::metadata::NetworkConfig; use crate::metadata::auth::{Auth, AuthHeader, Credentials}; use crate::services::{ - self, ConfigureAccountParams, GetDeltaParams, GetDeltaProposalParams, GetDeltaProposalsParams, - GetDeltaSinceParams, GetStateParams, LookupAccountParams, PushDeltaParams, - PushDeltaProposalParams, SignDeltaProposalParams, + self, AbandonCandidateParams, ConfigureAccountParams, GetDeltaParams, GetDeltaProposalParams, + GetDeltaProposalsParams, GetDeltaSinceParams, GetStateParams, LookupAccountParams, + PushDeltaParams, PushDeltaProposalParams, SignDeltaProposalParams, }; use crate::state::AppState; use crate::state_object::StateObject; @@ -94,6 +94,28 @@ pub struct DeltaProposalRequest { pub delta_payload: serde_json::Value, } +#[derive(Deserialize, Serialize, utoipa::ToSchema)] +pub struct AbandonCandidateRequest { + pub account_id: String, + /// Nonce of the candidate delta to abandon. Requiring the explicit + /// target prevents a stale request from releasing a newer candidate. + pub nonce: u64, +} + +#[derive(Debug, Serialize, utoipa::ToSchema)] +pub struct AbandonCandidateResponse { + pub account_id: String, + pub nonce: u64, + /// `"pending"` while the worker still has to resolve the intent + /// (the account stays locked until then); `"abandoned"` once the + /// delta is discarded as client-abandoned and the account released. + pub state: String, + /// RFC 3339 UTC timestamp of the recorded abandon request. Retries + /// return the original timestamp; absent once resolved. + #[serde(skip_serializing_if = "Option::is_none")] + pub abandon_requested_at: Option, +} + #[derive(Deserialize, Serialize, utoipa::ToSchema)] pub struct SignProposalRequest { pub account_id: String, @@ -458,6 +480,60 @@ pub async fn push_delta_proposal( })) } +/// Request abandonment of a pending canonicalization candidate whose +/// transaction will never land on-chain (issue #319). +/// +/// The request records an abandon *intent* and returns `202 Accepted`; +/// the delta stays a candidate — the account stays locked — until the +/// canonicalization worker confirms over the abandon quarantine that the +/// transaction did not land, then discards the delta as +/// `client_abandoned` and releases the account (typically well under a +/// minute, versus the full submission grace + retry window). +/// +/// Refused with `GUARDIAN_CANDIDATE_LANDED` (409) when the transaction +/// already landed. Retries are idempotent and preserve the original +/// request timestamp. Poll `GET /delta` for the resolution: still +/// `candidate` → waiting; `canonical` → landed after all; `discarded` +/// with reason `client_abandoned` → abandoned. +#[utoipa::path( + post, + path = "/delta/candidate/abandon", + tag = "client", + security(("x-pubkey" = [], "x-signature" = [], "x-timestamp" = [])), + request_body = AbandonCandidateRequest, + responses( + (status = 202, description = "Abandon intent accepted (or already resolved)", body = AbandonCandidateResponse), + (status = 401, description = "Authentication failed", body = crate::openapi::ApiErrorResponse), + (status = 404, description = "No candidate delta at this nonce", body = crate::openapi::ApiErrorResponse), + (status = 409, description = "Candidate already landed on-chain, or account paused/released", body = crate::openapi::ApiErrorResponse), + ) +)] +pub async fn abandon_candidate( + State(state): State, + AuthHeader(credentials): AuthHeader, + Json(payload): Json, +) -> Result<(StatusCode, Json), GuardianError> { + let request_payload = + request_payload_from_serializable(&payload).map_err(GuardianError::InvalidInput)?; + + let params = AbandonCandidateParams { + account_id: payload.account_id, + nonce: payload.nonce, + credentials: request_payload.apply_to(credentials), + }; + + let response = services::abandon_candidate(&state, params).await?; + Ok(( + StatusCode::ACCEPTED, + Json(AbandonCandidateResponse { + account_id: response.account_id, + nonce: response.nonce, + state: response.state.as_str().to_string(), + abandon_requested_at: response.abandon_requested_at, + }), + )) +} + /// List all in-flight multisig proposals for an account. #[utoipa::path( get, @@ -844,6 +920,104 @@ mod tests { assert!(!response.commitment.is_empty()); } + fn abandon_test_fixtures( + storage: &MockStorageBackend, + network: &MockNetworkClient, + metadata: &MockMetadataStore, + account_id: &str, + signer: &TestSigner, + landed: bool, + ) { + let account_json: serde_json::Value = serde_json::from_str(fixtures::ACCOUNT_JSON).unwrap(); + let mut candidate = create_test_delta(account_id, 1); + candidate.status = DeltaStatus::candidate("2024-11-14T12:00:00Z".to_string()); + + let _ = metadata.clone().with_get(Ok(Some(create_account_metadata( + account_id.to_string(), + vec![signer.commitment_hex.clone()], + )))); + let _ = storage + .clone() + .with_pull_delta(Ok(candidate)) + .with_pull_state(Ok(create_state_object( + account_id.to_string(), + "0x780aa2edb983c1baab3c81edcfe400bc54b516d5cb51f2a7cec4690667329392".to_string(), + account_json, + ))); + let verify = if landed { + Ok(crate::network::StateVerification::Match) + } else { + Ok(crate::network::StateVerification::Mismatch { + on_chain: "0x780aa2edb983c1baab3c81edcfe400bc54b516d5cb51f2a7cec4690667329392" + .to_string(), + }) + }; + let _ = network + .clone() + .with_apply_delta(Ok((serde_json::json!({"new": true}), "0xnew".to_string()))) + .with_verify_commitment(verify); + } + + #[tokio::test] + async fn test_abandon_candidate_success() { + let (state, storage, network, metadata) = create_test_state(); + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b".to_string(); + let signer = TestSigner::new(); + abandon_test_fixtures(&storage, &network, &metadata, &account_id, &signer, false); + + let request = AbandonCandidateRequest { + account_id: account_id.clone(), + nonce: 1, + }; + let credentials = signed_credentials(&signer, &account_id, &request); + let (status, Json(response)) = + abandon_candidate(State(state), AuthHeader(credentials), Json(request)) + .await + .expect("abandon_candidate should succeed"); + + assert_eq!(status, StatusCode::ACCEPTED); + assert_eq!(response.account_id, account_id); + assert_eq!(response.nonce, 1); + assert_eq!(response.state, "pending"); + assert!(response.abandon_requested_at.is_some()); + // Intent only: nothing is deleted at request time. + assert!(storage.get_delete_delta_calls().is_empty()); + } + + #[tokio::test] + async fn test_abandon_candidate_landed_maps_to_409_envelope() { + use axum::body::to_bytes; + use axum::response::IntoResponse; + + let (state, storage, network, metadata) = create_test_state(); + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b".to_string(); + let signer = TestSigner::new(); + abandon_test_fixtures(&storage, &network, &metadata, &account_id, &signer, true); + + let request = AbandonCandidateRequest { + account_id: account_id.clone(), + nonce: 1, + }; + let credentials = signed_credentials(&signer, &account_id, &request); + let err = abandon_candidate(State(state), AuthHeader(credentials), Json(request)) + .await + .expect_err("landed candidate must refuse the abandon"); + + let response = err.into_response(); + assert_eq!(response.status(), StatusCode::CONFLICT); + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body bytes"); + let parsed: serde_json::Value = serde_json::from_slice(&bytes).expect("JSON envelope"); + assert_eq!(parsed["code"], "GUARDIAN_CANDIDATE_LANDED"); + assert!( + parsed["message"].as_str().is_some_and(|m| !m.is_empty()), + "user-safe message present" + ); + assert_eq!(parsed["meta"]["retryable"], serde_json::Value::Bool(false)); + assert!(storage.get_delete_delta_calls().is_empty()); + } + #[tokio::test] async fn test_push_delta_proposal_missing_tx_summary() { let (state, storage, _network, metadata) = create_test_state(); diff --git a/crates/server/src/builder/canonicalization.rs b/crates/server/src/builder/canonicalization.rs index abf5f1fd..57e03e72 100644 --- a/crates/server/src/builder/canonicalization.rs +++ b/crates/server/src/builder/canonicalization.rs @@ -26,6 +26,18 @@ pub struct CanonicalizationConfig { /// bypasses the submission grace period. pub divergence_confirmations: u32, + /// Minimum age a client abandon request (issue #319) must reach before + /// the worker may finalize it. Together with + /// `abandon_quarantine_checks` this quarantine reduces the risk of + /// abandoning a transaction that lands late; like the divergence + /// discard, abandon resolution bypasses the submission grace period. + pub abandon_quarantine_seconds: u64, + + /// Consecutive worker ticks that must observe the on-chain commitment + /// still at the candidate's base after an abandon request before the + /// worker finalizes the abandon. A divergent observation resets the + /// streak, mirroring `divergence_confirmations`. + pub abandon_quarantine_checks: u32, /// How many accounts one canonicalization pass processes concurrently. /// Candidates within an account are always sequential (nonce order); /// this only overlaps the per-account work — dominated by the Miden @@ -44,6 +56,8 @@ impl Default for CanonicalizationConfig { max_retries: 18, // 18 attempts (total: ~3 minutes) submission_grace_period_seconds: 600, // Allow proving/submission to settle first divergence_confirmations: 2, // Two ticks to rule out a stale read + abandon_quarantine_seconds: 15, // Let a late-landing tx surface first + abandon_quarantine_checks: 2, // Two ticks to rule out a stale read max_concurrent_accounts: 10, // Overlaps per-account chain RPCs; prod Terraform sets 50 } } @@ -82,6 +96,19 @@ impl CanonicalizationConfig { self } + /// Override the abandon quarantine duration. + pub fn with_abandon_quarantine_seconds(mut self, seconds: u64) -> Self { + self.abandon_quarantine_seconds = seconds; + self + } + + /// Override the number of consecutive at-base observations required + /// before an abandon request is finalized. + pub fn with_abandon_quarantine_checks(mut self, checks: u32) -> Self { + self.abandon_quarantine_checks = checks; + self + } + /// Override how many accounts one pass processes concurrently. /// `1` reproduces the fully sequential pass. pub fn with_max_concurrent_accounts(mut self, accounts: usize) -> Self { diff --git a/crates/server/src/builder/handle.rs b/crates/server/src/builder/handle.rs index 70c7c654..568c1a4d 100644 --- a/crates/server/src/builder/handle.rs +++ b/crates/server/src/builder/handle.rs @@ -24,8 +24,9 @@ use crate::api::grpc::GuardianService; use crate::api::grpc::guardian::FILE_DESCRIPTOR_SET; use crate::api::grpc::guardian::guardian_server::GuardianServer; use crate::api::http::{ - configure, get_delta, get_delta_proposal, get_delta_proposals, get_delta_since, get_pubkey, - get_state, lookup, push_delta, push_delta_proposal, sign_delta_proposal, status, + abandon_candidate, configure, get_delta, get_delta_proposal, get_delta_proposals, + get_delta_since, get_pubkey, get_state, lookup, push_delta, push_delta_proposal, + sign_delta_proposal, status, }; use crate::builder::startup::StartupInfo; use crate::dashboard::require_dashboard_session; @@ -237,6 +238,7 @@ impl ServerHandle { .route("/delta/proposal", get(get_delta_proposals)) .route("/delta/proposal/single", get(get_delta_proposal)) .route("/delta/proposal", put(sign_delta_proposal)) + .route("/delta/candidate/abandon", post(abandon_candidate)) .route("/configure", post(configure)) .route("/state", get(get_state)) .route("/state/lookup", get(lookup)) diff --git a/crates/server/src/builder/startup.rs b/crates/server/src/builder/startup.rs index 37b09528..fc1d809f 100644 --- a/crates/server/src/builder/startup.rs +++ b/crates/server/src/builder/startup.rs @@ -180,6 +180,8 @@ mod tests { max_retries: 48, submission_grace_period_seconds: 600, divergence_confirmations: 2, + abandon_quarantine_seconds: 15, + abandon_quarantine_checks: 2, max_concurrent_accounts: 4, }), 3, diff --git a/crates/server/src/delta_object.rs b/crates/server/src/delta_object.rs index 9526b453..9979b1a5 100644 --- a/crates/server/src/delta_object.rs +++ b/crates/server/src/delta_object.rs @@ -9,6 +9,18 @@ pub struct CosignerSignature { pub signer_id: String, } +/// Why a delta ended in `Discarded`. Absent on discards that predate +/// the field (and on the worker's retry/divergence discards, which +/// delete the delta instead of transitioning it). +#[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq, utoipa::ToSchema)] +#[serde(rename_all = "snake_case")] +pub enum DiscardReason { + /// The client abandoned the candidate via the abandon endpoint + /// (issue #319) and the worker confirmed the transaction never + /// landed over the abandon quarantine. + ClientAbandoned, +} + /// Delta status state machine #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, utoipa::ToSchema)] #[serde(tag = "status", rename_all = "snake_case")] @@ -31,12 +43,26 @@ pub enum DeltaStatus { /// before discarding shields against stale RPC reads. #[serde(default)] divergence_count: u32, + /// RFC 3339 UTC timestamp of the client's abandon request + /// (issue #319). While set, the status remains `candidate` — the + /// account stays locked — until the canonicalization worker + /// resolves the intent after the abandon quarantine. + #[serde(default, skip_serializing_if = "Option::is_none")] + abandon_requested_at: Option, + /// Consecutive worker ticks that observed the on-chain commitment + /// still at this candidate's base after the abandon was requested. + /// Reset on a divergent observation, so only an unbroken streak + /// counts — the same stale-RPC shield as `divergence_count`. + #[serde(default)] + abandon_confirm_count: u32, }, Canonical { timestamp: String, }, Discarded { timestamp: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + reason: Option, }, } @@ -54,6 +80,8 @@ impl DeltaStatus { timestamp, retry_count: 0, divergence_count: 0, + abandon_requested_at: None, + abandon_confirm_count: 0, } } @@ -62,6 +90,8 @@ impl DeltaStatus { timestamp, retry_count, divergence_count: 0, + abandon_requested_at: None, + abandon_confirm_count: 0, } } @@ -70,7 +100,117 @@ impl DeltaStatus { } pub fn discarded(timestamp: String) -> Self { - Self::Discarded { timestamp } + Self::Discarded { + timestamp, + reason: None, + } + } + + pub fn discarded_client_abandoned(timestamp: String) -> Self { + Self::Discarded { + timestamp, + reason: Some(DiscardReason::ClientAbandoned), + } + } + + pub fn is_client_abandoned(&self) -> bool { + matches!( + self, + Self::Discarded { + reason: Some(DiscardReason::ClientAbandoned), + .. + } + ) + } + + pub fn abandon_requested_at(&self) -> Option<&str> { + match self { + Self::Candidate { + abandon_requested_at, + .. + } => abandon_requested_at.as_deref(), + _ => None, + } + } + + pub fn abandon_confirm_count(&self) -> u32 { + match self { + Self::Candidate { + abandon_confirm_count, + .. + } => *abandon_confirm_count, + _ => 0, + } + } + + /// Record the client's abandon intent, preserving every worker-owned + /// counter. Idempotent: an already-set request timestamp is kept so + /// retries cannot restart the quarantine. + pub fn with_abandon_requested(&self, now: String) -> Self { + match self { + Self::Candidate { + timestamp, + retry_count, + divergence_count, + abandon_requested_at, + abandon_confirm_count, + } => Self::Candidate { + timestamp: timestamp.clone(), + retry_count: *retry_count, + divergence_count: *divergence_count, + abandon_requested_at: Some(abandon_requested_at.clone().unwrap_or(now)), + abandon_confirm_count: *abandon_confirm_count, + }, + _ => self.clone(), + } + } + + /// Carry a concurrently-recorded abandon request into a status that is + /// about to overwrite the stored row. Worker counter writes are + /// computed from a tick-start snapshot, so without this a client + /// intent recorded mid-tick would be silently wiped. Only + /// `abandon_requested_at` is preserved — confirmation-streak resets + /// are legitimate worker writes, and non-candidate statuses drop the + /// intent by design (terminal states resolve it). + pub fn with_abandon_request_preserved_from(&self, stored_requested_at: Option<&str>) -> Self { + match (self, stored_requested_at) { + ( + Self::Candidate { + timestamp, + retry_count, + divergence_count, + abandon_requested_at: None, + abandon_confirm_count, + }, + Some(stored), + ) => Self::Candidate { + timestamp: timestamp.clone(), + retry_count: *retry_count, + divergence_count: *divergence_count, + abandon_requested_at: Some(stored.to_string()), + abandon_confirm_count: *abandon_confirm_count, + }, + _ => self.clone(), + } + } + + pub fn with_incremented_abandon_confirm(&self) -> Self { + match self { + Self::Candidate { + timestamp, + retry_count, + divergence_count, + abandon_requested_at, + abandon_confirm_count, + } => Self::Candidate { + timestamp: timestamp.clone(), + retry_count: *retry_count, + divergence_count: *divergence_count, + abandon_requested_at: abandon_requested_at.clone(), + abandon_confirm_count: abandon_confirm_count + 1, + }, + _ => self.clone(), + } } pub fn is_pending(&self) -> bool { @@ -94,7 +234,7 @@ impl DeltaStatus { Self::Pending { timestamp, .. } => timestamp, Self::Candidate { timestamp, .. } => timestamp, Self::Canonical { timestamp } => timestamp, - Self::Discarded { timestamp } => timestamp, + Self::Discarded { timestamp, .. } => timestamp, } } @@ -120,28 +260,39 @@ impl DeltaStatus { timestamp, retry_count, divergence_count, + abandon_requested_at, + abandon_confirm_count, } => { let _ = new_timestamp; Self::Candidate { timestamp: timestamp.clone(), retry_count: retry_count + 1, divergence_count: *divergence_count, + abandon_requested_at: abandon_requested_at.clone(), + abandon_confirm_count: *abandon_confirm_count, } } _ => self.clone(), } } + /// A divergent observation also resets the abandon-confirmation + /// streak: the account moved, so any prior "still at base" reads no + /// longer form consecutive evidence that the transaction is dead. pub fn with_incremented_divergence(&self) -> Self { match self { Self::Candidate { timestamp, retry_count, divergence_count, + abandon_requested_at, + abandon_confirm_count: _, } => Self::Candidate { timestamp: timestamp.clone(), retry_count: *retry_count, divergence_count: divergence_count + 1, + abandon_requested_at: abandon_requested_at.clone(), + abandon_confirm_count: 0, }, _ => self.clone(), } @@ -153,10 +304,14 @@ impl DeltaStatus { timestamp, retry_count, divergence_count: _, + abandon_requested_at, + abandon_confirm_count, } => Self::Candidate { timestamp: timestamp.clone(), retry_count: *retry_count, divergence_count: 0, + abandon_requested_at: abandon_requested_at.clone(), + abandon_confirm_count: *abandon_confirm_count, }, _ => self.clone(), } @@ -169,6 +324,8 @@ impl Default for DeltaStatus { timestamp: String::new(), retry_count: 0, divergence_count: 0, + abandon_requested_at: None, + abandon_confirm_count: 0, } } } @@ -285,6 +442,102 @@ impl<'de> Deserialize<'de> for DeltaObject { #[cfg(all(test, not(any(feature = "integration", feature = "e2e"))))] mod tests { + #[test] + fn candidate_json_without_abandon_fields_deserializes() { + // Wire/storage compatibility: candidates persisted before the + // abandon-intent fields existed must keep deserializing. + let json = serde_json::json!({ + "status": "candidate", + "timestamp": "2026-07-01T00:00:00Z", + "retry_count": 3, + "divergence_count": 1 + }); + let status: DeltaStatus = serde_json::from_value(json).unwrap(); + assert!(status.is_candidate()); + assert_eq!(status.retry_count(), 3); + assert_eq!(status.abandon_requested_at(), None); + assert_eq!(status.abandon_confirm_count(), 0); + } + + #[test] + fn discarded_json_without_reason_deserializes() { + let json = serde_json::json!({ + "status": "discarded", + "timestamp": "2026-07-01T00:00:00Z" + }); + let status: DeltaStatus = serde_json::from_value(json).unwrap(); + assert!(status.is_discarded()); + assert!(!status.is_client_abandoned()); + } + + #[test] + fn client_abandoned_discard_roundtrips() { + let status = DeltaStatus::discarded_client_abandoned("2026-07-01T00:00:00Z".to_string()); + let json = serde_json::to_value(&status).unwrap(); + assert_eq!(json["reason"], "client_abandoned"); + let back: DeltaStatus = serde_json::from_value(json).unwrap(); + assert!(back.is_client_abandoned()); + } + + #[test] + fn abandon_request_is_idempotent_and_preserves_counters() { + let status = DeltaStatus::candidate("2026-07-01T00:00:00Z".to_string()) + .with_incremented_retry("ignored".to_string()) + .with_abandon_requested("2026-07-01T00:01:00Z".to_string()); + assert_eq!(status.retry_count(), 1); + assert_eq!(status.abandon_requested_at(), Some("2026-07-01T00:01:00Z")); + + // A retried request must not restart the quarantine clock. + let retried = status.with_abandon_requested("2026-07-01T00:09:00Z".to_string()); + assert_eq!(retried.abandon_requested_at(), Some("2026-07-01T00:01:00Z")); + } + + #[test] + fn stored_abandon_request_is_preserved_into_stale_counter_writes() { + // A worker counter write computed before the intent existed must + // carry the stored request forward instead of wiping it. + let stale_write = DeltaStatus::candidate("2026-07-01T00:00:00Z".to_string()) + .with_incremented_divergence(); + let merged = stale_write.with_abandon_request_preserved_from(Some("2026-07-01T00:01:00Z")); + assert_eq!(merged.abandon_requested_at(), Some("2026-07-01T00:01:00Z")); + assert_eq!(merged.divergence_count(), 1, "counter write still applies"); + + // A write that already carries its own intent keeps it. + let with_own = DeltaStatus::candidate("2026-07-01T00:00:00Z".to_string()) + .with_abandon_requested("2026-07-01T00:02:00Z".to_string()) + .with_abandon_request_preserved_from(Some("2026-07-01T00:01:00Z")); + assert_eq!( + with_own.abandon_requested_at(), + Some("2026-07-01T00:02:00Z") + ); + + // Terminal statuses drop the intent by design. + let discarded = DeltaStatus::discarded_client_abandoned("2026-07-01T00:03:00Z".to_string()) + .with_abandon_request_preserved_from(Some("2026-07-01T00:01:00Z")); + assert_eq!(discarded.abandon_requested_at(), None); + } + + #[test] + fn divergent_observation_resets_abandon_confirmations() { + let status = DeltaStatus::candidate("2026-07-01T00:00:00Z".to_string()) + .with_abandon_requested("2026-07-01T00:01:00Z".to_string()) + .with_incremented_abandon_confirm() + .with_incremented_abandon_confirm(); + assert_eq!(status.abandon_confirm_count(), 2); + + let diverged = status.with_incremented_divergence(); + assert_eq!( + diverged.abandon_confirm_count(), + 0, + "a divergent read breaks the consecutive at-base streak" + ); + assert_eq!( + diverged.abandon_requested_at(), + Some("2026-07-01T00:01:00Z"), + "the intent itself persists across divergence" + ); + } + use super::*; #[test] @@ -418,6 +671,8 @@ mod tests { timestamp: "2024-01-02".to_string(), retry_count: 0, divergence_count: 0, + abandon_requested_at: None, + abandon_confirm_count: 0, }; assert!(!candidate.is_pending()); assert!(candidate.is_candidate()); @@ -434,6 +689,7 @@ mod tests { let discarded = DeltaStatus::Discarded { timestamp: "2024-01-04".to_string(), + reason: None, }; assert!(!discarded.is_pending()); assert!(!discarded.is_candidate()); diff --git a/crates/server/src/error.rs b/crates/server/src/error.rs index 1606bf02..d29fb456 100644 --- a/crates/server/src/error.rs +++ b/crates/server/src/error.rs @@ -111,6 +111,15 @@ pub enum GuardianError { AccountReleased { released_at: DateTime, }, + /// A candidate abandon was refused because the candidate's transaction + /// already landed on-chain (or the candidate already canonicalized); + /// the worker will canonicalize it shortly. Abandoning it would leave + /// guardian state behind chain. Stable code `GUARDIAN_CANDIDATE_LANDED`. + /// HTTP 409 Conflict, gRPC `FAILED_PRECONDITION`. Issue #319. + CandidateLanded { + account_id: String, + nonce: u64, + }, } /// Signing-specific error type for Miden Falcon RPO operations @@ -167,6 +176,7 @@ impl GuardianError { GuardianError::DataUnavailable(_) => StatusCode::SERVICE_UNAVAILABLE, GuardianError::AccountPaused { .. } => StatusCode::CONFLICT, GuardianError::AccountReleased { .. } => StatusCode::CONFLICT, + GuardianError::CandidateLanded { .. } => StatusCode::CONFLICT, } } @@ -213,6 +223,7 @@ impl GuardianError { GuardianError::DataUnavailable(_) => tonic::Code::Unavailable, GuardianError::AccountPaused { .. } => tonic::Code::FailedPrecondition, GuardianError::AccountReleased { .. } => tonic::Code::FailedPrecondition, + GuardianError::CandidateLanded { .. } => tonic::Code::FailedPrecondition, } } @@ -258,6 +269,7 @@ impl GuardianError { GuardianError::DataUnavailable(_) => "data_unavailable", GuardianError::AccountPaused { .. } => "GUARDIAN_ACCOUNT_PAUSED", GuardianError::AccountReleased { .. } => "GUARDIAN_ACCOUNT_RELEASED", + GuardianError::CandidateLanded { .. } => "GUARDIAN_CANDIDATE_LANDED", } } @@ -332,6 +344,9 @@ impl GuardianError { GuardianError::AccountReleased { .. } => { "This account has moved to a different guardian. Reconnect it to continue." } + GuardianError::CandidateLanded { .. } => { + "This transaction already went through, so it can't be abandoned." + } GuardianError::AccountDataUnavailable(_) => { "This account's data is temporarily unavailable. Please try again." } @@ -476,6 +491,11 @@ impl fmt::Display for GuardianError { "Account was released: it switched to a different guardian. \ Re-onboard via /configure to reactivate" ), + GuardianError::CandidateLanded { account_id, nonce } => write!( + f, + "Candidate at nonce {nonce} for account '{account_id}' already \ + landed on-chain; cannot abandon" + ), } } } @@ -1253,6 +1273,21 @@ mod tests { assert_eq!(details["meta"]["retryable"], serde_json::Value::Bool(false)); } + // -- Issue #319: CandidateLanded -- + + #[test] + fn candidate_landed_pins_http_grpc_and_code() { + let err = GuardianError::CandidateLanded { + account_id: "0xabc".into(), + nonce: 7, + }; + assert_eq!(err.http_status(), StatusCode::CONFLICT); + assert_eq!(err.grpc_status(), tonic::Code::FailedPrecondition); + assert_eq!(err.code(), "GUARDIAN_CANDIDATE_LANDED"); + assert!(err.to_string().contains("0xabc")); + assert!(err.to_string().contains("nonce 7")); + } + #[test] fn plain_error_body_has_code_message_meta_and_no_legacy_fields() { use axum::body::to_bytes; @@ -1345,6 +1380,13 @@ mod tests { paused_at, paused_reason: Some("0xSIGNER compromise".into()), }, + GuardianError::AccountReleased { + released_at: paused_at, + }, + GuardianError::CandidateLanded { + account_id: "0xDEADBEEFACCOUNT".into(), + nonce: 42, + }, ] } diff --git a/crates/server/src/jobs/canonicalization/processor.rs b/crates/server/src/jobs/canonicalization/processor.rs index 2521c110..109b006e 100644 --- a/crates/server/src/jobs/canonicalization/processor.rs +++ b/crates/server/src/jobs/canonicalization/processor.rs @@ -72,6 +72,8 @@ struct DeltasProcessorBase { max_retries: u32, submission_grace_period_seconds: u64, divergence_confirmations: u32, + abandon_quarantine_seconds: u64, + abandon_quarantine_checks: u32, max_concurrent_accounts: usize, } @@ -190,7 +192,7 @@ impl DeltasProcessorBase { } async fn process_account(&self, account_id: &str) -> Result<()> { - let _account_metadata = self + let account_metadata = self .state .metadata .get(account_id) @@ -205,6 +207,34 @@ impl DeltasProcessorBase { .await .map_err(|e| GuardianError::StorageError(format!("Failed to pull deltas: {e}")))?; + // Heal a stale pending-candidate flag: the flag can be left set + // when a best-effort flag-clear fails after the candidate delta + // was already removed. Without this, the account stays in + // `list_with_pending_candidates` and is rescanned on every tick + // forever. The conditional clear re-checks the delta store, so a + // candidate committed concurrently is never masked. + if candidates.is_empty() && account_metadata.has_pending_candidate { + tracing::warn!( + account_id = %account_id, + "Account flagged with pending candidate but no candidate delta \ + exists; clearing stale flag" + ); + let now = self.state.clock.now_rfc3339(); + if let Err(e) = self + .state + .metadata + .clear_pending_candidate_if_none(account_id, &now) + .await + { + tracing::warn!( + account_id = %account_id, + error = %e, + "Failed to clear stale has_pending_candidate flag" + ); + } + return Ok(()); + } + tracing::info!( account_id = %account_id, candidates = candidates.len(), @@ -303,6 +333,12 @@ impl DeltasProcessorBase { // may have started. Ok(StateVerification::Absent) => { let delta = self.reset_divergence_streak(delta).await?; + // An absent account is equally strong "did not land" + // evidence as an at-base read: a dead FIRST transaction + // must be abandonable too, not held for the grace window. + if delta.status.abandon_requested_at().is_some() { + return self.handle_abandon_confirmation(delta).await; + } self.handle_unverified_candidate(delta, "account not yet on chain") .await } @@ -316,12 +352,18 @@ impl DeltasProcessorBase { self.handle_diverged_candidate(delta, &on_chain).await } // On-chain still shows the candidate's base state: the - // transaction simply has not landed yet. Defer within the - // grace period, then consume retry budget, as before. This - // read also proves any earlier diverged observation was - // stale, so the divergence streak starts over. + // transaction simply has not landed yet. This read also proves + // any earlier diverged observation was stale, so the + // divergence streak starts over. A client abandon request is + // resolved on this arm — an at-base observation is exactly the + // evidence the abandon quarantine counts — and takes + // precedence over the grace/retry deferral, which would + // otherwise hold the abandon for the full grace window. Ok(StateVerification::Mismatch { on_chain }) => { let delta = self.reset_divergence_streak(delta).await?; + if delta.status.abandon_requested_at().is_some() { + return self.handle_abandon_confirmation(delta).await; + } self.handle_unverified_candidate( delta, &format!("on-chain commitment still at candidate base {on_chain}"), @@ -335,6 +377,173 @@ impl DeltasProcessorBase { } } + /// Count one at-base observation toward resolving a client abandon + /// request (issue #319), and finalize once the quarantine is + /// satisfied: enough consecutive at-base observations AND enough wall + /// time since the request for a late-landing transaction to surface. + async fn handle_abandon_confirmation(&self, delta: DeltaObject) -> Result<()> { + let requested_at = delta + .status + .abandon_requested_at() + .unwrap_or_default() + .to_string(); + let confirmations = delta.status.abandon_confirm_count() + 1; + + let now = self.state.clock.now(); + let request_age_seconds = DateTime::parse_from_rfc3339(&requested_at) + .map(|at| { + now.signed_duration_since(at.with_timezone(&Utc)) + .num_seconds() + .max(0) as u64 + }) + .unwrap_or(u64::MAX); + + if confirmations >= self.abandon_quarantine_checks + && request_age_seconds >= self.abandon_quarantine_seconds + { + return self.finalize_abandoned_candidate(delta).await; + } + + tracing::info!( + account_id = %delta.account_id, + nonce = delta.nonce, + confirmations, + abandon_quarantine_checks = self.abandon_quarantine_checks, + request_age_seconds, + abandon_quarantine_seconds = self.abandon_quarantine_seconds, + "Abandon requested and on-chain still at candidate base; \ + deferring until the quarantine is satisfied" + ); + + let new_status = delta.status.with_incremented_abandon_confirm(); + let outcome = self + .state + .storage + .update_candidate_status( + &delta.account_id, + delta.nonce, + new_status, + self.fence().as_ref(), + ) + .await + .map_err(|e| { + GuardianError::StorageError(format!("Failed to update delta status: {e}")) + })?; + match outcome { + CanonicalWrite::Applied => {} + CanonicalWrite::StaleLease => return Err(Self::stale_lease_error(&delta)), + CanonicalWrite::NotCandidate => Self::log_not_candidate(&delta, "abandon_confirm"), + } + + Ok(()) + } + + /// Resolve a confirmed abandon: finalize the matching proposal first, + /// transition the delta to `Discarded { reason: ClientAbandoned }` + /// (kept as history), then release the pending-candidate flag. Any + /// cleanup failure leaves the candidate in place for the next worker + /// run to retry. + async fn finalize_abandoned_candidate(&self, delta: DeltaObject) -> Result<()> { + let storage_backend = self.state.storage.clone(); + + let proposal_id = self.state.network_client.delta_proposal_id( + &delta.account_id, + delta.nonce, + &delta.delta_payload, + ); + match proposal_id { + Ok(id) => { + match storage_backend + .pull_delta_proposal(&delta.account_id, &id) + .await + { + Ok(_existing) => { + if let Err(e) = storage_backend + .delete_delta_proposal(&delta.account_id, &id) + .await + { + tracing::warn!( + account_id = %delta.account_id, + proposal_id = %id, + error = %e, + "Failed to delete proposal for abandoned candidate; \ + retrying on the next worker run" + ); + return Ok(()); + } + } + Err(e) if crate::storage::is_storage_not_found(&e) => {} + Err(e) => { + tracing::warn!( + account_id = %delta.account_id, + proposal_id = %id, + error = %e, + "Failed to check proposal for abandoned candidate; \ + retrying on the next worker run" + ); + return Ok(()); + } + } + } + Err(e) => { + tracing::warn!( + account_id = %delta.account_id, + nonce = delta.nonce, + error = %e, + "Could not derive proposal id for abandoned candidate; \ + retrying on the next worker run" + ); + return Ok(()); + } + } + + let now = self.state.clock.now_rfc3339(); + let outcome = self + .state + .storage + .update_candidate_status( + &delta.account_id, + delta.nonce, + DeltaStatus::discarded_client_abandoned(now.clone()), + self.fence().as_ref(), + ) + .await + .map_err(|e| { + GuardianError::StorageError(format!("Failed to discard abandoned delta: {e}")) + })?; + match outcome { + CanonicalWrite::Applied => {} + CanonicalWrite::StaleLease => return Err(Self::stale_lease_error(&delta)), + CanonicalWrite::NotCandidate => { + Self::log_not_candidate(&delta, "abandon_finalize"); + return Ok(()); + } + } + + if let Err(e) = self + .state + .metadata + .clear_pending_candidate_if_none(&delta.account_id, &now) + .await + { + tracing::warn!( + account_id = %delta.account_id, + error = %e, + "Failed to clear has_pending_candidate flag after abandon; \ + the stale-flag heal clears it on a later run" + ); + } + + record_candidate_outcome(crate::metrics::labels::CandidateOutcome::Abandoned); + tracing::info!( + account_id = %delta.account_id, + nonce = delta.nonce, + "Client-abandoned candidate discarded; account released" + ); + + Ok(()) + } + /// Reset a candidate's persisted divergence streak after a read showed /// the account still at the candidate's base: divergence must be /// observed on *consecutive* ticks, so a non-diverged observation in @@ -793,6 +1002,8 @@ impl DeltasProcessor { max_retries: config.max_retries, submission_grace_period_seconds: config.submission_grace_period_seconds, divergence_confirmations: config.divergence_confirmations, + abandon_quarantine_seconds: config.abandon_quarantine_seconds, + abandon_quarantine_checks: config.abandon_quarantine_checks, max_concurrent_accounts: config.max_concurrent_accounts, }, } @@ -823,7 +1034,9 @@ impl TestDeltasProcessor { max_retries: u32::MAX, // Test processor doesn't discard on retries submission_grace_period_seconds: 0, divergence_confirmations: u32::MAX, // ...nor on divergence - max_concurrent_accounts: 1, // ...and stays deterministic + abandon_quarantine_seconds: 0, // ...and resolves abandons immediately + abandon_quarantine_checks: 1, + max_concurrent_accounts: 1, // ...and stays deterministic }, } } @@ -1055,6 +1268,66 @@ mod tests { assert!(result.is_ok()); } + #[tokio::test] + async fn test_process_account_clears_stale_pending_flag() { + // Account is listed with a pending candidate and its metadata flag + // is set, but no candidate delta exists (e.g. a best-effort + // flag-clear failed after the delta was deleted). The worker must + // heal the stale flag so the account leaves the scan list. + let account_id = "0xtest_account"; + + // The indexed candidate read returns nothing (mock default). + let mock_storage = MockStorageBackend::new(); + let mock_network = MockNetworkClient::new(); + let mock_metadata = MockMetadataStore::new() + .with_list_with_pending_candidates(Ok(vec![account_id.to_string()])) + .with_get(Ok(Some(create_test_metadata(account_id)))); + + let state = create_test_app_state_with_mocks( + Arc::new(mock_storage), + Arc::new(mock_network), + Arc::new(mock_metadata.clone()), + ); + + let config = CanonicalizationConfig::default(); + let processor = DeltasProcessor::new(state, config); + + processor.process_all_accounts().await.unwrap(); + + let set_calls = mock_metadata.get_set_calls(); + assert_eq!(set_calls.len(), 1, "expected exactly one healing write"); + assert!(!set_calls[0].has_pending_candidate); + } + + #[tokio::test] + async fn test_process_account_no_candidates_and_clear_flag_skips_write() { + // Same as above but the metadata flag is already clear (the account + // reached the worker through a stale listing): no healing write. + let account_id = "0xtest_account"; + + let mut metadata_obj = create_test_metadata(account_id); + metadata_obj.has_pending_candidate = false; + + let mock_storage = MockStorageBackend::new().with_pull_deltas_after(Ok(vec![])); + let mock_network = MockNetworkClient::new(); + let mock_metadata = MockMetadataStore::new() + .with_list_with_pending_candidates(Ok(vec![account_id.to_string()])) + .with_get(Ok(Some(metadata_obj))); + + let state = create_test_app_state_with_mocks( + Arc::new(mock_storage), + Arc::new(mock_network), + Arc::new(mock_metadata.clone()), + ); + + let config = CanonicalizationConfig::default(); + let processor = DeltasProcessor::new(state, config); + + processor.process_all_accounts().await.unwrap(); + + assert!(mock_metadata.get_set_calls().is_empty()); + } + #[tokio::test] async fn test_process_candidate_verification_succeeds() { let account_id = "0xtest_account"; @@ -1273,6 +1546,255 @@ mod tests { assert!(metadata.get_set_calls().is_empty()); } + #[tokio::test] + async fn test_abandon_intent_confirms_and_bypasses_grace() { + // A candidate with a recorded abandon intent, observed still at + // its base INSIDE the submission grace period: the abandon + // quarantine takes precedence over the grace deferral, so a + // confirmation is persisted instead of nothing happening. + let account_id = "0xtest_account"; + let mut candidate = create_candidate_delta(account_id, 1); + candidate.status = candidate + .status + .with_abandon_requested("2024-01-01T00:00:00Z".to_string()); + + let storage = Arc::new( + MockStorageBackend::new() + .with_pull_deltas_after(Ok(vec![candidate])) + .with_pull_state(Ok(create_test_state(account_id))), + ); + + let mock_network = MockNetworkClient::new() + .with_apply_delta(Ok(( + serde_json::json!({"new": "state"}), + "new_commitment".to_string(), + ))) + .with_verify_commitment(Ok(StateVerification::Mismatch { + on_chain: "prev_commitment".to_string(), + })); + + let mock_metadata = MockMetadataStore::new() + .with_list_with_pending_candidates(Ok(vec![account_id.to_string()])) + .with_get(Ok(Some(create_test_metadata(account_id)))); + let metadata = Arc::new(mock_metadata); + + // 5s after the request: age is far under the 600s grace AND under + // the quarantine, and only one at-base observation exists — defer, + // but persist the confirmation. + let clock = Arc::new(MockClock::new( + Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 5).unwrap(), + )); + let state = create_test_app_state_with_clock( + storage.clone(), + Arc::new(mock_network), + metadata.clone(), + clock, + ); + + let config = CanonicalizationConfig::new(10, 18) + .with_submission_grace_period_seconds(600) + .with_abandon_quarantine_seconds(30) + .with_abandon_quarantine_checks(2); + let processor = DeltasProcessor::new(state, config); + + processor.process_all_accounts().await.unwrap(); + + let status_writes = storage.get_update_delta_status_calls(); + assert_eq!( + status_writes.len(), + 1, + "the abandon confirmation must be persisted despite the grace period" + ); + let (_, _, written) = &status_writes[0]; + assert!(written.is_candidate(), "status must remain candidate"); + assert_eq!(written.abandon_confirm_count(), 1); + assert_eq!(written.abandon_requested_at(), Some("2024-01-01T00:00:00Z")); + assert!(storage.get_delete_delta_calls().is_empty()); + } + + #[tokio::test] + async fn test_abandon_finalizes_after_quarantine() { + // Quarantine satisfied: enough consecutive at-base confirmations + // and enough wall time since the request. The delta transitions to + // Discarded { reason: ClientAbandoned } — preserved as history — + // and the pending-candidate flag is released. + let account_id = "0xtest_account"; + let mut candidate = create_candidate_delta(account_id, 1); + candidate.status = candidate + .status + .with_abandon_requested("2024-01-01T00:00:00Z".to_string()) + .with_incremented_abandon_confirm(); + + let storage = Arc::new( + MockStorageBackend::new() + .with_pull_deltas_after(Ok(vec![candidate])) + .with_pull_state(Ok(create_test_state(account_id))), + ); + + let mock_network = MockNetworkClient::new() + .with_apply_delta(Ok(( + serde_json::json!({"new": "state"}), + "new_commitment".to_string(), + ))) + .with_verify_commitment(Ok(StateVerification::Mismatch { + on_chain: "prev_commitment".to_string(), + })); + + let mock_metadata = MockMetadataStore::new() + .with_list_with_pending_candidates(Ok(vec![account_id.to_string()])) + .with_get(Ok(Some(create_test_metadata(account_id)))); + let metadata = Arc::new(mock_metadata); + + // 60s after the request: second at-base observation, age >= 30s. + let clock = Arc::new(MockClock::new( + Utc.with_ymd_and_hms(2024, 1, 1, 0, 1, 0).unwrap(), + )); + let state = create_test_app_state_with_clock( + storage.clone(), + Arc::new(mock_network), + metadata.clone(), + clock, + ); + + let config = CanonicalizationConfig::new(10, 18) + .with_submission_grace_period_seconds(600) + .with_abandon_quarantine_seconds(30) + .with_abandon_quarantine_checks(2); + let processor = DeltasProcessor::new(state, config); + + processor.process_all_accounts().await.unwrap(); + + let status_writes = storage.get_update_delta_status_calls(); + assert_eq!(status_writes.len(), 1, "exactly the discard transition"); + let (_, _, written) = &status_writes[0]; + assert!( + written.is_client_abandoned(), + "delta must be discarded as client-abandoned, got {written:?}" + ); + // History preserved, not deleted. + assert!(storage.get_delete_delta_calls().is_empty()); + // The pending-candidate flag was released. + let set_calls = metadata.get_set_calls(); + assert!( + set_calls.iter().any(|m| !m.has_pending_candidate), + "flag must be cleared after the abandon finalizes" + ); + } + + #[tokio::test] + async fn test_abandon_quarantine_waits_for_request_age() { + // Confirmation count is satisfied but the request is too fresh: + // the quarantine keeps waiting so a late-landing transaction can + // still surface. + let account_id = "0xtest_account"; + let mut candidate = create_candidate_delta(account_id, 1); + candidate.status = candidate + .status + .with_abandon_requested("2024-01-01T00:00:00Z".to_string()) + .with_incremented_abandon_confirm(); + + let storage = Arc::new( + MockStorageBackend::new() + .with_pull_deltas_after(Ok(vec![candidate])) + .with_pull_state(Ok(create_test_state(account_id))), + ); + + let mock_network = MockNetworkClient::new() + .with_apply_delta(Ok(( + serde_json::json!({"new": "state"}), + "new_commitment".to_string(), + ))) + .with_verify_commitment(Ok(StateVerification::Mismatch { + on_chain: "prev_commitment".to_string(), + })); + + let mock_metadata = MockMetadataStore::new() + .with_list_with_pending_candidates(Ok(vec![account_id.to_string()])) + .with_get(Ok(Some(create_test_metadata(account_id)))); + let metadata = Arc::new(mock_metadata); + + // Only 10s after the request: under the 30s quarantine. + let clock = Arc::new(MockClock::new( + Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 10).unwrap(), + )); + let state = create_test_app_state_with_clock( + storage.clone(), + Arc::new(mock_network), + metadata.clone(), + clock, + ); + + let config = CanonicalizationConfig::new(10, 18) + .with_abandon_quarantine_seconds(30) + .with_abandon_quarantine_checks(2); + let processor = DeltasProcessor::new(state, config); + + processor.process_all_accounts().await.unwrap(); + + let status_writes = storage.get_update_delta_status_calls(); + assert_eq!(status_writes.len(), 1); + let (_, _, written) = &status_writes[0]; + assert!(written.is_candidate(), "must still be a candidate"); + assert_eq!(written.abandon_confirm_count(), 2); + } + + #[tokio::test] + async fn test_landed_candidate_canonicalizes_despite_abandon_intent() { + // The transaction landed while the abandon was pending: the landed + // outcome wins — the delta canonicalizes exactly as without intent. + let account_id = "0xtest_account"; + let mut candidate = create_candidate_delta(account_id, 1); + candidate.status = candidate + .status + .with_abandon_requested("2024-01-01T00:00:00Z".to_string()); + + let storage = Arc::new( + MockStorageBackend::new() + .with_pull_deltas_after(Ok(vec![candidate])) + .with_pull_state(Ok(create_test_state(account_id))) + .with_pull_state(Ok(create_test_state(account_id))) + .with_pull_state(Ok(create_test_state(account_id))) + .with_submit_state(Ok(())) + .with_submit_delta(Ok(())), + ); + + let mock_network = MockNetworkClient::new() + .with_apply_delta(Ok(( + serde_json::json!({"new": "state"}), + "new_commitment".to_string(), + ))) + .with_verify_commitment(Ok(StateVerification::Match)) + .with_should_update_auth(Ok(None)); + + let mock_metadata = MockMetadataStore::new() + .with_list_with_pending_candidates(Ok(vec![account_id.to_string()])) + .with_get(Ok(Some(create_test_metadata(account_id)))) + .with_get(Ok(Some(create_test_metadata(account_id)))) + .with_set(Ok(())); + let metadata = Arc::new(mock_metadata); + + let clock = Arc::new(MockClock::new( + Utc.with_ymd_and_hms(2024, 1, 1, 0, 1, 0).unwrap(), + )); + let state = create_test_app_state_with_clock( + storage.clone(), + Arc::new(mock_network), + metadata.clone(), + clock, + ); + + let config = CanonicalizationConfig::new(10, 18); + let processor = DeltasProcessor::new(state, config); + + processor.process_all_accounts().await.unwrap(); + + let submitted = storage.get_submit_delta_calls(); + assert!( + submitted.iter().any(|d| d.status.is_canonical()), + "the landed delta must canonicalize despite the abandon intent" + ); + } + #[tokio::test] async fn test_diverged_candidate_first_observation_defers() { // On-chain matches neither the candidate's base nor its expected new @@ -1338,6 +1860,8 @@ mod tests { timestamp: "2024-01-01T00:00:00Z".to_string(), retry_count: 0, divergence_count: 1, + abandon_requested_at: None, + abandon_confirm_count: 0, }; let storage = Arc::new( @@ -1513,6 +2037,8 @@ mod tests { timestamp: "2024-01-01T00:00:00Z".to_string(), retry_count: 0, divergence_count: 1, + abandon_requested_at: None, + abandon_confirm_count: 0, }; // Response queues are LIFO (`Vec::pop`), so tick 3's responses are diff --git a/crates/server/src/metrics/labels.rs b/crates/server/src/metrics/labels.rs index d1df0092..7a1e4e96 100644 --- a/crates/server/src/metrics/labels.rs +++ b/crates/server/src/metrics/labels.rs @@ -111,6 +111,10 @@ pub enum CandidateOutcome { /// Discarded because the account advanced past the candidate's base /// state on-chain, making verification permanently unsatisfiable. Diverged, + /// Discarded because the client abandoned it via the abandon-candidate + /// endpoint (issue #319): the client knows its transaction will never + /// land and releases the account instead of waiting out grace+retries. + Abandoned, /// Promotion rolled back because the stored state moved off the /// candidate's base commitment during the pass; the candidate is /// re-verified against the new base next tick. @@ -126,6 +130,7 @@ impl CandidateOutcome { Self::GraceDeferred => "grace_deferred", Self::DivergenceDeferred => "divergence_deferred", Self::Diverged => "diverged", + Self::Abandoned => "abandoned", Self::StaleBase => "stale_base", } } @@ -192,6 +197,7 @@ mod tests { CandidateOutcome::GraceDeferred.as_str(), CandidateOutcome::DivergenceDeferred.as_str(), CandidateOutcome::Diverged.as_str(), + CandidateOutcome::Abandoned.as_str(), CandidateOutcome::StaleBase.as_str(), AccountKind::Miden.as_str(), PoolKind::Storage.as_str(), diff --git a/crates/server/src/metrics/names.rs b/crates/server/src/metrics/names.rs index 2750e2f8..90b05358 100644 --- a/crates/server/src/metrics/names.rs +++ b/crates/server/src/metrics/names.rs @@ -412,6 +412,7 @@ const KNOWN_GRPC_METHODS: &[(&str, &str)] = &[ ("guardian.Guardian", "GetDeltaProposals"), ("guardian.Guardian", "GetDeltaProposal"), ("guardian.Guardian", "SignDeltaProposal"), + ("guardian.Guardian", "AbandonDeltaCandidate"), ("guardian.Guardian", "GetAccountByKeyCommitment"), // Served alongside Guardian via tonic-reflection (v1 and v1alpha). ( diff --git a/crates/server/src/metrics/storage.rs b/crates/server/src/metrics/storage.rs index cb6ccb29..d53c1e6c 100644 --- a/crates/server/src/metrics/storage.rs +++ b/crates/server/src/metrics/storage.rs @@ -26,9 +26,10 @@ use super::names::{ use crate::delta_object::{DeltaObject, DeltaStatus}; use crate::state_object::StateObject; use crate::storage::{ - AccountDeltaCursor, AccountProposalCursor, CandidatePromotion, CandidateSubmission, - CanonicalWrite, DeltaStatusCounts, DeltaStatusKind, GlobalDeltaCursor, GlobalDeltaRow, - GlobalProposalCursor, LeaseFence, PromoteWrite, ProposalRecord, StorageBackend, StorageType, + AbandonIntent, AccountDeltaCursor, AccountProposalCursor, CandidatePromotion, + CandidateSubmission, CanonicalWrite, DeltaStatusCounts, DeltaStatusKind, GlobalDeltaCursor, + GlobalDeltaRow, GlobalProposalCursor, LeaseFence, PromoteWrite, ProposalRecord, StorageBackend, + StorageType, }; /// Record one storage operation: duration histogram plus an @@ -212,6 +213,19 @@ impl StorageBackend for InstrumentedStorage { timed("delete_delta", self.inner.delete_delta(account_id, nonce)).await } + async fn request_candidate_abandon( + &self, + account_id: &str, + nonce: u64, + now: &str, + ) -> Result { + timed( + "request_candidate_abandon", + self.inner.request_candidate_abandon(account_id, nonce, now), + ) + .await + } + async fn submit_candidate( &self, metadata: &dyn crate::metadata::MetadataStore, diff --git a/crates/server/src/openapi.rs b/crates/server/src/openapi.rs index 636805e1..a54b2060 100644 --- a/crates/server/src/openapi.rs +++ b/crates/server/src/openapi.rs @@ -198,6 +198,7 @@ impl Modify for CommonResponsesAddon { crate::api::http::get_delta_proposals, crate::api::http::get_delta_proposal, crate::api::http::sign_delta_proposal, + crate::api::http::abandon_candidate, ), components(schemas(ApiErrorResponse, ApiErrorMeta, crate::services::StatusResponse)), modifiers(&ClientSecurityAddon, &CommonResponsesAddon), diff --git a/crates/server/src/services/abandon_candidate.rs b/crates/server/src/services/abandon_candidate.rs new file mode 100644 index 00000000..4299f7ce --- /dev/null +++ b/crates/server/src/services/abandon_candidate.rs @@ -0,0 +1,577 @@ +use crate::builder::state::AppState; +use crate::delta_object::DeltaStatus; +use crate::error::{GuardianError, Result}; +use crate::metadata::auth::Credentials; +use crate::network::StateVerification; +use crate::services::account_status::ensure_account_active_metadata; +use crate::services::resolve_account; +use crate::storage::AbandonIntent; +use tracing::info; + +#[derive(Debug, Clone)] +pub struct AbandonCandidateParams { + pub account_id: String, + pub nonce: u64, + pub credentials: Credentials, +} + +/// Where the abandon stands from the client's point of view. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AbandonState { + /// The intent is recorded; the delta remains a candidate (the account + /// stays locked) until the canonicalization worker resolves it after + /// the abandon quarantine. + Pending, + /// The worker already resolved the abandon: the delta is + /// `Discarded { reason: ClientAbandoned }` and the account released. + Abandoned, +} + +impl AbandonState { + pub fn as_str(self) -> &'static str { + match self { + Self::Pending => "pending", + Self::Abandoned => "abandoned", + } + } +} + +#[derive(Debug, Clone)] +pub struct AbandonCandidateResult { + pub account_id: String, + pub nonce: u64, + pub state: AbandonState, + /// RFC 3339 UTC timestamp of the recorded abandon request. Retries + /// return the original timestamp — the quarantine never restarts. + /// `None` once the abandon is already resolved. + pub abandon_requested_at: Option, +} + +/// Read the delta at `nonce` while distinguishing "absent" from "backend +/// failure": a storage outage must surface as a 5xx `StorageError`, never +/// as a spurious 404. +async fn pull_delta_at_nonce( + storage: &std::sync::Arc, + account_id: &str, + nonce: u64, +) -> Result { + storage.pull_delta(account_id, nonce).await.map_err(|e| { + if crate::storage::is_storage_not_found(&e) { + GuardianError::DeltaNotFound { + account_id: account_id.to_string(), + nonce, + } + } else { + GuardianError::StorageError(format!("Failed to read delta: {e}")) + } + }) +} + +/// Client-initiated abandon of a pending canonicalization candidate +/// (issue #319), as an intent: the request records +/// `abandon_requested_at` on the candidate and returns immediately; the +/// canonicalization worker — the sole owner of candidate lifecycles under +/// the lease model — resolves the intent after the abandon quarantine +/// (consecutive at-base observations plus a minimum age), transitioning +/// the delta to `Discarded { reason: ClientAbandoned }` and releasing the +/// account. +/// +/// A candidate whose transaction died client-side after approval (RPC +/// submit failure, prover timeout, crash) looks identical to one that is +/// slowly proving, so without this the account stays locked for the full +/// submission grace period plus retry budget. Only the client knows its +/// transaction will never land. +/// +/// The request is refused with [`GuardianError::CandidateLanded`] when +/// the on-chain state already matches the candidate's expected state. An +/// on-chain read failure does NOT block the request — recording intent is +/// non-destructive, and the worker independently re-verifies against +/// chain before finalizing. Retries are idempotent: the original request +/// timestamp is preserved so the quarantine never restarts. +pub async fn abandon_candidate( + state: &AppState, + params: AbandonCandidateParams, +) -> Result { + let AbandonCandidateParams { + account_id, + nonce, + credentials, + } = params; + + let resolved = resolve_account(state, &account_id, &credentials).await?; + ensure_account_active_metadata(&resolved.metadata)?; + + let delta = pull_delta_at_nonce(&resolved.storage, &account_id, nonce).await?; + + match &delta.status { + DeltaStatus::Candidate { .. } => {} + DeltaStatus::Canonical { .. } => { + return Err(GuardianError::CandidateLanded { account_id, nonce }); + } + status if status.is_client_abandoned() => { + // Already resolved: a retried abandon reports success. + return Ok(AbandonCandidateResult { + account_id, + nonce, + state: AbandonState::Abandoned, + abandon_requested_at: None, + }); + } + _ => { + return Err(GuardianError::DeltaNotFound { account_id, nonce }); + } + } + + // Landed guard, best-effort: refuse when the transaction demonstrably + // landed (the worker will canonicalize it shortly; abandoning would + // put guardian behind chain). An RPC failure only skips the guard — + // the intent write is non-destructive and the worker re-verifies + // against chain before finalizing. + let current_state = resolved + .storage + .pull_state(&account_id) + .await + .map_err(|_| GuardianError::StateNotFound(account_id.clone()))?; + + let (_, expected_commitment) = { + let client = state.network_client.clone(); + let prev_state_json = current_state.state_json; + let delta_payload = std::sync::Arc::new(delta.delta_payload.clone()); + crate::network::reconstructor() + .run_background(move || client.apply_delta(&prev_state_json, &delta_payload)) + .await? + }; + + let verify_result = state + .network_client + .verify_commitment(&account_id, &expected_commitment) + .await; + match verify_result { + Ok(StateVerification::Match) => { + return Err(GuardianError::CandidateLanded { account_id, nonce }); + } + // Mismatch: on-chain differs from the candidate's expected state + // (tx not landed, or the account diverged). Absent: the account + // has no on-chain state at all, so the tx certainly did not land. + // Abandoning is safe in both cases. + Ok(StateVerification::Mismatch { .. }) | Ok(StateVerification::Absent) => {} + Err(e) => { + tracing::warn!( + account_id = %account_id, + nonce, + error = %e, + "Could not verify on-chain state before abandon; recording \ + intent anyway (worker re-verifies before finalizing)" + ); + } + } + + let now = state.clock.now_rfc3339(); + let intent = resolved + .storage + .request_candidate_abandon(&account_id, nonce, &now) + .await + .map_err(|e| { + GuardianError::StorageError(format!("Failed to record abandon request: {e}")) + })?; + + match intent { + AbandonIntent::Recorded => { + info!( + account_id = %account_id, + nonce, + "Abandon requested; worker resolves after the quarantine" + ); + Ok(AbandonCandidateResult { + account_id, + nonce, + state: AbandonState::Pending, + abandon_requested_at: Some(now), + }) + } + AbandonIntent::AlreadyRequested { requested_at } => Ok(AbandonCandidateResult { + account_id, + nonce, + state: AbandonState::Pending, + abandon_requested_at: Some(requested_at), + }), + AbandonIntent::NotCandidate => { + // Resolved between our read and the write: classify precisely. + let delta = pull_delta_at_nonce(&resolved.storage, &account_id, nonce).await?; + if delta.status.is_client_abandoned() { + Ok(AbandonCandidateResult { + account_id, + nonce, + state: AbandonState::Abandoned, + abandon_requested_at: None, + }) + } else { + Err(GuardianError::CandidateLanded { account_id, nonce }) + } + } + } +} + +#[cfg(all(test, not(any(feature = "integration", feature = "e2e"))))] +mod tests { + use super::*; + use crate::delta_object::DeltaObject; + use crate::metadata::AccountMetadata; + use crate::metadata::auth::Auth; + use crate::state_object::StateObject; + use crate::testing::fixtures; + use crate::testing::helpers::create_test_app_state_with_mocks; + use crate::testing::mocks::{MockMetadataStore, MockNetworkClient, MockStorageBackend}; + use std::sync::Arc; + + fn create_account_metadata(account_id: String, auth: Auth) -> AccountMetadata { + AccountMetadata { + account_id, + auth, + network_config: crate::metadata::NetworkConfig::miden_default(), + created_at: "2024-11-14T12:00:00Z".to_string(), + updated_at: "2024-11-14T12:00:00Z".to_string(), + has_pending_candidate: true, + last_auth_timestamp: None, + paused_at: None, + paused_reason: None, + released_at: None, + } + } + + fn create_state_object(account_id: String, commitment: String) -> StateObject { + let account_json: serde_json::Value = serde_json::from_str(fixtures::ACCOUNT_JSON).unwrap(); + StateObject { + account_id, + commitment, + state_json: account_json, + created_at: "2024-11-14T12:00:00Z".to_string(), + updated_at: "2024-11-14T12:00:00Z".to_string(), + auth_scheme: String::new(), + } + } + + fn create_candidate_delta(account_id: &str, nonce: u64) -> DeltaObject { + let delta_fixture: serde_json::Value = + serde_json::from_str(fixtures::DELTA_1_JSON).unwrap(); + DeltaObject { + account_id: account_id.to_string(), + nonce, + prev_commitment: "0x123".to_string(), + new_commitment: Some("0x456".to_string()), + delta_payload: delta_fixture["delta_payload"].clone(), + ack_sig: String::new(), + ack_pubkey: String::new(), + ack_scheme: String::new(), + status: DeltaStatus::candidate("2024-11-14T12:00:00Z".to_string()), + metadata: None, + } + } + + struct TestSetup { + state: AppState, + storage: MockStorageBackend, + params: AbandonCandidateParams, + } + + /// Common scaffolding: authenticated account with a candidate delta at + /// nonce 1. Individual tests override mock responses before calling + /// the service. + fn setup(network: MockNetworkClient) -> TestSetup { + let storage = MockStorageBackend::new(); + let metadata = MockMetadataStore::new(); + let state = create_test_app_state_with_mocks( + Arc::new(storage.clone()), + Arc::new(network.clone()), + Arc::new(metadata.clone()), + ); + + let delta_fixture: serde_json::Value = + serde_json::from_str(fixtures::DELTA_1_JSON).unwrap(); + let account_id = delta_fixture["account_id"].as_str().unwrap().to_string(); + + let (test_pubkey, test_commitment_hex, test_signature, test_timestamp) = + crate::testing::helpers::generate_falcon_signature(&account_id); + + let _metadata = metadata.clone().with_get(Ok(Some(create_account_metadata( + account_id.clone(), + Auth::MidenFalconRpo { + cosigner_commitments: vec![test_commitment_hex], + }, + )))); + + let _storage = storage + .clone() + .with_pull_state(Ok(create_state_object( + account_id.clone(), + "0x123".to_string(), + ))) + .with_pull_delta(Ok(create_candidate_delta(&account_id, 1))); + + let _network = + network.with_apply_delta(Ok((serde_json::json!({"new": true}), "0x456".to_string()))); + + let params = AbandonCandidateParams { + account_id, + nonce: 1, + credentials: Credentials::signature(test_pubkey, test_signature, test_timestamp), + }; + + TestSetup { + state, + storage, + params, + } + } + + #[tokio::test] + async fn test_abandon_records_intent_and_deletes_nothing() { + let network = + MockNetworkClient::new().with_verify_commitment(Ok(StateVerification::Mismatch { + on_chain: "0x123".to_string(), + })); + let t = setup(network); + + let result = abandon_candidate(&t.state, t.params.clone()).await; + assert!(result.is_ok(), "Expected success, got: {:?}", result); + let result = result.unwrap(); + assert_eq!(result.account_id, t.params.account_id); + assert_eq!(result.nonce, 1); + assert_eq!(result.state, AbandonState::Pending); + assert!(result.abandon_requested_at.is_some()); + + // Intent only: the endpoint never deletes or discards anything — + // resolution belongs to the canonicalization worker. + assert!(t.storage.get_delete_delta_calls().is_empty()); + assert!(t.storage.get_delete_delta_proposal_calls().is_empty()); + assert!(t.storage.get_update_delta_status_calls().is_empty()); + } + + #[tokio::test] + async fn test_abandon_retry_preserves_original_request_timestamp() { + let network = + MockNetworkClient::new().with_verify_commitment(Ok(StateVerification::Mismatch { + on_chain: "0x123".to_string(), + })); + let t = setup(network); + let _ = t.storage.clone().with_request_candidate_abandon(Ok( + crate::storage::AbandonIntent::AlreadyRequested { + requested_at: "2024-11-14T12:01:00Z".to_string(), + }, + )); + + let result = abandon_candidate(&t.state, t.params).await.unwrap(); + assert_eq!(result.state, AbandonState::Pending); + assert_eq!( + result.abandon_requested_at.as_deref(), + Some("2024-11-14T12:01:00Z"), + "retries must return the original request timestamp" + ); + } + + #[tokio::test] + async fn test_abandon_refused_when_candidate_landed_on_chain() { + let network = MockNetworkClient::new().with_verify_commitment(Ok(StateVerification::Match)); + let t = setup(network); + + let result = abandon_candidate(&t.state, t.params).await; + match result.unwrap_err() { + GuardianError::CandidateLanded { nonce, .. } => assert_eq!(nonce, 1), + e => panic!("Expected CandidateLanded, got: {:?}", e), + } + } + + #[tokio::test] + async fn test_abandon_records_intent_despite_rpc_failure() { + // The intent write is non-destructive and the worker re-verifies + // against chain before finalizing, so an RPC failure must not + // block the request. + let network = + MockNetworkClient::new().with_verify_commitment(Err("rpc timeout".to_string())); + let t = setup(network); + + let result = abandon_candidate(&t.state, t.params).await; + assert!(result.is_ok(), "Expected success, got: {:?}", result); + assert_eq!(result.unwrap().state, AbandonState::Pending); + } + + #[tokio::test] + async fn test_abandon_missing_delta_maps_to_delta_not_found() { + // No pull_delta response is canned, so the mock returns its + // "delta not found" default. + let network = MockNetworkClient::new(); + let storage = MockStorageBackend::new(); + let metadata = MockMetadataStore::new(); + let state = create_test_app_state_with_mocks( + Arc::new(storage.clone()), + Arc::new(network.clone()), + Arc::new(metadata.clone()), + ); + + let delta_fixture: serde_json::Value = + serde_json::from_str(fixtures::DELTA_1_JSON).unwrap(); + let account_id = delta_fixture["account_id"].as_str().unwrap().to_string(); + let (pubkey, commitment, signature, timestamp) = + crate::testing::helpers::generate_falcon_signature(&account_id); + let _ = metadata.with_get(Ok(Some(create_account_metadata( + account_id.clone(), + Auth::MidenFalconRpo { + cosigner_commitments: vec![commitment], + }, + )))); + + let params = AbandonCandidateParams { + account_id, + nonce: 9, + credentials: Credentials::signature(pubkey, signature, timestamp), + }; + let result = abandon_candidate(&state, params).await; + match result.unwrap_err() { + GuardianError::DeltaNotFound { nonce, .. } => assert_eq!(nonce, 9), + e => panic!("Expected DeltaNotFound, got: {:?}", e), + } + } + + #[tokio::test] + async fn test_abandon_storage_read_failure_is_not_delta_not_found() { + // A backend outage must surface as a 5xx StorageError, never as + // DeltaNotFound. + let network = MockNetworkClient::new(); + let t = setup(network); + let _ = t + .storage + .clone() + .with_pull_delta(Err("connection refused".to_string())); + + let result = abandon_candidate(&t.state, t.params).await; + match result.unwrap_err() { + GuardianError::StorageError(msg) => assert!(msg.contains("connection refused")), + e => panic!("Expected StorageError, got: {:?}", e), + } + } + + #[tokio::test] + async fn test_abandon_already_canonical_maps_to_candidate_landed() { + let network = MockNetworkClient::new(); + let t = setup(network); + let mut canonical = create_candidate_delta(&t.params.account_id, 1); + canonical.status = DeltaStatus::canonical("2024-11-14T12:05:00Z".to_string()); + let _ = t.storage.clone().with_pull_delta(Ok(canonical)); + + let result = abandon_candidate(&t.state, t.params).await; + match result.unwrap_err() { + GuardianError::CandidateLanded { .. } => {} + e => panic!("Expected CandidateLanded, got: {:?}", e), + } + } + + #[tokio::test] + async fn test_abandon_already_resolved_reports_abandoned() { + let network = MockNetworkClient::new(); + let t = setup(network); + let mut resolved = create_candidate_delta(&t.params.account_id, 1); + resolved.status = + DeltaStatus::discarded_client_abandoned("2024-11-14T12:05:00Z".to_string()); + let _ = t.storage.clone().with_pull_delta(Ok(resolved)); + + let result = abandon_candidate(&t.state, t.params).await.unwrap(); + assert_eq!(result.state, AbandonState::Abandoned); + assert!(result.abandon_requested_at.is_none()); + } + + #[tokio::test] + async fn test_abandon_paused_account_rejected() { + let network = MockNetworkClient::new(); + let storage = MockStorageBackend::new(); + let metadata = MockMetadataStore::new(); + let state = create_test_app_state_with_mocks( + Arc::new(storage.clone()), + Arc::new(network.clone()), + Arc::new(metadata.clone()), + ); + + let delta_fixture: serde_json::Value = + serde_json::from_str(fixtures::DELTA_1_JSON).unwrap(); + let account_id = delta_fixture["account_id"].as_str().unwrap().to_string(); + let (pubkey, commitment, signature, timestamp) = + crate::testing::helpers::generate_falcon_signature(&account_id); + + let mut account_metadata = create_account_metadata( + account_id.clone(), + Auth::MidenFalconRpo { + cosigner_commitments: vec![commitment], + }, + ); + account_metadata.paused_at = Some( + chrono::DateTime::parse_from_rfc3339("2026-05-19T14:23:00Z") + .unwrap() + .with_timezone(&chrono::Utc), + ); + let _ = metadata.with_get(Ok(Some(account_metadata))); + + let params = AbandonCandidateParams { + account_id, + nonce: 1, + credentials: Credentials::signature(pubkey, signature, timestamp), + }; + let result = abandon_candidate(&state, params).await; + match result.unwrap_err() { + GuardianError::AccountPaused { .. } => {} + e => panic!("Expected AccountPaused, got: {:?}", e), + } + } + + #[tokio::test] + async fn test_abandon_race_resolved_as_abandoned_between_read_and_write() { + // The worker resolves the candidate between the service's read and + // the intent write: classify via re-read. + let network = + MockNetworkClient::new().with_verify_commitment(Ok(StateVerification::Mismatch { + on_chain: "0x123".to_string(), + })); + let t = setup(network); + let mut resolved = create_candidate_delta(&t.params.account_id, 1); + resolved.status = + DeltaStatus::discarded_client_abandoned("2024-11-14T12:05:00Z".to_string()); + // LIFO: the initial read pops the candidate canned by setup() only + // after this re-read response, so push the re-read FIRST. + let _ = t + .storage + .clone() + .with_request_candidate_abandon(Ok(crate::storage::AbandonIntent::NotCandidate)); + // Re-read response must be popped AFTER setup's candidate: push + // order is [setup candidate, resolved] -> pops resolved first. + // Re-can both reads explicitly to control order. + let _ = t + .storage + .clone() + .with_pull_delta(Ok(resolved)) + .with_pull_delta(Ok(create_candidate_delta(&t.params.account_id, 1))); + + let result = abandon_candidate(&t.state, t.params).await.unwrap(); + assert_eq!(result.state, AbandonState::Abandoned); + } + + #[tokio::test] + async fn test_abandon_race_canonicalized_between_read_and_write() { + let network = + MockNetworkClient::new().with_verify_commitment(Ok(StateVerification::Mismatch { + on_chain: "0x123".to_string(), + })); + let t = setup(network); + let mut canonical = create_candidate_delta(&t.params.account_id, 1); + canonical.status = DeltaStatus::canonical("2024-11-14T12:05:00Z".to_string()); + let _ = t + .storage + .clone() + .with_request_candidate_abandon(Ok(crate::storage::AbandonIntent::NotCandidate)) + .with_pull_delta(Ok(canonical)) + .with_pull_delta(Ok(create_candidate_delta(&t.params.account_id, 1))); + + let result = abandon_candidate(&t.state, t.params).await; + match result.unwrap_err() { + GuardianError::CandidateLanded { .. } => {} + e => panic!("Expected CandidateLanded, got: {:?}", e), + } + } +} diff --git a/crates/server/src/services/dashboard_account_deltas.rs b/crates/server/src/services/dashboard_account_deltas.rs index 4d779420..89b9489a 100644 --- a/crates/server/src/services/dashboard_account_deltas.rs +++ b/crates/server/src/services/dashboard_account_deltas.rs @@ -94,7 +94,7 @@ pub(crate) fn decode_delta_status( DeltaStatus::Canonical { timestamp } => { Some((DashboardDeltaStatus::Canonical, None, timestamp.clone())) } - DeltaStatus::Discarded { timestamp } => { + DeltaStatus::Discarded { timestamp, .. } => { Some((DashboardDeltaStatus::Discarded, None, timestamp.clone())) } } @@ -246,6 +246,8 @@ mod tests { timestamp: format!("2026-05-08T12:0{nonce}:00Z"), retry_count: retries, divergence_count: 0, + abandon_requested_at: None, + abandon_confirm_count: 0, }, ) } @@ -265,6 +267,7 @@ mod tests { nonce, DeltaStatus::Discarded { timestamp: format!("2026-05-08T12:0{nonce}:00Z"), + reason: None, }, ) } diff --git a/crates/server/src/services/dashboard_info.rs b/crates/server/src/services/dashboard_info.rs index 8bffbbb5..c05100b6 100644 --- a/crates/server/src/services/dashboard_info.rs +++ b/crates/server/src/services/dashboard_info.rs @@ -614,6 +614,8 @@ mod tests { ) .await; state.canonicalization = Some(crate::canonicalization::CanonicalizationConfig { + abandon_quarantine_seconds: 15, + abandon_quarantine_checks: 2, check_interval_seconds: 7, max_retries: 13, submission_grace_period_seconds: 42, diff --git a/crates/server/src/services/mod.rs b/crates/server/src/services/mod.rs index 4baca9fc..c9c7f25e 100644 --- a/crates/server/src/services/mod.rs +++ b/crates/server/src/services/mod.rs @@ -7,6 +7,7 @@ use base64::Engine; use serde_json::Value; use std::sync::Arc; +mod abandon_candidate; pub mod account_status; mod configure_account; mod dashboard_account_delta_detail; @@ -33,6 +34,10 @@ mod sign_delta_proposal; mod status; pub mod unpause_account; +pub use abandon_candidate::{ + AbandonCandidateParams, AbandonCandidateResult, AbandonState, abandon_candidate, +}; + pub use crate::jobs::canonicalization::{ process_canonicalizations_now, start_canonicalization_worker, }; diff --git a/crates/server/src/services/push_delta_proposal.rs b/crates/server/src/services/push_delta_proposal.rs index 92d73553..d2f0a21a 100644 --- a/crates/server/src/services/push_delta_proposal.rs +++ b/crates/server/src/services/push_delta_proposal.rs @@ -826,6 +826,8 @@ mod tests { timestamp: "2024-11-14T12:00:00Z".to_string(), retry_count: 0, divergence_count: 0, + abandon_requested_at: None, + abandon_confirm_count: 0, }, metadata: None, }; diff --git a/crates/server/src/storage/encryption/decorator.rs b/crates/server/src/storage/encryption/decorator.rs index cc0fe228..6a6deaa7 100644 --- a/crates/server/src/storage/encryption/decorator.rs +++ b/crates/server/src/storage/encryption/decorator.rs @@ -9,9 +9,10 @@ use super::envelope::RecordAad; use crate::delta_object::{DeltaObject, DeltaStatus}; use crate::state_object::StateObject; use crate::storage::{ - AccountDeltaCursor, AccountProposalCursor, CandidatePromotion, CandidateSubmission, - CanonicalWrite, DeltaStatusCounts, DeltaStatusKind, GlobalDeltaCursor, GlobalDeltaRow, - GlobalProposalCursor, LeaseFence, PromoteWrite, ProposalRecord, StorageBackend, StorageType, + AbandonIntent, AccountDeltaCursor, AccountProposalCursor, CandidatePromotion, + CandidateSubmission, CanonicalWrite, DeltaStatusCounts, DeltaStatusKind, GlobalDeltaCursor, + GlobalDeltaRow, GlobalProposalCursor, LeaseFence, PromoteWrite, ProposalRecord, StorageBackend, + StorageType, }; use crate::utils::normalize_commitment_hex; @@ -259,6 +260,17 @@ impl StorageBackend for EncryptedStorage { self.inner.delete_delta(account_id, nonce).await } + async fn request_candidate_abandon( + &self, + account_id: &str, + nonce: u64, + now: &str, + ) -> Result { + self.inner + .request_candidate_abandon(account_id, nonce, now) + .await + } + async fn update_delta_status( &self, account_id: &str, diff --git a/crates/server/src/storage/filesystem.rs b/crates/server/src/storage/filesystem.rs index 2df33a27..14e4cb2a 100644 --- a/crates/server/src/storage/filesystem.rs +++ b/crates/server/src/storage/filesystem.rs @@ -3,7 +3,7 @@ use crate::state_object::StateObject; use crate::storage::StorageBackend; use crate::storage::encryption::marker::{EncryptionMarker, MarkerStore}; use crate::storage::{ - AccountDeltaCursor, AccountProposalCursor, DeltaStatusCounts, DeltaStatusKind, + AbandonIntent, AccountDeltaCursor, AccountProposalCursor, DeltaStatusCounts, DeltaStatusKind, GlobalDeltaCursor, GlobalDeltaRow, GlobalProposalCursor, ProposalRecord, StorageType, }; use crate::utils::normalize_commitment_hex; @@ -16,6 +16,13 @@ use tokio::io::AsyncWriteExt; pub struct FilesystemService { app_path: PathBuf, + /// Serializes delta-status writes against the conditional candidate + /// delete (issue #319): the filesystem has no transactions, so + /// `delete_delta_if_candidate`'s read-check-delete and the status + /// writes it races (`submit_delta`, `update_delta_status`) take this + /// lock. The backend is single-process, so an in-process mutex is + /// sufficient. + delta_write_lock: std::sync::Arc>, } impl FilesystemService { @@ -26,7 +33,10 @@ impl FilesystemService { .await .map_err(|e| format!("Failed to create app directory: {e}"))?; - Ok(Self { app_path }) + Ok(Self { + app_path, + delta_write_lock: std::sync::Arc::new(tokio::sync::Mutex::new(())), + }) } /// Atomically write a file @@ -409,6 +419,7 @@ impl StorageBackend for FilesystemService { let app_path = self.get_delta_path(&delta.account_id, delta.nonce); + let _guard = self.delta_write_lock.lock().await; self.write(&app_path, &content).await } @@ -635,6 +646,47 @@ impl StorageBackend for FilesystemService { Ok(()) } + async fn request_candidate_abandon( + &self, + account_id: &str, + nonce: u64, + now: &str, + ) -> Result { + let path = self.get_delta_path(account_id, nonce); + + // Read-check-write under the delta write lock: status writes + // (`submit_delta`, `update_delta_status`) take the same lock, so + // the intent annotation can neither clobber a concurrent status + // transition nor lose worker-owned counters. + let _guard = self.delta_write_lock.lock().await; + + let content = match fs::read_to_string(&path).await { + Ok(content) => content, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return Ok(AbandonIntent::NotCandidate); + } + Err(e) => return Err(format!("Failed to read delta file: {e}")), + }; + let mut delta: DeltaObject = serde_json::from_str(&content) + .map_err(|e| format!("Failed to deserialize delta: {e}"))?; + + if !delta.status.is_candidate() { + return Ok(AbandonIntent::NotCandidate); + } + if let Some(requested_at) = delta.status.abandon_requested_at() { + return Ok(AbandonIntent::AlreadyRequested { + requested_at: requested_at.to_string(), + }); + } + + delta.status = delta.status.with_abandon_requested(now.to_string()); + let updated = serde_json::to_string_pretty(&delta) + .map_err(|e| format!("Failed to serialize delta: {e}"))?; + self.write(&path, &updated).await?; + + Ok(AbandonIntent::Recorded) + } + async fn update_delta_status( &self, account_id: &str, @@ -643,6 +695,7 @@ impl StorageBackend for FilesystemService { ) -> Result<(), String> { let path = self.get_delta_path(account_id, nonce); + let _guard = self.delta_write_lock.lock().await; let content = fs::read_to_string(&path) .await .map_err(|e| format!("Failed to read delta file: {e}"))?; @@ -698,7 +751,35 @@ impl StorageBackend for FilesystemService { status: DeltaStatus, _fence: Option<&crate::storage::LeaseFence>, ) -> Result { - crate::storage::update_candidate_status_sequential(self, account_id, nonce, status).await + let path = self.get_delta_path(account_id, nonce); + + // Read-modify-write under the delta write lock (the same lock + // `request_candidate_abandon` takes): the new status is computed + // from the worker's tick-start snapshot, so a concurrently + // recorded abandon request must be carried into the overwrite. + let _guard = self.delta_write_lock.lock().await; + + let content = match fs::read_to_string(&path).await { + Ok(content) => content, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return Ok(crate::storage::CanonicalWrite::NotCandidate); + } + Err(e) => return Err(format!("Failed to read delta file: {e}")), + }; + let mut delta: DeltaObject = serde_json::from_str(&content) + .map_err(|e| format!("Failed to deserialize delta: {e}"))?; + + if !delta.status.is_candidate() { + return Ok(crate::storage::CanonicalWrite::NotCandidate); + } + + delta.status = + status.with_abandon_request_preserved_from(delta.status.abandon_requested_at()); + let updated = serde_json::to_string_pretty(&delta) + .map_err(|e| format!("Failed to serialize delta: {e}"))?; + self.write(&path, &updated).await?; + + Ok(crate::storage::CanonicalWrite::Applied) } // ---------------------------------------------------------------------- @@ -1077,6 +1158,163 @@ mod tests { } } + #[tokio::test] + async fn test_request_candidate_abandon_records_intent() { + let temp_dir = env::temp_dir().join(format!("guardian_test_{}", uuid::Uuid::new_v4())); + let storage = FilesystemService::new(temp_dir.clone()) + .await + .expect("Failed to create storage"); + + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b"; + let mut delta = create_test_delta(account_id, 1); + delta.status = DeltaStatus::candidate("2024-11-14T12:00:00Z".to_string()); + storage.submit_delta(&delta).await.expect("submit works"); + + let intent = storage + .request_candidate_abandon(account_id, 1, "2024-11-14T12:05:00Z") + .await + .expect("intent recording works"); + assert_eq!(intent, AbandonIntent::Recorded); + + let stored = storage.pull_delta(account_id, 1).await.expect("readable"); + assert!(stored.status.is_candidate(), "status must stay candidate"); + assert_eq!( + stored.status.abandon_requested_at(), + Some("2024-11-14T12:05:00Z") + ); + } + + #[tokio::test] + async fn test_request_candidate_abandon_is_idempotent() { + let temp_dir = env::temp_dir().join(format!("guardian_test_{}", uuid::Uuid::new_v4())); + let storage = FilesystemService::new(temp_dir.clone()) + .await + .expect("Failed to create storage"); + + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b"; + let mut delta = create_test_delta(account_id, 1); + delta.status = DeltaStatus::candidate("2024-11-14T12:00:00Z".to_string()); + storage.submit_delta(&delta).await.expect("submit works"); + + storage + .request_candidate_abandon(account_id, 1, "2024-11-14T12:05:00Z") + .await + .expect("first request works"); + let retry = storage + .request_candidate_abandon(account_id, 1, "2024-11-14T12:09:00Z") + .await + .expect("retry works"); + assert_eq!( + retry, + AbandonIntent::AlreadyRequested { + requested_at: "2024-11-14T12:05:00Z".to_string() + }, + "retries must preserve the original request timestamp" + ); + } + + #[tokio::test] + async fn test_request_candidate_abandon_spares_non_candidates() { + let temp_dir = env::temp_dir().join(format!("guardian_test_{}", uuid::Uuid::new_v4())); + let storage = FilesystemService::new(temp_dir.clone()) + .await + .expect("Failed to create storage"); + + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b"; + // create_test_delta is canonical by default. + let delta = create_test_delta(account_id, 1); + storage.submit_delta(&delta).await.expect("submit works"); + + let intent = storage + .request_candidate_abandon(account_id, 1, "2024-11-14T12:05:00Z") + .await + .expect("call works"); + assert_eq!(intent, AbandonIntent::NotCandidate); + let stored = storage.pull_delta(account_id, 1).await.expect("readable"); + assert!(stored.status.is_canonical(), "canonical delta untouched"); + } + + #[tokio::test] + async fn test_stale_counter_write_preserves_concurrent_abandon_intent() { + // The clobber race: the worker computes a counter write from its + // tick-start snapshot (no intent), a client records the intent in + // between, then the worker's write lands. The stored intent must + // survive. + let temp_dir = env::temp_dir().join(format!("guardian_test_{}", uuid::Uuid::new_v4())); + let storage = FilesystemService::new(temp_dir.clone()) + .await + .expect("Failed to create storage"); + + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b"; + let mut delta = create_test_delta(account_id, 1); + delta.status = DeltaStatus::candidate("2024-11-14T12:00:00Z".to_string()); + storage.submit_delta(&delta).await.expect("submit works"); + + // Worker snapshot taken here (no intent yet). + let stale_counter_write = delta.status.with_incremented_divergence(); + + // Client records the intent. + storage + .request_candidate_abandon(account_id, 1, "2024-11-14T12:05:00Z") + .await + .expect("intent recording works"); + + // Worker's stale write lands. + let outcome = storage + .update_candidate_status(account_id, 1, stale_counter_write, None) + .await + .expect("status update works"); + assert_eq!(outcome, crate::storage::CanonicalWrite::Applied); + + let stored = storage.pull_delta(account_id, 1).await.expect("readable"); + assert_eq!( + stored.status.abandon_requested_at(), + Some("2024-11-14T12:05:00Z"), + "the concurrently recorded intent must survive the counter write" + ); + assert_eq!(stored.status.divergence_count(), 1); + } + + #[tokio::test] + async fn test_update_candidate_status_spares_non_candidates() { + let temp_dir = env::temp_dir().join(format!("guardian_test_{}", uuid::Uuid::new_v4())); + let storage = FilesystemService::new(temp_dir.clone()) + .await + .expect("Failed to create storage"); + + let account_id = "0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b"; + // create_test_delta is canonical by default. + let delta = create_test_delta(account_id, 1); + storage.submit_delta(&delta).await.expect("submit works"); + + let outcome = storage + .update_candidate_status( + account_id, + 1, + DeltaStatus::candidate("2024-11-14T12:00:00Z".to_string()), + None, + ) + .await + .expect("call works"); + assert_eq!(outcome, crate::storage::CanonicalWrite::NotCandidate); + let stored = storage.pull_delta(account_id, 1).await.expect("readable"); + assert!(stored.status.is_canonical(), "canonical delta untouched"); + } + + #[tokio::test] + async fn test_request_candidate_abandon_missing_is_not_candidate() { + let temp_dir = env::temp_dir().join(format!("guardian_test_{}", uuid::Uuid::new_v4())); + let storage = FilesystemService::new(temp_dir.clone()) + .await + .expect("Failed to create storage"); + + let intent = storage + .request_candidate_abandon("0x7b7b7b7a7b7b7b017b7b7b7b7b7b7b", 1, "now") + .await + .expect("call works"); + assert_eq!(intent, AbandonIntent::NotCandidate); + } + #[tokio::test] async fn test_submit_and_pull_state() { let temp_dir = env::temp_dir().join(format!("guardian_test_{}", uuid::Uuid::new_v4())); diff --git a/crates/server/src/storage/mod.rs b/crates/server/src/storage/mod.rs index 44e1755a..deab9f13 100644 --- a/crates/server/src/storage/mod.rs +++ b/crates/server/src/storage/mod.rs @@ -155,6 +155,17 @@ pub enum CanonicalWrite { NotCandidate, } +/// Outcome of recording a client abandon request on a candidate. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AbandonIntent { + /// The request timestamp was recorded; the delta remains a candidate. + Recorded, + /// The candidate already carries an abandon request; nothing written. + AlreadyRequested { requested_at: String }, + /// The delta is absent or no longer a candidate; nothing written. + NotCandidate, +} + /// Outcome of a fenced candidate promotion. Promotion overwrites the /// account state, so it carries one gate the other canonicalization /// writes do not: the stored state must still sit at the candidate's @@ -278,12 +289,27 @@ pub(crate) async fn discard_candidate_sequential( } /// Single-process fallback for [`StorageBackend::update_candidate_status`]. +/// Best-effort merge of a concurrently recorded abandon request: the read +/// and the write are separate steps here, acceptable only where no +/// concurrent replica can observe the gap. Its sole consumer is the +/// cfg-gated mock backend — the filesystem backend overrides the method +/// with a locked read-modify-write — hence the dead-code allowance for +/// non-test builds. +#[allow(dead_code)] pub(crate) async fn update_candidate_status_sequential( storage: &dyn StorageBackend, account_id: &str, nonce: u64, status: DeltaStatus, ) -> Result { + let status = match storage.pull_delta(account_id, nonce).await { + Ok(current) if current.status.is_candidate() => { + status.with_abandon_request_preserved_from(current.status.abandon_requested_at()) + } + Ok(_) => return Ok(CanonicalWrite::NotCandidate), + // Mocks without a canned read keep the historical plain-write path. + Err(_) => status, + }; storage .update_delta_status(account_id, nonce, status) .await?; @@ -443,6 +469,19 @@ pub trait StorageBackend: Send + Sync { async fn delete_delta_proposal(&self, account_id: &str, commitment: &str) -> Result<(), String>; async fn delete_delta(&self, account_id: &str, nonce: u64) -> Result<(), String>; + /// Atomically record a client's abandon request (issue #319) on the + /// candidate at `nonce`: sets `abandon_requested_at` while preserving + /// every worker-owned counter, only while the delta is still a + /// candidate. Unfenced by design — the write is a non-destructive + /// annotation; the lease-holding worker resolves the intent. An + /// existing request timestamp is kept so retries cannot restart the + /// abandon quarantine. + async fn request_candidate_abandon( + &self, + account_id: &str, + nonce: u64, + now: &str, + ) -> Result; async fn update_delta_status( &self, account_id: &str, diff --git a/crates/server/src/storage/postgres.rs b/crates/server/src/storage/postgres.rs index 6d6c172f..ca6c5cc8 100644 --- a/crates/server/src/storage/postgres.rs +++ b/crates/server/src/storage/postgres.rs @@ -5,9 +5,9 @@ use crate::state_object::StateObject; use crate::storage::StorageBackend; use crate::storage::encryption::marker::{EncryptionMarker, MarkerStore}; use crate::storage::{ - AccountDeltaCursor, AccountProposalCursor, CandidatePromotion, CandidateSubmission, - CanonicalWrite, DeltaStatusCounts, DeltaStatusKind, GlobalDeltaCursor, GlobalDeltaRow, - GlobalProposalCursor, LeaseFence, PromoteWrite, ProposalRecord, StorageType, + AbandonIntent, AccountDeltaCursor, AccountProposalCursor, CandidatePromotion, + CandidateSubmission, CanonicalWrite, DeltaStatusCounts, DeltaStatusKind, GlobalDeltaCursor, + GlobalDeltaRow, GlobalProposalCursor, LeaseFence, PromoteWrite, ProposalRecord, StorageType, }; use async_trait::async_trait; use diesel::ConnectionError; @@ -1296,6 +1296,71 @@ impl StorageBackend for PostgresService { Ok(()) } + async fn request_candidate_abandon( + &self, + account_id: &str, + nonce: u64, + now: &str, + ) -> Result { + use diesel::OptionalExtension; + + let mut conn = self + .pool + .get() + .await + .map_err(|e| format!("Failed to get connection: {e}"))?; + + let account_id = account_id.to_string(); + let now = now.to_string(); + + // Row-locked read + conditional in-place JSONB update: the intent + // annotation touches only `abandon_requested_at`, so worker-owned + // counters in the same status blob are never overwritten, and the + // `status_kind` filter guarantees a concurrently promoted or + // discarded delta is left untouched. + conn.transaction::(|conn| { + async move { + let existing: Option> = deltas::table + .filter(deltas::account_id.eq(&account_id)) + .filter(deltas::nonce.eq(nonce as i64)) + .filter(deltas::status_kind.eq("candidate")) + .select(diesel::dsl::sql::< + diesel::sql_types::Nullable, + >("status->>'abandon_requested_at'")) + .for_update() + .first(conn) + .await + .optional()?; + + match existing { + None => Ok(AbandonIntent::NotCandidate), + Some(Some(requested_at)) => { + Ok(AbandonIntent::AlreadyRequested { requested_at }) + } + Some(None) => { + diesel::update(deltas::table) + .filter(deltas::account_id.eq(&account_id)) + .filter(deltas::nonce.eq(nonce as i64)) + .filter(deltas::status_kind.eq("candidate")) + .set( + deltas::status.eq(diesel::dsl::sql::( + "jsonb_set(status, '{abandon_requested_at}', to_jsonb(", + ) + .bind::(now) + .sql("::text))")), + ) + .execute(conn) + .await?; + Ok(AbandonIntent::Recorded) + } + } + } + .scope_boxed() + }) + .await + .map_err(|e| format!("Failed to record abandon request: {e}")) + } + async fn update_delta_status( &self, account_id: &str, @@ -1604,6 +1669,38 @@ impl StorageBackend for PostgresService { return Ok(CanonicalWrite::StaleLease); } + // Row-locked read of the stored abandon request: the new + // status is computed from the worker's tick-start snapshot, + // so an intent recorded concurrently by + // `request_candidate_abandon` (which takes the same row + // lock) must be carried into the overwrite or it would be + // silently wiped. + use diesel::OptionalExtension; + let stored_requested_at: Option> = deltas::table + .filter(deltas::account_id.eq(&account_id)) + .filter(deltas::nonce.eq(nonce as i64)) + .filter(deltas::status_kind.eq("candidate")) + .select(diesel::dsl::sql::< + diesel::sql_types::Nullable, + >("status->>'abandon_requested_at'")) + .for_update() + .first(conn) + .await + .optional()?; + let Some(stored_requested_at) = stored_requested_at else { + return Ok(CanonicalWrite::NotCandidate); + }; + + let mut status_json = status_json; + if status_kind == "candidate" + && status_json + .get("abandon_requested_at") + .is_none_or(serde_json::Value::is_null) + && let Some(stored) = stored_requested_at + { + status_json["abandon_requested_at"] = serde_json::Value::String(stored); + } + let updated = diesel::update(deltas::table) .filter(deltas::account_id.eq(&account_id)) .filter(deltas::nonce.eq(nonce as i64)) diff --git a/crates/server/src/testing/e2e/abandon_candidate.rs b/crates/server/src/testing/e2e/abandon_candidate.rs new file mode 100644 index 00000000..c7e51c34 --- /dev/null +++ b/crates/server/src/testing/e2e/abandon_candidate.rs @@ -0,0 +1,469 @@ +//! End-to-end reproduction of issue #319 and confirmation of the fix. +//! +//! The issue's scenario: an approved candidate whose transaction dies +//! client-side after approval (RPC submit failure, prover timeout, crash) +//! holds the account's pending-candidate lock for the full submission +//! grace period plus retry budget, and every new proposal in that window +//! is answered `409 conflict_pending_delta`. This test replays that +//! sequence against a real mock chain: +//! +//! 1. Build a real 2-of-2 multisig account (guardian key = this server's +//! ack key), obtain a valid abort `TransactionSummary` exactly like the +//! wallet does, and configure the account on the server. +//! 2. Push the delta so it becomes a candidate, with the registered +//! on-chain answer pinned at the account's INITIAL commitment — the +//! transaction never lands, mirroring the client-side death. +//! 3. Run the canonicalization worker and assert the candidate survives +//! (submission grace period), then assert a new proposal is refused +//! with `ConflictPendingDelta` — the issue's 409 loop. +//! 4. Record the abandon intent via the client-initiated service (202 +//! semantics: the account stays locked), run the worker to resolve the +//! quarantine, and assert the delta transitions to +//! `Discarded { reason: ClientAbandoned }` (kept as history), the flag +//! clears, and the SAME proposal is accepted afterwards. +//! +//! A second test covers the safety guard: when the registered on-chain +//! answer equals the candidate's expected post-state (the transaction +//! actually landed), abandon must refuse with `CandidateLanded` and leave +//! the candidate for the worker to canonicalize. + +use std::sync::Arc; + +use guardian_shared::auth_request_message::AuthRequestMessage; +use guardian_shared::auth_request_payload::AuthRequestPayload; +use guardian_shared::hex::IntoHex; +use guardian_shared::{SignatureScheme, ToJson}; +use miden_confidential_contracts::masm_builder::get_guardian_library; +use miden_confidential_contracts::multisig_guardian::{ + MultisigGuardianBuilder, MultisigGuardianConfig, +}; +use miden_protocol::account::{Account, AccountType}; +use miden_protocol::crypto::dsa::falcon512_poseidon2::SecretKey; +use miden_protocol::utils::serde::Serializable; +use miden_protocol::vm::AdviceInputs; +use miden_protocol::{Felt, Word}; +use miden_standards::code_builder::CodeBuilder; +use miden_testing::MockChainBuilder; +use miden_tx::TransactionExecutorError; + +use crate::delta_object::DeltaObject; +use crate::error::GuardianError; +use crate::metadata::NetworkConfig; +use crate::metadata::auth::{Auth, Credentials}; +use crate::network::NetworkType; +use crate::network::miden::MidenNetworkClient; +use crate::services::{ + AbandonCandidateParams, AbandonState, ConfigureAccountParams, PushDeltaParams, + PushDeltaProposalParams, abandon_candidate, configure_account, process_canonicalizations_now, + push_delta, push_delta_proposal, +}; +use crate::state::AppState; +use crate::testing::helpers::{IntegrationMockNetworkClient, create_test_app_state}; + +fn commitment_hex(account: &Account) -> String { + format!("0x{}", hex::encode(account.to_commitment().as_bytes())) +} + +fn word_from_hex(hex_str: &str) -> Word { + use miden_protocol::utils::serde::Deserializable; + let bytes = hex::decode(hex_str.trim_start_matches("0x")).expect("valid hex"); + Word::read_from_bytes(&bytes).expect("valid word bytes") +} + +fn falcon_credentials( + key: &SecretKey, + pubkey_hex: &str, + account_id_hex: &str, + timestamp: i64, +) -> Credentials { + let message = AuthRequestMessage::from_account_id_hex( + account_id_hex, + timestamp, + AuthRequestPayload::empty(), + ) + .expect("valid account ID") + .to_word(); + let signature = key.sign(message); + let signature_hex = format!("0x{}", hex::encode(signature.to_bytes())); + Credentials::signature(pubkey_hex.to_string(), signature_hex, timestamp) +} + +/// Scaffolding shared by both #319 e2e tests: a configured 2-of-2 +/// multisig account with a pushed candidate delta whose on-chain answer +/// is pinned to `registered_commitment` (computed from the setup's own +/// artifacts by the caller-provided selector). +struct StrandedCandidateSetup { + state: AppState, + account_id_hex: String, + candidate_nonce: u64, + cosigner_key: SecretKey, + api_pubkey_hex: String, + delta_payload: serde_json::Value, + /// Commitment of the candidate's expected post-state, as the server + /// itself computes it via `apply_delta`. + expected_commitment_hex: String, + /// Strictly-increasing auth timestamp source (replay protection). + next_timestamp: i64, +} + +impl StrandedCandidateSetup { + fn credentials(&mut self) -> Credentials { + self.next_timestamp += 1; + falcon_credentials( + &self.cosigner_key, + &self.api_pubkey_hex, + &self.account_id_hex, + self.next_timestamp, + ) + } + + /// The wallet-shaped proposal payload the issue's 409 loop keeps + /// retrying: the same valid `TransactionSummary`, wrapped as a + /// multisig proposal. + fn proposal_payload(&self) -> serde_json::Value { + serde_json::json!({ + "tx_summary": self.delta_payload.clone(), + "signatures": [], + "metadata": { + "proposal_type": "custom", + "description": "retried while candidate was stuck" + } + }) + } +} + +/// `landed`: whether the registered on-chain commitment is the candidate's +/// expected post-state (transaction landed) or the account's initial +/// commitment (transaction never landed — the issue's scenario). +async fn stranded_candidate_setup(landed: bool) -> StrandedCandidateSetup { + let mut state = create_test_app_state().await; + + let scheme = SignatureScheme::Falcon; + let ack_commitment_hex = state.ack.commitment(&scheme); + let ack_commitment_word = word_from_hex(&ack_commitment_hex); + + let cosigner_keys: Vec = (0..2).map(|_| SecretKey::new()).collect(); + let cosigner_pubkeys: Vec<_> = cosigner_keys.iter().map(|k| k.public_key()).collect(); + let signer_commitments: Vec = cosigner_pubkeys + .iter() + .map(|pk| pk.to_commitment()) + .collect(); + + let config = MultisigGuardianConfig::new(2, signer_commitments, ack_commitment_word) + .with_account_type(AccountType::Public) + .with_guardian_enabled(true) + .with_signature_scheme(SignatureScheme::Falcon); + let multisig_account = MultisigGuardianBuilder::new(config) + .build_existing() + .expect("multisig account builds"); + + let account_id_hex = multisig_account.id().to_hex(); + let initial_commitment = commitment_hex(&multisig_account); + + let mock_chain = MockChainBuilder::with_accounts([multisig_account.clone()]) + .expect("mock chain accepts account") + .build() + .expect("mock chain builds"); + + // Any valid transaction works; the guardian-key update script is the + // one with existing scaffolding. The no-signature abort run yields the + // TransactionSummary the wallet pushes as the delta payload. + let new_guardian_key = SecretKey::new(); + let new_guardian_commitment = new_guardian_key.public_key().to_commitment(); + let advice_inputs = + AdviceInputs::default().with_stack(new_guardian_commitment.as_elements().iter().copied()); + let guardian_library = get_guardian_library().expect("guardian library compiles"); + let tx_script = CodeBuilder::new() + .with_dynamically_linked_library(&guardian_library) + .expect("library links") + .compile_tx_script( + r#" + use oz_guardian::guardian + begin + call.guardian::update_guardian_public_key + end + "#, + ) + .expect("tx script compiles"); + let salt = Word::from([Felt::new_unchecked(7); 4]); + + let abort_summary = match mock_chain + .build_tx_context(multisig_account.id(), &[], &[]) + .expect("tx context builds") + .authenticator(None) + .tx_script(tx_script) + .extend_advice_inputs(advice_inputs) + .auth_args(salt) + .build() + .expect("tx builds") + .execute() + .await + .unwrap_err() + { + TransactionExecutorError::Unauthorized(tx_effects) => tx_effects, + error => panic!("expected abort with tx effects: {error:?}"), + }; + let delta_payload = abort_summary.as_ref().to_json(); + + // The candidate's expected post-state commitment, computed the same + // way the server computes it (apply_delta on the initial state). + let miden_client = MidenNetworkClient::lazy_for_test(NetworkType::MidenLocal); + let (_, expected_commitment_hex) = { + use crate::network::NetworkClient; + miden_client + .apply_delta(&multisig_account.to_json(), &delta_payload) + .expect("delta applies to initial state") + }; + + let registered_commitment = if landed { + expected_commitment_hex.clone() + } else { + initial_commitment.clone() + }; + let mut integration_client = IntegrationMockNetworkClient::new(miden_client); + integration_client.register_account(account_id_hex.clone(), registered_commitment); + state.network_client = Arc::new(integration_client); + + let api_pubkey_hex = cosigner_pubkeys[0].clone().into_hex(); + let api_commitment_hex = format!( + "0x{}", + hex::encode(cosigner_pubkeys[0].to_commitment().to_bytes()) + ); + + let mut setup = StrandedCandidateSetup { + state, + account_id_hex: account_id_hex.clone(), + candidate_nonce: multisig_account.nonce().as_canonical_u64() + 1, + cosigner_key: cosigner_keys[0].clone(), + api_pubkey_hex, + delta_payload: delta_payload.clone(), + expected_commitment_hex, + next_timestamp: chrono::Utc::now().timestamp_millis(), + }; + + let creds = setup.credentials(); + configure_account( + &setup.state, + ConfigureAccountParams { + account_id: account_id_hex.clone(), + auth: Auth::MidenFalconRpo { + cosigner_commitments: vec![api_commitment_hex], + }, + network_config: NetworkConfig::miden_default(), + initial_state: multisig_account.to_json(), + credential: creds, + }, + ) + .await + .expect("configure_account succeeds"); + + // Push the approved delta: it becomes the pending candidate whose + // transaction (in the `landed = false` case) will never land. + let creds = setup.credentials(); + let push_result = push_delta( + &setup.state, + PushDeltaParams { + delta: DeltaObject { + account_id: account_id_hex.clone(), + nonce: setup.candidate_nonce, + prev_commitment: initial_commitment, + new_commitment: None, + delta_payload, + ack_sig: String::new(), + ack_pubkey: String::new(), + ack_scheme: String::new(), + status: Default::default(), + metadata: None, + }, + credentials: creds, + }, + ) + .await + .expect("push_delta accepts the delta"); + assert!( + push_result.delta.status.is_candidate(), + "delta should await canonicalization, got {:?}", + push_result.delta.status + ); + + setup +} + +#[tokio::test] +async fn test_stranded_candidate_locks_account_until_abandoned() { + let mut setup = stranded_candidate_setup(false).await; + + // The worker cannot release the candidate: on-chain still shows the + // account's base state, indistinguishable from slow proving, so the + // candidate sits in the submission grace period. + process_canonicalizations_now(&setup.state) + .await + .expect("canonicalization run succeeds"); + let deltas = setup + .state + .storage + .pull_deltas_after(&setup.account_id_hex, 0) + .await + .expect("deltas readable"); + assert!( + deltas + .iter() + .any(|d| d.nonce == setup.candidate_nonce && d.status.is_candidate()), + "candidate must survive the worker run within the grace period" + ); + + // The issue's 409 loop: every new proposal is refused while the + // stranded candidate holds the account. + let proposal_payload = setup.proposal_payload(); + let creds = setup.credentials(); + let refused = push_delta_proposal( + &setup.state, + PushDeltaProposalParams { + account_id: setup.account_id_hex.clone(), + nonce: setup.candidate_nonce + 1, + delta_payload: proposal_payload.clone(), + credentials: creds, + }, + ) + .await + .expect_err("proposal must be refused while the candidate is pending"); + assert!( + matches!(refused, GuardianError::ConflictPendingDelta), + "expected ConflictPendingDelta, got {refused:?}" + ); + + // The fix: the client knows its transaction died and records the + // abandon intent. The account stays locked until the worker resolves + // the quarantine. + let creds = setup.credentials(); + let abandoned = abandon_candidate( + &setup.state, + AbandonCandidateParams { + account_id: setup.account_id_hex.clone(), + nonce: setup.candidate_nonce, + credentials: creds, + }, + ) + .await + .expect("abandon_candidate accepts the intent for a never-landing candidate"); + assert_eq!(abandoned.nonce, setup.candidate_nonce); + assert_eq!(abandoned.state, AbandonState::Pending); + assert!(abandoned.abandon_requested_at.is_some()); + + // Still locked: intent alone releases nothing. + let creds = setup.credentials(); + let still_refused = push_delta_proposal( + &setup.state, + PushDeltaProposalParams { + account_id: setup.account_id_hex.clone(), + nonce: setup.candidate_nonce + 1, + delta_payload: proposal_payload.clone(), + credentials: creds, + }, + ) + .await + .expect_err("account must stay locked until the worker resolves the intent"); + assert!(matches!(still_refused, GuardianError::ConflictPendingDelta)); + + // The worker resolves the intent (the e2e worker runs with a zero + // quarantine): the delta becomes Discarded { ClientAbandoned } — + // preserved as history — and the account is released. + process_canonicalizations_now(&setup.state) + .await + .expect("canonicalization run succeeds"); + + let deltas = setup + .state + .storage + .pull_deltas_after(&setup.account_id_hex, 0) + .await + .expect("deltas readable"); + let resolved = deltas + .iter() + .find(|d| d.nonce == setup.candidate_nonce) + .expect("abandoned delta must be preserved as history"); + assert!( + resolved.status.is_client_abandoned(), + "delta must be discarded as client-abandoned, got {:?}", + resolved.status + ); + let metadata = setup + .state + .metadata + .get(&setup.account_id_hex) + .await + .expect("metadata readable") + .expect("metadata present"); + assert!( + !metadata.has_pending_candidate, + "pending-candidate flag must be cleared by the resolution" + ); + + // The SAME proposal that was refused moments ago is now accepted — + // no grace-period or retry-budget wait. + let creds = setup.credentials(); + let accepted = push_delta_proposal( + &setup.state, + PushDeltaProposalParams { + account_id: setup.account_id_hex.clone(), + nonce: setup.candidate_nonce + 1, + delta_payload: proposal_payload, + credentials: creds, + }, + ) + .await + .expect("proposal must be accepted immediately after the abandon"); + assert!(accepted.delta.status.is_pending()); +} + +#[tokio::test] +async fn test_abandon_refused_when_candidate_transaction_landed() { + let mut setup = stranded_candidate_setup(true).await; + + // On-chain already shows the candidate's expected post-state: the + // transaction landed, so the client's abandon must be refused — the + // worker will canonicalize shortly. + let creds = setup.credentials(); + let refused = abandon_candidate( + &setup.state, + AbandonCandidateParams { + account_id: setup.account_id_hex.clone(), + nonce: setup.candidate_nonce, + credentials: creds, + }, + ) + .await + .expect_err("abandon must be refused when the transaction landed"); + assert!( + matches!(refused, GuardianError::CandidateLanded { .. }), + "expected CandidateLanded, got {refused:?}" + ); + + // The candidate is untouched and the worker canonicalizes it against + // the landed on-chain state. + process_canonicalizations_now(&setup.state) + .await + .expect("canonicalization run succeeds"); + let deltas = setup + .state + .storage + .pull_deltas_after(&setup.account_id_hex, 0) + .await + .expect("deltas readable"); + let candidate = deltas + .iter() + .find(|d| d.nonce == setup.candidate_nonce) + .expect("delta still stored"); + assert!( + candidate.status.is_canonical(), + "landed candidate must canonicalize, got {:?}", + candidate.status + ); + let final_state = setup + .state + .storage + .pull_state(&setup.account_id_hex) + .await + .expect("state readable"); + assert_eq!(final_state.commitment, setup.expected_commitment_hex); +} diff --git a/crates/server/src/testing/e2e/mod.rs b/crates/server/src/testing/e2e/mod.rs index 20b8feed..56bb924b 100644 --- a/crates/server/src/testing/e2e/mod.rs +++ b/crates/server/src/testing/e2e/mod.rs @@ -1,5 +1,6 @@ // E2E tests (enabled with `--features e2e`) #![cfg(feature = "e2e")] +mod abandon_candidate; mod configure_account; mod switch_guardian_canonicalization; diff --git a/crates/server/src/testing/mocks.rs b/crates/server/src/testing/mocks.rs index d93906a8..fbdfee52 100644 --- a/crates/server/src/testing/mocks.rs +++ b/crates/server/src/testing/mocks.rs @@ -282,6 +282,8 @@ pub struct MockStorageBackend { pub delete_delta_proposal_responses: Arc>>>, pub delete_delta_proposal_calls: Arc>>, pub delete_delta_calls: Arc>>, + pub request_candidate_abandon_responses: + Arc>>>, pub update_delta_status_calls: Arc>>, // Canonicalization lifecycle writes. When a scripted outcome is @@ -443,6 +445,17 @@ impl MockStorageBackend { self.delete_delta_proposal_calls.lock().unwrap().clone() } + pub fn with_request_candidate_abandon( + self, + response: StdResult, + ) -> Self { + self.request_candidate_abandon_responses + .lock() + .unwrap() + .push(response); + self + } + pub fn get_delete_delta_calls(&self) -> Vec<(String, u64)> { self.delete_delta_calls.lock().unwrap().clone() } @@ -621,7 +634,7 @@ impl StorageBackend for MockStorageBackend { .lock() .unwrap() .pop() - .unwrap_or_else(|| Err("No delta found".to_string())) + .unwrap_or_else(|| Err("Mock: delta not found".to_string())) } async fn pull_deltas_after( @@ -682,7 +695,7 @@ impl StorageBackend for MockStorageBackend { .lock() .unwrap() .pop() - .unwrap_or_else(|| Err("Mock: No proposal found".to_string())) + .unwrap_or_else(|| Err("Mock: proposal not found".to_string())) } async fn pull_all_delta_proposals( @@ -746,6 +759,19 @@ impl StorageBackend for MockStorageBackend { Ok(()) } + async fn request_candidate_abandon( + &self, + _account_id: &str, + _nonce: u64, + _now: &str, + ) -> Result { + self.request_candidate_abandon_responses + .lock() + .unwrap() + .pop() + .unwrap_or(Ok(crate::storage::AbandonIntent::Recorded)) + } + async fn update_delta_status( &self, account_id: &str, diff --git a/docs/CONCEPTS.md b/docs/CONCEPTS.md index 4ce6ca70..ddaa3357 100644 --- a/docs/CONCEPTS.md +++ b/docs/CONCEPTS.md @@ -194,6 +194,7 @@ flowchart TB | Guardian unreachable | gRPC `Unavailable` / HTTP 5xx, no ACK | Continue locally, retry; rotate operator if persistent. | | Stale delta (`commitment_mismatch`) | `400` with `code: commitment_mismatch` | `GET /delta/since` → replay canonical chain → retry the local transaction. | | Candidate discarded | Delta status flips `candidate` → `discarded` | Refetch state, rebuild and resubmit. Usually means the Miden proof was never submitted or the on-chain commitment diverged. | +| Transaction died after approval (stranded candidate) | New proposals answered `409 conflict_pending_delta` while the candidate waits out the grace + retry window | Call `POST /delta/candidate/abandon` (SDKs: `abandonCandidate` / `abandon_candidate`). The worker confirms over a short quarantine that the transaction did not land, flips the delta to `discarded` with reason `client_abandoned`, and releases the account — typically well under a minute. Poll via `abandonStatus` / `abandon_status`. | | Operator censors / withholds | Other cosigners see stale state | Use the user's cold key to rotate Guardian; the new operator inherits canonical state from Miden. | | Account paused by operator | State-transition, proposal, and EVM mutation paths return `409 GUARDIAN_ACCOUNT_PAUSED` with `paused_reason` (reads and `ConfigureAccount` keep working) | Operator-driven safety lever, not a fault. An operator with `accounts:pause` clears it via `POST /dashboard/accounts/{id}/unpause`. See [`DASHBOARD.md`](./DASHBOARD.md#account-pausing). | | Account switched to another guardian | After the `switch_guardian` delta canonicalizes on this server, mutation paths return `409 GUARDIAN_ACCOUNT_RELEASED` with `released_at` (reads and `ConfigureAccount` keep working); the dashboard shows `released_at` | Expected outcome of a guardian switch, not a fault. Terminal until the wallet re-onboards via `/configure`, which re-validates the guardian binding. An operator unpause never reactivates a released account. | diff --git a/docs/MULTISIG_SDK.md b/docs/MULTISIG_SDK.md index 3ccbe7e9..db60a181 100644 --- a/docs/MULTISIG_SDK.md +++ b/docs/MULTISIG_SDK.md @@ -489,6 +489,8 @@ await multisig.executeProposal(signedProposal.id); | `fetchState()` | Fetch latest state from GUARDIAN | | `registerOnGuardian()` | Register new account with GUARDIAN | | `syncProposals()` | Sync proposals from GUARDIAN | +| `abandonCandidate(nonce)` | Record an abandon intent for a stuck candidate (worker resolves after a short quarantine) | +| `abandonStatus(nonce)` | Poll the abandon resolution: `waiting` / `landed` / `abandoned` / `unexpected` | | `listProposals()` | Get cached proposals | | `createP2idProposal(recipient, faucet, amount, nonce?)` | Create transfer proposal | | `createConsumeNotesProposal(noteIds, nonce?)` | Create note consumption proposal | @@ -749,6 +751,8 @@ for note in notes { | `list_proposals()` | List pending proposals | | `sign_proposal(id)` | Sign a proposal | | `execute_proposal(id)` | Execute ready proposal | +| `abandon_candidate(nonce)` | Record an abandon intent for a stuck candidate (worker resolves after a short quarantine) | +| `abandon_status(nonce)` | Poll the abandon resolution: `Waiting` / `Landed` / `Abandoned` / `Unexpected` | | `create_proposal_offline(tx)` | Create offline proposal | | `sign_imported_proposal(exported)` | Sign offline proposal | | `execute_imported_proposal(exported)` | Execute offline proposal | diff --git a/docs/openapi-client.json b/docs/openapi-client.json index 35904246..69ef7f3b 100644 --- a/docs/openapi-client.json +++ b/docs/openapi-client.json @@ -252,6 +252,95 @@ ] } }, + "/delta/candidate/abandon": { + "post": { + "tags": [ + "client" + ], + "summary": "Request abandonment of a pending canonicalization candidate whose\ntransaction will never land on-chain (issue #319).", + "description": "The request records an abandon *intent* and returns `202 Accepted`;\nthe delta stays a candidate — the account stays locked — until the\ncanonicalization worker confirms over the abandon quarantine that the\ntransaction did not land, then discards the delta as\n`client_abandoned` and releases the account (typically well under a\nminute, versus the full submission grace + retry window).\n\nRefused with `GUARDIAN_CANDIDATE_LANDED` (409) when the transaction\nalready landed. Retries are idempotent and preserve the original\nrequest timestamp. Poll `GET /delta` for the resolution: still\n`candidate` → waiting; `canonical` → landed after all; `discarded`\nwith reason `client_abandoned` → abandoned.", + "operationId": "abandon_candidate", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AbandonCandidateRequest" + } + } + }, + "required": true + }, + "responses": { + "202": { + "description": "Abandon intent accepted (or already resolved)", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AbandonCandidateResponse" + } + } + } + }, + "401": { + "description": "Authentication failed", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "404": { + "description": "No candidate delta at this nonce", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "409": { + "description": "Candidate already landed on-chain, or account paused/released", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "413": { + "description": "Request body exceeds the configured size limit", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "429": { + "description": "Rate limit exceeded", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + } + }, + "security": [ + { + "x-pubkey": [], + "x-signature": [], + "x-timestamp": [] + } + ] + } + }, "/delta/proposal": { "get": { "tags": [ @@ -937,6 +1026,53 @@ }, "components": { "schemas": { + "AbandonCandidateRequest": { + "type": "object", + "required": [ + "account_id", + "nonce" + ], + "properties": { + "account_id": { + "type": "string" + }, + "nonce": { + "type": "integer", + "format": "int64", + "description": "Nonce of the candidate delta to abandon. Requiring the explicit\ntarget prevents a stale request from releasing a newer candidate.", + "minimum": 0 + } + } + }, + "AbandonCandidateResponse": { + "type": "object", + "required": [ + "account_id", + "nonce", + "state" + ], + "properties": { + "abandon_requested_at": { + "type": [ + "string", + "null" + ], + "description": "RFC 3339 UTC timestamp of the recorded abandon request. Retries\nreturn the original timestamp; absent once resolved." + }, + "account_id": { + "type": "string" + }, + "nonce": { + "type": "integer", + "format": "int64", + "minimum": 0 + }, + "state": { + "type": "string", + "description": "`\"pending\"` while the worker still has to resolve the intent\n(the account stays locked until then); `\"abandoned\"` once the\ndelta is discarded as client-abandoned and the account released." + } + } + }, "ApiErrorMeta": { "type": "object", "description": "Structured machine-readable side-data on the error wire object\n(feature `009-human-readable-errors`). `retryable` is always present;\nthe rest appear only for the codes that carry them.", @@ -1415,6 +1551,19 @@ "status" ], "properties": { + "abandon_confirm_count": { + "type": "integer", + "format": "int32", + "description": "Consecutive worker ticks that observed the on-chain commitment\nstill at this candidate's base after the abandon was requested.\nReset on a divergent observation, so only an unbroken streak\ncounts — the same stale-RPC shield as `divergence_count`.", + "minimum": 0 + }, + "abandon_requested_at": { + "type": [ + "string", + "null" + ], + "description": "RFC 3339 UTC timestamp of the client's abandon request\n(issue #319). While set, the status remains `candidate` — the\naccount stays locked — until the canonicalization worker\nresolves the intent after the abandon quarantine." + }, "divergence_count": { "type": "integer", "format": "int32", @@ -1462,6 +1611,16 @@ "status" ], "properties": { + "reason": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/DiscardReason" + } + ] + }, "status": { "type": "string", "enum": [ @@ -1476,6 +1635,13 @@ ], "description": "Delta status state machine" }, + "DiscardReason": { + "type": "string", + "description": "Why a delta ended in `Discarded`. Absent on discards that predate\nthe field (and on the worker's retry/divergence discards, which\ndelete the delta instead of transitioning it).", + "enum": [ + "client_abandoned" + ] + }, "LookupAccount": { "type": "object", "description": "Single match in a lookup response. Wraps `account_id` so the response shape\ncan be extended in a forward-compatible way (e.g. adding role tags or\nper-account metadata) without breaking existing clients.", diff --git a/docs/openapi.json b/docs/openapi.json index d9e6c29a..ae21ced5 100644 --- a/docs/openapi.json +++ b/docs/openapi.json @@ -1612,6 +1612,95 @@ ] } }, + "/delta/candidate/abandon": { + "post": { + "tags": [ + "client" + ], + "summary": "Request abandonment of a pending canonicalization candidate whose\ntransaction will never land on-chain (issue #319).", + "description": "The request records an abandon *intent* and returns `202 Accepted`;\nthe delta stays a candidate — the account stays locked — until the\ncanonicalization worker confirms over the abandon quarantine that the\ntransaction did not land, then discards the delta as\n`client_abandoned` and releases the account (typically well under a\nminute, versus the full submission grace + retry window).\n\nRefused with `GUARDIAN_CANDIDATE_LANDED` (409) when the transaction\nalready landed. Retries are idempotent and preserve the original\nrequest timestamp. Poll `GET /delta` for the resolution: still\n`candidate` → waiting; `canonical` → landed after all; `discarded`\nwith reason `client_abandoned` → abandoned.", + "operationId": "abandon_candidate", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AbandonCandidateRequest" + } + } + }, + "required": true + }, + "responses": { + "202": { + "description": "Abandon intent accepted (or already resolved)", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AbandonCandidateResponse" + } + } + } + }, + "401": { + "description": "Authentication failed", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "404": { + "description": "No candidate delta at this nonce", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "409": { + "description": "Candidate already landed on-chain, or account paused/released", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "413": { + "description": "Request body exceeds the configured size limit", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + }, + "429": { + "description": "Rate limit exceeded", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + } + } + }, + "security": [ + { + "x-pubkey": [], + "x-signature": [], + "x-timestamp": [] + } + ] + } + }, "/delta/proposal": { "get": { "tags": [ @@ -3105,6 +3194,53 @@ }, "components": { "schemas": { + "AbandonCandidateRequest": { + "type": "object", + "required": [ + "account_id", + "nonce" + ], + "properties": { + "account_id": { + "type": "string" + }, + "nonce": { + "type": "integer", + "format": "int64", + "description": "Nonce of the candidate delta to abandon. Requiring the explicit\ntarget prevents a stale request from releasing a newer candidate.", + "minimum": 0 + } + } + }, + "AbandonCandidateResponse": { + "type": "object", + "required": [ + "account_id", + "nonce", + "state" + ], + "properties": { + "abandon_requested_at": { + "type": [ + "string", + "null" + ], + "description": "RFC 3339 UTC timestamp of the recorded abandon request. Retries\nreturn the original timestamp; absent once resolved." + }, + "account_id": { + "type": "string" + }, + "nonce": { + "type": "integer", + "format": "int64", + "minimum": 0 + }, + "state": { + "type": "string", + "description": "`\"pending\"` while the worker still has to resolve the intent\n(the account stays locked until then); `\"abandoned\"` once the\ndelta is discarded as client-abandoned and the account released." + } + } + }, "AccountStatus": { "type": "string", "enum": [ @@ -4601,6 +4737,19 @@ "status" ], "properties": { + "abandon_confirm_count": { + "type": "integer", + "format": "int32", + "description": "Consecutive worker ticks that observed the on-chain commitment\nstill at this candidate's base after the abandon was requested.\nReset on a divergent observation, so only an unbroken streak\ncounts — the same stale-RPC shield as `divergence_count`.", + "minimum": 0 + }, + "abandon_requested_at": { + "type": [ + "string", + "null" + ], + "description": "RFC 3339 UTC timestamp of the client's abandon request\n(issue #319). While set, the status remains `candidate` — the\naccount stays locked — until the canonicalization worker\nresolves the intent after the abandon quarantine." + }, "divergence_count": { "type": "integer", "format": "int32", @@ -4648,6 +4797,16 @@ "status" ], "properties": { + "reason": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/DiscardReason" + } + ] + }, "status": { "type": "string", "enum": [ @@ -4662,6 +4821,13 @@ ], "description": "Delta status state machine" }, + "DiscardReason": { + "type": "string", + "description": "Why a delta ended in `Discarded`. Absent on discards that predate\nthe field (and on the worker's retry/divergence discards, which\ndelete the delta instead of transitioning it).", + "enum": [ + "client_abandoned" + ] + }, "EvmProposal": { "type": "object", "required": [ diff --git a/packages/guardian-client/README.md b/packages/guardian-client/README.md index 7bd29f49..5d3fdf90 100644 --- a/packages/guardian-client/README.md +++ b/packages/guardian-client/README.md @@ -69,6 +69,22 @@ console.log('Commitment:', state.commitment); console.log('State data:', state.state_json.data); ``` +### Abandon a Stuck Candidate + +If an approved transaction died client-side after guardian approval, its +candidate keeps the account locked (`409 conflict_pending_delta` on new +proposals). Record an abandon intent and poll for the resolution: + +```typescript +const accepted = await client.abandonCandidate(accountId, nonce); +console.log(accepted.state); // 'pending' + +// The guardian's worker confirms over a short quarantine that the tx did +// not land, then releases the account. +const status = await client.abandonStatus(accountId, nonce); +// 'waiting' | 'landed' | 'abandoned' | 'unexpected' +``` + ### Look Up An Account By Key Commitment When a wallet only holds a signing key, it cannot derive the account ID diff --git a/packages/guardian-client/src/conversion.ts b/packages/guardian-client/src/conversion.ts index ae2e8fe9..515ee23b 100644 --- a/packages/guardian-client/src/conversion.ts +++ b/packages/guardian-client/src/conversion.ts @@ -85,7 +85,7 @@ export function fromServerDeltaStatus(server: ServerDeltaStatus): DeltaStatus { case 'canonical': return { status: 'canonical', timestamp: server.timestamp }; case 'discarded': - return { status: 'discarded', timestamp: server.timestamp }; + return { status: 'discarded', timestamp: server.timestamp, reason: server.reason }; } } @@ -194,7 +194,7 @@ export function toServerDeltaStatus(status: DeltaStatus): ServerDeltaStatus { case 'canonical': return { status: 'canonical', timestamp: status.timestamp }; case 'discarded': - return { status: 'discarded', timestamp: status.timestamp }; + return { status: 'discarded', timestamp: status.timestamp, reason: status.reason }; } } diff --git a/packages/guardian-client/src/http.test.ts b/packages/guardian-client/src/http.test.ts index e4ad95e7..edef3cf6 100644 --- a/packages/guardian-client/src/http.test.ts +++ b/packages/guardian-client/src/http.test.ts @@ -455,6 +455,156 @@ describe('GuardianHttpClient', () => { }); }); + describe('abandonCandidate', () => { + it('should record an abandon intent and map the response to camelCase', async () => { + client.setSigner(mockSigner); + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + account_id: '0x' + 'a'.repeat(30), + nonce: 7, + state: 'pending', + abandon_requested_at: '2026-07-14T12:00:00Z', + }), + }); + + const result = await client.abandonCandidate('0x' + 'a'.repeat(30), 7); + + expect(result).toEqual({ + accountId: '0x' + 'a'.repeat(30), + nonce: 7, + state: 'pending', + abandonRequestedAt: '2026-07-14T12:00:00Z', + }); + + // Wire format is snake_case + expect(mockFetch).toHaveBeenCalledWith( + 'http://localhost:3000/delta/candidate/abandon', + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({ + account_id: '0x' + 'a'.repeat(30), + nonce: 7, + }), + }) + ); + }); + + it('surfaces 409 GUARDIAN_CANDIDATE_LANDED with a parseable error envelope', async () => { + client.setSigner(mockSigner); + + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 409, + statusText: 'Conflict', + text: async () => + JSON.stringify({ + code: 'GUARDIAN_CANDIDATE_LANDED', + message: "This transaction already went through, so it can't be abandoned.", + meta: { retryable: false }, + }), + }); + + const error = await client + .abandonCandidate('0x' + 'a'.repeat(30), 7) + .catch((e) => e); + expect(error).toBeInstanceOf(GuardianHttpError); + expect(error.status).toBe(409); + expect(error.code).toBe('GUARDIAN_CANDIDATE_LANDED'); + }); + + it('surfaces 404 delta_not_found when no candidate exists at the nonce', async () => { + client.setSigner(mockSigner); + + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + statusText: 'Not Found', + text: async () => + JSON.stringify({ + code: 'delta_not_found', + message: "We couldn't find that. It may have been completed or removed.", + meta: { retryable: false }, + }), + }); + + const error = await client + .abandonCandidate('0x' + 'a'.repeat(30), 7) + .catch((e) => e); + expect(error).toBeInstanceOf(GuardianHttpError); + expect(error.status).toBe(404); + expect(error.code).toBe('delta_not_found'); + }); + }); + + describe('abandonStatus', () => { + const serverDelta = (status: object) => ({ + account_id: '0x' + 'a'.repeat(30), + nonce: 7, + prev_commitment: '0x' + 'b'.repeat(64), + delta_payload: { tx_summary: { data: 'base64summary' }, signatures: [] }, + status, + }); + + it('classifies a still-pending candidate as waiting', async () => { + client.setSigner(mockSigner); + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => + serverDelta({ status: 'candidate', timestamp: '2026-07-14T12:00:00Z' }), + }); + expect(await client.abandonStatus('0x' + 'a'.repeat(30), 7)).toBe('waiting'); + }); + + it('classifies a canonicalized delta as landed', async () => { + client.setSigner(mockSigner); + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => + serverDelta({ status: 'canonical', timestamp: '2026-07-14T12:00:00Z' }), + }); + expect(await client.abandonStatus('0x' + 'a'.repeat(30), 7)).toBe('landed'); + }); + + it('classifies a client-abandoned discard as abandoned', async () => { + client.setSigner(mockSigner); + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => + serverDelta({ + status: 'discarded', + timestamp: '2026-07-14T12:00:00Z', + reason: 'client_abandoned', + }), + }); + expect(await client.abandonStatus('0x' + 'a'.repeat(30), 7)).toBe('abandoned'); + }); + + it('classifies a reasonless discard and a missing delta as unexpected', async () => { + client.setSigner(mockSigner); + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => + serverDelta({ status: 'discarded', timestamp: '2026-07-14T12:00:00Z' }), + }); + expect(await client.abandonStatus('0x' + 'a'.repeat(30), 7)).toBe('unexpected'); + + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + statusText: 'Not Found', + text: async () => + JSON.stringify({ + code: 'delta_not_found', + message: "We couldn't find that. It may have been completed or removed.", + meta: { retryable: false }, + }), + }); + expect(await client.abandonStatus('0x' + 'a'.repeat(30), 7)).toBe('unexpected'); + }); + }); + describe('signDeltaProposal', () => { it('should sign a delta proposal', async () => { client.setSigner(mockSigner); diff --git a/packages/guardian-client/src/http.ts b/packages/guardian-client/src/http.ts index 547ecaa1..dfcb55df 100644 --- a/packages/guardian-client/src/http.ts +++ b/packages/guardian-client/src/http.ts @@ -1,4 +1,6 @@ import type { + AbandonCandidateResponse, + AbandonStatus, ConfigureRequest, ConfigureResponse, DeltaObject, @@ -16,6 +18,8 @@ import type { } from './types.js'; import { RequestAuthPayload } from './auth-request.js'; import type { + ServerAbandonCandidateRequest, + ServerAbandonCandidateResponse, ServerDeltaObject, ServerDeltaProposalResponse, ServerLookupResponse, @@ -266,6 +270,65 @@ export class GuardianHttpClient { }; } + /** + * Request abandonment of a pending canonicalization candidate whose + * transaction will never land on-chain (issue #319). + * + * Records an abandon *intent* (`202 Accepted`): the delta stays a + * candidate — the account stays locked — until the guardian's worker + * confirms over the abandon quarantine that the transaction did not + * land, then discards the delta as `client_abandoned` and releases the + * account (typically well under a minute). Poll {@link abandonStatus} + * for the resolution. Refused with `GUARDIAN_CANDIDATE_LANDED` (409) + * when the transaction actually landed. Retries are idempotent and + * preserve the original request timestamp. + */ + async abandonCandidate(accountId: string, nonce: number): Promise { + const serverRequest: ServerAbandonCandidateRequest = { account_id: accountId, nonce }; + const response = await this.fetchAuthenticated('/delta/candidate/abandon', { + method: 'POST', + body: JSON.stringify(serverRequest), + }, accountId, serverRequest); + const server = (await response.json()) as ServerAbandonCandidateResponse; + return { + accountId: server.account_id, + nonce: server.nonce, + state: server.state, + abandonRequestedAt: server.abandon_requested_at, + }; + } + + /** + * Poll the resolution of an abandon request made with + * {@link abandonCandidate}: `'waiting'` while the quarantine runs, + * `'landed'` if the transaction landed after all (the delta + * canonicalized), `'abandoned'` once the delta is discarded as + * client-abandoned and the account released, `'unexpected'` for any + * state no abandon flow produces (including a missing delta). + */ + async abandonStatus(accountId: string, nonce: number): Promise { + let delta: DeltaObject; + try { + delta = await this.getDelta(accountId, nonce); + } catch (e) { + if (e instanceof GuardianHttpError && e.code === 'delta_not_found') { + return 'unexpected'; + } + throw e; + } + switch (delta.status.status) { + case 'candidate': + return 'waiting'; + case 'canonical': + return 'landed'; + case 'discarded': + return delta.status.reason === 'client_abandoned' ? 'abandoned' : 'unexpected'; + default: + return 'unexpected'; + } + } + + async signDeltaProposal(request: SignProposalRequest): Promise { const serverRequest = toServerSignProposalRequest(request); const response = await this.fetchAuthenticated('/delta/proposal', { diff --git a/packages/guardian-client/src/index.ts b/packages/guardian-client/src/index.ts index 0d590af0..e4238d1f 100644 --- a/packages/guardian-client/src/index.ts +++ b/packages/guardian-client/src/index.ts @@ -3,6 +3,8 @@ export type { GuardianErrorMeta } from './http.js'; export { RequestAuthPayload } from './auth-request.js'; export type { + AbandonCandidateResponse, + AbandonStatus, Signer, FalconSignature, EcdsaSignature, diff --git a/packages/guardian-client/src/server-types.ts b/packages/guardian-client/src/server-types.ts index 57e0f6e6..dbda1b10 100644 --- a/packages/guardian-client/src/server-types.ts +++ b/packages/guardian-client/src/server-types.ts @@ -21,7 +21,7 @@ export type ServerDeltaStatus = | { status: 'pending'; timestamp: string; proposer_id: string; cosigner_sigs: ServerCosignerSignature[] } | { status: 'candidate'; timestamp: string } | { status: 'canonical'; timestamp: string } - | { status: 'discarded'; timestamp: string }; + | { status: 'discarded'; timestamp: string; reason?: string }; export type ServerProposalType = | 'add_signer' @@ -137,6 +137,18 @@ export interface ServerProposalsResponse { proposals: ServerDeltaObject[]; } +export interface ServerAbandonCandidateRequest { + account_id: string; + nonce: number; +} + +export interface ServerAbandonCandidateResponse { + account_id: string; + nonce: number; + state: 'pending' | 'abandoned'; + abandon_requested_at?: string; +} + export interface ServerSignProposalRequest { account_id: string; commitment: string; diff --git a/packages/guardian-client/src/types.ts b/packages/guardian-client/src/types.ts index 1282d2dc..d07f05a4 100644 --- a/packages/guardian-client/src/types.ts +++ b/packages/guardian-client/src/types.ts @@ -65,7 +65,7 @@ export type DeltaStatus = | { status: 'pending'; timestamp: string; proposerId: string; cosignerSigs: CosignerSignature[] } | { status: 'candidate'; timestamp: string } | { status: 'canonical'; timestamp: string } - | { status: 'discarded'; timestamp: string }; + | { status: 'discarded'; timestamp: string; reason?: string }; export type ProposalType = | 'add_signer' @@ -187,6 +187,26 @@ export interface SignProposalRequest { signature: ProposalSignature; } +export interface AbandonCandidateResponse { + accountId: string; + nonce: number; + /** + * `'pending'` while the guardian's worker still has to resolve the + * abandon intent (the account stays locked until then); `'abandoned'` + * once the delta is discarded as client-abandoned and the account + * released. + */ + state: 'pending' | 'abandoned'; + /** + * RFC 3339 UTC timestamp of the recorded abandon request. Retries + * return the original timestamp; absent once resolved. + */ + abandonRequestedAt?: string; +} + +/** Resolution of an abandon request, as observed via the delta feed. */ +export type AbandonStatus = 'waiting' | 'landed' | 'abandoned' | 'unexpected'; + export interface PushDeltaResponse { accountId: string; nonce: number; diff --git a/packages/miden-multisig-client/README.md b/packages/miden-multisig-client/README.md index 9e47244a..f906e66a 100644 --- a/packages/miden-multisig-client/README.md +++ b/packages/miden-multisig-client/README.md @@ -153,6 +153,23 @@ for (const p of proposals) { } ``` +### Recover From a Dead Transaction (Abandon) + +If an approved transaction died client-side after guardian approval, the +candidate keeps the account locked on GUARDIAN. Record an abandon intent +and poll for the resolution: + +```typescript +const accepted = await multisig.abandonCandidate(nonce); +console.log(accepted.state); // 'pending' + +// The guardian's worker confirms over a short quarantine (typically well +// under a minute) that the transaction did not land, then releases the +// account. +const status = await multisig.abandonStatus(nonce); +// 'waiting' | 'landed' | 'abandoned' | 'unexpected' +``` + ### Check Proposal Status Returns cached proposals without making a network request: diff --git a/packages/miden-multisig-client/src/multisig.ts b/packages/miden-multisig-client/src/multisig.ts index 9f05ab8f..d4644f63 100644 --- a/packages/miden-multisig-client/src/multisig.ts +++ b/packages/miden-multisig-client/src/multisig.ts @@ -5,7 +5,7 @@ * for proposal management. */ -import { GuardianHttpClient, type DeltaObject, type ProposalSignature, type Signer, type AuthConfig, type StateObject } from '@openzeppelin/guardian-client'; +import { GuardianHttpClient, type AbandonCandidateResponse, type AbandonStatus, type DeltaObject, type ProposalSignature, type Signer, type AuthConfig, type StateObject } from '@openzeppelin/guardian-client'; import type { ConsumableNote, ExportedProposal, @@ -903,6 +903,38 @@ export class Multisig { * * @param proposalId - The proposal commitment/ID (this is also what gets signed) */ + /** + * Request abandonment of a pending canonicalization candidate whose + * transaction will never land on-chain (issue #319) — e.g. after an + * approved transaction died client-side (RPC submit failure, prover + * timeout, crash). + * + * Records an abandon *intent* on GUARDIAN: the account stays locked + * until the guardian's canonicalization worker confirms over a short + * quarantine (typically well under a minute) that the transaction did + * not land, then releases the account. Poll {@link abandonStatus} for + * the resolution. + * + * `nonce` pins the exact candidate to release; it is the nonce the + * proposal was pushed with. Retries are idempotent and preserve the + * original request timestamp. Refused with `GUARDIAN_CANDIDATE_LANDED` + * (409) when the transaction actually landed. + */ + async abandonCandidate(nonce: number): Promise { + return this.guardian.abandonCandidate(this._accountId, nonce); + } + + /** + * Poll the resolution of an abandon request made with + * {@link abandonCandidate}: `'waiting'` while the quarantine runs, + * `'landed'` if the transaction landed after all, `'abandoned'` once + * the account is released, `'unexpected'` for any state no abandon + * flow produces. + */ + async abandonStatus(nonce: number): Promise { + return this.guardian.abandonStatus(this._accountId, nonce); + } + async signProposal(proposalId: string): Promise { const normalizedProposalId = normalizeHexWord(proposalId); const existingProposal = await this.getProposalForSigning(proposalId, normalizedProposalId); diff --git a/spec/api.md b/spec/api.md index dc3baa72..0f9d65c2 100644 --- a/spec/api.md +++ b/spec/api.md @@ -270,6 +270,7 @@ component schemas. | client | `GET /delta/proposal` | signed headers | List pending proposals | | client | `GET /delta/proposal/single` | signed headers | Fetch one proposal by commitment | | client | `PUT /delta/proposal` | signed headers | Add a cosigner signature | +| client | `POST /delta/candidate/abandon` | signed headers | Record an abandon intent for a stuck candidate (202; worker resolves after quarantine) | | dashboard | `GET /auth/challenge` | public | Operator login challenge | | dashboard | `POST /auth/verify` | public | Verify challenge, establish session | | dashboard | `POST /auth/logout` | session | Invalidate the operator session | diff --git a/spec/processes.md b/spec/processes.md index 74ad6c56..623cf6be 100644 --- a/spec/processes.md +++ b/spec/processes.md @@ -254,6 +254,18 @@ sequenceDiagram landed yet), or the comparison itself failed (RPC error): defer within `submission_grace_period_seconds`, then consume retry budget each tick and discard after `max_retries`. + - Matches the candidate's `prev_commitment` AND the candidate carries a + client abandon intent (`abandon_requested_at`, recorded by + `POST /delta/candidate/abandon`): count the observation toward the + abandon quarantine instead — this takes precedence over the grace + deferral. After `abandon_quarantine_checks` consecutive at-base + observations (default 2) AND `abandon_quarantine_seconds` since the + request (default 15, so a late-landing transaction can surface), + delete the matching proposal, transition the delta to + `discarded` with reason `client_abandoned` (preserved as history), + and clear the pending-candidate flag. A divergent observation resets + the abandon-confirmation streak; a landed transaction always wins + and canonicalizes normally. - Matches neither — the account advanced past the candidate's base state, so the candidate can never verify: after `divergence_confirmations` consecutive such observations (default 2,