Skip to content

scrolls_bitcoin: vetKD-derived canister-specific secret prefix for address derivation#189

Open
imikushin wants to merge 6 commits into
CharmsDev:mainfrom
imikushin:scrolls-secret
Open

scrolls_bitcoin: vetKD-derived canister-specific secret prefix for address derivation#189
imikushin wants to merge 6 commits into
CharmsDev:mainfrom
imikushin:scrolls-secret

Conversation

@imikushin
Copy link
Copy Markdown
Member

Summary

  • Bootstraps a 32-byte canister-specific secret prefix via ICP vetKeys (vetkd_derive_key) on the first signing call: samples transport_sk from raw_rand, derives transport_pk = sk · G1 on BLS12-381, calls vetKD, and uses SHA-256(sk_bytes || encrypted_key) as the prefix. transport_sk is dropped — nobody, including this canister, can decrypt the ciphertext after.
  • Caches the prefix in stable memory directly (marker byte + 32 bytes at the start of stable memory) and in a thread_local! RAM cache for cheap reads on the hot path.
  • No pre_upgrade hook. Stable memory is written at derive time, not at upgrade time — a pre_upgrade trap would brick the canister; a post_upgrade failure is always recoverable by another upgrade.
  • Prepends the cached prefix in derivation_path_for_output before SCROLLS, so every derivation path becomes [secret_prefix, "scrolls", tx_in_0_bytes, out_i_le_bytes]. addresses_impl and do_sign both await ensure_secret_prefix() before any derivation.
  • Adds direct deps ark-bls12-381, ark-ec, ark-ff, ark-serialize (all already in the workspace lockfile via charms-client, so no new transitive crates).

⚠️ Breaking: changes every derived Bitcoin address for scrolls_bitcoin going forward. Existing UTXOs derived under the old [SCROLLS, ...] path will no longer match this canister's derivation — requires a fresh deploy / new canister version.

Test plan

  • cargo check --package scrolls_bitcoin clean
  • cargo check --package scrolls_bitcoin --target wasm32-unknown-unknown clean
  • cargo test --package scrolls_bitcoin passes
  • Deploy to local replica with vetKD enabled; call addresses and confirm a 32-byte prefix is persisted to stable memory
  • Upgrade the canister and verify the RAM cache is restored from stable memory in post_upgrade (addresses stay stable across the upgrade)
  • Confirm addresses returns the same address for the same (tx_in_0, out_i) on repeated calls, but a different address than a freshly-installed canister

🤖 Generated with Claude Code

…for address derivation

Bootstraps a 32-byte secret via `vetkd_derive_key` on first signing call:
sample `transport_sk` from `raw_rand`, derive transport pk = sk·G1, call
vetKD, and store `SHA-256(sk_bytes || encrypted_key)` directly in stable
memory. RAM cache (`thread_local!`) is restored from stable memory in
`post_upgrade`; no `pre_upgrade` hook (avoids the upgrade-bricking risk).
`derivation_path_for_output` now prepends the secret prefix before `SCROLLS`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 20, 2026 00:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates scrolls_bitcoin address derivation to incorporate a canister-specific secret prefix derived via ICP vetKD, so derived Bitcoin addresses become unique per canister and stable across upgrades (via stable-memory persistence).

Changes:

  • Derive and cache a 32-byte canister-specific secret prefix on first use via raw_rand + vetkd_derive_key, persist it in stable memory, and restore it on upgrade.
  • Prepend the cached prefix to the ECDSA derivation path used for per-output address/key derivation.
  • Add arkworks dependencies needed to compute the BLS12-381 transport public key for vetKD.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
scrolls/src/scrolls_bitcoin/src/lib.rs Implements vetKD-based prefix derivation, stable-memory persistence, RAM cache, and derivation-path changes.
scrolls/src/scrolls_bitcoin/Cargo.toml Adds arkworks crates used for BLS12-381 scalar/curve operations and serialization.
scrolls/Cargo.lock Records the new direct dependencies for scrolls_bitcoin.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scrolls/src/scrolls_bitcoin/src/lib.rs Outdated
Comment on lines +181 to +205
async fn ensure_secret_prefix() -> anyhow::Result<()> {
if SECRET_PREFIX.with(|cell| cell.borrow().is_some()) {
return Ok(());
}

let rand = raw_rand()
.await
.map_err(|e| anyhow!("System error: raw_rand failed: {}", e))?;
let transport_sk = Fr::from_le_bytes_mod_order(&rand);
let transport_pk = G1Projective::generator() * transport_sk;
let mut transport_pk_bytes = Vec::new();
transport_pk
.into_affine()
.serialize_compressed(&mut transport_pk_bytes)
.context("System error: serializing transport public key")?;

let args = VetKDDeriveKeyArgs {
input: VETKD_SECRET_PREFIX_INPUT.to_vec(),
context: VETKD_SECRET_PREFIX_CONTEXT.to_vec(),
transport_public_key: transport_pk_bytes,
key_id: vetkd_key_id(),
};
let result = vetkd_derive_key(&args)
.await
.map_err(|e| anyhow!("System error: vetkd_derive_key failed: {}", e))?;
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR bootstraps a 32-byte canister-specific secret prefix via ICP vetKeys and prepends it to every Bitcoin address derivation path, preventing key reuse across separately-deployed canisters. The prefix is derived once on first run, persisted to stable memory using a write-data-before-marker pattern, and cached in an OnceLock for zero-cost reads on the hot path.

  • Timer-driven bootstrapping (schedule_secret_prefix_bootstrap) replaces the previous user-call-driven ensure_secret_prefix, eliminating the concurrent derivation race flagged in earlier reviews; an exponential-backoff retry (1 s → 2 s → … → 1 h) ensures the prefix is eventually populated even when vetKD is transiently unavailable.
  • OnceLock<[u8; 32]> semantics make SECRET_PREFIX truly write-once: cached_secret_prefix() always returns the same value once set, closing the mid-loop inconsistency concern from earlier review rounds.
  • Stable memory layout (marker byte at offset 0, prefix at offset 1–32) lets post_upgrade restore the prefix without a pre_upgrade hook; the timer re-reads from stable memory on every upgrade, falling back to a fresh vetKD call only on a brand-new canister.

Confidence Score: 5/5

Safe to merge; the timer-only bootstrapping and OnceLock cache together close the concurrency and stale-prefix concerns raised in prior reviews.

The address derivation is guarded by require_secret_prefix() at both entry points. Only a single timer drives ensure_secret_prefix; user calls never trigger it, so there is no concurrent derivation path. OnceLock immutability means cached_secret_prefix() cannot return a stale or overwritten value mid-loop. The write-data-before-marker pattern in store_persisted_prefix ensures a torn write leaves the marker unset, causing a safe re-derive on the next attempt. The exponential-backoff retry covers transient vetKD failures without manual intervention.

No files require special attention. The raw stable-memory region (bytes 0-32) is the only area to watch in future upgrades that add more persistent state - a comment already documents this layout.

Important Files Changed

Filename Overview
scrolls/src/scrolls_bitcoin/src/lib.rs Core change: adds vetKD-derived prefix bootstrapping via a timer with exponential backoff, OnceLock RAM cache, and stable memory persistence. Timer-only derivation path eliminates the concurrent race from earlier reviews; OnceLock immutability removes the mid-loop stale-prefix concern.
scrolls/src/scrolls_bitcoin/Cargo.toml Adds ic-cdk-timers 1.0 and ic-vetkeys 0.7 as direct dependencies; all transitive crates were already present in the workspace lockfile.
scrolls/Cargo.lock Lockfile update reflecting new direct and transitive deps (ic-vetkeys, ic-stable-structures, aes-gcm, ic_bls12_381, etc.); no unexpected version conflicts.

Reviews (6): Last reviewed commit: "refactor(scrolls_bitcoin): retry secret-..." | Re-trigger Greptile

Comment thread scrolls/src/scrolls_bitcoin/src/lib.rs
Comment thread scrolls/src/scrolls_bitcoin/src/lib.rs
imikushin and others added 4 commits May 19, 2026 17:27
…nister via vetKD decryption

Switch from the throwaway-transport-secret design to a fully deterministic
vetKD flow using the official `ic-vetkeys` crate:

  1. Seed the transport secret key from a BIP-340 Schnorr signature over a
     fixed message under a dedicated derivation path — only this canister
     can produce it, and the signature is deterministic per (chain key,
     message), so the seed is reproducible.
  2. Build a `TransportSecretKey` from that seed, call `vetkd_derive_key`
     and `vetkd_public_key`, then decrypt-and-verify the ciphertext.
  3. The decrypted vetKey is deterministic per (canister_id, input,
     context) by design, so the resulting 32-byte secret prefix (HKDF
     domain-separated) can always be regenerated by this canister.

Drops the direct `ark-*` deps in favor of `ic-vetkeys`, which encapsulates
all the BLS12-381 IBE machinery. Stable-memory cache and lack of
`pre_upgrade` hook are unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…et prefix

The decrypted vetKey is canonical for (canister_id, input, context) — the
transport keypair only wraps it in transit. So a fresh `raw_rand` seed on
every call yields the same vetKey after decryption, and the secret prefix
remains reproducible without needing a deterministic transport sk.

Drops the Schnorr-signature seeding ceremony and its dedicated derivation
path / message constants.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cache has write-once, read-many semantics and IC canisters execute
single-threaded per message, so `thread_local! RefCell<Option<...>>` was
overkill. A plain `static OnceLock<[u8; SECRET_PREFIX_LEN]>` matches the
use exactly: cheap reads, `set()` tolerates the (impossible-in-practice)
double-init race, and no closure boilerplate at the call sites.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, fail fast on calls

Lifecycle hooks can't make inter-canister calls, so schedule
`ensure_secret_prefix` on a `Duration::ZERO` timer from `do_init` (covers
both `init` and `post_upgrade`). The timer's first action is to read the
prefix from stable memory if present — only on a fresh canister does it
actually call vetKD.

`addresses` and `sign_and_submit` no longer try to lazy-init themselves;
they call `require_secret_prefix` up front and return a clear "service
unavailable, retry shortly" error if the timer hasn't completed yet.
This decouples the hot signing path from any vetKD latency and keeps each
call's blast radius small.

Adds `ic-cdk-timers = "1.0"`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread scrolls/src/scrolls_bitcoin/src/lib.rs Outdated
…tial backoff

If `ensure_secret_prefix` fails (transient subnet error, vetKD outage,
etc.), reschedule the timer with a doubling delay (1s, 2s, 4s, …, capped
at one hour) instead of leaving the canister unable to sign forever.
The chain stops on success because the cache short-circuit at the top
of `ensure_secret_prefix` turns any subsequent call into a no-op.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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