Check for spendable UTXOs before payjoin operations#1071
Conversation
48358f9 to
1d48deb
Compare
Pull Request Test Coverage Report for Build 17735121824Details
💛 - Coveralls |
arminsabouri
left a comment
arminsabouri
left a comment
There was a problem hiding this comment.
cACK
New validation and its use look correct to me. I had one suggestion for the error messages. Lastly, do you mind re-writing your commit message to include the rationale for the change. Not just what changes were made.
Great work 🔥
| if candidate_inputs.is_empty() { | ||
| return Err(ImplementationError::from( | ||
| anyhow::anyhow!( | ||
| "No spendable UTXOs available in wallet. Cannot contribute inputs to payjoin." |
There was a problem hiding this comment.
Nit: Lets add a call to action.
| "No spendable UTXOs available in wallet. Cannot contribute inputs to payjoin." | |
| "No spendable UTXOs available in wallet. Please fund your wallet before resuming this session" |
This gives the user two thing todo:
- Fund the wallet
- Resume the session
Thank you @arminsabouri |
Previously, attempting payjoin operations with an empty wallet resulted in a generic "500 Internal Server Error" from bitcoind. This created a poor user experience because users could not understand why their payjoin failed or what action to take.
1d48deb to
213f98b
Compare
|
Is this actually a payjoin-cli issue or was our RPC client (ripped out in #1063) just just not displaying the bitcoind error properly? |
arminsabouri
left a comment
arminsabouri
left a comment
There was a problem hiding this comment.
We have one follow up task for the receiver. Otherwise the error messaging seems like a strict improvement over what we have.
| if candidate_inputs.is_empty() { | ||
| return Err(anyhow::anyhow!( | ||
| "No spendable UTXOs available in wallet. Cannot contribute inputs to payjoin." | ||
| )); | ||
| } |
There was a problem hiding this comment.
This check if fine as the internal error is opaque and this is checked in the input selection logic https://github.com/payjoin/rust-payjoin/blob/3f057e130d45c423a22c5e41007529cc0ee6894f/payjoin/src/core/receive/common/mod.rs#L273C51-L273C87
But ideally the receiver should fallback here instead. And we propogate the internally error somehow so we don't have a duplicate check
| let candidate_inputs = | ||
| wallet.list_unspent().map_err(|e| ImplementationError::from(e.into_boxed_dyn_error()))?; | ||
|
|
||
| if candidate_inputs.is_empty() { |
There was a problem hiding this comment.
Same comment as v2 applies here
4 participants
This PR improves error handling when a wallet has no UTXOs.
Previously, attempting to send or receive without UTXOs returned a generic Error: Obtained failure status(500): Internal Server Error.
This change lets the code now performs UTXO checks and provides informative error messages for both send and receive operations.
This makes failures more user-friendly and easier to debug.
Closes #1070
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.