From a38f63359d02f508dc987c8a985d34e393d831a8 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Fri, 3 Mar 2023 22:54:47 +1100 Subject: [PATCH] Make bdk_file_store use bincode v1 --- crates/bdk/Cargo.toml | 2 - crates/chain/src/keychain.rs | 2 +- crates/file_store/Cargo.toml | 2 +- crates/file_store/src/file_store.rs | 158 ++++++++++++++++++--- crates/file_store/tests/test_file_store.rs | 116 --------------- 5 files changed, 143 insertions(+), 137 deletions(-) diff --git a/crates/bdk/Cargo.toml b/crates/bdk/Cargo.toml index 77b577400..332197da4 100644 --- a/crates/bdk/Cargo.toml +++ b/crates/bdk/Cargo.toml @@ -24,7 +24,6 @@ bdk_chain = { path = "../chain", version = "0.3.1", features = ["miniscript", "s # Optional dependencies hwi = { version = "0.5", optional = true, features = [ "use-miniscript"] } bip39 = { version = "1.0.1", optional = true } -bdk_file_store = { path = "../file_store", version = "0.0.1", optional = true } [target.'cfg(target_arch = "wasm32")'.dependencies] getrandom = "0.2" @@ -34,7 +33,6 @@ js-sys = "0.3" [features] default = ["std"] std = [] -file-store = [ "std", "bdk_file_store"] compiler = ["miniscript/compiler"] all-keys = ["keys-bip39"] keys-bip39 = ["bip39"] diff --git a/crates/chain/src/keychain.rs b/crates/chain/src/keychain.rs index 41110833e..68d5dd10f 100644 --- a/crates/chain/src/keychain.rs +++ b/crates/chain/src/keychain.rs @@ -56,7 +56,7 @@ pub use txout_index::*; ) )] #[must_use] -pub struct DerivationAdditions(BTreeMap); +pub struct DerivationAdditions(pub BTreeMap); impl DerivationAdditions { /// Returns whether the additions are empty. diff --git a/crates/file_store/Cargo.toml b/crates/file_store/Cargo.toml index 38a98f0e4..09ed12130 100644 --- a/crates/file_store/Cargo.toml +++ b/crates/file_store/Cargo.toml @@ -7,7 +7,7 @@ repository = "https://github.com/bitcoindevkit/bdk" [dependencies] bdk_chain = { path = "../chain", version = "0.3.1", features = [ "serde", "miniscript" ] } -bincode = { version = "2.0.0-rc.2", features = [ "serde" ] } +bincode = { version = "1" } serde = { version = "1", features = ["derive"] } [dev-dependencies] diff --git a/crates/file_store/src/file_store.rs b/crates/file_store/src/file_store.rs index 1247e4568..378555062 100644 --- a/crates/file_store/src/file_store.rs +++ b/crates/file_store/src/file_store.rs @@ -7,6 +7,7 @@ use bdk_chain::{ keychain::{KeychainChangeSet, KeychainTracker}, sparse_chain, AsTransaction, }; +use bincode::{DefaultOptions, Options}; use core::marker::PhantomData; use std::{ fs::{File, OpenOptions}, @@ -15,10 +16,10 @@ use std::{ }; /// BDK File Store magic bytes length. -pub const MAGIC_BYTES_LEN: usize = 12; +const MAGIC_BYTES_LEN: usize = 12; /// BDK File Store magic bytes. -pub const MAGIC_BYTES: [u8; MAGIC_BYTES_LEN] = [98, 100, 107, 102, 115, 48, 48, 48, 48, 48, 48, 48]; +const MAGIC_BYTES: [u8; MAGIC_BYTES_LEN] = [98, 100, 107, 102, 115, 48, 48, 48, 48, 48, 48, 48]; /// Persists an append only list of `KeychainChangeSet` to a single file. /// [`KeychainChangeSet`] record the changes made to a [`KeychainTracker`]. @@ -28,6 +29,10 @@ pub struct KeychainStore { changeset_type_params: core::marker::PhantomData<(K, P, T)>, } +fn bincode() -> impl bincode::Options { + DefaultOptions::new().with_varint_encoding() +} + impl KeychainStore where K: Ord + Clone + core::fmt::Debug, @@ -58,7 +63,7 @@ where /// Creates or loads a a store from `db_path`. If no file exists there it will be created. pub fn new_from_path>(db_path: D) -> Result { - let already_exists = db_path.as_ref().try_exists()?; + let already_exists = db_path.as_ref().exists(); let mut db_file = OpenOptions::new() .read(true) @@ -143,15 +148,12 @@ where return Ok(()); } - bincode::encode_into_std_write( - bincode::serde::Compat(changeset), - &mut self.db_file, - bincode::config::standard(), - ) - .map_err(|e| match e { - bincode::error::EncodeError::Io { inner, .. } => inner, - unexpected_err => panic!("unexpected bincode error: {}", unexpected_err), - })?; + bincode() + .serialize_into(&mut self.db_file, changeset) + .map_err(|e| match *e { + bincode::ErrorKind::Io(inner) => inner, + unexpected_err => panic!("unexpected bincode error: {}", unexpected_err), + })?; // truncate file after this changeset addition // if this is not done, data after this changeset may represent valid changesets, however @@ -205,7 +207,7 @@ pub enum IterError { /// Failure to read from file. Io(io::Error), /// Failure to decode data from file. - Bincode(bincode::error::DecodeError), + Bincode(bincode::ErrorKind), } impl core::fmt::Display for IterError { @@ -251,10 +253,10 @@ where let result = (|| { let pos = self.db_file.stream_position()?; - match bincode::decode_from_std_read(self.db_file, bincode::config::standard()) { - Ok(bincode::serde::Compat(changeset)) => Ok(Some(changeset)), + match bincode().deserialize_from(&mut self.db_file) { + Ok(changeset) => Ok(Some(changeset)), Err(e) => { - if let bincode::error::DecodeError::Io { inner, .. } = &e { + if let bincode::ErrorKind::Io(inner) = &*e { if inner.kind() == io::ErrorKind::UnexpectedEof { let eof = self.db_file.seek(io::SeekFrom::End(0))?; if pos == eof { @@ -264,7 +266,7 @@ where } self.db_file.seek(io::SeekFrom::Start(pos))?; - Err(IterError::Bincode(e)) + Err(IterError::Bincode(*e)) } } })(); @@ -284,3 +286,125 @@ impl From for IterError { IterError::Io(value) } } + +#[cfg(test)] +mod test { + use super::*; + use bdk_chain::{ + bitcoin::Transaction, + keychain::{DerivationAdditions, KeychainChangeSet}, + TxHeight, + }; + use std::{ + io::{Read, Write}, + vec::Vec, + }; + use tempfile::NamedTempFile; + #[derive( + Debug, + Clone, + Copy, + PartialOrd, + Ord, + PartialEq, + Eq, + Hash, + serde::Serialize, + serde::Deserialize, + )] + enum TestKeychain { + External, + Internal, + } + + impl core::fmt::Display for TestKeychain { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::External => write!(f, "external"), + Self::Internal => write!(f, "internal"), + } + } + } + + #[test] + fn magic_bytes() { + assert_eq!(&MAGIC_BYTES, "bdkfs0000000".as_bytes()); + } + + #[test] + fn new_fails_if_file_is_too_short() { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(&MAGIC_BYTES[..MAGIC_BYTES_LEN - 1]) + .expect("should write"); + + match KeychainStore::::new(file.reopen().unwrap()) { + Err(FileError::Io(e)) => assert_eq!(e.kind(), std::io::ErrorKind::UnexpectedEof), + unexpected => panic!("unexpected result: {:?}", unexpected), + }; + } + + #[test] + fn new_fails_if_magic_bytes_are_invalid() { + let invalid_magic_bytes = "ldkfs0000000"; + + let mut file = NamedTempFile::new().unwrap(); + file.write_all(invalid_magic_bytes.as_bytes()) + .expect("should write"); + + match KeychainStore::::new(file.reopen().unwrap()) { + Err(FileError::InvalidMagicBytes(b)) => { + assert_eq!(b, invalid_magic_bytes.as_bytes()) + } + unexpected => panic!("unexpected result: {:?}", unexpected), + }; + } + + #[test] + fn append_changeset_truncates_invalid_bytes() { + // initial data to write to file (magic bytes + invalid data) + let mut data = [255_u8; 2000]; + data[..MAGIC_BYTES_LEN].copy_from_slice(&MAGIC_BYTES); + + let changeset = KeychainChangeSet { + derivation_indices: DerivationAdditions( + vec![(TestKeychain::External, 42)].into_iter().collect(), + ), + chain_graph: Default::default(), + }; + + let mut file = NamedTempFile::new().unwrap(); + file.write_all(&data).expect("should write"); + + let mut store = + KeychainStore::::new(file.reopen().unwrap()) + .expect("should open"); + match store.iter_changesets().expect("seek should succeed").next() { + Some(Err(IterError::Bincode(_))) => {} + unexpected_res => panic!("unexpected result: {:?}", unexpected_res), + } + + store.append_changeset(&changeset).expect("should append"); + + drop(store); + + let got_bytes = { + let mut buf = Vec::new(); + file.reopen() + .unwrap() + .read_to_end(&mut buf) + .expect("should read"); + buf + }; + + let expected_bytes = { + let mut buf = MAGIC_BYTES.to_vec(); + DefaultOptions::new() + .with_varint_encoding() + .serialize_into(&mut buf, &changeset) + .expect("should encode"); + buf + }; + + assert_eq!(got_bytes, expected_bytes); + } +} diff --git a/crates/file_store/tests/test_file_store.rs b/crates/file_store/tests/test_file_store.rs index 0ed089dfe..8b1378917 100644 --- a/crates/file_store/tests/test_file_store.rs +++ b/crates/file_store/tests/test_file_store.rs @@ -1,117 +1 @@ -use bdk_chain::{ - bitcoin::Transaction, - keychain::{KeychainChangeSet, KeychainTracker}, - TxHeight, -}; -use bdk_file_store::{FileError, IterError, KeychainStore, MAGIC_BYTES, MAGIC_BYTES_LEN}; -use std::{ - io::{Read, Write}, - vec::Vec, -}; -use tempfile::NamedTempFile; -#[derive( - Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize, -)] -enum TestKeychain { - External, - Internal, -} -impl core::fmt::Display for TestKeychain { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::External => write!(f, "external"), - Self::Internal => write!(f, "internal"), - } - } -} - -#[test] -fn magic_bytes() { - assert_eq!(&MAGIC_BYTES, "bdkfs0000000".as_bytes()); -} - -#[test] -fn new_fails_if_file_is_too_short() { - let mut file = NamedTempFile::new().unwrap(); - file.write_all(&MAGIC_BYTES[..MAGIC_BYTES_LEN - 1]) - .expect("should write"); - - match KeychainStore::::new(file.reopen().unwrap()) { - Err(FileError::Io(e)) => assert_eq!(e.kind(), std::io::ErrorKind::UnexpectedEof), - unexpected => panic!("unexpected result: {:?}", unexpected), - }; -} - -#[test] -fn new_fails_if_magic_bytes_are_invalid() { - let invalid_magic_bytes = "ldkfs0000000"; - - let mut file = NamedTempFile::new().unwrap(); - file.write_all(invalid_magic_bytes.as_bytes()) - .expect("should write"); - - match KeychainStore::::new(file.reopen().unwrap()) { - Err(FileError::InvalidMagicBytes(b)) => { - assert_eq!(b, invalid_magic_bytes.as_bytes()) - } - unexpected => panic!("unexpected result: {:?}", unexpected), - }; -} - -#[test] -fn append_changeset_truncates_invalid_bytes() { - use bdk_chain::miniscript; - use core::str::FromStr; - // initial data to write to file (magic bytes + invalid data) - let mut data = [255_u8; 2000]; - data[..MAGIC_BYTES_LEN].copy_from_slice(&MAGIC_BYTES); - - let descriptor = miniscript::Descriptor::from_str("tr([73c5da0a/86'/0'/0']xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)#rg247h69").unwrap(); - let mut tracker = KeychainTracker::::default(); - tracker.add_keychain(TestKeychain::External, descriptor); - let changeset = KeychainChangeSet { - derivation_indices: tracker - .txout_index - .reveal_to_target(&TestKeychain::External, 21) - .1, - chain_graph: Default::default(), - }; - - let mut file = NamedTempFile::new().unwrap(); - file.write_all(&data).expect("should write"); - - let mut store = - KeychainStore::::new(file.reopen().unwrap()) - .expect("should open"); - match store.iter_changesets().expect("seek should succeed").next() { - Some(Err(IterError::Bincode(_))) => {} - unexpected_res => panic!("unexpected result: {:?}", unexpected_res), - } - - store.append_changeset(&changeset).expect("should append"); - - drop(store); - - let got_bytes = { - let mut buf = Vec::new(); - file.reopen() - .unwrap() - .read_to_end(&mut buf) - .expect("should read"); - buf - }; - - let expected_bytes = { - let mut buf = MAGIC_BYTES.to_vec(); - bincode::encode_into_std_write( - bincode::serde::Compat(&changeset), - &mut buf, - bincode::config::standard(), - ) - .expect("should encode"); - buf - }; - - assert_eq!(got_bytes, expected_bytes); -}