Skip to content

Commit 1e3053e

Browse files
committed
Revert "policies: move duplicate key check into separate function"
This reverts commit 34bc408 (except for typo). BIP-388 prohibits duplicate keys across all keys in a Taproot descriptor, even across the internal key and leaf scripts, so we don't need to move this into a separate function (the reason for it was so we could call it once per leaf script, which is not necessary).
1 parent 81a59e5 commit 1e3053e

File tree

1 file changed

+35
-42
lines changed

1 file changed

+35
-42
lines changed

src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs

Lines changed: 35 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -137,47 +137,6 @@ fn get_change_and_address_index<R: core::convert::AsRef<str>, T: core::iter::Ite
137137
Err(Error::InvalidInput)
138138
}
139139

140-
/// Check that it is impossible to create a derivation with duplicate pubkeys, assuming all the keys
141-
/// in the key vector are distinct.
142-
///
143-
/// Even though the rust-miniscript library checks for duplicate keys, it does so on the raw
144-
/// miniscript, which would not catch e.g. that `wsh(or_b(pk(@0/<0;1>/*),s:pk(@0/<2;1>/*)))` has a
145-
/// duplicate change derivation if we derive at the receive path.
146-
///
147-
/// Also checks that each key is used, e.g. if there are 3 keys in the key vector, @0, @1 and @2
148-
/// must be present.
149-
fn check_dups<R: core::convert::AsRef<str>, T: core::iter::Iterator<Item = R>>(
150-
pubkeys: T,
151-
keys: &[pb::KeyOriginInfo],
152-
) -> Result<(), Error> {
153-
// in "@key_index/<left;right>", keeps track of (key_index,left) and
154-
// (key_index,right) to check for duplicates.
155-
let mut derivations_seen: Vec<(usize, u32)> = Vec::new();
156-
157-
let mut keys_seen: Vec<bool> = vec![false; keys.len()];
158-
159-
for pk in pubkeys {
160-
let (key_index, multipath_index_left, multipath_index_right) =
161-
parse_wallet_policy_pk(pk.as_ref()).or(Err(Error::InvalidInput))?;
162-
163-
if derivations_seen.contains(&(key_index, multipath_index_left)) {
164-
return Err(Error::InvalidInput);
165-
}
166-
derivations_seen.push((key_index, multipath_index_left));
167-
if derivations_seen.contains(&(key_index, multipath_index_right)) {
168-
return Err(Error::InvalidInput);
169-
}
170-
derivations_seen.push((key_index, multipath_index_right));
171-
172-
*keys_seen.get_mut(key_index).ok_or(Error::InvalidInput)? = true;
173-
}
174-
175-
if !keys_seen.into_iter().all(|b| b) {
176-
return Err(Error::InvalidInput);
177-
}
178-
Ok(())
179-
}
180-
181140
struct WalletPolicyPkTranslator<'a> {
182141
keys: &'a [pb::KeyOriginInfo],
183142
is_change: bool,
@@ -244,10 +203,44 @@ pub struct ParsedPolicy<'a> {
244203
}
245204

246205
impl<'a> ParsedPolicy<'a> {
206+
/// Check that it is impossible to create a derivation with duplicate pubkeys, assuming all the
207+
/// keys in the key vector are distinct.
208+
///
209+
/// Even though the rust-miniscript library checks for duplicate keys, it does so on the raw
210+
/// miniscript, which would not catch e.g. that `wsh(or_b(pk(@0/<0;1>/*),s:pk(@0/<2;1>/*)))` has
211+
/// a duplicate change derivation if we derive at the receive path.
212+
///
213+
/// Also checks that each key is used, e.g. if there are 3 keys in the key vector, @0, @1 and @2
214+
/// must be present.
247215
fn validate_keys(&self) -> Result<(), Error> {
248216
match &self.descriptor {
249217
Descriptor::Wsh(Wsh { miniscript_expr }) => {
250-
check_dups(miniscript_expr.iter_pk(), &self.policy.keys)
218+
// in "@key_index/<left;right>", keeps track of (key_index,left) and
219+
// (key_index,right) to check for duplicates.
220+
let mut derivations_seen: Vec<(usize, u32)> = Vec::new();
221+
222+
let mut keys_seen: Vec<bool> = vec![false; self.policy.keys.len()];
223+
224+
for pk in miniscript_expr.iter_pk() {
225+
let (key_index, multipath_index_left, multipath_index_right) =
226+
parse_wallet_policy_pk(&pk).or(Err(Error::InvalidInput))?;
227+
228+
if derivations_seen.contains(&(key_index, multipath_index_left)) {
229+
return Err(Error::InvalidInput);
230+
}
231+
derivations_seen.push((key_index, multipath_index_left));
232+
if derivations_seen.contains(&(key_index, multipath_index_right)) {
233+
return Err(Error::InvalidInput);
234+
}
235+
derivations_seen.push((key_index, multipath_index_right));
236+
237+
*keys_seen.get_mut(key_index).ok_or(Error::InvalidInput)? = true;
238+
}
239+
240+
if !keys_seen.into_iter().all(|b| b) {
241+
return Err(Error::InvalidInput);
242+
}
243+
Ok(())
251244
}
252245
}
253246
}

0 commit comments

Comments
 (0)