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: bouncer btc client mutex for vault swaps #5436

Merged
merged 5 commits into from
Nov 27, 2024
Merged

Conversation

j4m1ef0rd
Copy link
Contributor

@j4m1ef0rd j4m1ef0rd commented Nov 21, 2024

Pull Request

Closes: PRO-1832

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have written sufficient tests.
  • I have written and tested required migrations.
  • I have updated documentation where appropriate.

Summary

Using the btc client mutex around selecting inputs and sending transaction to avoid double spending error.

@j4m1ef0rd j4m1ef0rd requested a review from kylezs November 21, 2024 02:20
@j4m1ef0rd j4m1ef0rd self-assigned this Nov 21, 2024
Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72%. Comparing base (277b152) to head (bdf6514).
Report is 3 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff           @@
##            main   #5436    +/-   ##
======================================
- Coverage     72%     72%    -0%     
======================================
  Files        490     490            
  Lines      87053   86929   -124     
  Branches   87053   86929   -124     
======================================
- Hits       62423   62234   -189     
- Misses     21675   21731    +56     
- Partials    2955    2964     +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bouncer/shared/send_btc.ts Outdated Show resolved Hide resolved
@j4m1ef0rd j4m1ef0rd requested a review from dandanlen November 25, 2024 02:48
@j4m1ef0rd j4m1ef0rd changed the title fix: bouncer lock utxos after selection fix: bouncer btc client mutex for vault swaps Nov 25, 2024
@j4m1ef0rd j4m1ef0rd force-pushed the fix/bouncer-utxo-selection branch from 1714311 to 59cf85a Compare November 25, 2024 02:53
@dandanlen
Copy link
Collaborator

I was playing around with this on Friday, and it looks like the selectInputs method is not necessary.

Bitcoin nodes have a fundRawTransaction method that selects the inputs for us.

This would also solve the (still remaining) issue that we can only select a single utxo. I'll see if it works on localnet and if yes, will push it.

@dandanlen dandanlen enabled auto-merge November 25, 2024 12:37
@dandanlen dandanlen added this pull request to the merge queue Nov 25, 2024
@dandanlen dandanlen removed this pull request from the merge queue due to a manual request Nov 25, 2024
j4m1ef0rd and others added 2 commits November 26, 2024 11:00
- lockUnspents: true to ensure we don't try to double-spend
- changePosition: 2 on vault swaps to ensure a valid transaction
@dandanlen dandanlen added this pull request to the merge queue Nov 26, 2024
@kylezs kylezs removed this pull request from the merge queue due to a manual request Nov 26, 2024
@kylezs kylezs added this pull request to the merge queue Nov 27, 2024
Merged via the queue into main with commit 4851f8a Nov 27, 2024
48 of 49 checks passed
@kylezs kylezs deleted the fix/bouncer-utxo-selection branch November 27, 2024 10:50
nmammeri pushed a commit that referenced this pull request Nov 27, 2024
* fix: bouncer lock utxos after selection

* refactor: use mutex instead of locking

* feat: use bitcoin node's input selection.

* chore: set correct timeouts for LP-API bouncer test

* fix: additional btc tx options:

- lockUnspents: true to ensure we don't try to double-spend
- changePosition: 2 on vault swaps to ensure a valid transaction

---------

Co-authored-by: Daniel <daniel@chainflip.io>
nmammeri pushed a commit that referenced this pull request Nov 27, 2024
* fix: bouncer lock utxos after selection

* refactor: use mutex instead of locking

* feat: use bitcoin node's input selection.

* chore: set correct timeouts for LP-API bouncer test

* fix: additional btc tx options:

- lockUnspents: true to ensure we don't try to double-spend
- changePosition: 2 on vault swaps to ensure a valid transaction

---------

Co-authored-by: Daniel <daniel@chainflip.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants