[bisq-network/bisq] Add coin input control (#5142)

sqrrm notifications at github.com
Wed Feb 10 15:27:40 CET 2021


@sqrrm commented on this pull request.

Testing issue:
1. Start with a BSQ wallet with no BTC balance
2. BSQ selection popup shows when button is pressed
3. Send BTC to BSQ receive address (remove leading "B")
4. BTC balance section shows in BSQ wallet but neither "select inputs" button work.

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

```suggestion
    // Prevent usage of dust attack utxos
```

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

```suggestion
            // We reuse the selectors. Reset the transactionOutputCandidates field
```

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

```suggestion
        // We reset the input selection at activate to have all inputs selected, otherwise the user
```

> +            // 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());

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 `bsqUtxoCandidates` are still all spendable, if they are then all is good, if not, then reset the selection to allow choosing between all `unspentTransactionOutputs` again.

> +            btcUtxoCandidates = btcUtxoCandidates.stream().
+                    filter(e -> unspentTransactionOutputs.contains(e)).
+                    collect(Collectors.toSet());

Same comment as for the BSQ, could be confusing if the selection changes without the users knowing about it

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/5142#pullrequestreview-587622253
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20210210/4bb198b5/attachment-0001.htm>


More information about the bisq-github mailing list