Fall back to first candidate if avoid_uih fails#533
Conversation
DanGould
force-pushed
the
only-try-preserving-privacy
branch
3 times, most recently
from
649d305 to
b88511c
Compare
|
It's not yet clear to me whether we want to expose the private coin selection methods in-place as public, or whether we want to make public pure functions that a sophisticated downstream receiver can implement themselves. I think the latter is a whole lot cleaner but is also more code to maintain. It might be a good place for us to start addressing the coin selection problem in general in this repo. Using Psbt V2 Inputs would make the interface a whole lot cleaner instead of using our custom |
payjoin/src/receive/v1/mod.rs
Outdated
| /// To ensure the resemblance, a number of heuristics must be avoided. | ||
| /// | ||
| /// UIH "Unnecessary input heuristic" is avoided for multi-output transactions. | ||
| /// Attempt to avoid UIH (Unnecessary input heuristic) for multi-output transactions. |
There was a problem hiding this comment.
Nit: specify "two-output transaction" instead of "multi-output transaction".
DanGould
force-pushed
the
only-try-preserving-privacy
branch
from
b88511c to
1201492
Compare
try_preserving_privacy is about a best-effort attempt, not a guaranteed success. This implementation is what was already implied by the docstring. So it's a fix. Making avoid_uih and select_first_candidate public later can be public to allow for more granular downstream control.
It can be too short too.
spacebear21
left a comment
spacebear21
left a comment
There was a problem hiding this comment.
Not directly related to this change, I noticed select_first_candidate returns a InternalSelectionError::NotFound if there are no candidates - that should be InternalSelectionError::Empty instead.
Pull Request Test Coverage Report for Build 13294028995Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
4 participants
try_preserving_privacy is about a best-effort attempt, not a guaranteed success. This implementation is what was already implied by the docstring. So it's a fix.
Making avoid_uih and select_first_candidate public later can be public to allow for more granular downstream control.