From b62c35cb8d0510e045314a45414bbf1910f95101 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 15 Jun 2026 21:36:20 +0000 Subject: [PATCH] feat(core): allow serialization of SyncResponse Add `serde::Serialize`/`Deserialize` for `SyncResponse` so a connected device can serialize a sync response and hand it to an air-gapped device to apply independently. `SyncResponse` holds a `TxUpdate` and an `Option`: - `TxUpdate` gains a plain derive; its fields are flat collections so there is nothing self-referential to recurse over. - `CheckPoint` is a reference-counted linked list, so a derived impl would recurse through `prev`/`skip` once per checkpoint and overflow the stack on long chains -- the same hazard that forced the hand-written `Drop` (#1634). It is instead serialized iteratively as a flat sequence of `(height, data)` pairs and rebuilt with `CheckPoint::from_blocks`, which re-derives the `skip`/`index` topology deterministically. Tests cover round-tripping `CheckPoint`, `TxUpdate` and `SyncResponse`, and assert (de)serialization is not recursive by round-tripping a long chain on a deliberately small thread stack. https://claude.ai/code/session_014BXMFRQP8qoGBghJcWzakG --- crates/core/Cargo.toml | 1 + crates/core/src/checkpoint.rs | 108 ++++++++++++++++++++++++++++++++++ crates/core/src/spk_client.rs | 75 +++++++++++++++++++++++ crates/core/src/tx_update.rs | 58 ++++++++++++++++++ 4 files changed, 242 insertions(+) diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index ed313f3ae..78709d00c 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -25,6 +25,7 @@ bdk_chain = { path = "../chain" } bdk_testenv = { path = "../testenv", default-features = false } criterion = { version = "0.7" } proptest = "1.2.0" +serde_json = "1" [[bench]] name = "checkpoint_skiplist" diff --git a/crates/core/src/checkpoint.rs b/crates/core/src/checkpoint.rs index a343e0dc9..f24438eac 100644 --- a/crates/core/src/checkpoint.rs +++ b/crates/core/src/checkpoint.rs @@ -618,6 +618,60 @@ impl IntoIterator for CheckPoint { } } +/// Serializes a [`CheckPoint`] as a flat sequence of `(height, data)` pairs, ordered from tip to +/// base (descending height). +/// +/// A [`CheckPoint`] is a reference-counted linked list, so a derived implementation would recurse +/// through `prev` (and `skip`) once per checkpoint and overflow the stack on long chains — the same +/// hazard that forced the hand-written [`Drop`] (see +/// ). Iterating with [`CheckPoint::iter`] keeps +/// serialization flat, and the `skip`/`index` topology is omitted because it is rebuilt +/// deterministically on deserialization. +#[cfg(feature = "serde")] +impl serde::Serialize for CheckPoint +where + D: serde::Serialize, +{ + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + use serde::ser::SerializeSeq; + // `index` is the number of checkpoints below the tip, so the chain has `index + 1` nodes. + let mut seq = serializer.serialize_seq(Some(self.0.index as usize + 1))?; + for cp in self.iter() { + seq.serialize_element(&(cp.height(), cp.data_ref()))?; + } + seq.end() + } +} + +/// Deserializes a [`CheckPoint`] from the flat sequence of `(height, data)` pairs produced by its +/// [`Serialize`](serde::Serialize) implementation. +/// +/// The chain is rebuilt iteratively with [`CheckPoint::from_blocks`] (which re-derives the +/// `skip`/`index` topology), so deserialization never recurses regardless of chain length. +#[cfg(feature = "serde")] +impl<'de, D> serde::Deserialize<'de> for CheckPoint +where + D: serde::Deserialize<'de> + ToBlockHash + Clone + fmt::Debug, +{ + fn deserialize(deserializer: De) -> Result + where + De: serde::Deserializer<'de>, + { + use serde::de::Error; + // Serialized tip-first (descending height); `from_blocks` expects ascending order. + let mut blocks = Vec::<(u32, D)>::deserialize(deserializer)?; + blocks.reverse(); + CheckPoint::from_blocks(blocks).map_err(|_| { + De::Error::custom( + "invalid checkpoint chain: blocks must be non-empty, ascending and well-linked", + ) + }) + } +} + #[cfg(test)] mod tests { use super::*; @@ -644,6 +698,60 @@ mod tests { .unwrap(); } + /// Round-tripping a checkpoint through serde must preserve the block ids and rebuild an + /// identical `index`/`skip` topology. + #[cfg(feature = "serde")] + #[test] + fn checkpoint_serde_round_trip() { + let mut cp = CheckPoint::new(0, bitcoin::hashes::Hash::hash(b"genesis")); + for height in 1u32..=100 { + let hash: BlockHash = bitcoin::hashes::Hash::hash(height.to_be_bytes().as_slice()); + cp = cp.push(height, hash).unwrap(); + } + + let json = serde_json::to_string(&cp).expect("serialization must succeed"); + let restored: CheckPoint = + serde_json::from_str(&json).expect("deserialization must succeed"); + + assert_eq!(cp, restored); + assert_eq!(cp.iter().count(), restored.iter().count()); + for (orig_cp, restored_cp) in cp.iter().zip(restored.iter()) { + assert_eq!(orig_cp.block_id(), restored_cp.block_id()); + assert_eq!(orig_cp.index(), restored_cp.index()); + assert_eq!( + orig_cp.skip().map(|cp| cp.block_id()), + restored_cp.skip().map(|cp| cp.block_id()), + ); + } + } + + /// Serialization and deserialization must walk the linked list iteratively. A recursive + /// implementation (e.g. a naive derive over `prev`) would overflow this deliberately small + /// stack on a long chain — the same hazard guarded against in + /// `checkpoint_drop_is_not_recursive`. + #[cfg(feature = "serde")] + #[test] + fn checkpoint_serde_is_not_recursive() { + let run = || { + let mut cp = CheckPoint::new(0, bitcoin::hashes::Hash::hash(b"genesis")); + for height in 1u32..=(1024 * 10) { + let hash: BlockHash = bitcoin::hashes::Hash::hash(height.to_be_bytes().as_slice()); + cp = cp.push(height, hash).unwrap(); + } + + let json = serde_json::to_string(&cp).expect("serialization must not recurse"); + let restored: CheckPoint = + serde_json::from_str(&json).expect("deserialization must not recurse"); + assert_eq!(cp, restored); + }; + std::thread::Builder::new() + .stack_size(128 * 1024) + .spawn(run) + .unwrap() + .join() + .unwrap(); + } + #[test] fn checkpoint_does_not_leak() { const CHAIN_LEN: u32 = 1000; diff --git a/crates/core/src/spk_client.rs b/crates/core/src/spk_client.rs index 64fe3dd8c..4aa7073c2 100644 --- a/crates/core/src/spk_client.rs +++ b/crates/core/src/spk_client.rs @@ -404,6 +404,15 @@ impl SyncRequest { /// See also [`SyncRequest`]. #[must_use] #[derive(Debug)] +#[cfg_attr( + feature = "serde", + derive(serde::Serialize, serde::Deserialize), + serde(bound( + serialize = "A: serde::Serialize, D: serde::Serialize", + deserialize = "A: Ord + serde::Deserialize<'de>, \ + D: crate::ToBlockHash + Clone + core::fmt::Debug + serde::Deserialize<'de>" + )) +)] pub struct SyncResponse { /// Relevant transaction data discovered during the scan. pub tx_update: crate::TxUpdate, @@ -689,3 +698,69 @@ impl Iterator for SyncIter<'_, I, D, OutPoint> { (remaining, Some(remaining)) } } + +#[cfg(all(test, feature = "serde"))] +mod test { + use super::*; + use crate::{alloc::sync::Arc, BlockId}; + use bitcoin::{hashes::Hash, Amount, Transaction, TxOut}; + + #[test] + fn sync_response_serde_round_trip() { + // A checkpoint chain to exercise the non-recursive `CheckPoint` (de)serialization. + let mut cp = CheckPoint::new(0, Hash::hash(b"genesis")); + for height in 1u32..=10 { + let hash: BlockHash = Hash::hash(height.to_be_bytes().as_slice()); + cp = cp.push(height, hash).unwrap(); + } + + // A `tx_update` exercising every field. + let tx = Transaction { + version: bitcoin::transaction::Version::non_standard(0), + lock_time: bitcoin::absolute::LockTime::ZERO, + input: Vec::new(), + output: Vec::new(), + }; + let txid = tx.compute_txid(); + let mut tx_update = crate::TxUpdate::::default(); + tx_update.txs.push(Arc::new(tx)); + tx_update.txouts.insert( + OutPoint::new(txid, 0), + TxOut { + value: Amount::from_sat(21_000), + script_pubkey: ScriptBuf::new(), + }, + ); + tx_update.anchors.insert(( + ConfirmationBlockTime { + block_id: BlockId { + height: 5, + hash: Hash::hash(b"anchor"), + }, + confirmation_time: 1_700_000_000, + }, + txid, + )); + tx_update.seen_ats.insert((txid, 1_700_000_001)); + tx_update.evicted_ats.insert((txid, 1_700_000_002)); + + let response = SyncResponse { + tx_update, + chain_update: Some(cp), + }; + + let json = serde_json::to_string(&response).expect("serialization must succeed"); + let restored: SyncResponse = + serde_json::from_str(&json).expect("deserialization must succeed"); + + assert_eq!(response.chain_update, restored.chain_update); + assert_eq!(response.tx_update.txs, restored.tx_update.txs); + assert_eq!(response.tx_update.txouts, restored.tx_update.txouts); + assert_eq!(response.tx_update.anchors, restored.tx_update.anchors); + assert_eq!(response.tx_update.seen_ats, restored.tx_update.seen_ats); + assert_eq!( + response.tx_update.evicted_ats, + restored.tx_update.evicted_ats + ); + } +} diff --git a/crates/core/src/tx_update.rs b/crates/core/src/tx_update.rs index f92cf3e92..b12990aa7 100644 --- a/crates/core/src/tx_update.rs +++ b/crates/core/src/tx_update.rs @@ -26,6 +26,14 @@ use bitcoin::{OutPoint, Transaction, TxOut, Txid}; /// The built-in chain-source crates (`bdk_electrum`, `bdk_esplora`, `bdk_bitcoind_rpc`) handle this /// automatically. Transactions lacking temporal context are stored but ignored by canonicalization. #[derive(Debug, Clone)] +#[cfg_attr( + feature = "serde", + derive(serde::Serialize, serde::Deserialize), + serde(bound( + serialize = "A: serde::Serialize", + deserialize = "A: Ord + serde::Deserialize<'de>" + )) +)] #[non_exhaustive] pub struct TxUpdate { /// Full transactions. These are transactions that were determined to be relevant to the wallet @@ -110,3 +118,53 @@ impl TxUpdate { self.evicted_ats.extend(other.evicted_ats); } } + +#[cfg(all(test, feature = "serde"))] +mod test { + use super::*; + use crate::{BlockId, ConfirmationBlockTime}; + use bitcoin::{hashes::Hash, Amount, ScriptBuf}; + + #[test] + fn tx_update_serde_round_trip() { + let tx = Transaction { + version: bitcoin::transaction::Version::non_standard(0), + lock_time: bitcoin::absolute::LockTime::ZERO, + input: Vec::new(), + output: Vec::new(), + }; + let txid = tx.compute_txid(); + + let mut tx_update = TxUpdate::::default(); + tx_update.txs.push(Arc::new(tx)); + tx_update.txouts.insert( + OutPoint::new(txid, 0), + TxOut { + value: Amount::from_sat(42_000), + script_pubkey: ScriptBuf::new(), + }, + ); + tx_update.anchors.insert(( + ConfirmationBlockTime { + block_id: BlockId { + height: 7, + hash: Hash::hash(b"anchor"), + }, + confirmation_time: 1_700_000_000, + }, + txid, + )); + tx_update.seen_ats.insert((txid, 1_700_000_001)); + tx_update.evicted_ats.insert((txid, 1_700_000_002)); + + let json = serde_json::to_string(&tx_update).expect("serialization must succeed"); + let restored: TxUpdate = + serde_json::from_str(&json).expect("deserialization must succeed"); + + assert_eq!(tx_update.txs, restored.txs); + assert_eq!(tx_update.txouts, restored.txouts); + assert_eq!(tx_update.anchors, restored.anchors); + assert_eq!(tx_update.seen_ats, restored.seen_ats); + assert_eq!(tx_update.evicted_ats, restored.evicted_ats); + } +}