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

Revert replacing BTreeMap to HashMap in SingersContainer

Browse Source
This commit is contained in:
Evgenii P 2020-12-14 17:50:47 +07:00
parent 4ede4a4ad0
commit c075183a7b
No known key found for this signature in database
GPG Key ID: 46717E4E65912EF7
1 changed files with 33 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
src/wallet/signer.rs
View File

@ -91,8 +91,9 @@
//! ``` //! ```
use std::cmp::Ordering; use std::cmp::Ordering;
use std::collections::HashMap; use std::collections::BTreeMap;
use std::fmt; use std::fmt;
use std::ops::Bound::Included;
use std::sync::Arc; use std::sync::Arc;
use bitcoin::blockdata::opcodes; use bitcoin::blockdata::opcodes;
@ -295,7 +296,7 @@ impl Signer for PrivateKey {
/// The default value is `100`. Signers with an ordering above that will be called later, /// The default value is `100`. Signers with an ordering above that will be called later,
/// and they will thus see the partial signatures added to the transaction once they get to sign /// and they will thus see the partial signatures added to the transaction once they get to sign
/// themselves. /// themselves.
#[derive(Debug, Clone, Hash, PartialOrd, PartialEq, Ord, Eq)] #[derive(Debug, Clone, PartialOrd, PartialEq, Ord, Eq)]
pub struct SignerOrdering(pub usize); pub struct SignerOrdering(pub usize);
impl std::default::Default for SignerOrdering { impl std::default::Default for SignerOrdering {
@ -304,7 +305,7 @@ impl std::default::Default for SignerOrdering {
} }
} }
#[derive(Debug, Clone, Hash, Eq, PartialEq)] #[derive(Debug, Clone)]
struct SignersContainerKey { struct SignersContainerKey {
id: SignerId, id: SignerId,
ordering: SignerOrdering, ordering: SignerOrdering,
@ -321,7 +322,7 @@ impl From<(SignerId, SignerOrdering)> for SignersContainerKey {
/// Container for multiple signers /// Container for multiple signers
#[derive(Debug, Default, Clone)] #[derive(Debug, Default, Clone)]
pub struct SignersContainer(HashMap<SignersContainerKey, Arc<dyn Signer>>); pub struct SignersContainer(BTreeMap<SignersContainerKey, Arc<dyn Signer>>);
impl SignersContainer { impl SignersContainer {
pub fn as_key_map(&self, secp: &SecpCtx) -> KeyMap { pub fn as_key_map(&self, secp: &SecpCtx) -> KeyMap {
@ -394,18 +395,18 @@ impl SignersContainer {
/// Returns the list of signers in the container, sorted by lowest to highest `ordering` /// Returns the list of signers in the container, sorted by lowest to highest `ordering`
pub fn signers(&self) -> Vec<&Arc<dyn Signer>> { pub fn signers(&self) -> Vec<&Arc<dyn Signer>> {
let mut items = self.0.iter().collect::<Vec<_>>(); self.0.values().collect()
items.sort_unstable_by_key(|(key, _)| *key);
items.into_iter().map(|(_, v)| v).collect()
} }
/// Finds the signer with lowest ordering for a given id in the container. /// Finds the signer with lowest ordering for a given id in the container.
pub fn find(&self, id: SignerId) -> Option<&Arc<dyn Signer>> { pub fn find(&self, id: SignerId) -> Option<&Arc<dyn Signer>> {
self.0 self.0
.iter() .range((
.filter(|(key, _)| key.id == id) Included(&(id.clone(), SignerOrdering(0)).into()),
.min_by(|(k_a, _), (k_b, _)| k_a.cmp(k_b)) Included(&(id, SignerOrdering(usize::MAX)).into()),
))
.map(|(_, v)| v) .map(|(_, v)| v)
.next()
} }
} }
@ -525,18 +526,26 @@ impl Ord for SignersContainerKey {
} }
} }
impl PartialEq for SignersContainerKey {
fn eq(&self, other: &Self) -> bool {
self.ordering == other.ordering
}
}
impl Eq for SignersContainerKey {}
#[cfg(test)] #[cfg(test)]
mod signers_container_tests { mod signers_container_tests {
use super::*; use super::*;
use crate::descriptor; use crate::descriptor;
use crate::descriptor::ToWalletDescriptor;
use crate::keys::{DescriptorKey, ToDescriptorKey};
use bitcoin::secp256k1::All;
use bitcoin::util::bip32;
use bitcoin::util::psbt::PartiallySignedTransaction;
use bitcoin::Network;
use miniscript::ScriptContext; use miniscript::ScriptContext;
use crate::keys::{DescriptorKey, ToDescriptorKey};
use bitcoin::util::bip32;
use bitcoin::secp256k1::All;
use std::str::FromStr; use std::str::FromStr;
use crate::descriptor::ToWalletDescriptor;
use bitcoin::Network;
use bitcoin::util::psbt::PartiallySignedTransaction;
// Signers added with the same ordering (like `Ordering::default`) created from `KeyMap` // Signers added with the same ordering (like `Ordering::default`) created from `KeyMap`
// should be preserved and not overwritten. // should be preserved and not overwritten.
@ -562,21 +571,9 @@ mod signers_container_tests {
let signer2 = Arc::new(DummySigner); let signer2 = Arc::new(DummySigner);
let signer3 = Arc::new(DummySigner); let signer3 = Arc::new(DummySigner);
signers.add_external( signers.add_external(SignerId::Fingerprint(b"cafe"[..].into()), SignerOrdering(1), signer1.clone());
SignerId::Fingerprint(b"cafe"[..].into()), signers.add_external(SignerId::Fingerprint(b"babe"[..].into()), SignerOrdering(2), signer2.clone());
SignerOrdering(1), signers.add_external(SignerId::Fingerprint(b"feed"[..].into()), SignerOrdering(3), signer3.clone());
signer1.clone(),
);
signers.add_external(
SignerId::Fingerprint(b"babe"[..].into()),
SignerOrdering(2),
signer2.clone(),
);
signers.add_external(
SignerId::Fingerprint(b"feed"[..].into()),
SignerOrdering(3),
signer3.clone(),
);
// Check that signers are sorted from lowest to highest ordering // Check that signers are sorted from lowest to highest ordering
let signers = signers.signers(); let signers = signers.signers();
@ -602,22 +599,14 @@ mod signers_container_tests {
signers.add_external(id2.clone(), SignerOrdering(2), signer2.clone()); signers.add_external(id2.clone(), SignerOrdering(2), signer2.clone());
signers.add_external(id3.clone(), SignerOrdering(3), signer3.clone()); signers.add_external(id3.clone(), SignerOrdering(3), signer3.clone());
assert!( assert!(matches!(signers.find(id1), Some(signer) if Arc::as_ptr(&signer1) == Arc::as_ptr(signer)));
matches!(signers.find(id1), Some(signer) if Arc::as_ptr(&signer1) == Arc::as_ptr(signer)) assert!(matches!(signers.find(id2), Some(signer) if Arc::as_ptr(&signer2) == Arc::as_ptr(signer)));
); assert!(matches!(signers.find(id3.clone()), Some(signer) if Arc::as_ptr(&signer3) == Arc::as_ptr(signer)));
assert!(
matches!(signers.find(id2), Some(signer) if Arc::as_ptr(&signer2) == Arc::as_ptr(signer))
);
assert!(
matches!(signers.find(id3.clone()), Some(signer) if Arc::as_ptr(&signer3) == Arc::as_ptr(signer))
);
// The `signer4` has the same ID as `signer3` but lower ordering. // The `signer4` has the same ID as `signer3` but lower ordering.
// It should be found by `id3` instead of `signer3`. // It should be found by `id3` instead of `signer3`.
signers.add_external(id3.clone(), SignerOrdering(2), signer4.clone()); signers.add_external(id3.clone(), SignerOrdering(2), signer4.clone());
assert!( assert!(matches!(signers.find(id3), Some(signer) if Arc::as_ptr(&signer4) == Arc::as_ptr(signer)));
matches!(signers.find(id3), Some(signer) if Arc::as_ptr(&signer4) == Arc::as_ptr(signer))
);
// Can't find anything with ID that doesn't exist // Can't find anything with ID that doesn't exist
assert!(matches!(signers.find(id_nonexistent), None)); assert!(matches!(signers.find(id_nonexistent), None));
@ -626,12 +615,7 @@ mod signers_container_tests {
#[derive(Debug)] #[derive(Debug)]
struct DummySigner; struct DummySigner;
impl Signer for DummySigner { impl Signer for DummySigner {
fn sign( fn sign(&self, _psbt: &mut PartiallySignedTransaction, _input_index: Option<usize>, _secp: &SecpCtx) -> Result<(), SignerError> {
&self,
_psbt: &mut PartiallySignedTransaction,
_input_index: Option<usize>,
_secp: &SecpCtx,
) -> Result<(), SignerError> {
Ok(()) Ok(())
} }