Batching Payments in Coinjoin

[lontivero]

PR goal

To allow batching payments in coinjoins

Term definitions

• A payment in coinjoin is a payment made by registering the payees' scriptPubKeys in the coinjoin outputs side.
• A batch of payments is simply a list of payment intents (destinations, amounts) that the user wants to make in one or more coinjoins.

---

For more detailed explanation about the design considerations see: #10570

[lontivero]

Notes for testers:

1. Run Wasabi with the RPC server enabled. You can run the GUI or the Daemon but I will show only the daemon:

$ dotnet run \
    --network=testnet \
    --jsonrpcserverenabled=true \
    --jsonrpcuser="" \
    --jsonrpcpassword=""  \
    --wallet=<MyTestNetWallet>

After your wallet is started (fully synchronized) you can create a new pending payment:

$ curl -s --data-binary '{"jsonrpc":"2.0", "id":"curltext", "method":"payincoinjoin", "params":["<TestNetBitcoinAddress>", <Satoshis>]}' -H 'content-type: text/plain;' http://127.0.0.1:37128/<MyTestNetWallet>

In case your wallet was not configured to start coinjoining automatically you can force it by doing:

$ curl -s --data-binary '{"jsonrpc":"2.0", "id":"curltext", "method":"startcoinjoin", "params":[]}' -H 'content-type: text/plain;' http://127.0.0.1:37128/<MyTestNetWallet>

Happy paycoinjoining!

[MaxHillebrand]

EDIT: unrelated bug here #10621

[lontivero]

The GUI doesn't react to changes in the underlying model, that's an UI problem. This PR is for payments in coinjoins only. All bugs you find that are not related (can be reproduced in master) please open a bug report in a new issue.

[MaxHillebrand]

tACK 6a82a31

It works like a charm!

[kiminuo]

Partial review

[kiminuo]

Suggested change

	new (Amount, Destination);
	new(Amount, Destination);

[kiminuo]

Another partial review.

[stale]

This has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[lontivero]

Can we merge this feature?

[MaxHillebrand]

tACK aa57dd9

the UI has this tx history icon overlay bug.

[turbolay]

Interestingly, this already happens from time to time on master #9171

[MarnixCroes]

Succesfully did a few paymentincoinjoin 's, both test and mainnet🎉 aa57dd9

• What am I/the user supposed to do when all coins are private and want to do a paymentincoinjoin? If I do startcoinjoin with stopwhenallmixed:false parameter, it doesn't start coinjoin, neither when I schedule a payment. Coinjoin status stays stuck at In Schedule
  (Maybe out of scope of this PR and more related to startcoinjoin method)
• Maybe add a log when paymentincoinjoin was succesful?
• useful follow-up would be listpendingpayments rpc method
• I got this ERROR in the log:

log ERROR, exception: overall balance must not be negative

[28] INFO	PaymentBatch.LogPaymentSetDetails (103)Best payment set contains 0 payments.
2023-10-17 20:26:23.690 [28] ERROR	CoinJoinManager.HandleCoinJoinFinalizationAsync (549)	Wallet (Testint): CoinJoinClient failed with exception: 'System.ArgumentException: Overall balance must not be negative.
   at WalletWasabi.WabiSabi.Client.CredentialDependencies.DependencyGraph.FromValues(IEnumerable`1 inputValues, IEnumerable`1 outputValues) in WalletWasabi/WabiSabi/Client/CredentialDependencies/DependencyGraph.cs:line 96
   at WalletWasabi.WabiSabi.Client.CredentialDependencies.DependencyGraph.ResolveCredentialDependencies(IEnumerable`1 inputValues, IEnumerable`1 outputValues) in WalletWasabi/WabiSabi/Client/CredentialDependencies/DependencyGraph.cs:line 74
   at WalletWasabi.WabiSabi.Client.CredentialDependencies.DependencyGraph.ResolveCredentialDependencies(IEnumerable`1 effectiveValuesAndSizes, IEnumerable`1 outputs, FeeRate feeRate, Int64 vsizeAllocationPerInput) in WalletWasabi/WabiSabi/Client/CredentialDependencies/DependencyGraph.cs:line 67
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ProceedWithOutputRegistrationPhaseAsync(uint256 roundId, ImmutableArray`1 registeredAliceClients, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 770
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ProceedWithRoundAsync(RoundState roundState, IEnumerable`1 smartCoins, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 351
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartRoundAsync(IEnumerable`1 smartCoins, RoundState roundState, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 249
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartRoundAsync(IEnumerable`1 smartCoins, RoundState roundState, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 329
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartCoinJoinAsync(Func`1 coinCandidatesFunc, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 198
   at WalletWasabi.WabiSabi.Client.CoinJoinManager.HandleCoinJoinFinalizationAsync(CoinJoinTracker finishedCoinJoin, ConcurrentDictionary`2 trackedCoinJoins, ConcurrentDictionary`2 trackedAutoStarts, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinManager.cs:line 499'

while my balance is more than 100k sats

another ERROR in the logs, value cannot be null exception

[22] ERROR	CoinJoinManager.HandleCoinJoinFinalizationAsync (549)	Wallet (Testint): CoinJoinClient failed with exception: 'System.ArgumentNullException: Value cannot be null. (Parameter 'left')
   at NBitcoin.Money.op_GreaterThan(Money left, Money right)
   at WalletWasabi.WabiSabi.Client.AmountDecomposer.Decompose(IEnumerable`1 myInputCoinEffectiveValues, IEnumerable`1 othersInputCoinEffectiveValues) in WalletWasabi/WabiSabi/Client/AmountDecomposer.cs:line 87
   at WalletWasabi.WabiSabi.Client.OutputProvider.GetOutputs(uint256 roundId, RoundParameters roundParameters, IEnumerable`1 registeredCoinEffectiveValues, IEnumerable`1 theirCoinEffectiveValues, Int32 availableVsize) in WalletWasabi/WabiSabi/Client/OutputProvider.cs:line 30
   at WalletWasabi.WabiSabi.Client.Batching.PaymentAwareOutputProvider.<>n__0(uint256 roundId, RoundParameters roundParameters, IEnumerable`1 registeredCoinEffectiveValues, IEnumerable`1 theirCoinEffectiveValues, Int32 availableVsize)
   at WalletWasabi.WabiSabi.Client.Batching.PaymentAwareOutputProvider.GetOutputs(uint256 roundId, RoundParameters roundParameters, IEnumerable`1 registeredCoinEffectiveValues, IEnumerable`1 theirCoinEffectiveValues, Int32 availableVsize)+MoveNext() in WalletWasabi/WabiSabi/Client/Batching/PaymentAwareOutputProvider.cs:line 63
   at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items)
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ProceedWithOutputRegistrationPhaseAsync(uint256 roundId, ImmutableArray`1 registeredAliceClients, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 768
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ProceedWithRoundAsync(RoundState roundState, IEnumerable`1 smartCoins, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 351
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartRoundAsync(IEnumerable`1 smartCoins, RoundState roundState, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 249
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartRoundAsync(IEnumerable`1 smartCoins, RoundState roundState, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 329
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartCoinJoinAsync(Func`1 coinCandidatesFunc, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinClient.cs:line 198
   at WalletWasabi.WabiSabi.Client.CoinJoinManager.HandleCoinJoinFinalizationAsync(CoinJoinTracker finishedCoinJoin, ConcurrentDictionary`2 trackedCoinJoins, ConcurrentDictionary`2 trackedAutoStarts, CancellationToken cancellationToken) in WalletWasabi/WabiSabi/Client/CoinJoinManager.cs:line 499'

and left a few nits

[lontivero]

@MarnixCroes the second error that you see in the log is solved by nopara73/Sake#44 after we merge it in Sake we can merge it here.

[lontivero]

@MarnixCroes do you know how the first error happened? It is in testnet or main? Do you remember what you did? Is it something related to small payments?

[MarnixCroes]

@MarnixCroes do you know how the first error happened? It is in testnet or main? Do you remember what you did? Is it something related to small payments?

It was on testnet. I was testing what happens when registering 3 payments, of which 2 are to the same address. All 3 of them were 10k sats each. I'm not 100% sure if that's what caused it, as the error came later in time, after it successfully did my "test case".

[turbolay]

utACK overall, maybe some weird edge cases but nothing significant, I think.

I have a conceptual "doubt" about this feature, is it better to choose outputs depending on payments as you are doing or would it be better to select inputs depending on pending payments?

[turbolay]

Could the fact that this is deterministic pose any privacy concerns? I've given it some thought and couldn't identify any problems

[lontivero]

is it better to choose outputs depending on payments as you are doing or would it be better to select inputs depending on pending payments?

This point was addressed, but not at a thoroughly enough detail, in the design document section "Implementation approaches".

This PR implements what in the document we called the Pay while coinjoining approach because it is the simplest one and consists just in providing transaction outputs. This design has the cons that it can be implemented with a non-intrusive style that required to change only one line in the CoinJoinClient and five in the CoinJoinManager.

The more powerful approach, that includes what you mentioned here, is the called Pay regardless of coinjoin approach that allows you not only to provide outputs but also control every single aspect of the payment, from coin selection, notification system, change avoidance among many other desirable characteristics but it would require much deeper changes (I believed it would require to create another CoinJoinManager or PayInCoinJoinManager)

[turbolay]

I believe that this should be merged.