Skip to content

fix: QA findings - #95

Open
olga24912 wants to merge 8 commits into
devfrom
cleanup
Open

olga24912 wants to merge 8 commits into
devfrom
cleanup

Conversation

@olga24912

Copy link
Copy Markdown

Fixes for the QA / Informational findings from the audit.

  • QA-02: the user's withdrawal output must now equal amount - withdraw_fee - gas_fee exactly; the min_change_amount relaxation is removed, so no unaccounted nBTC remainder is left on the bridge.
  • QA-03: release workflows hardened:
    actions are pinned to commit SHAs;
    cargo-near is pinned to a specific version and its SHA-256 is verified;
    build (contents: read) and publish (contents: write) run as separate jobs;
    releases now include checksums.sha256.
  • QA-04: assert_valid requires min_deposit_amount > deposit_bridge_fee.fee_min; documented the zero values of rbf_num_limit and the refund timelocks.
  • QA-06: new Zcash-only view list_btc_pending_expiry_heights exposes the expiry height of pending transactions.
  • QA-08: set_withdraw_relayer_address in nBTC now requires one yocto.
  • QA-10: updated CLAUDE.md, doc/refund-flow.md and the verify_deposit_v2 doc comment to match the current entrypoints, the withdraw flow, the safe-deposit callback semantics and the refund permissions.

@olga24912
olga24912 requested a review from karim-en October 14, 2026 14:42
#[near]
impl Contract {
/// Expiry height of each pending transaction (`None` if not found). 0 means it never expires.
pub fn list_btc_pending_expiry_heights(

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.

why do we need this?

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.

I was unsure whether to add this method or not, but in the end I decided to add it.

It's needed in case a transaction gets stuck and doesn't even submitted it into the mempool — this way it will be easiest to check its expiry_height. Otherwise, we would have to parse the transaction.

I don't have a strong opinion about this method.

require!(
actual_received_amount >= min_received_amount
&& actual_received_amount <= max_received_amount,
actual_received_amount == expected_received_amount,

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.

is this correct?

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.

Yes, and I think it's fairer to the user this way.

@github-actions

Copy link
Copy Markdown

WASM size

Contract Size Base Δ Δ %
bitcoin 1230.2 KiB 1255.9 KiB -25.7 KiB -2.05%
zcash 1347.4 KiB 1369.7 KiB -22.4 KiB -1.63%

Fails above +2% growth (ignoring deltas under 1024 bytes).

aecb805 vs base 1cf39b1 · run

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.

2 participants