Skip to content

Commit 7429fda

Browse files
heap-coderheap-coder
committed
Merge rust-bitcoin/rust-miniscript#564: Remove hashbrown dependency
49fd7b9ad80064de42b2ff2ca63fe750b609d478 Remove hashbrown dependency (Tobin C. Harding) 4e174e1432f1a23d418adcc8c6cf41a4bd42ce43 Use BTreeMap in compile test code (Tobin C. Harding) eefd3377d33057d3576acc94f847993804d0722e Use BTreeMap in test code (Tobin C. Harding) 13b9ec9a0843da4a5b894d937e08d83fa0dae878 Use BTreeSet internally (Tobin C. Harding) 023f13b95576d94de9e1eef5cf4c2175973d281a Fix stale variable names and comments (Tobin C. Harding) cef9d461a2716b2362ffabf10ee99bd1bb13268f Use BTreeMap internally (Tobin C. Harding) 4e2ee866a082bb26d5977fdad5da2fdcc2fc218e Use BTreeMap for descriptor::KeyMap (Tobin C. Harding) 49a2bdfb4e93bf901f93798ab7b5175220966e66 Add Satisfier macro for ((hash160, leaf hash), (pk, tap sig)) (Tobin C. Harding) df11203e4f01e7ca90d3b1f3c4bef04ce802588b Add Satisfier macro for (hash160, (pk, ecdsa sig)) (Tobin C. Harding) 42f559f9613c978ca413188378c36619ba24e01f Add Satisfier macro for ((pk, leaf hash), taproot sig) (Tobin C. Harding) 20ca6a5d6f2760d27432d27194bf0af27841235c 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 49fd7b9ad80064de42b2ff2ca63fe750b609d478 Tree-SHA512: 73d0e29812b91c18a10e6dd57363a7b188985c62d74500134b41879539ce2ecc6cf8d638ed27ba4986ddbe3e70f7f5bc0d3cf70491da521b5f724b818cb5334c
2 parents 26380fc + f18be03 commit 7429fda

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)