Skip to content

Multisig proposal polling#513

Merged
dewabisma merged 34 commits into
multisig-implementationfrom
beast/multisig-proposal-polling
Jun 12, 2026
Merged

Multisig proposal polling#513
dewabisma merged 34 commits into
multisig-implementationfrom
beast/multisig-proposal-polling

Conversation

@dewabisma

@dewabisma dewabisma commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add fetch proposals to polling in multisig account
  • Pull to refresh home page also fetch proposals in multisig account

Note

Low Risk
Read-only provider invalidation/refetch scoped to the active multisig account; no changes to signing, auth, or transaction submission.

Overview
Multisig proposal data now stays in sync with the same refresh paths that already update balances and transaction history when a multisig account is selected.

polling_refresh_scope gains multisigAccountFromActive, invalidateActiveMultisigProposals, and refreshActiveMultisigProposals (invalidates open/past proposal providers and current block, then awaits reload). Background global history polls invalidate proposals; manual global refresh, home pull-to-refresh, silent refresh via the history polling manager, and account switch all hook into these helpers (pull-to-refresh and manual global refresh wait for fresh proposal data).

Unit tests cover multisigAccountFromActive for multisig vs regular/entrusted/null active accounts.

Reviewed by Cursor Bugbot for commit f4f175f. Configure here.

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

n13 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Review of PR #513

The PR's overall approach is right — it does not add a new polling timer. It hooks multisig proposal invalidation into the four existing refresh paths (background poll, manual refresh, silent refresh on app-resume, account switch) and reuses the existing invalidateMultisigProposals helper from multisig_providers.dart. That's the DRY-friendly design. However, there are three real problems, one of them a crash.

1. Critical: ref as Ref in home_screen.dart will crash at runtime

await refreshActiveMultisigProposals(ref as Ref);

The repo is on flutter_riverpod 3.3.1. In Riverpod 3, WidgetRef and Ref are unrelated sealed classesConsumerStatefulElement implements WidgetRef only:

base class ConsumerStatefulElement extends StatefulElement
    implements WidgetRef {

So this cast throws TypeError every time, and since it runs unconditionally (before the multisig null-check inside the helper), pull-to-refresh on home breaks for all accounts, not just multisig ones. This was clearly never run on a device.

2. The duplication you asked about: the same refresh is wired twice, once into dead code

GlobalHistoryPollingService.triggerManualRefresh() gets await refreshActiveMultisigProposals(_ref) — but that method is only reachable via HistoryPollingManager.triggerManualRefresh(), which is called from nowhere (its own comment even says // This is not called from anywhere!). The real pull-to-refresh path is _HomeScreenState._refresh(), which got its own (broken) copy.

This perpetuates a pre-existing duplication: _refresh() in the home screen already reimplements most of triggerManualRefresh() (balance invalidation + loadingRefresh on the active account's pagination). The clean fix that solves both #1 and #2 at once: have _refresh() delegate to ref.read(globalHistoryPollingServiceProvider).triggerManualRefresh() — that runs with a real Ref, already awaits the proposal refresh after this PR, and removes the duplicated refresh logic instead of adding a third copy.

3. Minor: the two new helpers duplicate each other

refreshActiveMultisigProposals repeats invalidateActiveMultisigProposals's body (active-account lookup, null guard, invalidate) instead of building on it. Trivially restructured, e.g. have the invalidate helper return the MultisigAccount? and have refresh call it.

What's fine

  • Reusing invalidateMultisigProposals rather than re-invalidating providers inline (notably, the existing confirm sheets like multisig_approve_confirm_sheet.dart invalidate inline and even miss multisigPastProposalsProvider — pre-existing, not this PR).
  • No overlap with the existing multisig_*_polling_service.dart files — those track pending submitted extrinsics, a different concern; this PR covers external changes from other signers.
  • Invalidate-only on background/silent paths vs. awaited refresh on user-initiated paths is the correct split, and matches how balances are handled.
  • multisigAccountFromActive doesn't duplicate anything — existing code only had inline is MultisigDisplayAccount checks.

Verdict: architecture is sound and non-duplicative in intent, but it shouldn't merge as-is — the pull-to-refresh cast is a guaranteed crash, and the fix for it (delegating _refresh to triggerManualRefresh) also removes the one genuine duplication the PR introduces. Per your rules I haven't commented on GitHub; let me know if you want me to post this review on the PR.

@dewabisma dewabisma changed the title Beast/multisig proposal polling Multisig proposal polling Jun 11, 2026

@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/cancel-proposal-flow)

Verdict: GTG — small, focused diff; three minor, non-blocking notes.

Minor

  1. Manual refresh skips reconciliation on proposal-refresh failure — in GlobalHistoryPollingService.triggerManualRefresh, await refreshActiveMultisigProposals(_ref) runs before reconcilePendingTransactions(). If any proposal/block provider future fails, the method throws and reconciliation never runs for that refresh. The home screen catches the error, but consider try/catching the proposal refresh (or reordering) so reconciliation still happens.

  2. Silent catch in timezone fallbacklocal_notifications_service.dart has a last-resort catch (_) {} with no logging when even the UTC fallback fails. Per repo convention (no silent failures), this should at least debugPrint/telemetry the error. Also, the message 'Failed to set device timezone "$e"' interpolates the exception where the timezone identifier is implied — minor wording nit.

  3. Transient duplicate proposal rows possible — the background poll now invalidates open proposals (invalidateActiveMultisigProposals) independently of pending-event cleanup, so a just-indexed proposal can briefly render alongside its still-pending "Proposing…" row in the Open Proposals section until reconciliation removes the pending event. Cosmetic and self-healing, but worth knowing.

Checked and fine

  • multisigAccountFromActive helper + unit tests cover multisig/regular/entrusted/null cases.
  • Pull-to-refresh, manual global refresh, silent refresh, and account switch all consistently route through the new polling_refresh_scope helpers — good DRY consolidation (also removes the dead triggerManualRefresh in HistoryPollingManager).
  • Reordering polling-service registration before notification init in AppInitializer makes listener attachment robust to notification init failures.

@dewabisma dewabisma changed the base branch from beast/cancel-proposal-flow to multisig-implementation June 12, 2026 03:14
@dewabisma dewabisma merged commit bbd4053 into multisig-implementation Jun 12, 2026
dewabisma added a commit that referenced this pull request Jun 12, 2026
* msig implementation 1

* linter fixes and package updates

* Create multisig flow (#505)

* feat: localize copy writing

* Format and regrenate runtime apis

* feat: finish create multisig flow

* fix: code reviews

* fix: code reviews

* feat: finish adding in-flight tracking for multisig creation

- reconcilate pending multisig creation
- track multisig creation

* feat: improve multisig creation

- make nonce calculated on the fly
- only show history for creator

* feat: add checksum and not truncate checksum

* fix: create multisig screen

* fix: add logging on error

* feat: improve UX multisig creation pending and details sheet

* fix: preflight balance check

* fix: DRY violation

* fix: silent or masking bad data

* fix: persisting pending multisig creation

* chore: formatting

* Discover multisig flow (#506)

* feat: finish adding discover multisig

* fix: code review issues

* fix: another code review issues

* feat: add checksum on discover list item

* fix: listview item non properly ordered without key

* feat: refactor repeating multisig graphql handling

* feat: make json reading throw early instead of silently droping it. Also have intFromJson for supporting hasura stringify number.

* fix: use variable instead of string interpolation

* feat: propagate error as to not confuse user

* Create proposal flow (#507)

* wip: create proposal

- finish create flow
- create history of proposal

* feat: improve UX of propose amount and review screen

* feat: finish improving the lifecycle of proposal creation and event listing presentation

* feat: address code review issues

* feat: another code reviews fixes

* chore: formatting

* fix: dart rule violation

* fix: redudancy

* fix: graphql query for multisig

* fix: circle dependency

* feat: explicitly show failed fee fetch

- add retry button
- show message
- silently refetch network fee unless failed

* fix: reduce risk of double proposal creation

* fix: handle last effort before timeout

* chore: formatting

* Sign proposal flow (#510)

* feat: finish approve proposal flow from multisig account

* feat: show approve event in signer account

* feat: resolve DRY and simplicity issues

* feat: best effort timout and balance refresh

* chore: formatting

* chore: revert constants

* chore: remove dead code

* Execute proposal flow (#511)

* feat: finish approve proposal flow from multisig account

* feat: show approve event in signer account

* feat: resolve DRY and simplicity issues

* feat: best effort timout and balance refresh

* chore: formatting

* wip:  add execute flow

* feat: regenerate polkadart metadata, clean up deposit field and rename interceptor to guardian

* fix: live state resolution and update executed proposal UI

* feat: improve UX executed proposals

* chore: formatting

* chore: revert constants

* feat: show activity in executor

* chore: formatting

* chore: revert back chain url

* fix: code reviews

* fix: execute event query and data model

* chore: revert constants

* chore: formatting

* Cancel proposal flow (#512)

* feat: finish approve proposal flow from multisig account

* feat: show approve event in signer account

* feat: resolve DRY and simplicity issues

* feat: best effort timout and balance refresh

* chore: formatting

* wip:  add execute flow

* feat: regenerate polkadart metadata, clean up deposit field and rename interceptor to guardian

* fix: live state resolution and update executed proposal UI

* feat: improve UX executed proposals

* chore: formatting

* chore: revert constants

* feat: show activity in executor

* chore: formatting

* feat: add cancel proposal flow

* feat: display cancel proposal event in proposer activity

* chore: formatting

* chore: revert constants

* chore: revert back chain url

* fix: code reviews

* fix: execute event query and data model

* chore: revert constants

* chore: formatting

* fix: code review issues

* Multisig proposal polling (#513)

* feat: finish approve proposal flow from multisig account

* feat: show approve event in signer account

* feat: resolve DRY and simplicity issues

* feat: best effort timout and balance refresh

* chore: formatting

* wip:  add execute flow

* feat: regenerate polkadart metadata, clean up deposit field and rename interceptor to guardian

* fix: live state resolution and update executed proposal UI

* feat: improve UX executed proposals

* chore: formatting

* chore: revert constants

* feat: show activity in executor

* chore: formatting

* feat: add cancel proposal flow

* feat: display cancel proposal event in proposer activity

* chore: formatting

* chore: revert constants

* feat: finish implementing polling and refresh proposals

* chore: formatting

* chore: revert back chain url

* fix: code reviews

* fix: execute event query and data model

* chore: revert constants

* chore: formatting

* fix: code review issues

* fix: code review issues

* fix: broken initialization of timezone

* chore: add debug on error

* fix: code review issues

---------

Co-authored-by: Nikolaus Heger <nheger@gmail.com>
@dewabisma dewabisma deleted the beast/multisig-proposal-polling branch June 22, 2026 03:41
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