Simplify banner scope to asset and wallet id - #986
Merged
Conversation
DRadmir
marked this pull request as draft
August 27, 2026 08:48
DRadmir
force-pushed
the
wt2/banner-drop-chain
branch
2 times, most recently
from
August 27, 2026 10:29
b2d165e to
93979b9
Compare
An AssetId already carries the chain, so a banner scoped to a chain is expressible as the chain's native asset. Multi-signature banners now carry that asset instead of a separate chain, which also fills the chain name in the Android warning text that rendered empty. Banner.wallet held a whole Wallet so that two call sites could read its id and its nil-ness; it becomes a WalletId. Together this drops both joins from the banner query, removes the chain column whose name collided with the association key and forced a hand-written row decoder, and lets NewBanner stop fabricating a Wallet just to read its id back.
The enable-notifications banner was unreachable on both platforms: iOS never called active_events and Android required no wallet at all, so no row for it has ever been written. The system prompt shown right after wallet creation covers the case, so the event goes away entirely, and with it is_available, closes_on_action, which then always returned false, and the notification permissions the banner service held only for it. That leaves active_events producing nothing the seed and extra_banners do not already provide, so both platforms now read banners the same way: rows from the database filtered by visible_banners. Android drops its generate-and-merge branch. This also fixes a banner needing two closes there: the generator looked up a wallet-scoped key while the seed and the close wrote a global one, so the first close was never seen.
DRadmir
force-pushed
the
wt2/banner-drop-chain
branch
from
August 28, 2026 06:14
93979b9 to
1de52ee
Compare
DRadmir
marked this pull request as ready for review
August 28, 2026 06:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A banner was carrying both an asset and a chain, and a whole wallet where only its id was ever used.
The chain is already part of the asset, so banners scoped to a network now use that network's own asset. This also fixes the multi-signature warning on Android, which showed an empty network name.
The wallet is now just a wallet id. Together this removes a database column, both joins from the banner query, and a hand-written decoder that only existed to work around a name clash.