Skip to content

Cancel proposal flow#512

Open
dewabisma wants to merge 6 commits into
beast/execute-proposal-flowfrom
beast/cancel-proposal-flow
Open

Cancel proposal flow#512
dewabisma wants to merge 6 commits into
beast/execute-proposal-flowfrom
beast/cancel-proposal-flow

Conversation

@dewabisma

@dewabisma dewabisma commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add cancel button and implementation to proposal details sheet
  • Show cancel proposal event in proposer activity

Screenshots

  • Details sheet
Simulator Screenshot - iPhone 8 - 2026-06-10 at 13 01 30
  • Confirm cancelling
Simulator Screenshot - iPhone 8 - 2026-06-10 at 13 01 33

Note

Medium Risk
Submits signed chain extrinsics and adjusts effective balance for pending cancel fees; behavior mirrors existing multisig execution paths with moderate surface area across SDK and wallet UI.

Overview
Adds an end-to-end multisig proposal cancel path for the proposer, aligned with existing approve/execute flows.

SDK: multisig.cancel (fee estimate + submit), MultisigProposalCancelledEvent / PendingMultisigCancellationEvent, indexer GraphQL for cancelled proposals, and lookup by extrinsic hash.

App: Proposers get a Cancel Proposal action on the detail sheet (confirm sheet with fee + biometrics) → optimistic pending state → indexer polling → reconciliation into the proposer’s activity. Pending cancellations feed activity lists, effective balance (member cost), deduplication in combineAndDeduplicateTransactions, toasts on submit failure/timeout, and EN/ID strings.

Proposal detail UI is refactored so primary actions and status notes are separate; cancelled/executed terminal proposals hide action buttons.

Reviewed by Cursor Bugbot for commit c9c6792. Configure here.

@dewabisma dewabisma requested a review from n13 June 10, 2026 05:02

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c9c6792. Configure here.

return event;
}
}
return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pending cancel ignored for signers

Medium Severity

findPendingCancellationForProposal only matches when the viewer’s account equals event.proposerId, so co-signers never see an in-flight cancellation. Approve and execute buttons still use canApprove / canExecute without checking any proposal-level pending cancel, so signers (and a proposer on a threshold-ready proposal) can submit conflicting extrinsics while cancellation is confirming.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c9c6792. Configure here.

@n13

n13 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

The review is complete. Here's my assessment of PR #512.

Review: Cancel proposal flow (PR #512)

Setup: Reviewed in worktree /private/tmp/pr-reviews/quantus-pr-512-review at c9c67921, diffed against base beast/execute-proposal-flow (+1282/−82, 34 files).

Verification:

  • flutter analyze clean on both quantus_sdk and mobile-app
  • mobile-app unit tests pass (8/8)
  • quantus_sdk/test/multisig_service_test.dart has 4 failures (FormatException: Cannot parse DateTime from Null in MultisigProposal.fromIndexerJson) — but these fail identically on the base branch, so they're pre-existing, not from this PR. The new buildCancelCall test passes.

Overall

Solid, well-structured PR. It faithfully mirrors the existing approve/execute architecture: optimistic pending event → background submit → indexer polling with soft-timeout fallback → reconciliation into activity. Gating is correct: cancel is proposer-only, hidden on terminal proposals, and disabled for expired ones (correct, since the pallet has a separate removeExpired path for deposit reclamation). Effective balance accounts for the pending cancel fee, dedup keys handle the pending→indexed swap, and both EN and ID locales are complete.

Issue 1 — Race: approve/execute stay enabled while cancel is in flight (confirms Bugbot, medium)

In _actionButtons, the primary button doesn't know about the pending cancellation:

    final isPending = pendingApproval != null;
    final canApprove = isActionable && !didApprove && !isPending && hasLocalSigner;

canApprove/canExecute ignore pendingCancellation, so after the proposer submits a cancel, the Execute button (on a threshold-ready proposal) remains tappable until the indexer confirms — allowing conflicting extrinsics from the same device. The chain will reject one, but the user pays a failed-extrinsic fee and gets a confusing error. Fix: pass pendingCancellation into _approveButton/_executeButton and fold it into the canApprove/canExecute conditions (and symmetrically, the execute flow should probably disable cancel — it already does via isPending).

Note: Bugbot's "co-signers never see it" angle is largely moot — pending cancellations are device-local state, so other signers' devices can't see them regardless. The same-device race is the real defect.

Issue 2 — DRY: third copy of the confirm sheet

multisig_cancel_confirm_sheet.dart is ~85% identical to the approve and execute confirm sheets (after normalizing names, only ~27 of ~180 lines differ). This is now the third copy of the same fee-estimate → biometric-auth → submit → error-handling sheet. Given the repo's strict DRY rule, these should be collapsed into one parameterized sheet (title/body/labels, fee estimator callback, submit callback, button variant). Same applies to searchCancelledByExtrinsicHash in chain_history_service.dart — the fourth structurally identical search-by-hash method; a single generic helper taking the query + parser would remove all four.

Minor (mostly inherited from the copied pattern)

  • Silent fee-estimate failure: _loadNetworkFee swallows errors and just hides the fee row; the user confirms a cancellation without seeing a fee, with no error surfaced. Conflicts with the "no silent failures" rule, though copied verbatim from the approve sheet.
  • setState after unmount: in _confirm, if (!authed || !mounted) { setState(...) } will throw if the sheet was dismissed during biometric auth. Pre-existing in approve/execute sheets too.
  • String reuse: the cancel sheet shows multisigApproveAuthRequired on auth failure — works, but the approve-specific key is misleading.
  • _isSkippedMultisigAccountEventId uses prefix 'ae-ms-cancel' without the trailing - that all other prefixes have — fine if intentional (covers ae-ms-cancelled-), but inconsistent.

Recommendation

Request changes for Issue 1 (the in-flight cancel race) — it's a one-line-per-button fix. Issue 2 is worth doing now while all three sheets are fresh, but could be a fast-follow. The worktree is kept at /private/tmp/pr-reviews/quantus-pr-512-review if you want to dig further or test fixes there.

@n13 n13 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.

Review (agent-assisted, diffed against beast/execute-proposal-flow)

Verdict: GTG — no blockers found; one minor, non-blocking note.

Minor

  1. Logout doesn't clear pending cancellation state — the new in-memory pendingMultisigCancellationsProvider is not cleared in mobile-app/lib/services/logout_service.dart (same pre-existing gap as the proposals/approvals/executions pending providers). After logout + re-login in the same process, stale "Cancelling…" rows could remain in activity and memberCost could keep being subtracted from effective balance.

Checked and fine

  • _executeButton and _approveButton both correctly block while a cancellation is pending (pendingCancellation == null guard), so the same device can't race conflicting cancel + execute/approve extrinsics.
  • Cancel-only-for-proposer gating, terminal-proposal handling (action buttons hidden), confirm sheet with fee + biometrics, optimistic pending state, polling, and reconciliation all mirror the established approve/execute patterns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants