JoinMarket-Org / joinmarket-clientserver

Bitcoin CoinJoin implementation with incentive structure to convince people to take part
GNU General Public License v3.0
725 stars 178 forks source link

[QT-GUI improvement] tick/untick coins to unfreeze/freeze #1010

Open openoms opened 3 years ago

openoms commented 3 years ago

Right now one needs to right click on each of the coins to freeze/unfreeze although multiple selection is possible.

A quicker and more intuitive way to do this would be to able tick/untick utxos which then can be spent in the Coinjoins tab somewhat similar to the Electrum interface.

@wukong1971 's use of QT Designer seems very promising in https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/932, this might be another thing which could be improved.

BitcoinWukong commented 3 years ago

Is this about adding a new "Spend from selected coins" feature?

If so, I think it's not just a QT-GUI improvement, but also a new feature for the cli as well, so that a user can python sendpayment.py [coin1] [coin2] [dest_address] [amount].

openoms commented 3 years ago

The sendpayment.py or Coinjoins tab already only spends the unfrozen coins so the behaviour is there, just not shown in the GUI. Freezing is the way to use coin control in JM currently.

BitcoinWukong commented 3 years ago

The sendpayment.py or Coinjoins tab already only spends the unfrozen coins so the behaviour is there, just not shown in the GUI.

Hmm, the Coins tab is part of the GUI, and it does show what coins are "frozen", so why is it "not shown in the GUI"?

My understanding is that this issue wants to change the behavior of the GUI regarding Coin Control, but it's not clear to me what the new behavior should look like.

to able tick/untick utxos which then can be spent in the Coinjoins tab

By reading this comment, I thought what was asked is: "select a few coins in the Coins tab", and then "right click", and in the popup menu, in addition to "freeze/unfreeze", there is a new option of "spend from these coins", and those coins will be used to do the coinjoin if selected.

It seems that my understanding above might be incorrect. Could you elaborate what the desired behavior should look like? Thanks!

openoms commented 3 years ago

Example aim: spend two coins out of 10 in a mixdepth.

Current behaviour: Coins tab: freeze 8 coins by selecting - right clicking - and choosing freeze Coinjoin tab - spend all not frozen coins by setting 0 as the amount (sweeping spend).

Desired change: instead of needing to right click for a context menu, could have a tickbox next to all coins in the Coins tab which is:

Spending can be still done from the Coinjoins tab which currently has no indication of the (unfrozen) amount available for spending.

BitcoinWukong commented 3 years ago

I see, thanks for clarifying the use case.

I don't really like this proposed design for the following reasons:

  1. In almost all other Bitcoin wallets, the freeze / unfreeze feature is intended to be used for a longer term. I.e., when a user freeze the coins, it means they don't want to spend those coins for quite some time, not just the next Send transaction alone. I understand that freezing all other coins and sweeping a specific mixdepth is the current approach of doing coin-control in Joinmarket, but I also think this is a short term workaround, not the ideal long term solution.

  2. The feature of "Freeze / Unfreeze" coins is used widely in many places of JoinMarket. For example, the YieldGenerator depends on this feature to exclude unlocked Fidelity Bond from being included in the maker UTXO set. So I think it's a good idea to separate the concept of "selected coins for next coin join" and the concept of "frozen coins that should not be used in any transaction".

  3. In the code base, the Frozen Coins are actually marked as disabled coins. And I think disabled coins has very different meaning from unselected coins. Although we can potentially rename & repurpose Frozen Coins into Deselected Coins, I'm not sure that's what everybody wants.

So, given these concerns, my proposal is to support both "Freeze Coins for long term" and "Select Coins for the next Spend transaction" at the same time.

And here is what it could look like:

  1. Add a checkbox next to all coins in the Coins tab
  2. When a checkbox is checked, that particular coin is "Selected", and will be used in the next CoinJoin transaction.
  3. Multiple checkboxes of the same mixdepth can be checked at the same time.
  4. Checkboxes of different mixdepths can not be selected at the same time. For example, if a user selected checkbox of Coin A from mixdepth 0, all the checkboxes in mixdepth 1 ~ 4 would be disabled automatically.
  5. If a coin is Frozen, its checkbox will be disabled, and that particular coin can not be selected until it is unfrozen.
  6. After the coin selection, the user switches to the Coinjoin tab, and it will only use those selected coins to perform the Coinjoin transaction.
  7. Once the transaction is signed and broadcast, all checkboxes in the Coins tab will be reset to unselected status.

Please let me know what you think about this, thanks!

openoms commented 3 years ago

You made a very good point about the difference between Disabled and Unselected coins and that both are called being called Frozen currently.

I very much agree with the method of making a difference between the long term Disabled ones by disabling the checkbox and the coin control for the next transaction. The steps you described utilising checkboxes would provide an intuitive way of coin control.

I really like the idea of deselecting the checkboxes in the other mixdepths when one is used as it explains that one can spend from only one at a time at a glance.

ConceptACK on the comment above.

BitcoinWukong commented 3 years ago

Glad that we're on the same page :)

I plan to implement this feature in two steps:

  1. Update sendpayment.py, so that you can specify the spend_from_addresses or spend_from_utxos directly without the freeze/unfreeze dancing.
  2. Update the Qt GUI to present this new feature in JoinMarketQt