Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Zero-amount swap does not account for pair fee changes #602

Open
dangeross opened this issue Dec 13, 2024 · 4 comments · May be fixed by #615
Open

Zero-amount swap does not account for pair fee changes #602

dangeross opened this issue Dec 13, 2024 · 4 comments · May be fixed by #615
Assignees
@dangeross @danielgranhao

Comments

@dangeross
Copy link
Collaborator

If the pair fee hash changes during the zero-amount chain swap, the the server lockup quote validation fails:

[2024-12-13 11:08:18.433 DEBUG breez_sdk_liquid::chain_swap:414] Found lockup balance GetBalanceRes { confirmed: 0, unconfirmed: 50000 }
[2024-12-13 11:08:18.434 DEBUG breez_sdk_liquid::chain_swap:439] user_lockup_amount_sat = 50000, service_fees_sat = 50, server_fees_sat = 781, expected_server_lockup_amount_sat = 49169, quote_server_lockup_amount_sat = 49018
[2024-12-13 11:08:18.434 ERROR breez_sdk_liquid::chain_swap:339] Failed to accept the quote for swap qn2eEvvcxo76: Generic { err: "Invalid quote: expected at least 49169 sat, got 49018 sat" }

Creation pair fees:

{"hash":"592d31f2ebdcc3ae55aca9e1d9ea9f712498d9992fdcb35cdbce14b71c712748","rate":1.0,"limits":{"maximal":25000000,"minimal":50000,"maximalZeroConf":0},"fees":{"percentage":0.1,"minerFees":{"server":781,"user":{"lockup":770,"claim":14}}}}

Validation pair fees:

{"hash":"70979521c3bcd465364d8e1d0e3282409f38a36316b6508d915a3fa329263810","rate":1.0,"limits":{"maximal":25000000,"minimal":50000,"maximalZeroConf":0},"fees":{"percentage":0.1,"minerFees":{"server":932,"user":{"lockup":924,"claim":14}}}}
@roeierez
Copy link
Member

So the issue here is that the swapper treats differently amount less swaps with regards to fees.
When the swap is created with a specific amount the fees are decided and locked for that swap (service fees and onchain fees)
When an amount-less swap is created the swapper is not committed to a specific fee settings but rather waits for the user's lockup to be confirmed and returns the fees in the quote request by the user.
So from the user perspective the flow should be roughly like this:

  1. create an amount-less swap with some fees estimation (estimation only).
  2. user sends funds to the lockup address
  3. swapper notifies with TransactionLockupFailed (the swapper way to ask for user acceptance)
  4. SDK uses the swapper API (quote) to get the fees the swapper suggests for the swap.
  5. User accepts/rejects or decide to wait for the fees to reduce

On top of that we can add a setting to allow automatic acceptance with some threshold (up to 20% above the estimated fees).

Technically to support the above we need:

  1. Add event for PaymentRequireUserAcceptance
  2. Add API to get all payments require user acceptance with their suggested(updated) fees.
  3. Allow user to accept and complete payment or reject and initiate a refund.
  4. Since the TransactionLockupFailed might happen when the user is in the background we should probably register for notification of such event (swapper web hook) and add handling logic with specific message.

Let's discuss.

@danielgranhao
Copy link
Contributor

danielgranhao commented Dec 16, 2024 •
edited
Loading

@roeierez The general idea looks reasonable to me.

Besides the new, separate API to list payments waiting for user review, it may be helpful to somehow expose this state in Payment so that the displayed payment list can inform users about required reviews.

@roeierez
Copy link
Member

Besides the new, separate API to list payments waiting for user review, it may be helpful to somehow expose this state in Payment so that the displayed payment list can inform users about required reviews.

Good idea! This payment will be in pending state anyway so we can add the indication there that it is waiting for approval. Sounds good.

@danielgranhao
Copy link
Contributor

I'll prepare a PR

@danielgranhao danielgranhao linked a pull request Dec 17, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dangeross dangeross

@danielgranhao danielgranhao

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Expose updated swap fees for user review breez/breez-sdk-liquid
3 participants
@dangeross @roeierez @danielgranhao