frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoindevkit/bdk#1292: fix(store): Remove lifetime

Browse Source 
e6433fb2c1526cff37e3a17cc71986759ebac30f feat(persist): Add stage_and_commit to Persist (LLFourn)
0bee46e75bc99112abc15de59984e6059ff874e5 fix(store): Remove lifetime (LLFourn)

Pull request description:

  Remove gratuitous use of lifetimes in the main persistence struct `Store`. Having lifetimes on this means that you have to keep the magic bytes alive longer than the database which is particularly offensive if you have to send the database to another thread. On top of that the bytes aren't even read.

ACKs for top commit:
  evanlinjin:
    ACK e6433fb2c1526cff37e3a17cc71986759ebac30f

Tree-SHA512: 7f6d9d60951a8ceaee30719d0771e15632c6fad0702294af15409c5df492669a07299874ef5ee34e3d75bdecbbd41df29bced3ff16b2360d5d5c7687ef677ffc
This commit is contained in:
志宇 2024-01-19 23:28:19 +08:00
commit 3fa44a58ec
No known key found for this signature in database
GPG Key ID: F6345C9837C2BDE8
6 changed files with 36 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
crates/chain/src/persist.rs
View File

@ -55,6 +55,18 @@ where
// if written successfully, take and return `self.stage`
.map(|_| Some(core::mem::take(&mut self.stage)))
}
/// Stages a new changeset and commits it (along with any other previously staged changes) to
/// the persistence backend
///
/// Convience method for calling [`stage`] and then [`commit`].
///
/// [`stage`]: Self::stage
/// [`commit`]: Self::commit
pub fn stage_and_commit(&mut self, changeset: C) -> Result<Option<C>, B::WriteError> {
self.stage(changeset);
self.commit()
}
}
/// A persistence backend for [`Persist`].

20
crates/file_store/src/store.rs
View File

@ -15,13 +15,13 @@ use crate::{bincode_options, EntryIter, FileError, IterError};
///
/// The changesets are the results of altering a tracker implementation (`T`).
#[derive(Debug)]
pub struct Store<'a, C> {
magic: &'a [u8],
pub struct Store<C> {
magic_len: usize,
db_file: File,
marker: PhantomData<C>,
}
impl<'a, C> PersistBackend<C> for Store<'a, C>
impl<C> PersistBackend<C> for Store<C>
where
C: Append + serde::Serialize + serde::de::DeserializeOwned,
{
@ -38,7 +38,7 @@ where
}
}
impl<'a, C> Store<'a, C>
impl<C> Store<C>
where
C: Append + serde::Serialize + serde::de::DeserializeOwned,
{
@ -48,7 +48,7 @@ where
/// the `Store` in the future with [`open`].
///
/// [`open`]: Store::open
pub fn create_new<P>(magic: &'a [u8], file_path: P) -> Result<Self, FileError>
pub fn create_new<P>(magic: &[u8], file_path: P) -> Result<Self, FileError>
where
P: AsRef<Path>,
{
@ -67,7 +67,7 @@ where
.open(file_path)?;
f.write_all(magic)?;
Ok(Self {
magic,
magic_len: magic.len(),
db_file: f,
marker: Default::default(),
})
@ -83,7 +83,7 @@ where
/// [`FileError::InvalidMagicBytes`] error variant will be returned.
///
/// [`create_new`]: Store::create_new
pub fn open<P>(magic: &'a [u8], file_path: P) -> Result<Self, FileError>
pub fn open<P>(magic: &[u8], file_path: P) -> Result<Self, FileError>
where
P: AsRef<Path>,
{
@ -99,7 +99,7 @@ where
}
Ok(Self {
magic,
magic_len: magic.len(),
db_file: f,
marker: Default::default(),
})
@ -111,7 +111,7 @@ where
///
/// [`open`]: Store::open
/// [`create_new`]: Store::create_new
pub fn open_or_create_new<P>(magic: &'a [u8], file_path: P) -> Result<Self, FileError>
pub fn open_or_create_new<P>(magic: &[u8], file_path: P) -> Result<Self, FileError>
where
P: AsRef<Path>,
{
@ -132,7 +132,7 @@ where
/// always iterate over all entries until `None` is returned if you want your next write to go
/// at the end; otherwise, you will write over existing entries.
pub fn iter_changesets(&mut self) -> EntryIter<C> {
EntryIter::new(self.magic.len() as u64, &mut self.db_file)
EntryIter::new(self.magic_len as u64, &mut self.db_file)
}
/// Loads all the changesets that have been stored as one giant changeset.

4
example-crates/example_bitcoind_rpc_polling/src/main.rs
View File

@ -147,7 +147,7 @@ fn main() -> anyhow::Result<()> {
let rpc_cmd = match args.command {
example_cli::Commands::ChainSpecific(rpc_cmd) => rpc_cmd,
general_cmd => {
let res = example_cli::handle_commands(
return example_cli::handle_commands(
&graph,
&db,
&chain,
@ -160,8 +160,6 @@ fn main() -> anyhow::Result<()> {
},
general_cmd,
);
db.lock().unwrap().commit()?;
return res;
}
};

24
example-crates/example_cli/src/lib.rs
View File

@ -29,7 +29,7 @@ pub type KeychainChangeSet<A> = (
local_chain::ChangeSet,
indexed_tx_graph::ChangeSet<A, keychain::ChangeSet<Keychain>>,
);
pub type Database<'m, C> = Persist<Store<'m, C>, C>;
pub type Database<C> = Persist<Store<C>, C>;
#[derive(Parser)]
#[clap(author, version, about, long_about = None)]
@ -457,11 +457,10 @@ where
let ((spk_i, spk), index_changeset) = spk_chooser(index, &Keychain::External);
let db = &mut *db.lock().unwrap();
db.stage(C::from((
db.stage_and_commit(C::from((
local_chain::ChangeSet::default(),
indexed_tx_graph::ChangeSet::from(index_changeset),
)));
db.commit()?;
)))?;
let addr =
Address::from_script(spk, network).context("failed to derive address")?;
println!("[address @ {}] {}", spk_i, addr);
@ -601,11 +600,10 @@ where
// If we're unable to persist this, then we don't want to broadcast.
{
let db = &mut *db.lock().unwrap();
db.stage(C::from((
db.stage_and_commit(C::from((
local_chain::ChangeSet::default(),
indexed_tx_graph::ChangeSet::from(index_changeset),
)));
db.commit()?;
)))?;
}
// We don't want other callers/threads to use this address while we're using it
@ -627,10 +625,10 @@ where
// We know the tx is at least unconfirmed now. Note if persisting here fails,
// it's not a big deal since we can always find it again form
// blockchain.
db.lock().unwrap().stage(C::from((
db.lock().unwrap().stage_and_commit(C::from((
local_chain::ChangeSet::default(),
keychain_changeset,
)));
)))?;
Ok(())
}
Err(e) => {
@ -646,14 +644,14 @@ where
}
#[allow(clippy::type_complexity)]
pub fn init<'m, CS: clap::Subcommand, S: clap::Args, C>(
db_magic: &'m [u8],
pub fn init<CS: clap::Subcommand, S: clap::Args, C>(
db_magic: &[u8],
db_default_path: &str,
) -> anyhow::Result<(
Args<CS, S>,
KeyMap,
KeychainTxOutIndex<Keychain>,
Mutex<Database<'m, C>>,
Mutex<Database<C>>,
C,
)>
where
@ -681,7 +679,7 @@ where
index.add_keychain(Keychain::Internal, internal_descriptor);
}
let mut db_backend = match Store::<'m, C>::open_or_create_new(db_magic, &args.db_path) {
let mut db_backend = match Store::<C>::open_or_create_new(db_magic, &args.db_path) {
Ok(db_backend) => db_backend,
// we cannot return `err` directly as it has lifetime `'m`
Err(err) => return Err(anyhow::anyhow!("failed to init db backend: {:?}", err)),

5
example-crates/example_electrum/src/main.rs
View File

@ -122,7 +122,7 @@ fn main() -> anyhow::Result<()> {
let electrum_cmd = match &args.command {
example_cli::Commands::ChainSpecific(electrum_cmd) => electrum_cmd,
general_cmd => {
let res = example_cli::handle_commands(
return example_cli::handle_commands(
&graph,
&db,
&chain,
@ -135,9 +135,6 @@ fn main() -> anyhow::Result<()> {
},
general_cmd.clone(),
);
db.lock().unwrap().commit()?;
return res;
}
};

5
example-crates/example_esplora/src/main.rs
View File

@ -125,7 +125,7 @@ fn main() -> anyhow::Result<()> {
example_cli::Commands::ChainSpecific(esplora_cmd) => esplora_cmd,
// These are general commands handled by example_cli. Execute the cmd and return.
general_cmd => {
let res = example_cli::handle_commands(
return example_cli::handle_commands(
&graph,
&db,
&chain,
@ -140,9 +140,6 @@ fn main() -> anyhow::Result<()> {
},
general_cmd.clone(),
);
db.lock().unwrap().commit()?;
return res;
}
};