fix(app): finalize SIA integration and Trezor guards by CharlVS · Pull Request #3449 · GLEECBTC/gleec-wallet

CharlVS · 2026-03-18T18:52:13Z

Summary

finalize the app-side SIA integration in coin activation, coin manager, and withdrawal flows
prevent unsupported Trezor plus SIA flows in this release by hiding unsupported activation paths and surfacing clear errors where needed
update the SDK submodule pointer to the merged test-sia branch state required by this integration

Why

the app already has most of the SIA user flow, but it still needs the final activation/withdraw handling and hardware-wallet guardrails to behave correctly on dev
this PR is the app-side counterpart to the SDK SIA support PR and should land after that dependency is available in dev

Dependency

Depends on SDK PR: feat(sdk): add SIA activation and withdrawal support komodo-defi-sdk-flutter#320

Verification

flutter build web

Test plan

Review SIA coin activation behavior in the app
Review SIA coin manager visibility and selection behavior
Verify unsupported Trezor plus SIA flows show the expected guardrails
Confirm the app works correctly once the SDK PR is merged into dev

Point the app's SIA integration branch at the current sdk/test-sia tip so the submodule references a GitHub-visible commit and the dev PR can be reviewed alongside the SDK PR.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1dcd7db4e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-18T18:59:00Z

+      assets = assets
+          .where((asset) => asset.id.subClass != CoinSubClass.sia)
+          .toList();
+      if (assets.isEmpty) {
+        return;


Return an error for unsupported Trezor SIA activation

When the requested set is only SIA assets on a Trezor, activateAssetsSync() now filters them out and returns successfully. Callers such as activateCoinIfNeeded() in lib/views/dex/dex_helpers.dart:226-255 only surface a guardrail from the catch path, and that helper is reused by maker/taker/bridge/NFT entry points, so those flows will still let a Trezor user pick SIA and continue with an inactive coin instead of showing the intended unsupported-asset message.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-18T18:59:00Z

+    final visibleSelectedCoins = await _filterUnsupportedHardwareCoins(
+      selectedCoins,
+      event.action,
+    );


Keep active Trezor SIA coins visible in add-assets

This second filter removes wallet-owned SIA coins from selectedCoins immediately after _mergeWalletCoinsIfNeeded() re-added them. In this app, add-assets is the only reachable coin-management UI I could find (main_layout.dart:178-183, wallet_manage_section.dart:117-121, while CoinsManagerSwitchButton is unused), so any Trezor wallet that already has SIA in metadata from earlier dev builds will have those assets disappear from the list and lose its only in-app way to deselect/deactivate them.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb56887a4d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-18T19:12:37Z

+      assets = assets
+          .where((asset) => asset.id.subClass != CoinSubClass.sia)
+          .toList();
+      if (assets.isEmpty) {
+        return;


Emit a state change when Trezor SIA activation is skipped

If a Trezor wallet already has SIA in activated_coins from an older build, CoinsBloc._onLogin() still pre-populates that asset as CoinState.activating and waits for activation updates (lib/bloc/coins_bloc/coins_bloc.dart:381-392 and :597-615). The new early return here drops the request without broadcasting anything back through enabledAssetsChanges, so the SIA entry never gets removed from walletCoins and remains stuck as an "activating" asset for the rest of the session; it can also keep the initial 80% activation threshold from being reached until the one-minute timeout.

Useful? React with 👍 / 👎.

When a Trezor wallet has SIA in its activated coins from an older build, the early return in activateAssetsSync would leave those coins stuck in the "activating" state because no update was ever broadcast via enabledAssetsChanges. Now we broadcast CoinState.suspended for each filtered SIA asset before discarding them, so CoinsBloc correctly removes them from walletCoins.

chatgpt-codex-connector · 2026-03-18T19:26:03Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

CharlVS · 2026-03-18T19:26:19Z

Review comment audit

Triaged all 3 Codex review comments. Summary below.

C1 (P1) — "Emit a state change when Trezor SIA activation is skipped"

Verdict: Valid — fixed in 94b36cf

The concern was that if a Trezor wallet had SIA in activatedCoins from an older build, _prePopulateListWithActivatingCoins would mark it as CoinState.activating, but the early return in activateAssetsSync would never broadcast an update via enabledAssetsChanges, leaving SIA stuck in "activating" state and potentially delaying the 80% activation threshold.

In practice this is unlikely because the SDK's TrezorAssetFilterStrategy already excludes SIA from available, so getKnownCoinsMap() returns null for SIA and it's never pre-populated. However, the defensive gap was real — if the SDK filter ever changed, the stuck state would occur. The fix now broadcasts CoinState.suspended for each filtered SIA asset before discarding them.

C2 (P2) — "DEX helpers don't surface SIA unsupported message"

Verdict: Out of scope

The concern was that activateCoinIfNeeded in dex_helpers.dart would silently succeed if SIA were filtered. However:

The DEX helper code is not touched by this PR
SIA coins won't appear in DEX pair lists because the SDK's TrezorAssetFilterStrategy filters them from available for Trezor wallets
Even if getCoin('SC') somehow resolved, activateCoinsSync would throw (since available[siaId] is null), and the DEX helper's catch block would surface the error

This is a pre-existing concern about general DEX robustness, not introduced by this PR.

C3 (P2) — "Keep active Trezor SIA coins visible in add-assets"

Verdict: Out of scope

The concern was that _filterUnsupportedHardwareCoins removes SIA from selectedCoins, so Trezor users who had SIA from older builds couldn't deselect it. However:

getWalletCoins() calls _kdfSdk.getWalletAssets(), which goes through the SDK's asset filter — SIA is already excluded for Trezor
_mergeWalletCoinsIfNeeded therefore never re-adds SIA for Trezor users
The _filterUnsupportedHardwareCoins call on selectedCoins is a no-op for Trezor + SIA since SIA was never in the list

This is also a pre-existing concern, not introduced by this PR.

Resolve submodule conflict by pointing SDK to origin/dev (7a3f409) which now includes the merged SIA support (#320).

chatgpt-codex-connector · 2026-03-18T20:49:30Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

chatgpt-codex-connector · 2026-03-18T20:55:26Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

github-actions · 2026-03-18T21:01:03Z

Visit the preview URL for this PR (updated for commit 36eaecf):

https://walletrc--pull-3449-merge-4f9ktaxw.web.app

_{(expires Wed, 25 Mar 2026 21:00:49 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc}

CharlVS added 2 commits March 16, 2026 20:00

fix(sia): finalize app release integration

8e87464

chore(sdk): repoint app SIA branch to sdk/test-sia

e1dcd7d

Point the app's SIA integration branch at the current sdk/test-sia tip so the submodule references a GitHub-visible commit and the dev PR can be reviewed alongside the SDK PR.

chatgpt-codex-connector bot reviewed Mar 18, 2026

View reviewed changes

Merge branch 'dev' into test-sia-dev-pr

cb56887

CharlVS mentioned this pull request Mar 18, 2026

feat(sdk): add SIA activation and withdrawal support GLEECBTC/komodo-defi-sdk-flutter#320

Merged

4 tasks

CharlVS changed the title ~~fix(sia): finalize app integration for dev~~ fix(app): finalize SIA integration and Trezor guards Mar 18, 2026

chatgpt-codex-connector bot reviewed Mar 18, 2026

View reviewed changes

Merge branch 'dev' into test-sia-dev-pr

7a6b462

Resolve submodule conflict by pointing SDK to origin/dev (7a3f409) which now includes the merged SIA support (#320).

chore(sdk): update submodule to include withdrawal fix (#322)

36eaecf

CharlVS merged commit cdad98a into dev Mar 18, 2026
5 of 12 checks passed

CharlVS deleted the test-sia-dev-pr branch March 18, 2026 22:06

CharlVS mentioned this pull request Mar 23, 2026

chore(release): v0.9.4 release candidate #3461

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(app): finalize SIA integration and Trezor guards#3449

fix(app): finalize SIA integration and Trezor guards#3449
CharlVS merged 6 commits intodevfrom
test-sia-dev-pr

CharlVS commented Mar 18, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 18, 2026

Uh oh!

CharlVS commented Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 18, 2026

Uh oh!

github-actions bot commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

CharlVS commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Dependency

Verification

Test plan

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot commented Mar 18, 2026

Uh oh!

CharlVS commented Mar 18, 2026

Review comment audit

C1 (P1) — "Emit a state change when Trezor SIA activation is skipped"

C2 (P2) — "DEX helpers don't surface SIA unsupported message"

C3 (P2) — "Keep active Trezor SIA coins visible in add-assets"

Uh oh!

chatgpt-codex-connector bot commented Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 18, 2026

Uh oh!

github-actions bot commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

CharlVS commented Mar 18, 2026 •

edited

Loading