Skip to content

Support BTC refund flow#28

Open
olga24912 wants to merge 43 commits intoomni-mainfrom
btc_refund
Open

Support BTC refund flow#28
olga24912 wants to merge 43 commits intoomni-mainfrom
btc_refund

Conversation

@olga24912
Copy link
Copy Markdown

@olga24912 olga24912 commented Apr 1, 2026

Added a refund mechanism for cases when the relayer does not want to finalize a deposit transaction for any reason.

A user creates a refund request, and if the deposit is not finalized within a certain time period, execute_refund can be called to return the funds back to Bitcoin. DAO/Operator can reject the proposal.

@olga24912 olga24912 marked this pull request as ready for review April 7, 2026 08:51
#[cfg(not(feature = "zcash"))]
pub fn required_balance_for_execute_refund(&self) -> NearToken {
// execute_refund uses ~700 bytes of storage (BTCPendingInfo + Account + verified_deposit_utxo)
// At 0.00001 NEAR/byte, that's ~0.007 NEAR. We use 0.1 NEAR as a safe margin.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is margin too big?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed: e764521

///
/// # Arguments
///
/// * `deposit_msg` - The original deposit message (must contain `refund_address`).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This doesn't match function definition where refund_address is separate

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed: 22d199e

);

// Must not be already verified/finalized
require!(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why don't we only check it in the callback?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

To avoid unnecessary triggering of the check in btc_light_client. In general, it can be done only in the callback.

#[cfg(not(feature = "zcash"))]
#[payable]
#[access_control_any(roles(Role::DAO))]
pub fn set_refund_timelock_sec(&mut self, refund_timelock_sec: u64) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The number of setters has become huge, can we combine some of them under one setter, eg set_config?

vout: usize,
tx_block_blockhash: String,
tx_index: u64,
merkle_proof: Vec<String>,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

are we going to add coinbase proof here?
I would like. to refactor it and create specific struct for the proof arguments so it can be reused between methods.
Also please verify the performance of this call, maybe we can optimize it by using borsh, or at least make the tx_bytes readable by using hex or baseXX


// Must not have a pending refund request already
require!(
!self.data().refund_requests.contains_key(&utxo_storage_key),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we can move all these checks to the callback, the sdk should be smart enough to not call this method if it could fails

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.

3 participants