Skip to content

feat: cumulative confirmations tier per BTC block#48

Draft
olga24912 wants to merge 7 commits into
omni-mainfrom
block-limit
Draft

feat: cumulative confirmations tier per BTC block#48
olga24912 wants to merge 7 commits into
omni-mainfrom
block-limit

Conversation

@olga24912

Copy link
Copy Markdown

Confirmations tier now computed against the SUM of bridge tx amounts in a BTC
block, closing the split-deposit bypass (one big deposit chunked into many
small to fall under a lower tier).

Per-block sums in a fixed-capacity ring keyed by block_height % cap. Uses
verify_transaction_inclusion_with_heights (Near-One/btc-light-client-contract#140) to get tip + block height in one call; confirmations check moves to our callback.

pub fn set_confirmations_strategy(&mut self, range_upper_bound: U128, confirmations: u8) {
assert_one_yocto();
require!(
(2..=10).contains(&confirmations),

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.

We should not have such limitations for ZCash

// `verify_deposit_callback` takes 8 args (extra `confirmations_delta`); the `#[near]`
// proc-macro re-emits the signature in the ext-trait so the `clippy::too_many_arguments`
// lint fires from inside the macro expansion and an inner `#[allow]` doesn't reach it.
#![allow(clippy::too_many_arguments)]

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.

Remove comments

pub tx_id: H256,
pub tx_block_blockhash: H256,
pub tx_index: u64,
pub merkle_proof: Vec<H256>,

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.

coinbase_merkle_proof

&self,
#[serializer(borsh)] args: TxInclusionProof,
) -> Option<TxInclusionInfo>;
fn verify_transaction_inclusion_with_heights_v2(

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.

remove

/// required-confirmations value, so a tx near the top tier still has room left
/// to be tracked across in-flight verifies. Chosen heuristically; not tied to
/// any protocol parameter.
pub const BLOCK_AMOUNT_RING_CAPACITY_SLACK: usize = 5;

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.

reduce comment

pub fn max_tier_confirmations(&self) -> u8 {
self.confirmations_strategy
.values()
.copied()

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.

do we need copied?

refund_requests,
#[cfg(feature = "zcash")]
refund_requests: IterableMap::new(StorageKey::RefundRequests),
block_bridge_amounts: BlockAmountRing::new(ring_capacity),

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.

Do we need to add extra Contract version for update?

// SUM of bridge txs in a block, blocking an attacker from splitting one big deposit
// into many small ones to bypass the high-tier confirmations requirement.
// Capacity is derived from config; see `BlockAmountRing::capacity_for`.
pub block_bridge_amounts: BlockAmountRing,

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.

reduce comment

#[cfg(feature = "zcash")]
refund_requests: IterableMap::new(StorageKey::RefundRequests),
block_bridge_amounts: BlockAmountRing::new(ring_capacity),
}

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.

Add migration tests for the last contract versions

// Refund-request is rare and already gated by `refund_timelock_sec`, so we
// skip the cumulative-amount ring entirely and just demand max-tier depth
// (see `Config::max_required_confirmations`). The callback doesn't need to
// know about the deposit amount or whitelist deltas.

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.

reduce comment

let ext = ext_btc_light_client::ext(btc_light_client_account_id)
.with_static_gas(GAS_FOR_VERIFY_TRANSACTION_INCLUSION);
if let Some((coinbase_tx_id, coinbase_merkle_proof)) = coinbase_proof {
ext.verify_transaction_inclusion_with_heights_v2(TxInclusionProofV2::new(

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.

incorrect API

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.

1 participant