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.

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

Fix rev swap edge case caused by claim tx delays #493

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

ok300
Copy link
Contributor

@ok300 ok300 commented Sep 29, 2023

If the rev swap is created and proceeds successfully until the lockup tx, but the claim tx is not broadcasted until after the rev swap timeout is reached (for example, because the SDK node was offline), then the service provider re-claims the locked amount and cancels the HODL invoice.

This scenario resulted in a panic when the device came back online and tried to construct and broadcast the claim tx, because of how the claim tx amount was calculated.

This PR fixes that.

@ok300 ok300 requested a review from roeierez September 29, 2023 18:52
// on confirmed utxos to determine the claim tx amount will result in a panic (0 - fees < 0)
// Therefore we read the claim tx amount from the originally agreed upon onchain amount,
// confirmed by the service provider on rev swap creation.
let claim_amount_sat = rs.onchain_amount_sat;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it better to validate that the total utxo value is at least the claim_amount_sat before constructing the transaction?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also why the status wasn't updated to Canceled in the call to refresh_monitored_reverse_swaps?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway I agree that to make sure we claim exactly the amount we know we need to take is better than calculating the utxo and if the tx is not valid so we will keep trying untill timeout. LGTM.

Copy link
Member

@roeierez roeierez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ok300 ok300 marked this pull request as ready for review October 3, 2023 09:20
@ok300 ok300 merged commit c16163a into main Oct 3, 2023
5 checks passed
@ok300 ok300 deleted the ok300-fix-rev-swap-edge-case-lock-tx-reclaimed branch October 3, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants