Fix: Payjoin wasn't always properly choosing utxo for optimal change #4600
Conversation
NicolasDorier
force-pushed
the
fweioqn
branch
from
bb39367
to
967f76d
Compare
| { | ||
| // UIH2: Unnecessary input, let's try to get an optimal change by using a different output | ||
| continue; |
There was a problem hiding this comment.
I think this will loop without getting any closer to maxTries because it does not add to locked. Therefore maxTries is not the hard cap on tries it once was
There was a problem hiding this comment.
This was on purpose, as this line doesn't make any DB roundtrip.
Irrelevant now, as I use a timeout.
| // We don't want to make too many db roundtrip which would be inconvenient for the sender | ||
| int maxTries = 30; |
There was a problem hiding this comment.
If the goal here is to accommodate the sender a timeout may make more sense. Default HTTP request range timeouts are 30 to > 100 seconds. A timeout of say 10s would be well within the acceptable range for every HTTP client I know of. That would allow you to account for other slowdowns, too.
|
@DanGould indeed, will update code to use a timeout instead. |
|
Updated for timeout of 10s |
|
Looks like the update applied a timeout of 30s, not 10s, but if it takes that long to search through utxos selection it would seem extraordinary to find a solution |
This paper assert that lot's of payjoin transaction exhibit UIH2: Unnecessary input trait.
I was surprised as @Kukks made this PR a while ago #1473
By writing a proper test, I found out that the implementation was actually broken, and not always avoiding UIH2.
On top of it the implementation was suboptimal.
I also updated the tests.
Closing #4599