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

Unlock Coins on Anchor TX Broadcast Failure #1348

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Feb 3, 2025

This PR ensures that asset coins are properly unlocked if an error occurs during the anchor transaction broadcast process. Previously, failed broadcasts could leave coins locked, causing send attempts to be stuck.

Key changes:

  • Unlock asset coins before returning an error when publishing an anchor transaction.
  • Ensure inputs are unlocked if an error occurs before reaching the broadcast complete state.
  • Introduce SendStateBroadcastComplete to better define when a transfer can be canceled.
  • Improve logging by reporting the final fee rate of the anchoring transaction and logging the wallet's minimum relay fee for better debugging.

A user encountered an error where the anchor transaction fee was
reported to be lower than the wallet's minimum relay fee. To provide
more context, log the minimum relay fee as reported by the wallet.
The absolute fee is already reported. This change calculates and reports
the final fee rate using the virtual size of the completed transaction.
@ffranr ffranr self-assigned this Feb 3, 2025
@ffranr ffranr force-pushed the unlock-coins-on-broadcast-fail branch 2 times, most recently from 3d7d78e to 8dba8e6 Compare February 3, 2025 16:27
@coveralls
Copy link

coveralls commented Feb 3, 2025

Pull Request Test Coverage Report for Build 13118260360

Details

  • 0 of 24 (0.0%) changed or added relevant lines in 4 files are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.01%) to 40.713%

Changes Missing Coverage Covered Lines Changed/Added Lines %
itest/assertions.go 0 1 0.0%
tapfreighter/parcel.go 0 2 0.0%
tapfreighter/wallet.go 0 8 0.0%
tapfreighter/chain_porter.go 0 13 0.0%
Files with Coverage Reduction New Missed Lines %
tapdb/addrs.go 2 75.29%
tapfreighter/chain_porter.go 4 0.0%
tapgarden/caretaker.go 4 68.11%
Totals Coverage Status
Change from base Build 13075453647: 0.01%
Covered Lines: 26769
Relevant Lines: 65751

💛 - Coveralls

Introduce a new send state to act as a threshold for transfer
cancellation handling. After this state, the anchor transaction may
already be in the mempool or confirmed on-chain. Before reaching this
point, however, the broadcast may have failed, requiring different
handling.
Update unlock logic to ensure that errors encountered during the
anchor tx broadcast process, but before reaching the broadcast complete
state, result in coins being unlocked. This prevents assets from
remaining locked in case of intermediate failures.
Ensure locked asset coins are released before returning an error when
attempting to publish the anchor transaction. Without this change, if
the chain bridge logic determines the anchor transaction should not be
broadcast, the coins remain locked, leaving the send attempt stuck in
limbo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

2 participants