Skip to content

Block adding one of this wallet's multisigs as a multisig signer - #715

Open
exocognosis wants to merge 3 commits into
Quantus-Network:mainfrom
exocognosis:exocognosis/multisig-signer-guard
Open

exocognosis wants to merge 3 commits into
Quantus-Network:mainfrom
exocognosis:exocognosis/multisig-signer-guard

Conversation

@exocognosis

@exocognosis exocognosis commented Oct 20, 2026 •

Copy link
Copy Markdown

Fixes the creation half of #708.

Minimal version. No chain query, no new service.

Change:

  • In the add signer step, an address that matches a multisig saved or pending in this wallet is rejected with an inline error. The error says the address is a multisig account, a multisig cannot sign, a multisig that lists it as a signer can never execute from this wallet, and funds would need recovery through the CLI.
  • The check reuses _reservedAddresses(), which the screen already builds from saved and pending multisigs.

Files: the logic is one check in add_multisig_screen.dart. The other five files are the new string: app_en.arb, app_id.arb, and the three app_localizations*.dart files that gen-l10n regenerates and the repo commits.

Tradeoff: detection is local only. It misses multisigs created on another device or another wallet, since nothing in the wallet's store knows about them. That is accepted as the minimal scope. Anyone who still ends up there has the CLI path from Quantus-Network/quantus-cli#189.

Tests: none added. flutter analyze is clean on the changed files.

A multisig account holds no key, so it can never sign an approval from
the wallet. Adding one as a signer produced a multisig the app could not
execute (Quantus-Network#708).

When a signer is added, check it against local saved and pending
multisigs, then against Multisig.Multisigs on chain. A hit opens a
confirmation sheet that defaults to leaving the signer out; a failed
lookup blocks the add with an error instead of letting it through.
@exocognosis

Copy link
Copy Markdown
Author

Checked the lookup against a live chain (local dev node, quantus-node v1.0.2-Qm): Bell query.multisig.multisigs through KeepAliveHttpProvider, the same call isMultisigAccount makes, returns true for a created 2-of-2 and for a multisig that has a multisig signer, and false for two keyed accounts.

The screen itself has only run in widget tests with the service faked; not run on a device or simulator.

@n13

n13 commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

I think there's better fixes for this - this is an edge case of an edge case

The users who do such a thing, can recover through CLI

I was helping the original guy who did this, I just told him the prompt to give to his AI, and that resolved it - it involves constructing a complex msig

To be reviewed, ideally find a minimal solution

Drop the chain lookup, the confirm sheet and the helper. The add signer
step now rejects an address that matches a multisig saved or pending in
this wallet, with an inline explanation.
@exocognosis exocognosis changed the title Warn before adding a multisig account as a multisig signer Block adding one of this wallet's multisigs as a multisig signer Oct 21, 2026
@exocognosis

Copy link
Copy Markdown
Author

Fair. Trimmed to the minimal version: no chain query, no new service. The creation flow now checks the entered signer against multisig accounts this wallet itself knows, and blocks with an explanation if it matches. That misses multisigs created elsewhere, stated plainly in the description; those still have the CLI path from quantus-cli #189.

@exocognosis

Copy link
Copy Markdown
Author

Following up on the trimmed version from Wednesday: it is ready whenever you want another look. If you would rather leave this edge case to the CLI recovery path, I am fine closing this, your call.

@n13 n13 added the bot-review Request automated review from review-bot label Oct 25, 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.

Verdict: Approve
Reviewer: GPT-6.1 Sol

Reviewed head 107261d549edceb9c3d049a1fa41be28c7e5f4df against base 628583e24fb57629225e74814a183bdaeb7033dd. No blocking findings within the agreed wallet-local detection scope. The guard checks saved and pending multisigs before mutating the signer list; the Add Signer button and keyboard submission share this path.

Nonblocking wording correction: mobile-app/lib/l10n/app_en.arb:445 (and the Indonesian equivalent) says a multisig containing this signer can never execute from the wallet. It can still execute when the other keyed members meet the threshold, for example two keyed members of a 2-of-3 multisig. Consider saying this signer cannot approve in the wallet and CLI recovery may be needed if the remaining members cannot meet the threshold.

Validation: 73 focused tests passed: 19 mobile tests, including nine temporary reviewer widget tests, and 54 SDK multisig tests. The widget tests exercised saved/pending rejection through both input routes in English and Indonesian, whitespace trimming, error clearing, and ordinary/duplicate/invalid signer behavior, with validation and chain services faked. Formatting passed across 863 files; git diff --check passed; both generated localization getters match their ARB strings. No tracked files were changed.

Local workspace analysis was stopped at the required 10-second cap and remains incomplete. GitHub Analyze is still in progress; dependency-cooldown checks passed. No device or live-chain execution was performed.

@n13 n13 removed the bot-review Request automated review from review-bot label Oct 25, 2026

This branch has not been deployed

No deployments
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