Optimize multisig tx submission

[dewabisma]

Summary

Make extrinsic submission awaited. Display error toast on failed submission.

---

Note

Medium Risk
Changes timing and error handling for on-chain multisig flows; incorrect await/rethrow behavior could block navigation or leave pending UI state inconsistent, though scope is limited to submission orchestration.

Overview
Multisig creation and proposal lifecycle submissions (propose, approve, execute, cancel) no longer fire extrinsics in the background with unawaited. Public APIs now await chain acceptance of the signed extrinsic; indexer polling still runs afterward as before.

On submit failure, services still clear optimistic pending state and show the existing submit-failed toasts, but they also rethrow so UI layers can stay on the current screen instead of navigating away as if submission succeeded.

Add multisig stops showing a second generic error toaster when submission fails, since the creation toast listener already handles that case.

^{Reviewed by Cursor Bugbot for commit e428348. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

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

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

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

^{Reviewed by Cursor Bugbot for commit e428348. Configure here.}

[cursor]

Duplicate failure toasts and inline

Low Severity

After submission helpers began rethrowing on failure, approve/execute/cancel confirm sheets and the propose review screen still set inline error text when submit throws, while the submission service also fires the matching submitFailed toast. Users see the same failure message twice on the same screen.

 

^{Reviewed by Cursor Bugbot for commit e428348. Configure here.}

[cursor]

Creation errors lose user toast

Medium Severity

The generic catch after startMultisigCreation no longer shows any error toaster. Only submission failures get feedback via the creation toast listener; other failures (e.g. persisting pending creation) only hit quantusDebugPrint, so the user sees loading stop with no explanation.

 

^{Reviewed by Cursor Bugbot for commit e428348. Configure here.}

[n13]

Review: Optimize multisig tx submission (#533)

Summary

This PR switches multisig submission paths (creation, propose, approve, execute, cancel) from fire-and-forget unawaited(...) to awaited calls that complete only after the chain accepts the signed extrinsic, and adds rethrow so callers stay on the current screen and surface errors instead of optimistically navigating away. Indexer polling still runs in the background after acceptance. The AddMultisigScreen generic error toast is dropped to avoid duplicating the creation-toast listener.

The core change is a sound improvement: awaiting acceptance + rethrow gives correct, non-optimistic UX; the no-outer-retry reasoning (double-submit / double-spend avoidance) is correct; pending-state cleanup is preserved on every failure path; and the call sites (ProposeReviewScreen, MultisigActionConfirmSheet, AddMultisigScreen) all already catch the rethrown error. Nice work overall.

Findings

1. [major] AddMultisigScreen now silently swallows non-submission errors
The generic catch (e) (add_multisig_screen.dart ~L271-275) dropped its only user-facing feedback. But startMultisigCreation re-runs the full on-chain preflight internally (_runCreationPreflight: predictMultisigAddress / isMultisigIndexed / getFeeForCall / queryBalance) plus pendingMultisigCreationsProvider.add(...) before it ever submits. The creation toast listener only fires when the service sets MultisigCreationToastKind.submitFailed, i.e. on an actual submission failure. So a transient RPC failure during that second preflight (after the user has authenticated) now yields no toast, no navigation, and no release logging (quantusDebugPrint is kDebugMode-only) — a silent failure. This regresses prior behavior and conflicts with the repo's "never a silent failure / fail loud" rule. Suggest suppressing only for the known submission-failure case (or keeping a generic fallback toast) rather than swallowing all errors.

2. [minor] Double error reporting on propose/approve/execute/cancel
For these four flows the service both shows a toast (via the global Multisig*ToastListener mounted in HomeScreen) and rethrows, and the caller then also renders an inline error with the same message: ProposeReviewScreen._submit (_errorMessage = multisigProposeSubmitFailed, L111-118) and MultisigActionConfirmSheet._confirm (_errorMessage = failedMessage, L146-153). On failure the user now sees a toast plus an inline message. This is exactly the duplication you deliberately removed for the creation flow, so handling is now inconsistent across flows. Pick a single owner of the error UI (service toast or caller inline) per flow.

3. [minor / DRY] Dual-responsibility pattern duplicated across 5 submit methods
Every _submit* catch repeats: log -> remove pending -> show toast -> rethrow. That dual responsibility (toast + rethrow) is the root cause of #1 and #2. A single contract — service rethrows and callers own user feedback (or vice-versa) — would remove the duplication and the inconsistency.

4. [nit] Telemetry inconsistency (pre-existing)
Only multisig_create_submit_failed emits TelemetryService().sendError; the propose/approve/execute/cancel failure paths only quantusDebugPrint. Pre-existing and out of scope, but worth aligning for release observability.

5. [nit] import 'dart:async' removal in transaction_submission_service.dart is correct — no remaining unawaited/async-lib symbols in the file.

Tests

No tests cover MultisigSubmissionService / TransactionSubmissionService (only pending_multisig_creation_record_test.dart exists). Since this PR changes await/error semantics in sensitive signing flows, unit tests asserting "on submit failure: pending removed, toast set, rethrown" would add real value. Not blocking given there's no existing service-test harness.

Verdict: REQUEST CHANGES

Automated review via Cursor agent.

[n13]

Re-review: Optimize multisig tx submission (#533)

Re-reviewed at commit 5edab25 (fix: bad error handling). I pulled the branch into a clean worktree and ran flutter analyze on every touched file plus their consumers (toast listeners + call sites) — no issues found.

What changed since the last round

The fix took a cleaner route than a per-flow patch: it removes the submitFailed kind from all five toast providers, drops the matching arms from the three toast listeners, and removes the now-dead imports (dart:async + the four toast-provider imports) from transaction_submission_service.dart. The result is a single, consistent contract:

The submission service rethrows on failure (after clearing optimistic pending state); the caller owns all user-facing feedback.

• Propose → ProposeReviewScreen._submit shows inline multisigProposeSubmitFailed, stays on screen.
• Approve / Execute / Cancel → MultisigActionConfirmSheet._confirm shows inline failedMessage, sheet stays open.
• Create → AddMultisigScreen generic catch shows a single multisigCreateErrorCouldNotCreate toast for any failure, no navigation.

Previous findings — all resolved

• [major] #1 silent swallow in AddMultisigScreen → resolved. The generic catch (e) now always surfaces a toast (submission or the internal preflight/persist failures), so the post-auth "loading stops with no explanation" path is gone. Fixes the "fail loud" violation.
• [minor] #2 double error reporting (toast + inline) → resolved. The service no longer fires toasts, so propose/approve/execute/cancel now surface exactly one error (the caller's inline message).
• [minor / DRY] #3 dual-responsibility pattern → resolved. All five _submit* catch blocks are now uniform — log → remove pending → rethrow — and the toast-from-service responsibility is gone entirely.
• [nit] #5 dart:async removal → confirmed correct: removed from transaction_submission_service.dart (no unawaited left) and correctly kept in multisig_submission_service.dart (still used at L86).
• Both Cursor Bugbot items (duplicate toasts; creation errors losing a toast) are covered by the above.

The await-acceptance + background-poll boundary is intact and correct in every flow (await submit*Extrinsic(...) → then fire-and-forget startPolling(...)), and the no-outer-retry / double-submit reasoning is preserved.

Non-blocking nits (optional)

1. Loading affordance inconsistency. Now that submission is awaited (seconds until chain acceptance), the approve/execute/cancel confirm button (MultisigActionConfirmSheet, QuantusButton.simple ~L207) only sets isDisabled: _submitting — no spinner — whereas ProposeReviewScreen (isLoading: _submitting, L156) and AddMultisigScreen (isLoading: _isLoading, L338) both show one. Suggest passing isLoading: _submitting there for parity.
2. Telemetry asymmetry (pre-existing; #4 from last round still stands). Only multisig_create_submit_failed emits TelemetryService().sendError; the propose/approve/execute/cancel failure paths only quantusDebugPrint. Now that these failures are awaited and surfaced, emitting sendError on those four paths too would close the release-observability gap.
3. Tests. Still no coverage for the submission services. A small test per flow asserting "on submit failure → pending removed + rethrown" would lock in this contract. Not blocking given there's no existing service-test harness.

Verdict: APPROVE — every blocking and minor finding from the previous review is addressed; the remaining items are optional nits.

Automated review via Cursor agent.

[n13]

eh... yes appoved ;)