[ Feat ] Updated Direct-Send RPC-Api to accept list of UTXOs

[amitx13]

This PR fixes https://github.com/JoinMarket-Org/joinmarket-clientserver/issues/1712 and addresses https://github.com/joinmarket-webui/jam/issues/772 Changes Made:

• Updated the direct-send API to accept a list of UTXOs specified by the user.
• Implemented backend logic to handle the selection of these UTXOs for the transaction.
• Added necessary validation checks to ensure the specified UTXOs are part of the user's wallet.
• Ensured backward compatibility by making selected_utxos an optional property."

[amitx13]

Hey @AdamISZ and @kristapsk,

First of all, sorry for the disturbance. If you have a moment, could you please take a look at ~docs/api/wallet-rpc.yaml? I tried solving it on my own, but OpenAPI Diff keeps failing. I just want to add Coincontrol to JAM so that users can have more control over their funds. I apologize for being so annoying. I am in contract with JAM through the Summer of Bitcoin program and do not have much time left. I hope you understand, and thank you for your time and the great work you do.

[kristapsk]

@amitx13 Sorry, busy with other stuff, will try to find time and take a look.

[kristapsk]

I can't see the reason why CI is failing here. Bug in OpenAPI Diff? For some reason it thinks that selected_utxos is introduced as a required property, but it clearly isn't. Should we just ignore this?

@theborakompanioni Could you please test and confirm that this change does not break anything with Jam? That would be good enough for me to merge this.

[theborakompanioni]

@theborakompanioni Could you please test and confirm that this change does not break anything with Jam? That would be good enough for me to merge this.

I have now had time to test the changes. I exclusively tested on regtest via RPC api calls.

• Sending a single locked UTXO: Fails with "Transaction failed." :heavy_check_mark:
• Sending a single frozen UTXO: Fails with "Transaction failed." :heavy_check_mark:
• Sending an unfrozen and a frozen UTXO: Fails with "Transaction failed." :heavy_check_mark:
• Sending two unfrozen UTXO from different mixdepths: Fails with "Transaction failed." :heavy_check_mark:
• Sending a single locked but expired UTXO: Success! :heavy_check_mark:
• Sending selected_utxos as empty array: Success - as if no array was given - is this expected? :question:

When doing a sweep (amount_sats := 0), the selected_utxos is ignored! This should probably fail, or sweep with only the selected UTXOs! Which approach is preferred? I'd opt for failing, as a sweep with certain UTXOs can be done in a two phase: Freeze unselected UTXO, then sweep (don't provide selected_utxos).

So, from my point of view, the changes look good overall (except the sweep case). I am happy as this brings Coin Control to JM/Jam, albeit only for direct sends.

However, I'd love to see some tests covering this behaviour and verifying the above manual test cases. Great job @amitx13! And thanks for reviewing and considering this change @kristapsk! :pray:

[amitx13]

• Sending selected_utxos as empty array: Success - as if no array was given - is this expected? ❓

Yes, this is intentional. When no value is provided, selected_utxos is considered none, which ensures backward compatibility with the RPC-API.

When doing a sweep (amount_sats := 0), the selected_utxos is ignored! This should probably fail, or sweep with only the selected UTXOs! Which approach is preferred? I'd opt for failing, as a sweep with certain UTXOs can be done in a two phase: Freeze unselected UTXO, then sweep (don't provide selected_utxos).

I agree, failing makes more sense. Freezing unselected UTXOs and then sweeping with selected UTXOs would alter the original definition of a sweep. Instead of ignoring this case, we should fail it with Transaction failed.

Looking forward to hearing @kristapsk thoughts on this.

[amitx13]

Done with the changes requested by @roshii and fixed the sweep case that @theborakompanioni mentioned earlier. Please take a look. Thank you for your time, guys.