fix(bitcoin): restore confirmation-based finalize in header-only mode#3951
Conversation
- keep imported headers in pending storage and finalize only when confirmations are met - add pending_block pop/unpack helpers for ready header finalization - preserve bounded reorg rollback with ErrorReorgTooDeep for deep reorgs - add move tests for confirmation gating and reorg behavior Testing: - cargo run --bin rooch -- move test -p frameworks/bitcoin-move
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR restores confirmation-gated header finalization for Bitcoin header-only import, keeping headers in pending storage and finalizing them only once they are outside the configured reorg window.
Changes:
- Add pending-block APIs to pop/unpack a “ready” pending header for finalization after confirmations.
- Update Bitcoin header-only execution to finalize only confirmation-ready headers (and support bounded header reorg rollback).
- Add Move tests covering confirmation gating and bounded/deep reorg scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| frameworks/bitcoin-move/tests/bitcoin_test.move | Adds tests for confirmation-based finalization and reorg handling in header-only mode. |
| frameworks/bitcoin-move/sources/pending_block.move | Introduces ready-header pop/unpack APIs and a test-only reorg-window setter. |
| frameworks/bitcoin-move/sources/bitcoin.move | Switches header-only import to confirmation-gated finalization and adds bounded header rollback on reorg. |
| frameworks/bitcoin-move/doc/pending_block.md | Documents the new pending-block APIs and struct. |
| struct ReadyPendingBlockHeader has copy, drop, store { | ||
| block_height: u64, | ||
| block_hash: address, | ||
| header: Header, | ||
| } |
There was a problem hiding this comment.
ReadyPendingBlockHeader is returned by pop_ready_pending_block_header() after the pending header has been removed from storage. Because this struct has the drop ability, a (friend) caller can accidentally drop it and permanently lose the header without finalizing it. Consider removing the drop ability (and adjusting callers to explicitly consume the Option) or providing a single helper that both pops and processes/finalizes to avoid footguns.
| let ready_block = object::borrow(block_obj); | ||
| let ready_hash = ready_block.block_hash; | ||
| let ready_height = ready_block.block_height; | ||
| let ready_obj = take_pending_block(ready_hash); | ||
| simple_map::remove(&mut store.pending_blocks, &ready_height); | ||
| let header = remove_pending_block(ready_obj, true); | ||
| option::some(ReadyPendingBlockHeader { |
There was a problem hiding this comment.
pop_ready_pending_block_header() calls remove_pending_block(ready_obj, true), which will abort unless the pending block has processed_tx == tx_ids.len(). In header-only mode tx_ids is empty, but this API is public(friend) and can be called even for blocks that still have tx bodies, causing unexpected aborts. Add an explicit guard/assert that the pending block is header-only (e.g., tx_ids empty) or that it is fully processed before removing it.
| /// Roll back header mappings from `from_height` to current tip. | ||
| /// This is used to handle short reorgs in header-only mode. | ||
| fun rollback_headers_for_reorg(btc_block_store: &mut BitcoinBlockStore, from_height: u64) { | ||
| if (option::is_none(&btc_block_store.latest_block)) { | ||
| return | ||
| }; | ||
| let latest_block = *option::borrow(&btc_block_store.latest_block); | ||
| let (latest_height, _latest_hash) = types::unpack_block_height_hash(latest_block); | ||
| if (from_height > latest_height) { | ||
| return | ||
| }; | ||
|
|
||
| let reorg_depth = latest_height - from_height + 1; | ||
| let reorg_block_count = pending_block::get_reorg_block_count(); | ||
| assert!(reorg_depth <= reorg_block_count, ErrorReorgTooDeep); | ||
|
|
||
| let height = from_height; | ||
| while (height <= latest_height) { | ||
| if (table::contains(&btc_block_store.height_to_hash, height)) { | ||
| let stale_hash = table::remove(&mut btc_block_store.height_to_hash, height); | ||
| if (table::contains(&btc_block_store.hash_to_height, stale_hash)) { | ||
| let _old_height = table::remove(&mut btc_block_store.hash_to_height, stale_hash); | ||
| }; | ||
| if (table::contains(&btc_block_store.blocks, stale_hash)) { | ||
| let _old_header = table::remove(&mut btc_block_store.blocks, stale_hash); | ||
| }; |
There was a problem hiding this comment.
rollback_headers_for_reorg only rolls back header-indexing tables (height_to_hash, hash_to_height, blocks) and updates latest_block. If this code path ever runs after tx/UTXO state has been derived from these blocks, the module will end up with headers rolled back but tx/UTXO tables unchanged. Consider gating this rollback logic to header-only mode (and abort otherwise), or extending rollback to also revert any derived state for the removed heights.
| fun execute_l1_block(block_height: u64, block_hash: address, block_bytes: vector<u8>){ | ||
| let block = bcs::from_bytes<Block>(block_bytes); | ||
| let block_header = *types::header(&block); | ||
|
|
||
| // Keep pending_block topology for reorg detection (no tx bodies stored). | ||
| // Keep pending topology for reorg handling and confirmation-window finalize. | ||
| let _added = pending_block::add_pending_block_header_only(block_height, block_hash, block_header); |
There was a problem hiding this comment.
execute_l1_block trusts the block_hash argument but also parses block_bytes and has access to the header. To prevent inconsistent pending storage/finalization if upstream passes a mismatched hash (buggy relayer or corrupted input), recompute the hash from block_header and assert it matches block_hash before storing it in pending_block.
- remove header rollback logic from bitcoin::process_block_header - keep execute_l1_block as append -> pop-ready -> finalize - rely on pending_block append-time reorg handling and window gating Testing: - cargo run --bin rooch -- move test -p frameworks/bitcoin-move
- add explicit readiness guard in pop_ready_pending_block_header - require header-only block or fully processed tx-carrying block - avoid implicit abort-only behavior in remove_pending_block(processed=true) Testing: - cargo run --bin rooch -- move test -p frameworks/bitcoin-move
Summary
Testing