Swap: Handle monero under- and overfunding

[TheCharlatan]

Addresses #531 .
@zkao let me know what you think of Alice blocking the publishing of the buy transaction in case she overfunds. I'm not married to the idea and would be happy to remove this part of the logic.

[codecov-commenter]

Codecov Report

Merging #534 (6252d13) into main (57bc656) will decrease coverage by 0.1%.
The diff coverage is 0.0%.

@@           Coverage Diff           @@
##            main    #534     +/-   ##
=======================================
- Coverage   12.0%   11.9%   -0.1%     
=======================================
  Files         34      34             
  Lines      10308   10403     +95     
=======================================
+ Hits        1241    1243      +2     
- Misses      9067    9160     +93     

Impacted Files	Coverage Δ
src/swapd/runtime.rs	0.0% <0.0%> (ø)
src/swapd/swap_state.rs	0.0% <0.0%> (ø)
src/syncerd/syncer_state.rs	86.2% <0.0%> (-3.7%)	⬇️
src/syncerd/types.rs	24.1% <0.0%> (-1.3%)	⬇️
src/syncerd/bitcoin_syncer.rs	0.0% <0.0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57bc656...6252d13. Read the comment docs.

[zkao]

i was reviewing but i messed up and cancelled the review, so i just added 2 commits instead

[zkao]

@TheCharlatan can u diff the 4 last commits with ur version and review it?

[zkao]

@TheCharlatan, will the 5% funding error margin break the tests, here or in the bitcoin one?

[TheCharlatan]

@TheCharlatan, will the 5% funding error margin break the tests, here or in the bitcoin one?

It will break the test here: https://github.com/farcaster-project/farcaster-node/pull/534/files#diff-2dcb9e59d484446ce89de83ae51f011d9be045e37de5168024e0ab4134e9d07cR1073

I am not sure what this extra error margin is supposed to achieve. To my eyes, it just complicates everything even more.

[zkao]

i agree 5% is outrageous, that is why i put FIXME flags, just for now

i find it strange that one would abort a swap if the error is negligible such as 1 atomic unit or 1 sat

[TheCharlatan]

i agree 5% is outrageous, that is why i put FIXME flags, just for now

i find it strange that one would abort a swap if the error is negligible such as 1 atomic unit or 1 sat

The counterparty will reject the swap anyway, if you underfund, so aborting early will save time and fees. If you overfund, yeah, the counterparty is free to accept it, and a margin of error might make sense there. I'd rather have consistent behavior though and reject any user-induced error.

[zkao]

The counterparty will reject the swap anyway

This was not clear to me. Will the counterparty reject the swap?

[zkao]

if there is 1 sat missing?

[zkao]

The counterparty will reject the swap anyway

This was not clear to me. Will the counterparty reject the swap?

I guess you meant that the software is not handling it correctly elsewhere, I suppose

Not that the counterparty (as a human being) will deliberately choose to abort a swap <- this is what I read

[TheCharlatan]

I guess you meant that the software is not handling it correctly elsewhere, I suppose
Not that the counterparty (as a human being) will deliberately choose to abort a swap <- this is what I read

Yes, I mean the software. Here Bob rejects the funding amount if it does not match what he is expecting: https://github.com/farcaster-project/farcaster-node/blob/main/src/swapd/runtime.rs#L1141

[zkao]

hmm, on these kind of thing i always add an epsilon (error margin) for negligible errors.

i can fix the rest, but it seems you think its a bad idea, can you elaborate why

[TheCharlatan]

i can fix the rest, but it seems you think its a bad idea, can you elaborate why

I'll just rattle down some reasons here. As a swapper, I want to swap the exact amount, no more and no less. This is money, introducing error margins is usually a bad idea and leads to unknown side effects. If you add an error margin, then why not just swap at the limits all the time? We have an entire protocol to establish exact amounts and I don't support relaxing this here at the lowest level. If you want to trade within a margin, you should establish that in the negotiation and then commit to a fixed amount for the swap.

[zkao]

hmm, ok, we disagree on this one.

but i see what you're saying working for monero

but i dont see that currently working for bitcoin, as we have the 1st transaction from the external wallet, and we might not know exactly how much to pay on fees

[TheCharlatan]

but i dont see that currently working for bitcoin, as we have the 1st transaction from the external wallet, and we might not know exactly how much to pay on fees

It's not implemented currently, but Alice should check the exact amount in the Lock transaction's output and abort if it is incorrect in my opinion.

EDIT: Which means we have to explain to the user why he is paying extra in fees for the funding transaction.

[zkao]

so i guess what you're saying means: bob must pay fees in lock tx matching exactly what was requested and included in the amount of the funding tx?

[zkao]

removed the offending commits, but in general, i would be totally ok if someone robs me epsilon, since epsilon is negligible

[zkao]

@TheCharlatan if you're OK with the additional commits, please merge or signal me to merge

[zkao]

so i guess what you're saying means: bob must pay fees in lock tx matching exactly what was requested and included in the amount of the funding tx?

it also means we know exactly the size of the lock tx, is that a true assertion?

[h4sh3d]

I’m not against an epsilon if we want one. If we do it it should only account for fee variation until we use RBF and merge funding and lock txs.

IMO It is fair to check the exact amount against what was agreed in the trade in both lock txs outputs, for accordant that’s easy, for arbitrating it require bumping funding, which is what we already do IIRC.

we should be able to exactly know the lock size as it’s a 1-1 in-out tx with all the keys and scripts controlled by the protocol.

Currently core errors when creating a smaller lock output than what was requested in the offer. We have to fix this if epsilon allow lower. Also Alice while signing cancel checks the set of core arbitrating txs, so she already checks the exact amount in arbitrating lock.

If I try to do the sequence of actions to get the lock on chain:

• get the offer’s amount $a$
• estimate tx lock fee $f_0$ at time $t_0$
• display address and amount $a + f_0$ to the user
• wait until we see tx funding, see it at time $t_1$
• create the set of core arbitrating txs, for that we can:
• re-estimate fee $f_1$ for lock tx
• if between $t_0$ and $t_1$ the fees increased such that applying the newly calculated $f_1$ would go lower than $a$, then we need an epsilon

In my understanding having epsilon in the current setup (no use of RBF and interactive PSBT with the user) would allow to correct the lock fee at $t_1$. This time delta should be some minutes as we don’t wait on funding to be mined and between $t_1$ and broadcast it should be some seconds to complete interaction with counter-party.

Is there something I missed in the sequence?

For me the question is do we want to double estimate fees (at $t_0$ and $t_1$) and potentially adjust (what if $f_1 &lt; f_0$ do we give more to counter party? it is negligible tho) or we want to estimate only once.

[zkao]

@h4sh3d so vsize for the bitcoin Lock is always 94 vB?

[TheCharlatan]

Going by @h4sh3d 's argument for accounting for fee variation, I am even more against having an epsilon for Monero, since the fees don't matter in this case and should be burdened on the funder of the AccLock completely.

For fhe ArbLock and the Bitcoin funding I am still not sold, even by Joels argument. Establishing a sane epsilon value that accounts for fee variation is complex. If we introduce an epsilon it should be the difference in the estimated fee between time $t_0$ and $t_1$ and not some arbitrary value. If this really has to be introduced, and I stil hold the opinion that this is a largely unneeded complication, it should not be introduced right now, but rather in a separate PR once the fee estimation has properly landed.

[zkao]

re-estimate fee for lock tx
if between and the fees increased such that applying the newly calculated would go lower than , then we need an epsilon

If the size is deterministic, i'd just not take a new fee at $t_1$, basically commiting to the fee already at $t_0$, as u said, its a zero conf thing, only issue is if the funding is paying too low fee and lock should pay for parent

[zkao]

actually, the person could take ages to publish the funding tx, and maybe $t_1$ is necessary

[Lederstrumpf]

So I currently see the following three reasons why a party may underfund:

1. user error
2. in the arbitrating case, too low fee estimate for the locking transaction
3. doing so because the protocol tolerates it

The proposal to handle this by introducing an epsilon tolerance introduces more problems than it superficially solves and weakens the protocol from a few perspectives. In particular, error margins will handle 1. & 2. some of the time, but introduce the possibility of 3., and I vehemently object to this and we should be careful with anything that caters for such possibilities.
The arguments apply against it for both arbitrating & accordant, but even more so for accordant, since 2. is not even a concern there. So I will discuss this from the perspective that Bob underfunds only, since if it's a bad idea then, it's even more so for Alice underfunding:

High level objections:

1. It weakens Farcaster as a building block for composing with other protocols or usage with automated backends since its output is less deterministic. Any other software that utilises Farcaster in one form or another has to account for its increased flakiness.
2. My most significant issue with this proposal is that it weakens its game theory: On an ad-hoc level, Alice may not care about her received amount being off by a few sats. But on a systematic level giving the protocol's Bobs the freedom to push it to some error margin in their favour means this limit will be exercised by Bobs regularly - in particular by frequent Farcaster users where these sats adds. And these Bobs will regularly do so because, assuming that the Alices don't use software modified to remove epsilon tolerances, they will accept it. If the dominant Farcaster distribution does not have this tolerance, it will also not be exploited, since the exploiter should expect their counterparty to simply reject the attempt. So on a systematic level, by moving forward with this proposal, we'd introduce an opportunity for exploitation that weakens Farcaster.
3. Building on 2.: whenever Bob underfunds to the epsilon limit based on knowing that Alice's (most likely) software stack will accept it, this also removes the estimate tolerance that the margin was supposed to provide.

Specifically regarding user error:

i find it strange that one would abort a swap if the error is negligible such as
1 atomic unit or 1 sat

For bona fide user error, the error would typically be a typo. That this typo is a mere sat is very unlikely - I'm sure missing or adding a decimal is more likely. Likewise for any other typos that would fall within some error tolerance, even if the error tolerance is 5%, it's still a small subset of typos that are addressed by this proposal.

For handling fee fluctuations:

Although it might make us less efficient on fees, instead of burdening Alice with the cost of handling the fee estimate being off, you could also just increase the amount Bob funds with. That is: instead of Bob paying $a + f_0$, increase the fee estimate $f_0$ by some factor $k$ so that what Bob then funds, $a + k f_0$, already accounts for fluctuations up to some bound.
At $t_1$ with fee estimate $f_1$, you then have the following options how to handle the desired scenario, i.e. $f_1 \leq k f_0$:

1. Pay fee $k f_0$ anyway
2. increase the amount $a$ to $a + k f_0 - f_1$
3. create another tx output that sends $k f_0 - f_1$ back to Bob

My preference here would be 1. or 2..
1. still increases the likelihood of the tx being included by a miner sooner.
2. does not "waste" the surplus estimate on a miner, and the surplus is received by one of the swap participants, depending on swap protocol path.

My preference for this is also that it leaves responsibility & ownership of correctly handling fee fluctuations with Bob - who is in control of creating these transactions, instead of outsourcing the ultimate cost to Alice.

[zkao]

as far as i get, the vars of interest are: funding fee, funding amount, lock fee

and lock amount is constant

lock amount is funding amount minus lock fee and lock tx has always the exact same size

there is an implicit assumption that funding tx that is externally generated pays fees correctly, and that lock tx does not have to account for that, maybe check if funding fees are consistent?

this discussion belongs to #533

[zkao]

I approved this for merging, but since I added commits, I'd like ACK for merging, or someone else merges, thank you

[TheCharlatan]

@zkao ACK your changes 6252d13 , feel free to merge.