Skip to content

Commit 65a004c

Browse files
apoelstraapoelstra
committed
Merge #564: Remove hashbrown dependency
49fd7b9 Remove hashbrown dependency (Tobin C. Harding) 4e174e1 Use BTreeMap in compile test code (Tobin C. Harding) eefd337 Use BTreeMap in test code (Tobin C. Harding) 13b9ec9 Use BTreeSet internally (Tobin C. Harding) 023f13b Fix stale variable names and comments (Tobin C. Harding) cef9d46 Use BTreeMap internally (Tobin C. Harding) 4e2ee86 Use BTreeMap for descriptor::KeyMap (Tobin C. Harding) 49a2bdf Add Satisfier macro for ((hash160, leaf hash), (pk, tap sig)) (Tobin C. Harding) df11203 Add Satisfier macro for (hash160, (pk, ecdsa sig)) (Tobin C. Harding) 42f559f Add Satisfier macro for ((pk, leaf hash), taproot sig) (Tobin C. Harding) 20ca6a5 Implement Satisfier for map<Pk, bitcoin::ecdsa::Signature> (Tobin C. Harding) Pull request description: We can use `BTreeSet` instead of `HashSet` and `BTreeMap` instead of `HashMap` to remove the `hashbrown` dependency. ACKs for top commit: apoelstra: ACK 49fd7b9 Tree-SHA512: 73d0e29812b91c18a10e6dd57363a7b188985c62d74500134b41879539ce2ecc6cf8d638ed27ba4986ddbe3e70f7f5bc0d3cf70491da521b5f724b818cb5334c
2 parents d1c61e4 + 49fd7b9 commit 65a004c

File tree

8 files changed

+138
-91
lines changed

8 files changed

+138
-91
lines changed

‎Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ edition = "2018"
1313
[features]
1414
default = ["std"]
1515
std = ["bitcoin/std", "bitcoin/secp-recovery"]
16-
no-std = ["hashbrown", "bitcoin/no-std"]
16+
no-std = ["bitcoin/no-std"]
1717
compiler = []
1818
trace = []
1919

@@ -23,7 +23,6 @@ base64 = ["bitcoin/base64"]
2323

2424
[dependencies]
2525
bitcoin = { version = "0.30.0", default-features = false }
26-
hashbrown = { version = "0.11", optional = true }
2726
internals = { package = "bitcoin-private", version = "0.1.0", default_features = false }
2827

2928
# Do NOT use this as a feature! Use the `serde` feature instead.

‎src/descriptor/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub use self::key::{
5757
/// [`Descriptor::parse_descriptor`], since the descriptor will always only contain
5858
/// public keys. This map allows looking up the corresponding secret key given a
5959
/// public key from the descriptor.
60-
pub type KeyMap = HashMap<DescriptorPublicKey, DescriptorSecretKey>;
60+
pub type KeyMap = BTreeMap<DescriptorPublicKey, DescriptorSecretKey>;
6161

6262
/// Script descriptor
6363
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -656,7 +656,7 @@ impl Descriptor<DescriptorPublicKey> {
656656
Ok(public_key)
657657
}
658658

659-
let mut keymap_pk = KeyMapWrapper(HashMap::new(), secp);
659+
let mut keymap_pk = KeyMapWrapper(BTreeMap::new(), secp);
660660

661661
struct KeyMapWrapper<'a, C: secp256k1::Signing>(KeyMap, &'a secp256k1::Secp256k1<C>);
662662

@@ -1501,7 +1501,7 @@ mod tests {
15011501
witness: Witness::default(),
15021502
};
15031503
let satisfier = {
1504-
let mut satisfier = HashMap::with_capacity(2);
1504+
let mut satisfier = BTreeMap::new();
15051505

15061506
satisfier.insert(
15071507
a,

‎src/lib.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,6 @@ pub use bitcoin;
100100
#[macro_use]
101101
extern crate alloc;
102102

103-
#[cfg(not(feature = "std"))]
104-
extern crate hashbrown;
105-
106103
#[cfg(any(feature = "std", test))]
107104
extern crate core;
108105

@@ -961,9 +958,6 @@ mod prelude {
961958
vec::Vec,
962959
};
963960

964-
#[cfg(all(not(feature = "std"), not(test)))]
965-
pub use hashbrown::{HashMap, HashSet};
966-
967961
#[cfg(all(not(feature = "std"), not(test)))]
968962
pub use self::mutex::Mutex;
969963
}

‎src/miniscript/analyzable.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
213213
// to have an iterator
214214
let all_pkhs_len = self.iter_pk().count();
215215

216-
let unique_pkhs_len = self.iter_pk().collect::<HashSet<_>>().len();
216+
let unique_pkhs_len = self.iter_pk().collect::<BTreeSet<_>>().len();
217217

218218
unique_pkhs_len != all_pkhs_len
219219
}

‎src/miniscript/satisfy.rs

Lines changed: 115 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -153,71 +153,130 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for absolute::LockTime {
153153
}
154154
}
155155
}
156-
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for HashMap<Pk, bitcoin::ecdsa::Signature> {
157-
fn lookup_ecdsa_sig(&self, key: &Pk) -> Option<bitcoin::ecdsa::Signature> {
158-
self.get(key).copied()
159-
}
156+
157+
macro_rules! impl_satisfier_for_map_key_to_ecdsa_sig {
158+
($(#[$($attr:meta)*])* impl Satisfier<Pk> for $map:ident<$key:ty, $val:ty>) => {
159+
$(#[$($attr)*])*
160+
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk>
161+
for $map<Pk, bitcoin::ecdsa::Signature>
162+
{
163+
fn lookup_ecdsa_sig(&self, key: &Pk) -> Option<bitcoin::ecdsa::Signature> {
164+
self.get(key).copied()
165+
}
166+
}
167+
};
160168
}
161169

162-
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk>
163-
for HashMap<(Pk, TapLeafHash), bitcoin::taproot::Signature>
164-
{
165-
fn lookup_tap_leaf_script_sig(
166-
&self,
167-
key: &Pk,
168-
h: &TapLeafHash,
169-
) -> Option<bitcoin::taproot::Signature> {
170-
// Unfortunately, there is no way to get a &(a, b) from &a and &b without allocating
171-
// If we change the signature the of lookup_tap_leaf_script_sig to accept a tuple. We would
172-
// face the same problem while satisfying PkK.
173-
// We use this signature to optimize for the psbt common use case.
174-
self.get(&(key.clone(), *h)).copied()
175-
}
170+
impl_satisfier_for_map_key_to_ecdsa_sig! {
171+
impl Satisfier<Pk> for BTreeMap<Pk, bitcoin::ecdsa::Signature>
176172
}
177173

178-
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk>
179-
for HashMap<hash160::Hash, (Pk, bitcoin::ecdsa::Signature)>
180-
where
181-
Pk: MiniscriptKey + ToPublicKey,
182-
{
183-
fn lookup_ecdsa_sig(&self, key: &Pk) -> Option<bitcoin::ecdsa::Signature> {
184-
self.get(&key.to_pubkeyhash(SigType::Ecdsa)).map(|x| x.1)
185-
}
174+
impl_satisfier_for_map_key_to_ecdsa_sig! {
175+
#[cfg(feature = "std")]
176+
impl Satisfier<Pk> for HashMap<Pk, bitcoin::ecdsa::Signature>
177+
}
186178

187-
fn lookup_raw_pkh_pk(&self, pk_hash: &hash160::Hash) -> Option<bitcoin::PublicKey> {
188-
self.get(pk_hash).map(|x| x.0.to_public_key())
189-
}
179+
macro_rules! impl_satisfier_for_map_key_hash_to_taproot_sig {
180+
($(#[$($attr:meta)*])* impl Satisfier<Pk> for $map:ident<$key:ty, $val:ty>) => {
181+
$(#[$($attr)*])*
182+
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk>
183+
for $map<(Pk, TapLeafHash), bitcoin::taproot::Signature>
184+
{
185+
fn lookup_tap_leaf_script_sig(
186+
&self,
187+
key: &Pk,
188+
h: &TapLeafHash,
189+
) -> Option<bitcoin::taproot::Signature> {
190+
// Unfortunately, there is no way to get a &(a, b) from &a and &b without allocating
191+
// If we change the signature the of lookup_tap_leaf_script_sig to accept a tuple. We would
192+
// face the same problem while satisfying PkK.
193+
// We use this signature to optimize for the psbt common use case.
194+
self.get(&(key.clone(), *h)).copied()
195+
}
196+
}
197+
};
198+
}
190199

191-
fn lookup_raw_pkh_ecdsa_sig(
192-
&self,
193-
pk_hash: &hash160::Hash,
194-
) -> Option<(bitcoin::PublicKey, bitcoin::ecdsa::Signature)> {
195-
self.get(pk_hash)
196-
.map(|&(ref pk, sig)| (pk.to_public_key(), sig))
197-
}
200+
impl_satisfier_for_map_key_hash_to_taproot_sig! {
201+
impl Satisfier<Pk> for BTreeMap<(Pk, TapLeafHash), bitcoin::taproot::Signature>
198202
}
199203

200-
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk>
201-
for HashMap<(hash160::Hash, TapLeafHash), (Pk, bitcoin::taproot::Signature)>
202-
where
203-
Pk: MiniscriptKey + ToPublicKey,
204-
{
205-
fn lookup_tap_leaf_script_sig(
206-
&self,
207-
key: &Pk,
208-
h: &TapLeafHash,
209-
) -> Option<bitcoin::taproot::Signature> {
210-
self.get(&(key.to_pubkeyhash(SigType::Schnorr), *h))
211-
.map(|x| x.1)
212-
}
204+
impl_satisfier_for_map_key_hash_to_taproot_sig! {
205+
#[cfg(feature = "std")]
206+
impl Satisfier<Pk> for HashMap<(Pk, TapLeafHash), bitcoin::taproot::Signature>
207+
}
213208

214-
fn lookup_raw_pkh_tap_leaf_script_sig(
215-
&self,
216-
pk_hash: &(hash160::Hash, TapLeafHash),
217-
) -> Option<(XOnlyPublicKey, bitcoin::taproot::Signature)> {
218-
self.get(pk_hash)
219-
.map(|&(ref pk, sig)| (pk.to_x_only_pubkey(), sig))
220-
}
209+
macro_rules! impl_satisfier_for_map_hash_to_key_ecdsa_sig {
210+
($(#[$($attr:meta)*])* impl Satisfier<Pk> for $map:ident<$key:ty, $val:ty>) => {
211+
$(#[$($attr)*])*
212+
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk>
213+
for $map<hash160::Hash, (Pk, bitcoin::ecdsa::Signature)>
214+
where
215+
Pk: MiniscriptKey + ToPublicKey,
216+
{
217+
fn lookup_ecdsa_sig(&self, key: &Pk) -> Option<bitcoin::ecdsa::Signature> {
218+
self.get(&key.to_pubkeyhash(SigType::Ecdsa)).map(|x| x.1)
219+
}
220+
221+
fn lookup_raw_pkh_pk(&self, pk_hash: &hash160::Hash) -> Option<bitcoin::PublicKey> {
222+
self.get(pk_hash).map(|x| x.0.to_public_key())
223+
}
224+
225+
fn lookup_raw_pkh_ecdsa_sig(
226+
&self,
227+
pk_hash: &hash160::Hash,
228+
) -> Option<(bitcoin::PublicKey, bitcoin::ecdsa::Signature)> {
229+
self.get(pk_hash)
230+
.map(|&(ref pk, sig)| (pk.to_public_key(), sig))
231+
}
232+
}
233+
};
234+
}
235+
236+
impl_satisfier_for_map_hash_to_key_ecdsa_sig! {
237+
impl Satisfier<Pk> for BTreeMap<hash160::Hash, (Pk, bitcoin::ecdsa::Signature)>
238+
}
239+
240+
impl_satisfier_for_map_hash_to_key_ecdsa_sig! {
241+
#[cfg(feature = "std")]
242+
impl Satisfier<Pk> for HashMap<hash160::Hash, (Pk, bitcoin::ecdsa::Signature)>
243+
}
244+
245+
macro_rules! impl_satisfier_for_map_hash_tapleafhash_to_key_taproot_sig {
246+
($(#[$($attr:meta)*])* impl Satisfier<Pk> for $map:ident<$key:ty, $val:ty>) => {
247+
$(#[$($attr)*])*
248+
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk>
249+
for $map<(hash160::Hash, TapLeafHash), (Pk, bitcoin::taproot::Signature)>
250+
where
251+
Pk: MiniscriptKey + ToPublicKey,
252+
{
253+
fn lookup_tap_leaf_script_sig(
254+
&self,
255+
key: &Pk,
256+
h: &TapLeafHash,
257+
) -> Option<bitcoin::taproot::Signature> {
258+
self.get(&(key.to_pubkeyhash(SigType::Schnorr), *h))
259+
.map(|x| x.1)
260+
}
261+
262+
fn lookup_raw_pkh_tap_leaf_script_sig(
263+
&self,
264+
pk_hash: &(hash160::Hash, TapLeafHash),
265+
) -> Option<(XOnlyPublicKey, bitcoin::taproot::Signature)> {
266+
self.get(pk_hash)
267+
.map(|&(ref pk, sig)| (pk.to_x_only_pubkey(), sig))
268+
}
269+
}
270+
};
271+
}
272+
273+
impl_satisfier_for_map_hash_tapleafhash_to_key_taproot_sig! {
274+
impl Satisfier<Pk> for BTreeMap<(hash160::Hash, TapLeafHash), (Pk, bitcoin::taproot::Signature)>
275+
}
276+
277+
impl_satisfier_for_map_hash_tapleafhash_to_key_taproot_sig! {
278+
#[cfg(feature = "std")]
279+
impl Satisfier<Pk> for HashMap<(hash160::Hash, TapLeafHash), (Pk, bitcoin::taproot::Signature)>
221280
}
222281

223282
impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &'a S {

‎src/policy/compiler.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,11 +1393,12 @@ mod tests {
13931393
};
13941394
let sigvec = bitcoinsig.to_vec();
13951395

1396-
let no_sat = HashMap::<bitcoin::PublicKey, bitcoin::ecdsa::Signature>::new();
1397-
let mut left_sat = HashMap::<bitcoin::PublicKey, bitcoin::ecdsa::Signature>::new();
1398-
let mut right_sat =
1399-
HashMap::<hashes::hash160::Hash, (bitcoin::PublicKey, bitcoin::ecdsa::Signature)>::new(
1400-
);
1396+
let no_sat = BTreeMap::<bitcoin::PublicKey, bitcoin::ecdsa::Signature>::new();
1397+
let mut left_sat = BTreeMap::<bitcoin::PublicKey, bitcoin::ecdsa::Signature>::new();
1398+
let mut right_sat = BTreeMap::<
1399+
hashes::hash160::Hash,
1400+
(bitcoin::PublicKey, bitcoin::ecdsa::Signature),
1401+
>::new();
14011402

14021403
for i in 0..5 {
14031404
left_sat.insert(keys[i], bitcoinsig);

‎src/policy/concrete.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
327327
let mut prob = 0.;
328328
let semantic_policy = self.lift()?;
329329
let concrete_keys = self.keys();
330-
let key_prob_map: HashMap<_, _> = self
330+
let key_prob_map: BTreeMap<_, _> = self
331331
.to_tapleaf_prob_vec(1.0)
332332
.into_iter()
333333
.filter(|(_, ref pol)| matches!(*pol, Concrete::Key(..)))
@@ -347,7 +347,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
347347
internal_key = Some(key.clone());
348348
}
349349
}
350-
None => return Err(errstr("Key should have existed in the HashMap!")),
350+
None => return Err(errstr("Key should have existed in the BTreeMap!")),
351351
}
352352
}
353353
}
@@ -563,7 +563,7 @@ impl<Pk: MiniscriptKey> PolicyArc<Pk> {
563563
// owing to the current [policy element enumeration algorithm][`Policy::enumerate_pol`],
564564
// two passes of the algorithm might result in same sub-policy showing up. Currently, we
565565
// merge the nodes by adding up the corresponding probabilities for the same policy.
566-
let mut pol_prob_map = HashMap::<Arc<Self>, OrdF64>::new();
566+
let mut pol_prob_map = BTreeMap::<Arc<Self>, OrdF64>::new();
567567

568568
let arc_self = Arc::new(self);
569569
tapleaf_prob_vec.insert((Reverse(OrdF64(prob)), Arc::clone(&arc_self)));
@@ -791,7 +791,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
791791
pub fn check_duplicate_keys(&self) -> Result<(), PolicyError> {
792792
let pks = self.keys();
793793
let pks_len = pks.len();
794-
let unique_pks_len = pks.into_iter().collect::<HashSet<_>>().len();
794+
let unique_pks_len = pks.into_iter().collect::<BTreeSet<_>>().len();
795795

796796
if pks_len > unique_pks_len {
797797
Err(PolicyError::DuplicatePubKeys)

‎src/psbt/finalizer.rs

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,16 @@ fn construct_tap_witness(
3636
) -> Result<Vec<Vec<u8>>, InputError> {
3737
// When miniscript tries to finalize the PSBT, it doesn't have the full descriptor (which contained a pkh() fragment)
3838
// and instead resorts to parsing the raw script sig, which is translated into a "expr_raw_pkh" internally.
39-
let mut hash_map: BTreeMap<hash160::Hash, bitcoin::key::XOnlyPublicKey> = BTreeMap::new();
39+
let mut map: BTreeMap<hash160::Hash, bitcoin::key::XOnlyPublicKey> = BTreeMap::new();
4040
let psbt_inputs = &sat.psbt.inputs;
4141
for psbt_input in psbt_inputs {
4242
// We need to satisfy or dissatisfy any given key. `tap_key_origin` is the only field of PSBT Input which consist of
4343
// all the keys added on a descriptor and thus we get keys from it.
4444
let public_keys = psbt_input.tap_key_origins.keys();
4545
for key in public_keys {
4646
let bitcoin_key = *key;
47-
// Convert PubKeyHash into Hash::hash160
4847
let hash = bitcoin_key.to_pubkeyhash(SigType::Schnorr);
49-
// Insert pair in HashMap
50-
hash_map.insert(hash, bitcoin_key);
48+
map.insert(hash, bitcoin_key);
5149
}
5250
}
5351
assert!(spk.is_v1_p2tr());
@@ -72,7 +70,7 @@ fn construct_tap_witness(
7270
script,
7371
&ExtParams::allow_all(),
7472
) {
75-
Ok(ms) => ms.substitute_raw_pkh(&hash_map),
73+
Ok(ms) => ms.substitute_raw_pkh(&map),
7674
Err(..) => continue, // try another script
7775
};
7876
let mut wit = if allow_mall {
@@ -142,19 +140,15 @@ pub(super) fn prevouts(psbt: &Psbt) -> Result<Vec<&bitcoin::TxOut>, super::Error
142140
// we want to move the script is probably already created
143141
// and we want to satisfy it in any way possible.
144142
fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, InputError> {
145-
// Create a HashMap <Hash(Pk),Pk>
146-
let mut hash_map: BTreeMap<hash160::Hash, PublicKey> = BTreeMap::new();
143+
let mut map: BTreeMap<hash160::Hash, PublicKey> = BTreeMap::new();
147144
let psbt_inputs = &psbt.inputs;
148145
for psbt_input in psbt_inputs {
149146
// Use BIP32 Derviation to get set of all possible keys.
150147
let public_keys = psbt_input.bip32_derivation.keys();
151148
for key in public_keys {
152-
// Convert bitcoin::secp256k1::PublicKey into just bitcoin::PublicKey
153149
let bitcoin_key = bitcoin::PublicKey::new(*key);
154-
// Convert PubKeyHash into Hash::hash160
155150
let hash = bitcoin_key.pubkey_hash().to_raw_hash();
156-
// Insert pair in HashMap
157-
hash_map.insert(hash, bitcoin_key);
151+
map.insert(hash, bitcoin_key);
158152
}
159153
}
160154

@@ -214,7 +208,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
214208
witness_script,
215209
&ExtParams::allow_all(),
216210
)?;
217-
Ok(Descriptor::new_wsh(ms.substitute_raw_pkh(&hash_map))?)
211+
Ok(Descriptor::new_wsh(ms.substitute_raw_pkh(&map))?)
218212
} else {
219213
Err(InputError::MissingWitnessScript)
220214
}
@@ -241,7 +235,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
241235
witness_script,
242236
&ExtParams::allow_all(),
243237
)?;
244-
Ok(Descriptor::new_sh_wsh(ms.substitute_raw_pkh(&hash_map))?)
238+
Ok(Descriptor::new_sh_wsh(ms.substitute_raw_pkh(&map))?)
245239
} else {
246240
Err(InputError::MissingWitnessScript)
247241
}
@@ -285,7 +279,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
285279
&script_pubkey,
286280
&ExtParams::allow_all(),
287281
)?;
288-
Ok(Descriptor::new_bare(ms.substitute_raw_pkh(&hash_map))?)
282+
Ok(Descriptor::new_bare(ms.substitute_raw_pkh(&map))?)
289283
}
290284
}
291285

0 commit comments

Comments
 (0)