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

Properly advance state when swap is manually refunded #903

Closed
binarybaron opened this issue Dec 31, 2021 · 1 comment · Fixed by #934
Closed

Properly advance state when swap is manually refunded #903

binarybaron opened this issue Dec 31, 2021 · 1 comment · Fixed by #934

Comments

@binarybaron
Copy link
Collaborator

binarybaron commented Dec 31, 2021

Describe the bug
When manually refunding a swap, the state that is saved in the database is not advanced properly.

 ./swap --testnet refund --swap-id 417e8829-bc15-4f03-b7d8-482cb6583356 
Published Bitcoin transaction txid=4dfb63a139d5f00d31b55beeabcf229647f18d6f68c44e09d7750ee185a6b1f2 kind=refund
Waiting for Bitcoin transaction finality txid=4dfb63a139d5f00d31b55beeabcf229647f18d6f68c44e09d7750ee185a6b1f2 required_confirmation=2
Waiting for Bitcoin transaction finality txid=4dfb63a139d5f00d31b55beeabcf229647f18d6f68c44e09d7750ee185a6b1f2 seen_confirmations=1 needed_confirmations=2

./swap --testnet history
+--------------------------------------+------------------+
| SWAP ID                              | STATE            |
+=========================================================+
| 417e8829-bc15-4f03-b7d8-482cb6583356 | btc is cancelled |
+--------------------------------------+------------------+

Lost/trapped Funds
No

Platform (please complete the following information):

  • CLI (probably also ASB)
  • Software Version 0.10.2
  • Operating System: Ubuntu
@binarybaron binarybaron changed the title Properly advance state when swap is manually cancelled Properly advance state when swap is manually cancelled/refunded Dec 31, 2021
@binarybaron
Copy link
Collaborator Author

binarybaron commented Jan 2, 2022

pub async fn publish_refund_btc(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result<()> {
let signed_tx_refund = self.signed_refund_transaction()?;
let (_, subscription) = bitcoin_wallet.broadcast(signed_tx_refund, "refund").await?;
subscription.wait_until_final().await?;
Ok(())
}

pub async fn submit_tx_cancel(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result<Txid> {
let transaction = bitcoin::TxCancel::new(
&self.tx_lock,
self.cancel_timelock,
self.A,
self.b.public(),
self.tx_cancel_fee,
)
.complete_as_bob(self.A, self.b.clone(), self.tx_cancel_sig_a.clone())
.context("Failed to complete Bitcoin cancel transaction")?;
let (tx_id, _) = bitcoin_wallet.broadcast(transaction, "cancel").await?;
Ok(tx_id)
}

It seems that you have to wait for the refund transaction to be confirmed with 2 confirmations and only then the state will be advanced to BtcRefunded. This makes sense, but doesn't seem to be consistent with the way this is handled in regards to the cancel transaction. Also, if someone manually refunds their swap and then decides not to wait 20 minutes for the cli to pick up the two confirmations, there is no way for Bob to ever properly "finish" the swap (so that the state machine is in a "Done" state). I think it would make sense to remove that 2 confirmations requirement.

@binarybaron binarybaron changed the title Properly advance state when swap is manually cancelled/refunded Properly advance state when swap is manually refunded Jan 3, 2022
bors bot added a commit that referenced this issue Apr 14, 2022
934: Don't wait for refund transaction to receive confirmations r=binarybaron a=binarybaron

Don't wait for refund transaction to receive confirmations to mitigate a scenario where the swap is stuck in `BtcCancelled` because it's not resumable.

Closes #903

Co-authored-by: binarybaron <86064887+binarybaron@users.noreply.github.com>
@bors bors bot closed this as completed in #934 Apr 14, 2022
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 a pull request may close this issue.

1 participant