chore: migrate rust/basic_bitcoin to icp-cli#1397
Merged
Merged
Conversation
marc0olo
force-pushed
the
chore/migrate-rust-basic-bitcoin-to-icp-cli
branch
from
fd21b3e to
d1154c6
Compare
- Replace dfx.json with icp.yaml (Bitcoin integration via custom Docker image bundling bitcoind, matching motoko/basic_bitcoin pattern) - Move Rust source from src/ into backend/ subdirectory - Replace root Cargo.toml with workspace + backend/Cargo.toml (name=backend) - Copy Dockerfile and docker/start.sh from motoko/basic_bitcoin - Rewrite Makefile with icp-cli commands and 9 numbered tests - Update README with icp-cli instructions, preserve all domain content - Add rust-basic_bitcoin CI job to .github/workflows/basic_bitcoin.yml - Delete dfx.json, build.sh, basic_bitcoin.did, old src/ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
marc0olo
force-pushed
the
chore/migrate-rust-basic-bitcoin-to-icp-cli
branch
from
d1154c6 to
ccdf499
Compare
- backend/Cargo.toml: edition 2021→2024; bump bitcoin 0.32.7→0.32, candid 0.10.19→0.10, ic-cdk 0.20.0→0.20, ic-cdk-management-canister 0.1→0.1.1, serde/serde_bytes/leb128/hex to range specifiers; add cdylib comment - Add Makefile with build-image and topup targets (matches motoko/basic_bitcoin); README referenced make build-image but no Makefile existed - README: Node.js v18+, Rust v1.85+ with wasm32-unknown-unknown target; update Architecture links to docs.internetcomputer.org; replace "smart contract" with "canister" throughout Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace staging/production environments with a single `ic` environment (Bitcoin testnet4, test_key_1) — the default for IC network deployments - Add build-image.sh as a standalone alternative to `make build-image` - Fix key_name mapping: Network::Mainnet now correctly resolves to "key_1" instead of "test_key_1" (was a bug — mainnet and testnet used the same key) - Update README to reflect new environment structure and document how to switch to Bitcoin mainnet for production deployments - Remove remaining "smart contract"/"contract" jargon Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…coin - Add ic_cdk::export_candid!() to lib.rs so candid-extractor can find the get_candid_pointer export during the icp deploy build step - Bump ic-cdk to 0.20.2 (latest patch) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
export_candid!() generates Candid bindings using the types returned by all endpoint functions, so those types must be in scope at the crate root. Import GetBlockHeadersResponse, BlockchainInfo, GetUtxosResponse, and MillisatoshiPerByte from ic_cdk_bitcoin_canister. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
build-image is now covered by build-image.sh; the topup command is documented inline in the README instead of requiring make. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- docker/start.sh: bind bitcoind on 0.0.0.0 so the host can reach it through Docker's port mapping - icp.yaml: add 18443:18443 port mapping for bitcoind RPC - ord.yaml + bitcoin.conf: fix password to ic-btc-integration (was a different value, causing authentication failures when running ord) Run ./build-image.sh after pulling these changes to rebuild the image. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The old structure was confusing: it showed sending before explaining coinbase maturity, and omitted the confirmation block step. The new walkthrough guides the reader through the full flow: fund (101 blocks) → check balance → send → mine confirmation block → verify destination. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The ord homepage (/) has a "Latest Inscriptions" section that appears empty; /inscriptions correctly lists all indexed inscriptions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Store addresses and transaction IDs in variables so each step can reference them directly rather than requiring manual copy-paste of placeholder values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without turbo mode, short rune names (< 13 chars) are reserved until a high future block height (e.g. "ICPRUNE" unlocks at block 99,547 in regtest). Turbo mode makes the rune immediately active regardless of name length or current chain height. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… complete README walkthrough - Add transfer_rune endpoint: builds a Runestone with an edict to move rune tokens between addresses; verifiable via ord decode without the ord UI - Add mint_brc20 endpoint: creates a BRC-20 mint inscription at the canister's address - Add transfer_brc20 endpoint: chains commit → reveal to self → send inscription UTXO to recipient, following the BRC-20 two-step transfer spec - Extract commit_and_reveal helper in brc20.rs; refactor inscribe_brc20 to use it, eliminating code duplication across deploy/mint/transfer - Update README: complete deploy → mint → transfer walkthroughs for Runes and BRC-20; fix architecture links to use /ic-interface-spec/ paths matching the Motoko README; add get_utxos section; fix missing $CONTAINER setup in Bitcoin assets prerequisites Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… setup The brew install step alone is not enough — Homebrew installs LLVM as keg-only, so clang is not on PATH until explicitly added. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…subsections Matches the BRC-20 section structure so etch and transfer read as one cohesive flow rather than two disconnected top-level sections. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The ord server returns pretty-printed JSON ("id": "204:1" with a space),
so strip whitespace before grepping. Also tighten the pattern to [0-9]*:[0-9]*
to avoid false matches against other id fields in the response.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… in regtest Short rune names (< 12 chars) have unlock heights of 17,500–105,000+ blocks in regtest. ICPRUNE (7 chars) won't get an active rune ID until block ~87,500, making the transfer walkthrough impossible. BASICBITCOIN (12 chars) is valid from block 0, so ord assigns a rune ID immediately after confirmation. Also corrects the turbo mode comment in etch_rune.rs: turbo opts into future protocol upgrades, it does not bypass the name unlock schedule. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ngth Two protocol correctness fixes: 1. BRC-20 inscriptions must use `text/plain;charset=utf-8` as content type (not `application/json`). BRC-20 indexers check the content type field and will ignore inscriptions with the wrong MIME type. 2. Rune names need 13+ characters for immediate activation in regtest. The ord `minimum_at_height` formula uses linear interpolation between STEPS[] boundary values. All 13-char names have values above STEPS[12] (= 99,246,114,928,149,462), the highest minimum the formula produces, so they are active from block 0. 12-char names like BASICBITCOIN have their own unlock height (e.g. 16,808) that must be mined past first. Updated README to use BASICBITCOINS (13 chars) and corrected the note. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add data_dir: . to ord.yaml so the ord index (index.redb) is written into the example directory rather than ~/Library/Application Support/ord. This keeps each example's index isolated and makes cleanup simple: delete index.redb to start fresh. Also gitignore index.redb and update the README note with the correct path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Change data_dir from . to ./ord-db so the index lands in a clearly named directory (ord-db/regtest/, ord-db/testnet4/, etc. per chain). Gitignore the directory instead of the individual index.redb file and update the README note accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
replace_all on BASICBITCOIN→BASICBITCOINS also matched inside the already updated BASICBITCOINS in the etch command, producing BASICBITCOINSS. This caused the user to etch a different name than what the explorer URL referenced, making the rune appear unfound. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pport - Add transfer_ordinal endpoint: single-input Taproot key-path spend moves an inscription UTXO to recipient while preserving satoshi order - Add transfer_rune endpoint: Runestone with edict + pointer=2 so unallocated rune change goes to vout[2] (not the recipient at vout[0]) - Fix transfer_brc20: use actual reveal output value in Taproot sighash instead of hardcoded MIN_INSCRIPTION_VALUE (BIP341 prevout commitment) - Fix encode_rune_name: use u128 accumulator (u64 overflows at 14 chars) - Add encode_leb128_u128: build_etching_script now encodes rune name, premine, amount, and cap with the correct u128 LEB128 encoder - Fix etch_rune: enforce 6-confirmation requirement via MinConfirmations filter instead of checking height != 0 (which only checked 1 block) - Fix build_reveal_transaction_with_fee: use checked_sub to avoid panic on underflow if fee exceeds INSCRIPTION_OUTPUT_VALUE - Update README: full Ordinals/Runes/BRC-20 walkthrough with expected output, verification steps, and corrected structure - CI: install candid-extractor before icp deploy (not bundled on ubuntu-24.04 host runner) - test.sh: check tip_height is any positive value, not exactly 101, so tests pass after running the README walkthrough Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- runes.rs: encode flags field with encode_leb128_u128 (was silently truncated by `flags as u64`; Cenotaph flag at bit 127 would vanish) - runes.rs: use checked_sub for edict delta encoding so out-of-order edicts return Err instead of panicking or wrapping - ordinals.rs: replace saturating_sub with checked_sub + trap in build_rune_etch_transaction_with_fee to avoid silently burning the rune premine when fee exceeds commit output value - common.rs: type-annotate total_spent as u64 and use saturating_add to prevent theoretical overflow in UTXO accumulation - README.md: remove stale link to Motoko version Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the rust/basic_bitcoin example from dfx to icp-cli, restructures it into the canonical Rust workspace layout (backend/), and adds canister endpoints and local regtest tooling for Bitcoin asset protocols (Ordinals, Runes, BRC-20).
Changes:
- Migrates the example to
icp-cli(icp.yaml), introduces a custom network launcher Docker image, and replaces legacydfxbuild/deploy artifacts. - Adds/rewrites Rust backend modules and services for Ordinals / Runes / BRC-20 flows, plus supporting transaction-building utilities.
- Adds CI for the Rust variant and introduces a
test.shscript for basic end-to-end validation.
Reviewed changes
Copilot reviewed 35 out of 54 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/basic_bitcoin/test.sh | Adds an e2e smoke-test script for basic Bitcoin API functionality. |
| rust/basic_bitcoin/src/service/inscribe_brc20.rs | Removes legacy (pre-migration) service implementation. |
| rust/basic_bitcoin/src/service/etch_rune.rs | Removes legacy (pre-migration) service implementation. |
| rust/basic_bitcoin/src/runes.rs | Removes legacy runes encoding module (moved/replaced under backend/). |
| rust/basic_bitcoin/src/brc20.rs | Removes legacy BRC-20 script builder (moved/replaced under backend/). |
| rust/basic_bitcoin/README.md | Updates documentation for icp-cli deployment + asset protocol walkthroughs. |
| rust/basic_bitcoin/ord.yaml | Updates ord configuration (RPC password + ord db directory). |
| rust/basic_bitcoin/Makefile | Removes legacy dfx Makefile targets. |
| rust/basic_bitcoin/icp.yaml | Adds icp-cli project config + local network settings + env init args. |
| rust/basic_bitcoin/Dockerfile | Adds custom network launcher image layering bitcoind into the launcher base. |
| rust/basic_bitcoin/docker/start.sh | Adds container entrypoint to start bitcoind then exec the network launcher. |
| rust/basic_bitcoin/dfx.json | Removes legacy dfx project configuration. |
| rust/basic_bitcoin/Cargo.toml | Converts the project into a workspace rooted at backend/. |
| rust/basic_bitcoin/Cargo.lock | Updates lockfile for workspace rename/package changes and dependency bumps. |
| rust/basic_bitcoin/build.sh | Removes legacy build script. |
| rust/basic_bitcoin/build-image.sh | Adds helper script to build the custom network launcher Docker image. |
| rust/basic_bitcoin/bitcoin.conf | Updates bitcoind config (RPC password). |
| rust/basic_bitcoin/basic_bitcoin.did | Removes legacy candid file. |
| rust/basic_bitcoin/backend/src/service/transfer_rune.rs | Adds Rune transfer endpoint and tx construction/signing logic. |
| rust/basic_bitcoin/backend/src/service/transfer_ordinal.rs | Adds ordinal transfer endpoint spending the reveal UTXO. |
| rust/basic_bitcoin/backend/src/service/transfer_brc20.rs | Adds BRC-20 transfer flow (commit/reveal + send). |
| rust/basic_bitcoin/backend/src/service/send_from_p2wpkh_address.rs | Adds native SegWit (P2WPKH) sending endpoint. |
| rust/basic_bitcoin/backend/src/service/send_from_p2tr_script_path_enabled_address_script_spend.rs | Adds Taproot script-path spending send endpoint. |
| rust/basic_bitcoin/backend/src/service/send_from_p2tr_script_path_enabled_address_key_spend.rs | Adds Taproot key-path spending for script-path-enabled address. |
| rust/basic_bitcoin/backend/src/service/send_from_p2tr_key_path_only_address.rs | Adds BIP-86 style Taproot key-path-only send endpoint. |
| rust/basic_bitcoin/backend/src/service/send_from_p2pkh_address.rs | Adds legacy P2PKH send endpoint. |
| rust/basic_bitcoin/backend/src/service/mint_brc20.rs | Adds BRC-20 mint inscription endpoint. |
| rust/basic_bitcoin/backend/src/service/inscribe_ordinal.rs | Adds ordinal inscription (commit/reveal) endpoint. |
| rust/basic_bitcoin/backend/src/service/inscribe_brc20.rs | Adds BRC-20 deploy inscription endpoint. |
| rust/basic_bitcoin/backend/src/service/get_utxos.rs | Adds get-utxos endpoint passthrough to the Bitcoin canister. |
| rust/basic_bitcoin/backend/src/service/get_rune_address.rs | Adds helper endpoint returning the canister’s dedicated rune address. |
| rust/basic_bitcoin/backend/src/service/get_p2wpkh_address.rs | Adds P2WPKH address derivation endpoint. |
| rust/basic_bitcoin/backend/src/service/get_p2tr_script_path_enabled_address.rs | Adds Taproot script-path-enabled address derivation endpoint. |
| rust/basic_bitcoin/backend/src/service/get_p2tr_key_path_only_address.rs | Adds Taproot key-path-only address derivation endpoint. |
| rust/basic_bitcoin/backend/src/service/get_p2pkh_address.rs | Adds P2PKH address derivation endpoint. |
| rust/basic_bitcoin/backend/src/service/get_current_fee_percentiles.rs | Adds fee percentile query endpoint. |
| rust/basic_bitcoin/backend/src/service/get_blockchain_info.rs | Adds blockchain-info query endpoint. |
| rust/basic_bitcoin/backend/src/service/get_block_headers.rs | Adds block-headers query endpoint. |
| rust/basic_bitcoin/backend/src/service/get_balance.rs | Adds balance query endpoint. |
| rust/basic_bitcoin/backend/src/service/etch_rune.rs | Implements 2-step rune commit + etch flow with confirmation gating. |
| rust/basic_bitcoin/backend/src/service.rs | Wires newly added service modules into the backend crate. |
| rust/basic_bitcoin/backend/src/schnorr.rs | Adds Schnorr public key caching and signing helpers (+ mock signer). |
| rust/basic_bitcoin/backend/src/runes.rs | Adds Runes encoding utilities, commitment bytes, and runestone builders. |
| rust/basic_bitcoin/backend/src/p2wpkh.rs | Adds P2WPKH tx building + signing helpers with fee iteration. |
| rust/basic_bitcoin/backend/src/p2tr.rs | Adjusts Taproot tx builder logic to match updated primary output shape. |
| rust/basic_bitcoin/backend/src/p2pkh.rs | Adjusts P2PKH tx builder logic to match updated primary output shape. |
| rust/basic_bitcoin/backend/src/ordinals.rs | Improves fee underflow handling and adds rune etch tx builder helpers. |
| rust/basic_bitcoin/backend/src/lib.rs | Updates init logic, adds request types, and exports candid. |
| rust/basic_bitcoin/backend/src/ecdsa.rs | Adds ECDSA public key caching and signing helpers (+ mock signer). |
| rust/basic_bitcoin/backend/src/common.rs | Updates UTXO selection and simplifies PrimaryOutput to address-only. |
| rust/basic_bitcoin/backend/src/brc20.rs | Adds shared commit+reveal implementation and BRC-20 reveal script builder. |
| rust/basic_bitcoin/backend/Cargo.toml | Adds backend crate manifest and dependencies. |
| rust/basic_bitcoin/.gitignore | Adds ignore for ord index DB directory. |
| .github/workflows/basic_bitcoin.yml | Adds Rust CI job that builds the custom Docker image and runs test.sh. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace unwrap() with descriptive trap() on user-supplied address parsing in transfer_ordinal, transfer_brc20, and transfer_rune - Replace fixed sleep 5 with a 30s polling loop in test.sh so CI doesn't race against the IC↔Bitcoin sync - Use cargo install --locked candid-extractor for reproducible builds Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two bugs in the sync wait loop:
- Missing || true caused set -e to silently exit the script when the
bitcoin integration canister rejected calls during its sync window
("Canister state is not fully synced")
- grep -qE '[1-9]' false-positively matched the type annotation 'nat64'
in responses like '(0 : nat64)', causing the loop to exit immediately
even when the balance was still zero
Fix: add || true to survive rejection, and use ^\([1-9] which only
matches when the Candid value itself is non-zero. Apply the same pattern
fix to test 6.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
get_balance can return stale non-zero values from a previous run while
the bitcoin integration canister is still syncing new blocks (and
rejecting all fresh calls). Polling it gave a false positive, causing
subsequent tests to hit "Canister state is not fully synced".
Poll get_utxos instead: it throws on any transient sync window (the
|| { sleep; continue; } retries automatically), and only exits the
loop once tip_height is genuinely non-zero — definitive proof that the
IC has fully processed the newly mined blocks.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…st_key test_key_1 works for both regtest and testnet. dfx_test_key is dfx-specific and should not be used or advertised in icp-cli examples. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
raymondk
approved these changes
Jul 9, 2026
Removes Ordinals, Runes, and BRC-20 code from the migrated example. These were incomplete in the original example and the team lacks the domain expertise to maintain them long-term. The full implementation is preserved on the chore/migrate-rust-basic-bitcoin-to-icp-cli-with-assets branch. Also removes the now-dead select_one_utxo / SelectUtxosMode::Single and cleans up related Cargo deps (leb128, serde_bytes) and icp.yaml. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These are educational: they demonstrate different UTXO selection strategies (greedy accumulation vs. single large UTXO) which are relevant to Bitcoin development regardless of asset protocols. The Single variant is kept even though no current service uses it, so callers that need single-UTXO semantics have a clear pattern to follow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tests both SelectUtxosMode::Greedy (accumulate smallest UTXOs until covered) and SelectUtxosMode::Single (require one UTXO large enough on its own), including error cases for each. Also eliminates the dead-code warning on the Single variant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolves workflow conflict: master updated the motoko/basic_bitcoin test step to bash test.sh (PR #1429) while this branch still had make test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Member
Author
|
Update for reviewers — the scope of this PR changed significantly since the initial draft. Here's what to focus on: Dropped: All Ordinals/Runes/BRC-20 code ( Key things to review:
|
…Single doc comment The Ordinals implementation was dropped from this PR; the doc comment now uses the same generic wording as select_one_utxo() in common.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
rust/basic_bitcoinfrom dfx to icp-cli.Migration to icp-cli
dfx.json+Makefile+build.shwithicp.yaml+build-image.shDockerfileanddocker/start.shto self-contain the bitcoind environment inside a custom Docker image derived fromicp-cli-network-launcherbackend/src/to match the icp-cli project layouttest.shwith an end-to-end test suite covering all endpointsrust-basic_bitcoinCI job in.github/workflows/basic_bitcoin.ymlCode improvements
ic-cdkto 0.20.2,ic-cdk-bitcoin-canisterto 0.2,ic-cdk-management-canisterto 0.1.1export_candid!()withcandid-extractorin CI so the.didfile is auto-generated from the Rust typesBitcoinContextstruct and derivation path helpers with documentationSelectUtxosModeenum (Greedy/Single) top2tr::build_transaction— demonstrates the two standard UTXO selection strategies with unit tests for bothWhat was dropped
The original example contained Ordinals, Runes, and BRC-20 code that had been added at some point but was incomplete. This PR removes all of that (including
brc20.rs,runes.rs,ordinals.rs, and the associated service modules) and focuses on the core Bitcoin integration: address generation (P2PKH, P2WPKH, P2TR key-path, P2TR script-path), balance/UTXO/block queries, and sending transactions across all address types.The full asset implementation is preserved on the
chore/migrate-rust-basic-bitcoin-to-icp-cli-with-assetsbranch for future reference.🤖 Generated with Claude Code