Merge rust-bitcoin/rust-bitcoin#925: Require taproot tree depth argument always to be u8

e3f173e521 Require taproot tree depth argument always to be u8 (Dr Maxim Orlovsky)

Pull request description:

  There is an inconsistency in depth type: some places uses `u8` (which is reasonable, since the depth can't be >127), others use `usize`. This PR makes API consistent.

  I think this should be a RC fix, since it affects newly introduced API in this release and it will be bad to make a breaking changes the next version after its introduction.

ACKs for top commit:
  tcharding:
    Hey @sanket1729, I know you pointed me already to `bitcoin-maintainer-tools` for acking PRs but I'm not finding how to do it. If you get a sec can you tell me like I'm 5 please. I used `gh pr comment 925 --body "ACK e3f173e"` to ack this but it does not get picked up as an 'approve'. Thanks
  sanket1729:
    ACK e3f173e521

Tree-SHA512: 58797e5f0e295310a9fa409d1d497711e40d95a5ef8424d7ad4206d6ba00b89d9f981600293245282d5acf948d58674d97cd24ace8f0228ffcb62989f1464350
This commit is contained in:
sanket1729 2022-04-01 08:50:28 -07:00
commit c22f40f9f1
No known key found for this signature in database
GPG Key ID: 648FFB183E0870A2
2 changed files with 13 additions and 13 deletions

View File

@ -352,7 +352,7 @@ impl Deserialize for TapTree {
let leaf_version = LeafVersion::from_consensus(*version) let leaf_version = LeafVersion::from_consensus(*version)
.map_err(|_| encode::Error::ParseFailed("Leaf Version Error"))?; .map_err(|_| encode::Error::ParseFailed("Leaf Version Error"))?;
builder = builder.add_leaf_with_ver(usize::from(*depth), script, leaf_version) builder = builder.add_leaf_with_ver(*depth, script, leaf_version)
.map_err(|_| encode::Error::ParseFailed("Tree not in DFS order"))?; .map_err(|_| encode::Error::ParseFailed("Tree not in DFS order"))?;
} }
if builder.is_complete() { if builder.is_complete() {

View File

@ -423,7 +423,7 @@ impl TaprootBuilder {
/// are not provided in DFS walk order. The depth of the root node is 0. /// are not provided in DFS walk order. The depth of the root node is 0.
pub fn add_leaf_with_ver( pub fn add_leaf_with_ver(
self, self,
depth: usize, depth: u8,
script: Script, script: Script,
ver: LeafVersion, ver: LeafVersion,
) -> Result<Self, TaprootBuilderError> { ) -> Result<Self, TaprootBuilderError> {
@ -435,13 +435,13 @@ impl TaprootBuilder {
/// leaves are not provided in DFS walk order. The depth of the root node is 0. /// leaves are not provided in DFS walk order. The depth of the root node is 0.
/// ///
/// See [`TaprootBuilder::add_leaf_with_ver`] for adding a leaf with specific version. /// See [`TaprootBuilder::add_leaf_with_ver`] for adding a leaf with specific version.
pub fn add_leaf(self, depth: usize, script: Script) -> Result<Self, TaprootBuilderError> { pub fn add_leaf(self, depth: u8, script: Script) -> Result<Self, TaprootBuilderError> {
self.add_leaf_with_ver(depth, script, LeafVersion::TapScript) self.add_leaf_with_ver(depth, script, LeafVersion::TapScript)
} }
/// Adds a hidden/omitted node at `depth` to the builder. Errors if the leaves are not provided /// Adds a hidden/omitted node at `depth` to the builder. Errors if the leaves are not provided
/// in DFS walk order. The depth of the root node is 0. /// in DFS walk order. The depth of the root node is 0.
pub fn add_hidden(self, depth: usize, hash: sha256::Hash) -> Result<Self, TaprootBuilderError> { pub fn add_hidden(self, depth: u8, hash: sha256::Hash) -> Result<Self, TaprootBuilderError> {
let node = NodeInfo::new_hidden(hash); let node = NodeInfo::new_hidden(hash);
self.insert(node, depth) self.insert(node, depth)
} }
@ -473,20 +473,20 @@ impl TaprootBuilder {
} }
/// Inserts a leaf at `depth`. /// Inserts a leaf at `depth`.
fn insert(mut self, mut node: NodeInfo, mut depth: usize) -> Result<Self, TaprootBuilderError> { fn insert(mut self, mut node: NodeInfo, mut depth: u8) -> Result<Self, TaprootBuilderError> {
// early error on invalid depth. Though this will be checked later // early error on invalid depth. Though this will be checked later
// while constructing TaprootMerkelBranch // while constructing TaprootMerkelBranch
if depth > TAPROOT_CONTROL_MAX_NODE_COUNT { if depth as usize > TAPROOT_CONTROL_MAX_NODE_COUNT {
return Err(TaprootBuilderError::InvalidMerkleTreeDepth(depth)); return Err(TaprootBuilderError::InvalidMerkleTreeDepth(depth as usize));
} }
// We cannot insert a leaf at a lower depth while a deeper branch is unfinished. Doing // We cannot insert a leaf at a lower depth while a deeper branch is unfinished. Doing
// so would mean the add_leaf/add_hidden invocations do not correspond to a DFS traversal of a // so would mean the add_leaf/add_hidden invocations do not correspond to a DFS traversal of a
// binary tree. // binary tree.
if depth + 1 < self.branch.len() { if depth as usize + 1 < self.branch.len() {
return Err(TaprootBuilderError::NodeNotInDfsOrder); return Err(TaprootBuilderError::NodeNotInDfsOrder);
} }
while self.branch.len() == depth + 1 { while self.branch.len() == depth as usize + 1 {
let child = match self.branch.pop() { let child = match self.branch.pop() {
None => unreachable!("Len of branch checked to be >= 1"), None => unreachable!("Len of branch checked to be >= 1"),
Some(Some(child)) => child, Some(Some(child)) => child,
@ -507,14 +507,14 @@ impl TaprootBuilder {
depth -= 1; depth -= 1;
} }
if self.branch.len() < depth + 1 { if self.branch.len() < depth as usize + 1 {
// add enough nodes so that we can insert node at depth `depth` // add enough nodes so that we can insert node at depth `depth`
let num_extra_nodes = depth + 1 - self.branch.len(); let num_extra_nodes = depth as usize + 1 - self.branch.len();
self.branch self.branch
.extend((0..num_extra_nodes).into_iter().map(|_| None)); .extend((0..num_extra_nodes).into_iter().map(|_| None));
} }
// Push the last node to the branch // Push the last node to the branch
self.branch[depth] = Some(node); self.branch[depth as usize] = Some(node);
Ok(self) Ok(self)
} }
} }
@ -1270,7 +1270,7 @@ mod test {
v: &serde_json::Value, v: &serde_json::Value,
mut builder: TaprootBuilder, mut builder: TaprootBuilder,
leaves: &mut Vec<(Script, LeafVersion)>, leaves: &mut Vec<(Script, LeafVersion)>,
depth: usize, depth: u8,
) -> TaprootBuilder { ) -> TaprootBuilder {
if v.is_null() { if v.is_null() {
// nothing to push // nothing to push