<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>