Skip to content

abandon candidate endpoint#327

Merged
haseebrabbani merged 8 commits into
mainfrom
319-abandon-candidate
Jul 23, 2026
Merged

abandon candidate endpoint#327
haseebrabbani merged 8 commits into
mainfrom
319-abandon-candidate

Conversation

@haseebrabbani

@haseebrabbani haseebrabbani commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Closes #319

Problem

A candidate whose transaction dies client-side after approval (RPC submit failure, prover timeout, crash) looks identical to one that is slowly proving — on-chain stays at prev_commitment, so neither the grace period nor the #312 divergence release helps. The account stays locked for grace + retries (~18 min), with every new proposal answered 409 conflict_pending_delta. Only the client knows the transaction is dead.

What's new

POST /delta/candidate/abandon / gRPC AbandonDeltaCandidate — signature-authenticated (same header model as push_delta_proposal), body {account_id, nonce}; the nonce pins the exact candidate so a stale request can't release a newer one.

The service refuses with the new 409 GUARDIAN_CANDIDATE_LANDED when on-chain already matches the candidate's expected post-state (the tx landed; the worker will canonicalize it), and fails closed on an on-chain read error. Otherwise it deletes the candidate delta + matching proposal and clears has_pending_candidate, all as hard errors — a 200 means the account is released. delete_delta is the linearization point: a delta_not_found on retry after a 5xx means the abandon already succeeded.

Also included:

  • remove_candidate extracted to jobs/canonicalization/removal.rs with RemovalMode { BestEffort, Strict }; worker behavior unchanged.
  • Worker self-heal for a stale has_pending_candidate flag (deferred from the fix(server): release diverged canonicalization candidates instead of … #312 review).
  • Metric candidate_outcome="abandoned"; gRPC label allowlist entry; OpenAPI regenerated.
  • Client methods: GuardianClient::abandon_candidate, MultisigClient::abandon_candidate(nonce), and TS abandonCandidate(accountId, nonce).

Summary by CodeRabbit

  • New Features
    • Added support for abandoning pending canonicalization candidates through the multisig client and Guardian HTTP/gRPC APIs.
    • Added the POST /delta/candidate/abandon endpoint, accepting an account ID and nonce and returning abandonment details.
    • Added client library support with typed responses and consistent error handling.
  • Bug Fixes
    • Prevented abandonment when a candidate has already landed on-chain.
    • Automatically clears stale pending-candidate status when no candidate remains.
  • Documentation
    • Updated OpenAPI specifications for the new endpoint, request, response, and error conditions.

A candidate whose transaction dies client-side after approval (RPC submit
failure, prover timeout, crash) is indistinguishable from one that is
slowly proving, so the account stays locked for the full submission grace
period plus retry budget (~18 min) with every new proposal answered
409 conflict_pending_delta. Only the client knows the transaction is
dead: POST /delta/candidate/abandon (gRPC AbandonDeltaCandidate) lets it
release the account immediately.

- services/abandon_candidate: resolve_account signature auth, pause gate,
  fail-closed landed guard (on-chain Match -> 409 GUARDIAN_CANDIDATE_LANDED),
  pre-delete re-check, Strict removal so a 200 means released
- extract worker remove_candidate into jobs/canonicalization/removal.rs
  with RemovalMode {BestEffort, Strict}; worker behavior unchanged
- worker self-heals a stale has_pending_candidate flag (follow-up from
  the #312 review)
- new error CandidateLanded (409 / FailedPrecondition), metric label
  candidate_outcome=abandoned, gRPC method allowlist entry, OpenAPI regen
- e2e tests replay the issue's reproduction: stuck candidate -> 409 loop
  -> abandon -> same proposal accepted; landed -> refusal -> canonicalize
- guardian-client: AbandonDeltaCandidate rpc in the client proto copy,
  GuardianClient::abandon_candidate, mock service coverage
- miden-multisig-client: MultisigClient::abandon_candidate(nonce) for the
  wallet-side recovery path after a post-approval transaction death
- packages/guardian-client: abandonCandidate on GuardianHttpClient with
  wire-shape and error-envelope tests
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: deead5d9-0738-4e4c-833d-cc1d77eb846e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

This PR adds authenticated candidate-abandonment APIs across Guardian’s HTTP, gRPC, Rust, and TypeScript clients. The server validates on-chain state, removes stranded candidates with shared cleanup logic, records abandonment metrics, heals stale metadata, and adds unit and end-to-end tests.

Changes

Candidate abandonment flow

Layer / File(s) Summary
Contracts and client integrations
crates/client/proto/guardian.proto, crates/client/src/..., crates/miden-multisig-client/..., packages/guardian-client/src/...
Adds protobuf messages and RPCs, Rust and TypeScript client methods, response mappings, and mock support.
Strict candidate removal service
crates/server/src/services/..., crates/server/src/jobs/canonicalization/removal.rs, crates/server/src/error.rs, crates/server/src/metrics/...
Validates candidate state and on-chain commitments, prevents removal after landing, performs strict cleanup, adds error mappings, and records the abandoned outcome.
HTTP and gRPC endpoint wiring
crates/server/src/api/..., crates/server/src/builder/..., crates/server/src/openapi.rs, docs/openapi*.json
Registers authenticated HTTP and gRPC handlers, maps service results and errors, and documents the new endpoint and schemas.
Canonicalization cleanup integration
crates/server/src/jobs/canonicalization/...
Shares candidate-removal logic with the worker and clears stale pending-candidate metadata when no candidate exists.
End-to-end validation
crates/server/src/testing/e2e/..., packages/guardian-client/src/http.test.ts
Tests stranded-candidate release, landed-candidate refusal, client request mapping, and HTTP error-code handling.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant GuardianHTTP
  participant AbandonCandidateService
  participant Network
  participant CandidateStorage
  Client->>GuardianHTTP: POST /delta/candidate/abandon
  GuardianHTTP->>AbandonCandidateService: authenticated account_id and nonce
  AbandonCandidateService->>Network: verify expected post-state
  Network-->>AbandonCandidateService: commitment result
  AbandonCandidateService->>CandidateStorage: delete candidate and clear metadata
  CandidateStorage-->>AbandonCandidateService: cleanup result
  AbandonCandidateService-->>GuardianHTTP: abandonment result or error
  GuardianHTTP-->>Client: response or mapped error
Loading

Possibly related PRs

Suggested reviewers: zeljkox, mcarlomagno, zeljkox

Poem

I’m a rabbit, and candidates no longer stay,
When stranded in limbo, they hop away.
A nonce points true, the chain gives a sign,
Cleanup runs neatly, response fields align.
The account is released with a metrics-filled cheer—
“Abandoned!” I wiggle my whiskers and ears.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding an abandon candidate endpoint.
Linked Issues check ✅ Passed The PR implements the requested client-initiated abandon/release flow, including the endpoint, landed-state guard, and account release behavior from #319.
Out of Scope Changes check ✅ Passed The remaining changes are supporting pieces for the same feature, such as client updates, tests, OpenAPI regeneration, and removal refactoring.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 319-abandon-candidate

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
crates/server/src/testing/e2e/abandon_candidate.rs (1)

336-343: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Exercise the public HTTP and gRPC transports in the E2E coverage.

These calls invoke services::abandon_candidate directly, so the new route/method, signed request payload, HTTP 409, gRPC error fields, and response serialization remain untested. Add transport-level success and CandidateLanded cases for both APIs.

Also applies to: 395-402

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/server/src/testing/e2e/abandon_candidate.rs` around lines 336 - 343,
Update the E2E tests around abandon_candidate to call the public HTTP and gRPC
transports instead of invoking services::abandon_candidate directly. Add
transport-level success and CandidateLanded cases for both APIs, using signed
request payloads and asserting the HTTP 409 status, gRPC error fields, and
serialized responses as applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/server/src/jobs/canonicalization/processor.rs`:
- Around line 99-124: In the stale-flag handling within the candidate-processing
method, recheck candidate storage immediately before clearing
has_pending_candidate, and only clear the flag if candidates are still absent;
otherwise preserve the flag and continue processing the newly found candidate.
Ensure the existence check and conditional metadata update are atomic where
supported, avoiding a race between the recheck and set_has_pending_candidate.

In `@crates/server/src/jobs/canonicalization/removal.rs`:
- Around line 70-74: Update the proposal lookup logic in the removal flow around
storage_backend.pull_delta_proposal so it distinguishes a missing proposal from
other storage failures. Preserve the existing absent-proposal behavior, but
propagate backend errors as StorageError in Strict mode instead of treating them
as absent; only continue the cleanup path when the proposal is found or
genuinely not found.

In `@crates/server/src/services/abandon_candidate.rs`:
- Around line 58-65: Update the delta reads in the abandon-candidate flow,
including the `pull_delta` calls around both reported locations, to preserve
genuine storage/backend errors. Map only the explicit missing-record result to
`GuardianError::DeltaNotFound` with the existing account and nonce context;
propagate all other errors unchanged.
- Around line 110-125: Make the validation and deletion in the candidate-removal
flow atomic by updating remove_candidate or its storage operation to
conditionally delete only when the delta is still in candidate status, using an
account-scoped lock or storage transaction as needed. Preserve the existing
CandidateLanded error behavior when canonicalization occurs concurrently, and
ensure the proposal is removed only after the conditional candidate deletion
succeeds.

---

Nitpick comments:
In `@crates/server/src/testing/e2e/abandon_candidate.rs`:
- Around line 336-343: Update the E2E tests around abandon_candidate to call the
public HTTP and gRPC transports instead of invoking services::abandon_candidate
directly. Add transport-level success and CandidateLanded cases for both APIs,
using signed request payloads and asserting the HTTP 409 status, gRPC error
fields, and serialized responses as applicable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: af6ae204-eda7-4da3-8490-0d0352dae31d

📥 Commits

Reviewing files that changed from the base of the PR and between 20141df and 32c23b7.

📒 Files selected for processing (25)
  • crates/client/proto/guardian.proto
  • crates/client/src/client.rs
  • crates/client/src/testing/mocks.rs
  • crates/miden-multisig-client/src/client/proposals.rs
  • crates/server/proto/guardian.proto
  • crates/server/src/api/grpc.rs
  • crates/server/src/api/http.rs
  • crates/server/src/builder/handle.rs
  • crates/server/src/error.rs
  • crates/server/src/jobs/canonicalization/mod.rs
  • crates/server/src/jobs/canonicalization/processor.rs
  • crates/server/src/jobs/canonicalization/removal.rs
  • crates/server/src/metrics/labels.rs
  • crates/server/src/metrics/names.rs
  • crates/server/src/openapi.rs
  • crates/server/src/services/abandon_candidate.rs
  • crates/server/src/services/mod.rs
  • crates/server/src/testing/e2e/abandon_candidate.rs
  • crates/server/src/testing/e2e/mod.rs
  • docs/openapi-client.json
  • docs/openapi.json
  • packages/guardian-client/src/http.test.ts
  • packages/guardian-client/src/http.ts
  • packages/guardian-client/src/server-types.ts
  • packages/guardian-client/src/types.ts

Comment thread crates/server/src/jobs/canonicalization/processor.rs Outdated
Comment on lines +70 to +74
if let Some(ref id) = proposal_id
&& let Ok(_existing_proposal) = storage_backend
.pull_delta_proposal(&delta.account_id, id)
.await
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Propagate proposal lookup failures in Strict mode.

if let Ok(...) treats storage failure as “proposal absent.” The abandon request can return success while its matching proposal remains pending, violating strict cleanup semantics. Distinguish not-found from backend errors and return StorageError for the latter.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/server/src/jobs/canonicalization/removal.rs` around lines 70 - 74,
Update the proposal lookup logic in the removal flow around
storage_backend.pull_delta_proposal so it distinguishes a missing proposal from
other storage failures. Preserve the existing absent-proposal behavior, but
propagate backend errors as StorageError in Strict mode instead of treating them
as absent; only continue the cleanup path when the proposal is found or
genuinely not found.

Comment thread crates/server/src/services/abandon_candidate.rs Outdated
Comment thread crates/server/src/services/abandon_candidate.rs Outdated
Comment thread crates/server/src/jobs/canonicalization/processor.rs Fixed
@codecov-commenter

codecov-commenter commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.67939% with 109 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@a30ffa4). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #327   +/-   ##
=======================================
  Coverage        ?   78.60%           
=======================================
  Files           ?      168           
  Lines           ?    33202           
  Branches        ?        0           
=======================================
  Hits            ?    26100           
  Misses          ?     7102           
  Partials        ?        0           

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a30ffa4...96afd8a. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Distinguish missing records from backend failures on both delta reads
  in the abandon service: a storage outage now surfaces as a 5xx
  StorageError instead of delta_not_found, which the retry contract
  treats as proof of success. Reads go through pull_deltas_after, which
  errors only on backend failure.
- Strict-mode proposal cleanup no longer treats a proposal-store read
  failure as "no proposal": the check uses pull_all_delta_proposals and
  propagates backend errors as hard StorageErrors, while a genuinely
  absent proposal (direct-push candidates have none) still passes.
- Close the flag-clear vs concurrent-commit race: clearing
  has_pending_candidate now re-reads the delta store and restores the
  flag if a candidate was committed the moment the 409 gate opened
  (delta_commit writes the delta before setting the flag, so the re-read
  is guaranteed to see it). Applied to the worker discard path, the
  canonicalize-success path, and the stale-flag self-heal.
- Add transport-level tests: HTTP handler success + 409
  GUARDIAN_CANDIDATE_LANDED envelope, gRPC handler success + error_code,
  plus service tests for the storage-failure, strict proposal-check, and
  flag-restore behaviors.
… race

Complete fix for the delete-after-canonicalize race flagged in review:
remove_candidate's status check and delete were separate steps, so the
worker could canonicalize a delta between the abandon service's re-read
and the delete, and the abandon would then delete a canonical delta.

- New StorageBackend::delete_delta_if_candidate: Postgres implements it
  as a single conditional DELETE on the indexed status_kind column
  (atomic even across server replicas); the filesystem backend does a
  read-check-delete under a new delta write lock shared with
  submit_delta/update_delta_status (single-process backend, so an
  in-process mutex suffices). Metrics and encryption decorators
  delegate; the mock records into the existing delete recorder.
- remove_candidate deletes through the conditional op. Ok(false) means
  the delta is gone or no longer a candidate: BestEffort (worker) skips
  quietly - the concurrent actor owns the cleanup; Strict re-reads and
  maps to CandidateLanded or delta_not_found. The service's pre-delete
  re-read is gone - the atomic delete subsumes it.
- Delta and proposal reads use the shared is_storage_not_found heuristic
  to distinguish absent records from backend failures on the O(1)
  single-record reads (mock default errors updated to match).
- Tests: filesystem conditional-delete behaviors (candidate deleted,
  canonical spared, missing false), service race paths through the
  conditional delete (CandidateLanded / delta_not_found).
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Fixed
Comment thread crates/server/src/storage/filesystem.rs Fixed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Fixed
Comment thread crates/server/src/storage/filesystem.rs Fixed
Comment thread crates/server/src/storage/filesystem.rs Fixed

@zeljkoX zeljkoX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this one.

I am proposing changes and slightly different architecture. Main idea is that users should send intent to abandon some candidate but resolution should be left to the canonicalization worker.

Details:

Abandon-as-Intent Architecture

Extend DeltaStatus::Candidate with:

  • abandon_requested_at: Option<String>
  • abandon_confirm_count: u32

Use #[serde(default)] for compatibility. The status remains candidate, so the account stays locked until the worker resolves it.

Endpoint

POST /delta/candidate/abandon:

  1. Authenticates and validates the candidate nonce.
  2. Returns 409 GUARDIAN_CANDIDATE_LANDED if already landed.
  3. Atomically records abandonment intent without overwriting worker counters.
  4. Returns 202 Accepted.

Retries must preserve the original request time and confirmation count. RPC failure should not prevent marking intent because the operation is non-destructive.

Worker

The canonicalization worker remains the sole lifecycle owner:

  • Expected commitment → canonicalize.
  • Previous commitment → increment abandonment confirmations.
  • Previous commitment after the quarantine duration and required checks → abandon.
  • Divergent commitment → reset abandonment confirmations and use existing divergence handling.
  • RPC failure → retain and retry.

Suggested defaults:

  • abandon_quarantine_seconds = 30
  • abandon_quarantine_checks = 2

Terminal State

Preserve the delta as history instead of deleting it:

Discarded {
    timestamp: String,
    reason: DiscardReason::ClientAbandoned,
}

Delete/finalize the matching proposal first, transition the delta to discarded, then clear has_pending_candidate. Retry cleanup failures on the next worker run.

Clients

Rust and TypeScript clients should expose abandonment and polling helpers:
Candidate with intent → waiting
Canonical → landed
Discarded as client-abandoned → successfully abandoned
Missing delta → unexpected

The quarantine reduces late-landing risk but cannot eliminate it without a transaction expiration boundary.

/// 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<()> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the TS multisig client hasn't been extended with this new method yet, which breaks SDK parity.

Comment thread docs/openapi.json
]
}
},
"/delta/candidate/abandon": {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in addition to openapi files let's extend docs and spec dir where needed. Also README.md files for each crate/package should have updated docs with new methods.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a client-initiated “abandon candidate” capability to GUARDIAN (server + clients) so wallets can immediately release an account when an approved candidate tx dies client-side and would otherwise hold the conflict_pending_delta lock for the full grace+retry window (issue #319).

Changes:

  • Added POST /delta/candidate/abandon and gRPC AbandonDeltaCandidate, including a new stable conflict code GUARDIAN_CANDIDATE_LANDED when the candidate already landed on-chain.
  • Introduced StorageBackend::delete_delta_if_candidate as the linearization point for safe concurrent worker/client candidate removal; refactored candidate removal into jobs/canonicalization/removal.rs.
  • Updated Rust + TS clients and added tests (including an e2e reproduction) plus metrics label candidate_outcome="abandoned" and regenerated OpenAPI specs.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/guardian-client/src/types.ts Adds the client-facing AbandonCandidateResponse type.
packages/guardian-client/src/server-types.ts Adds snake_case wire request/response types for abandon-candidate.
packages/guardian-client/src/http.ts Implements abandonCandidate(accountId, nonce) with authenticated request signing.
packages/guardian-client/src/http.test.ts Adds HTTP client tests for success + 409/404 error envelope handling.
docs/openapi.json Regenerated OpenAPI including the new client endpoint and schemas.
docs/openapi-client.json Regenerated client-focused OpenAPI including the new endpoint and schemas.
crates/server/src/testing/mocks.rs Extends storage mock with delete_delta_if_candidate and adjusts not-found messages.
crates/server/src/testing/e2e/mod.rs Registers new e2e module for abandon-candidate.
crates/server/src/testing/e2e/abandon_candidate.rs Adds end-to-end tests reproducing #319 and validating the landed-guard behavior.
crates/server/src/storage/postgres.rs Implements atomic conditional delete for candidate rows in Postgres.
crates/server/src/storage/mod.rs Adds delete_delta_if_candidate to the StorageBackend trait and documents linearization semantics.
crates/server/src/storage/filesystem.rs Implements conditional delete with an async mutex to serialize status writes vs delete.
crates/server/src/storage/encryption/decorator.rs Forwards delete_delta_if_candidate through the encrypted storage decorator.
crates/server/src/services/mod.rs Wires the new abandon-candidate service module into the services surface.
crates/server/src/services/abandon_candidate.rs Implements the abandon-candidate service flow and unit tests.
crates/server/src/openapi.rs Adds the new HTTP handler into the OpenAPI generator set.
crates/server/src/metrics/storage.rs Instruments the new storage call for timing metrics.
crates/server/src/metrics/names.rs Adds gRPC method to the known-method allowlist.
crates/server/src/metrics/labels.rs Adds CandidateOutcome::Abandoned label value and extends label tests.
crates/server/src/jobs/canonicalization/removal.rs Extracts candidate removal + flag-clear logic with strict vs best-effort modes.
crates/server/src/jobs/canonicalization/processor.rs Uses extracted removal helpers and adds worker self-heal for stale pending-candidate flags.
crates/server/src/jobs/canonicalization/mod.rs Exposes removal helpers within the canonicalization job module.
crates/server/src/error.rs Introduces GuardianError::CandidateLanded with stable code and status mappings.
crates/server/src/builder/handle.rs Adds the new HTTP route /delta/candidate/abandon to the server router.
crates/server/src/api/http.rs Adds HTTP request/response shapes + handler + utoipa docs + tests for abandon-candidate.
crates/server/src/api/grpc.rs Adds gRPC implementation + tests for AbandonDeltaCandidate.
crates/server/proto/guardian.proto Adds gRPC service method and request/response messages for abandon-candidate.
crates/miden-multisig-client/src/client/proposals.rs Adds MultisigClient::abandon_candidate(nonce) convenience method.
crates/client/src/testing/mocks.rs Extends Rust gRPC client mock service with abandon-candidate support.
crates/client/src/client.rs Adds GuardianClient::abandon_candidate(account_id, nonce) gRPC client method.
crates/client/proto/guardian.proto Updates client-side proto definition to match server gRPC changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +93 to +97
let current_state = resolved
.storage
.pull_state(&account_id)
.await
.map_err(|_| GuardianError::StateNotFound(account_id.clone()))?;
@haseebrabbani

Copy link
Copy Markdown
Collaborator Author

@zeljkoX agreed — this is a better architecture, will adjust the PR to it. Two points to confirm:

  1. Grace-period precedence: handle_unverified_candidate currently defers any unverified candidate inside submission_grace_period_seconds without consuming retries. I'll make the abandon-resolution branch take precedence over that deferral (quarantine timing only), otherwise an abandon would still wait out the grace window — flagging in case you intended otherwise.
  2. Scope of Discarded { reason }: should the existing max-retries and divergence discards (which currently hard-delete the delta) also move to reason-carrying Discarded in this PR, or keep deletion there and introduce the status only for ClientAbandoned for now?

Also planning: intent write preserves retry_count/divergence_count and is idempotent on retry (original abandon_requested_at kept); a divergent observation resets abandon_confirm_count and defers to the existing divergence handling; endpoint returns 202 with the current candidate state so clients can poll.

Conflicts: jobs/canonicalization/processor.rs and metrics/storage.rs.
Resolution takes main's side for the processor internals — the worker's
remove_candidate now goes through the fenced, transactional
discard_candidate storage op, and promote_candidate owns the flag clear —
and keeps both sides' storage-trait surface.

Note: #301's candidate lifecycle primitives (discard_candidate with
conditional status delete + clear_pending_candidate_if_none, lease
fencing) supersede this branch's delete_delta_if_candidate /
removal.rs machinery, which is now only used by the abandon service's
synchronous path. That path is being reworked to the abandon-as-intent
architecture from PR review, at which point the superseded machinery
is removed.
…on (#319)

Rework the abandon endpoint to the intent architecture from PR review:
users send an intent to abandon; resolution belongs to the
canonicalization worker, the sole owner of candidate lifecycles under
the #301 lease model (fenced backends refuse candidate writes from
anything but the lease holder, so service-side deletion was a dead end).

- DeltaStatus::Candidate gains abandon_requested_at /
  abandon_confirm_count (serde defaults; wire/storage compatible);
  Discarded gains reason (DiscardReason::ClientAbandoned only - the
  worker's retry/divergence discards keep their delete behavior).
- POST /delta/candidate/abandon now returns 202 Accepted with
  {state: pending|abandoned, abandon_requested_at}. The write is the new
  unfenced-but-conditional StorageBackend::request_candidate_abandon
  (postgres: row-locked transaction touching only the intent field, so
  worker-owned counters are never overwritten; filesystem: under the
  delta write lock). Retries preserve the original request timestamp.
  The landed guard remains, but an RPC failure no longer blocks the
  request - the intent is non-destructive and the worker re-verifies.
- Worker resolution: an at-base observation on a candidate with intent
  counts toward the abandon quarantine (abandon_quarantine_seconds=30,
  abandon_quarantine_checks=2), taking precedence over the grace
  deferral; a divergent observation resets the streak; a landed
  transaction always wins and canonicalizes. Finalization deletes the
  proposal, transitions the delta to Discarded{client_abandoned}
  (preserved as history), and clears the pending-candidate flag;
  cleanup failures retry on the next run.
- Superseded machinery removed: removal.rs, delete_delta_if_candidate,
  the flag-clear helper (main's clear_pending_candidate_if_none and the
  fenced candidate ops own those concerns now).
- Wire: DeltaStatus proto gains additive discard_reason so clients can
  classify resolutions; clients (guardian-client, miden-multisig-client,
  TS packages guardian-client + miden-multisig-client) expose the
  abandon request plus polling helpers (waiting/landed/abandoned/
  unexpected). TS miden-multisig-client parity added per review.
- Docs per review: spec/api.md endpoint table, spec/processes.md worker
  behavior, CONCEPTS.md failure table, MULTISIG_SDK.md API tables, and
  READMEs for all four client crates/packages.
- Tests: serde compatibility for pre-intent statuses, service intent
  semantics, worker quarantine (confirm/finalize/age-gate/grace
  precedence/landed-wins), filesystem intent op, transport shapes, e2e
  repro reworked to the intent flow, TS suite extended.
@haseebrabbani

Copy link
Copy Markdown
Collaborator Author

@zeljkoX I reworked to the abandon-as-intent architecture. The intent write is a new unfenced-but-conditional request_candidate_abandon storage op that touches only abandon_requested_at — worker counters are never overwritten.

Implemented per your spec:

  • DeltaStatus::Candidate gains abandon_requested_at / abandon_confirm_count (#[serde(default)], so pre-existing candidates keep deserializing); the status stays candidate and the account stays locked until the worker resolves it.
  • Endpoint returns 202 with {state: "pending" | "abandoned", abandon_requested_at}; still 409 GUARDIAN_CANDIDATE_LANDED when already landed; retries are idempotent and preserve the original request timestamp; an RPC failure no longer blocks the request (the intent is non-destructive and the worker re-verifies before finalizing).
  • Worker: expected commitment → canonicalize (landed always wins, intent or not); at-base → count a confirmation; divergent → reset the confirmation streak and use the existing divergence handling; RPC failure → retain and retry. Defaults abandon_quarantine_seconds = 30, abandon_quarantine_checks = 2.
  • Terminal state: proposal finalized first, then Discarded { reason: ClientAbandoned } (preserved as history), then the pending-candidate flag clears; cleanup failures retry on the next run.
  • Clients: all four (Rust GuardianClient + MultisigClient, TS guardian-client + miden-multisig-client) expose the abandon request plus a polling helper classifying waiting / landed / abandoned / unexpected, via a new additive discard_reason field on the proto DeltaStatus.
  • Docs: spec/api.md, spec/processes.md (worker behavior), CONCEPTS.md failure table, MULTISIG_SDK.md API tables, READMEs for all four client crates/packages, OpenAPI regenerated.

Two judgment calls to confirm:

  1. Grace precedence: the abandon quarantine bypasses the submission-grace deferral (otherwise an abandon would wait out most of the 600s grace). Pinned by test_abandon_intent_confirms_and_bypasses_grace.
  2. Discarded { reason } scope: only ClientAbandoned, written solely by the abandon finalizer; the worker's retry/divergence discards keep their delete behavior.

Note on TS parity: packages/miden-multisig-client depends on the registry guardian-client@^0.15.0, so its new methods typecheck against the next published guardian-client (verified locally against the built dist).

Comment thread crates/server/src/jobs/canonicalization/processor.rs Dismissed
Comment thread crates/server/src/jobs/canonicalization/processor.rs Dismissed
Comment thread crates/server/src/jobs/canonicalization/processor.rs Dismissed
Comment thread crates/server/src/jobs/canonicalization/processor.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed

@zeljkoX zeljkoX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks for hard work here!

Left nit comment and seems like test are failing.

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: 30, // Let a late-landing tx surface first

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about lowering a bit, 15s? So it does not affect UX too much...

…te status writes

Review finding: worker counter writes (retry increment, divergence
increment/reset) overwrite the whole status blob from a tick-start
snapshot, so a client abandon intent recorded mid-tick could be silently
wiped - the client would then poll Waiting forever while the account
stayed locked for the full grace+retry window.

update_candidate_status now carries a stored abandon_requested_at into
any candidate-status overwrite that lacks one:
- postgres: row-locked SELECT of the stored intent inside the fenced
  transaction (serialized against request_candidate_abandon's row lock),
  merged before the UPDATE
- filesystem: the override does a read-modify-write under the delta
  write lock (also gaining the NotCandidate outcome it previously
  lacked)
- mock/sequential fallback: best-effort merge via pull_delta

Only abandon_requested_at is preserved: confirmation-streak resets are
legitimate worker writes, and terminal statuses drop the intent by
design. Also lowers abandon_quarantine_seconds 30 -> 15 per review.

Regression tests: DeltaStatus merge helper, filesystem clobber race
(intent recorded between snapshot and counter write survives), and
non-candidate NotCandidate outcome.
@haseebrabbani

Copy link
Copy Markdown
Collaborator Author

Thanks! Nit applied in 8c553df (abandon_quarantine_seconds 30 → 15, spec updated). That commit also fixes a race found in a follow-up review pass: worker counter writes overwrote the full status blob from a tick-start snapshot and could wipe a concurrently recorded abandon_requested_atupdate_candidate_status now merges the stored intent under the same row lock / delta write lock that request_candidate_abandon takes (regression-tested at the filesystem layer).

Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed
Comment thread crates/server/src/storage/filesystem.rs Dismissed

@zeljkoX zeljkoX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Let's resolve conflicts and merge this one.

…326)

Conflicts in nine files, all keep-both plus re-integration:

- #304 error contract: CandidateLanded registered in the new
  {code, message, meta} wire shape - user_message ('This transaction
  already went through, so it can't be abandoned.'), retryable=false,
  no extra meta - and added to the exhaustive sanitization test list
  (along with AccountReleased, which that list was missing). TS abandon
  tests updated to mock the new envelope.
- #326 lock-free NetworkClient: the abandon service's landed guard now
  computes the expected commitment on the background reconstructor and
  checks it via verify_commitment; the worker's abandon finalizer calls
  delta_proposal_id directly. StateVerification::Absent (account not
  yet on chain) counts as did-not-land evidence in both the service
  guard and the worker's abandon-confirmation dispatch - without the
  latter, a dead FIRST transaction could never be abandoned.
- #326 processor shape: the stale-flag self-heal re-ported onto the
  indexed pull_candidate_deltas read; abandon quarantine config fields
  merged alongside max_concurrent_accounts; the landed-wins test
  adapted to the fenced promote path (StaleBase gate needs the stored
  state at the candidate's base).
@haseebrabbani
haseebrabbani merged commit 5bc7e06 into main Jul 23, 2026
17 checks passed
@haseebrabbani
haseebrabbani deleted the 319-abandon-candidate branch July 23, 2026 14:37
@github-project-automation github-project-automation Bot moved this from Backlog to Release Candidate in OZ Development for Miden Jul 23, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Release Candidate

Development

Successfully merging this pull request may close these issues.

Stranded candidate (tx never lands) locks the account for grace+retries — follow-up to #312: client-initiated abandon/release

5 participants