<p></p>
<p><b>@sqrrm</b> commented on this pull request.</p>

<p>Testing issue:</p>
<ol>
<li>Start with a BSQ wallet with no BTC balance</li>
<li>BSQ selection popup shows when button is pressed</li>
<li>Send BTC to BSQ receive address (remove leading "B")</li>
<li>BTC balance section shows in BSQ wallet but neither "select inputs" button work.</li>
</ol><hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5142#discussion_r573739606">core/src/main/java/bisq/core/btc/wallet/NonBsqCoinSelector.java</a>:</p>
<pre style='color:#555'>> @@ -60,9 +64,9 @@ protected boolean isTxOutputSpendable(TransactionOutput output) {
         return !daoStateService.existsTxOutput(key) || daoStateService.isRejectedIssuanceOutput(key);
     }
 
-    // BTC utxo in the BSQ wallet are usually from rejected comp request so we don't expect dust attack utxos here.
+    // Prevent to use dust attack utxos
</pre>

⬇️ Suggested change
<pre style="color: #555">-    // Prevent to use dust attack utxos
+    // Prevent usage of dust attack utxos
</pre>


<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5142#discussion_r573745476">core/src/main/java/bisq/core/btc/wallet/BisqDefaultCoinSelector.java</a>:</p>
<pre style='color:#555'>> @@ -65,7 +74,16 @@ public BisqDefaultCoinSelector() {
     public CoinSelection select(Coin target, List<TransactionOutput> candidates) {
         ArrayList<TransactionOutput> selected = new ArrayList<>();
         // Sort the inputs by age*value so we get the highest "coin days" spent.
-        ArrayList<TransactionOutput> sortedOutputs = new ArrayList<>(candidates);
+
+        ArrayList<TransactionOutput> sortedOutputs;
+        if (utxoCandidates != null) {
+            sortedOutputs = new ArrayList<>(utxoCandidates);
+            // We we reuse the selectors we reset the transactionOutputCandidates field
</pre>

⬇️ Suggested change
<pre style="color: #555">-            // We we reuse the selectors we reset the transactionOutputCandidates field
+            // We reuse the selectors. Reset the transactionOutputCandidates field
</pre>


<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5142#discussion_r573752428">desktop/src/main/java/bisq/desktop/main/dao/wallet/send/BsqSendView.java</a>:</p>
<pre style='color:#555'>> @@ -171,11 +188,16 @@ protected void activate() {
 
         bsqWalletService.addBsqBalanceListener(this);
 
+        // We reset the input selection at active to have all inputs selected, otherwise the user
</pre>

⬇️ Suggested change
<pre style="color: #555">-        // We reset the input selection at active to have all inputs selected, otherwise the user
+        // We reset the input selection at activate to have all inputs selected, otherwise the user
</pre>


<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5142#discussion_r573761201">desktop/src/main/java/bisq/desktop/main/dao/wallet/send/BsqSendView.java</a>:</p>
<pre style='color:#555'>> +            // If we had some selection stored we need to update to already spent entries
+            bsqUtxoCandidates = bsqUtxoCandidates.stream().
+                    filter(e -> unspentTransactionOutputs.contains(e)).
+                    collect(Collectors.toSet());
</pre>
<p>This would remove transactions that are no longer spendable BSQ outputs from the selected outputs. I think that could be confusing. Better to check if the <code>bsqUtxoCandidates</code> are still all spendable, if they are then all is good, if not, then reset the selection to allow choosing between all <code>unspentTransactionOutputs</code> again.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5142#discussion_r573767167">desktop/src/main/java/bisq/desktop/main/dao/wallet/send/BsqSendView.java</a>:</p>
<pre style='color:#555'>> +            btcUtxoCandidates = btcUtxoCandidates.stream().
+                    filter(e -> unspentTransactionOutputs.contains(e)).
+                    collect(Collectors.toSet());
</pre>
<p>Same comment as for the BSQ, could be confusing if the selection changes without the users knowing about it</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/5142#pullrequestreview-587622253">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNRT7QLTQMNXWXUJWHLS6KJVZANCNFSM4W37UGLA">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNQLHACOSEVJFMNA3DTS6KJVZA5CNFSM4W37UGLKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOEMDGO3I.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/5142#pullrequestreview-587622253",
"url": "https://github.com/bisq-network/bisq/pull/5142#pullrequestreview-587622253",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>