Skip to content

Fix throw when toggling Tor with unopened cryptonote family wallets - #1275

Draft
sneurlax wants to merge 6 commits into
stagingfrom
fix/toggle-tor-w-null-wallet
Draft

Fix throw when toggling Tor with unopened cryptonote family wallets#1275
sneurlax wants to merge 6 commits into
stagingfrom
fix/toggle-tor-w-null-wallet

Conversation

@sneurlax

Copy link
Copy Markdown
Member

When you toggle Tor with an uninitialized cryptonote wallet, it throws. This isn't fatal. The fix uses the same guard pattern other functions use.

This fix is admittedly optional: it's not a critical fix but rather a cosmetic/log-cleaning fix.

@sneurlax
sneurlax marked this pull request as ready for review March 5, 2026 04:13
@sneurlaxsneurlax changed the title fix: guard against null wallet in updateNode() for Monero-family walletsFix throw when toggling Tor with unopened cryptonote family walletsMar 5, 2026
@sneurlax
sneurlax marked this pull request as draft May 13, 2026 21:25
@codecov

codecovBot commented May 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 131 lines in your changes missing coverage. Please review.
✅ Project coverage is 4.93%. Comparing base (2dece0a) to head (adbeade).
⚠️ Report is 179 commits behind head on staging.

Files with missing linesPatch %Lines
...wallets/wallet/intermediate/lib_monero_wallet.dart0.00%45 Missing ⚠️
...allets/wallet/intermediate/lib_wownero_wallet.dart0.00%44 Missing ⚠️
...allets/wallet/intermediate/lib_salvium_wallet.dart0.00%42 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## staging #1275 +/- ##
===========================================
+ Coverage 4.82% 4.93% +0.10% 
===========================================
Files 1064 1064 Lines 107929 108022 +93 ===========================================
+ Hits 5212 5330 +118 + Misses 102717 102692 -25 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sneurlax
sneurlax marked this pull request as ready for review May 13, 2026 21:49
@sneurlax
sneurlaxforce-pushed the fix/toggle-tor-w-null-wallet branch from db30ba4 to 4e93483CompareMay 15, 2026 15:41
@sneurlax

Copy link
Copy Markdown
MemberAuthor

Comments:
"""
does this null check https://github.com/cypherstack/stack_wallet/pull/1275/changes hide a problem elsewhere?
I'm looking at 1275 again. If the null check hides a problem elsewhere and that problem was fixed, why is the null check still there?
any chance introducing another mutex (_updateNodeMutex) could cause a deadlock?
Ok so I am generally in favor of writing less code and doing the same check in a single place instead of spreading it all over the place but I still think the null check is going a bit too far in catching every single call.

For example in https://www.github.com/cypherstack/stack_wallet/blob/211971b5d7c6cac19173071026388dbaf756e8ab/lib/pages/settings_views/global_settings_view/manage_nodes_views/add_edit_node_view.dart#L306-L339
The check should probably be handled here as refresh is called on the same wallets. Maybe I'm overthinking it though?
"""

@sneurlax
sneurlax marked this pull request as draft May 15, 2026 16:50
Sign up for freeto 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.

1 participant

@sneurlax