Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BDK shouldn't complain if there are duplicated keys in TapLeaves #760

Closed
danielabrozzoni opened this issue Sep 24, 2022 · 0 comments · Fixed by #761
Closed

BDK shouldn't complain if there are duplicated keys in TapLeaves #760

danielabrozzoni opened this issue Sep 24, 2022 · 0 comments · Fixed by #761
Assignees
Labels
bug Something isn't working

Comments

@danielabrozzoni
Copy link
Member

Describe the bug
BDK returns an error if there are duplicated keys in different tap leaves

To Reproduce
Trying to create a wallet using the descriptor

tr(020202020202020202020202020202020202020202020202020202020202020202,{thresh(1,pk(tpubD6NzVbkrYhZ4WrjhC6iLoEYLhozBrsUjMU4poViXC57heJ8Q3EimPpoUrUKvajtkgfuPpf8Hku7X4sBazDFKqTAiSzGUaK2iCga42pcaFWe),s:pk(tpubD6NzVbkrYhZ4Y2eyqr1oFLQN1c9WUBVu9PyPxKQKnffx5wC1sumGAV9KK2zUj46oDyNCD3GHcfR9e23nFaioBfoBN9a2TtdLoSkKXw9bW1r)),and_v(v:thresh(1,pk(tpubD6NzVbkrYhZ4WrjhC6iLoEYLhozBrsUjMU4poViXC57heJ8Q3EimPpoUrUKvajtkgfuPpf8Hku7X4sBazDFKqTAiSzGUaK2iCga42pcaFWe),s:pk(tpubD6NzVbkrYhZ4Y2eyqr1oFLQN1c9WUBVu9PyPxKQKnffx5wC1sumGAV9KK2zUj46oDyNCD3GHcfR9e23nFaioBfoBN9a2TtdLoSkKXw9bW1r)),older(1))})#kgu06739

will give back Error: Descriptor(DuplicatedKeys), even tho the duplicated keys are in different tap leaves, which should be fine

The descriptor has been created like this:

    let keys = [
        "tpubD6NzVbkrYhZ4WrjhC6iLoEYLhozBrsUjMU4poViXC57heJ8Q3EimPpoUrUKvajtkgfuPpf8Hku7X4sBazDFKqTAiSzGUaK2iCga42pcaFWe",
        "tpubD6NzVbkrYhZ4Y2eyqr1oFLQN1c9WUBVu9PyPxKQKnffx5wC1sumGAV9KK2zUj46oDyNCD3GHcfR9e23nFaioBfoBN9a2TtdLoSkKXw9bW1r",
    ];

    let keys_joined: String = keys.into_iter().map(|k| format!("pk({})", k)).collect::<Vec<_>>().join(",");

    let first_policy_str = format!("thresh({},{})", keys.len() - 1, keys_joined);
    let first_policy = Concrete::<String>::from_str(&first_policy_str)?.compile()?;
    let first_tap_leaf = TapTree::Leaf(Arc::new(first_policy));

    let second_policy_str = format!("and(older(1),thresh({},{}))",keys.len() / 2, keys_joined);
    let second_policy = Concrete::<String>::from_str(&second_policy_str)?.compile()?;
    let second_tap_leaf = TapTree::Leaf(Arc::new(second_policy));

    let dummy_internal = "020202020202020202020202020202020202020202020202020202020202020202".to_string();
    let tap_tree = TapTree::Tree(Arc::new(first_tap_leaf), Arc::new(second_tap_leaf));
    let descriptor = Descriptor::new_tr(dummy_internal, Some(tap_tree))?;
    println!("{}", descriptor);

Expected behavior
The wallet is created normally

Build environment

  • BDK tag/commit: v0.22
@danielabrozzoni danielabrozzoni added the bug Something isn't working label Sep 24, 2022
@afilini afilini changed the title BDK shouldn't complain if there are different keys in TapLeaves BDK shouldn't complain if there are duplicated keys in TapLeaves Sep 24, 2022
danielabrozzoni added a commit that referenced this issue Sep 26, 2022
e2bf973 Remove redundant duplicated keys check (Alekos Filini)

Pull request description:

  ### Description

  This check is redundant since it's already performed by miniscript (see https://docs.rs/miniscript/7.0.0/miniscript/miniscript/analyzable/enum.AnalysisError.html#variant.RepeatedPubkeys) and it was incorrectly failing on tr descriptors that contain duplicated keys across different taproot leaves

  Fixes #760

  ### Changelog notice

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [x] This pull request breaks the existing API
  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  danielabrozzoni:
    Code review ACK e2bf973 - the code looks good to me, but I didn't test manually

Tree-SHA512: 3c557d741e34abf6336c7e257867f2c6f91a4be0024317af834f08ba7c0c64557bd74b643005254c9f04e953350b104b0d4d287f0d0528134b357a4adf580f87
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants