From f08ae897e693a5c3af236b365bdb0e30f2f6a2fc Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sat, 31 Oct 2026 12:50:36 -0400 Subject: [PATCH] fix(wownero): hold the history mutex while reading transactions The 1.0.5 build 15 macOS test crashed with SIGSEGV in WOWNERO_TransactionInfo_amount on the main thread, near the end of a restore sync. TransactionHistory_refresh runs on a background isolate under txHistoryMutex and frees the TransactionInfo objects it replaces; getAllSubaddresses() and getTransaction() read those objects without the mutex, so a read that overlaps a refresh dereferences freed memory. The wallet cache had not been stored, so the next launch synced again. Upstream fixed the same race for Monero in #3188 and in the v6.4.5 coin-control change; this ports both to cw_wownero. getAllSubaddresses() and getTransaction() take the mutex and become async, which carries through the subaddress list, its app proxy and the configure interface. The subaddress list now fetches before clearing, so the observable list is never empty while the fetch is pending, and addSubaddress and setLabelSubaddress await the update before callers read the new address. The coin-control change check compared a Transaction with 1 and was always false; it now uses the transaction's direction, as Monero does. --- cw_wownero/lib/api/subaddress_list.dart | 45 ++++++++++++--------- cw_wownero/lib/api/transaction_history.dart | 10 ++++- cw_wownero/lib/wownero_subaddress_list.dart | 17 ++++---- cw_wownero/lib/wownero_wallet.dart | 7 +++- lib/wownero/cw_wownero.dart | 5 +-- tool/configure.dart | 2 +- 6 files changed, 51 insertions(+), 35 deletions(-) diff --git a/cw_wownero/lib/api/subaddress_list.dart b/cw_wownero/lib/api/subaddress_list.dart index ad54175c9..70a063fd5 100644 --- a/cw_wownero/lib/api/subaddress_list.dart +++ b/cw_wownero/lib/api/subaddress_list.dart @@ -57,27 +57,32 @@ int lastWptr = 0; int lastTxCount = 0; List ttDetails = []; -List getAllSubaddresses() { - txhistory = wownero.Wallet_history(wptr!); - final txCount = wownero.TransactionHistory_count(txhistory!); - if (lastTxCount != txCount && lastWptr != wptr!.address) { - final List newttDetails = []; - lastTxCount = txCount; - lastWptr = wptr!.address; - for (var i = 0; i < txCount; i++) { - final tx = wownero.TransactionHistory_transaction(txhistory!, index: i); - final subaddrs = wownero.TransactionInfo_subaddrIndex(tx).split(","); - final account = wownero.TransactionInfo_subaddrAccount(tx); - newttDetails.add(TinyTransactionDetails( - address: List.generate( - subaddrs.length, - (index) => getAddress( - accountIndex: account, addressIndex: int.tryParse(subaddrs[index]) ?? 0)), - amount: wownero.TransactionInfo_amount(tx), - )); +Future> getAllSubaddresses() async { + await txHistoryMutex.acquire(); + try { + txhistory = wownero.Wallet_history(wptr!); + final txCount = wownero.TransactionHistory_count(txhistory!); + if (lastTxCount != txCount && lastWptr != wptr!.address) { + final List newttDetails = []; + lastTxCount = txCount; + lastWptr = wptr!.address; + for (var i = 0; i < txCount; i++) { + final tx = wownero.TransactionHistory_transaction(txhistory!, index: i); + final subaddrs = wownero.TransactionInfo_subaddrIndex(tx).split(","); + final account = wownero.TransactionInfo_subaddrAccount(tx); + newttDetails.add(TinyTransactionDetails( + address: List.generate( + subaddrs.length, + (index) => getAddress( + accountIndex: account, addressIndex: int.tryParse(subaddrs[index]) ?? 0)), + amount: wownero.TransactionInfo_amount(tx), + )); + } + ttDetails.clear(); + ttDetails.addAll(newttDetails); } - ttDetails.clear(); - ttDetails.addAll(newttDetails); + } finally { + txHistoryMutex.release(); } final size = wownero.Wallet_numSubaddresses(wptr!, accountIndex: subaddress!.accountIndex); final list = List.generate(size, (index) { diff --git a/cw_wownero/lib/api/transaction_history.dart b/cw_wownero/lib/api/transaction_history.dart index 13dae3d71..a64c3ccf6 100644 --- a/cw_wownero/lib/api/transaction_history.dart +++ b/cw_wownero/lib/api/transaction_history.dart @@ -81,8 +81,14 @@ Future> getAllTransactions() async { } // TODO(mrcyjanek): ... -Transaction getTransaction(String txId) { - return Transaction(txInfo: wownero.TransactionHistory_transactionById(txhistory!, txid: txId)); +Future getTransaction(String txId) async { + await txHistoryMutex.acquire(); + try { + return Transaction( + txInfo: wownero.TransactionHistory_transactionById(txhistory!, txid: txId)); + } finally { + txHistoryMutex.release(); + } } Future createTransactionSync( diff --git a/cw_wownero/lib/wownero_subaddress_list.dart b/cw_wownero/lib/wownero_subaddress_list.dart index 4790c1ea8..067f26ba4 100644 --- a/cw_wownero/lib/wownero_subaddress_list.dart +++ b/cw_wownero/lib/wownero_subaddress_list.dart @@ -24,7 +24,7 @@ abstract class WowneroSubaddressListBase with Store { bool _isRefreshing; bool _isUpdating; - void update({required int accountIndex}) { + Future update({required int accountIndex}) async { refreshCoins(accountIndex); if (_isUpdating) { @@ -34,8 +34,9 @@ abstract class WowneroSubaddressListBase with Store { try { _isUpdating = true; refresh(accountIndex: accountIndex); + final newAddrs = await getAll(); subaddresses.clear(); - subaddresses.addAll(getAll()); + subaddresses.addAll(newAddrs); _isUpdating = false; } catch (e) { _isUpdating = false; @@ -43,8 +44,8 @@ abstract class WowneroSubaddressListBase with Store { } } - List getAll() { - var subaddresses = subaddress_list.getAllSubaddresses(); + Future> getAll() async { + var subaddresses = await subaddress_list.getAllSubaddresses(); if (subaddresses.length > 2) { final primary = subaddresses.first; @@ -74,14 +75,14 @@ abstract class WowneroSubaddressListBase with Store { Future addSubaddress({required int accountIndex, required String label}) async { await subaddress_list.addSubaddress(accountIndex: accountIndex, label: label); - update(accountIndex: accountIndex); + await update(accountIndex: accountIndex); } Future setLabelSubaddress( {required int accountIndex, required int addressIndex, required String label}) async { await subaddress_list.setLabelForSubaddress( accountIndex: accountIndex, addressIndex: addressIndex, label: label); - update(accountIndex: accountIndex); + await update(accountIndex: accountIndex); } void refresh({required int accountIndex}) { @@ -129,7 +130,7 @@ abstract class WowneroSubaddressListBase with Store { Future> _getAllUnusedAddresses( {required int accountIndex, required String label}) async { - final allAddresses = subaddress_list.getAllSubaddresses(); + final allAddresses = await subaddress_list.getAllSubaddresses(); final lastAddress = allAddresses.last.address; if (allAddresses.isEmpty || _usedAddresses.contains(lastAddress)) { final isAddressUnused = await _newSubaddress(accountIndex: accountIndex, label: label); @@ -156,7 +157,7 @@ abstract class WowneroSubaddressListBase with Store { Future _newSubaddress({required int accountIndex, required String label}) async { await subaddress_list.addSubaddress(accountIndex: accountIndex, label: label); - return subaddress_list.getAllSubaddresses().where((s) { + return (await subaddress_list.getAllSubaddresses()).where((s) { final address = s.address; return !_usedAddresses.contains(address); }).isNotEmpty; diff --git a/cw_wownero/lib/wownero_wallet.dart b/cw_wownero/lib/wownero_wallet.dart index 27a8db424..7b0fdcf26 100644 --- a/cw_wownero/lib/wownero_wallet.dart +++ b/cw_wownero/lib/wownero_wallet.dart @@ -583,7 +583,12 @@ abstract class WowneroWalletBase wownero.CoinsInfo_unlocked(coin), ); if (unspent.hash.isNotEmpty) { - unspent.isChange = transaction_history.getTransaction(unspent.hash) == 1; + try { + final tx = await transaction_history.getTransaction(unspent.hash); + unspent.isChange = tx.isSpend == true; + } catch (e) { + printV("isChange lookup failed for ${unspent.hash}: $e"); + } } unspentCoins.add(unspent); } diff --git a/lib/wownero/cw_wownero.dart b/lib/wownero/cw_wownero.dart index a0e9e33ea..111255118 100644 --- a/lib/wownero/cw_wownero.dart +++ b/lib/wownero/cw_wownero.dart @@ -83,10 +83,9 @@ class CWWowneroSubaddressList extends WowneroSubaddressList { } @override - List getAll(Object wallet) { + Future> getAll(Object wallet) async { final wowneroWallet = wallet as WowneroWallet; - return wowneroWallet.walletAddresses.subaddressList - .getAll() + return (await wowneroWallet.walletAddresses.subaddressList.getAll()) .map((sub) => Subaddress(id: sub.id, label: sub.label, address: sub.address)) .toList(); } diff --git a/tool/configure.dart b/tool/configure.dart index 3f90b62a2..5c43edb36 100644 --- a/tool/configure.dart +++ b/tool/configure.dart @@ -658,7 +658,7 @@ abstract class WowneroSubaddressList { ObservableList get subaddresses; void update(Object wallet, {required int accountIndex}); void refresh(Object wallet, {required int accountIndex}); - List getAll(Object wallet); + Future> getAll(Object wallet); Future addSubaddress(Object wallet, {required int accountIndex, required String label}); Future setLabelSubaddress(Object wallet, {required int accountIndex, required int addressIndex, required String label});