Dedup utxos locked method#1568
Conversation
Coverage Report for CI Build 26119539926Coverage increased (+0.05%) to 85.349%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
The verbage of this function suggests that this function returns the reciever's utxos to be locked. This comits moves this function to `PsbtContext` where we know the sender's inputs and can filter them out.
This is not a general comment describing the code but something to be potentially done in the future.
arminsabouri
force-pushed
the
dedup-utxos-locked
branch
from
848e7f5 to
301d280
Compare
| self, | ||
| wallet_process_psbt: impl Fn(&Psbt) -> Result<Psbt, ImplementationError>, | ||
| ) -> Result<PayjoinProposal, Error> { | ||
| let original_psbt = self.psbt_context.original_psbt.clone(); |
There was a problem hiding this comment.
So I think you can avoid this clone() by:
- updating
PsbtContext::finalize_proposal()to not consumeself - updating
PsbtContext::prepare_psbt()to not consumeself - move this assignment below
let finalized_psbt = ...assignment
|
canmt the function be completely removed because the functions that create / modify PSBTs already know what their utxos to lock are? is this fn used in any integration whatsoever? |
Yeah I guess so but might force callers to keep separate state of what inputs were contributed until the time of signing no? Or I guess looking up what utxos are signed on the proposal should be easy enough. |
Is this being done anywhere? Not that I know of. I could see it being a possibility for exchanges/services w/ async signing tho |
xstoicunicornx
left a comment
xstoicunicornx
left a comment
There was a problem hiding this comment.
utACK 301d280
Reviewed and tested the changes. I think it makes sense to make a resuable utxos_to_be_locked() method on PsbtContext that both v1 and v2 can leverage. Also think its a good method to provide to callers in the PayjoinProposal typestate. I don't actually feel strongly about the clone related comment earlier (but still think maybe worth doing?).
4 participants
Resolving this todo comment:
TODO: de-duplicate this with the v1 implementationby movingutxos_to_be_lockedtoPsbtContext.The bigger change is to filter out the sender inputs. If the prupose of this method is to inform the reciever what inputs they have to mark as "locked".
Not a fan of cloning the original PSBT. The only thing we need is sender input indecies.
In general,
PsbtContextonly needs avec<usize>for the sender inputs (for other methods:psbt_to_sign). The only place where we use the original tx is the in the v2 monitor typestate. This is where something like #1197 would be useful.Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.