search

payjoin / rust-payjoin

Supercharged payment batching to save you fees and preserve your privacy
https://payjoindevkit.org
85 stars 37 forks source link

Add amount checks to integration tests #351

Closed spacebear21 closed 3 months ago

spacebear21 commented 3 months ago

At the end of each integration test, check the resulting transaction inputs/outputs, and the resulting balances for the sender and receiver.

Note that the sender sweep test case requires the original PSBT fee rate to be high enough to cover the receiver input, because neither the sender nor the receiver can contribute additional fees in the payjoin PSBT due to current API limitations.

Originally added these changes to the tx-cut-through branch but pulled them out into a separate PR for easier review.

DanGould commented 3 months ago

Nightly changed from 1.77 to 1.78 so cargo fmt changed. grrr

DanGould commented 3 months ago

I'd rather we use rust-bitcoin's fee calculation instead of lopp's estimations, but I ran into some trouble when I tried. If you're confident this calculation is just a bit of repetition and the edge cases of variable signature size are palatable rather than tech debt I'm comfortable merging this request. If you see the issues I raised as problems, we should address them before this gets in.

spacebear21 commented 3 months ago

Ok let's hold off on merging, I'll take a stab at using rust-bitcoin fee calculations today.

DanGould commented 3 months ago

I believe the reason we want these tests is to make sure transaction construction doesn't regress. Is that correct? Has this caught bugs and enabled development in #332?

spacebear21 commented 3 months ago

Regression testing was the original reason for adding these checks, but I also like that it helps self-document what exactly is going on in each scenario. These changes were originally part of #332 but I pulled them out into a separate PR for independent review and to make #332 lighter.

I've found them very helpful in testing cut-through scenarios, e.g. receiver forwards payment. The fee checks are especially useful when the sender and the receiver are both on the hook for fees.

I ran these tests on a loop for ~an hour (187 iterations) and didn't get any errors, so I'm reasonably confident that they're not flaky.