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.

Sign up for GitHub

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

Jump to bottom

We should not be running pkh logic through rawpkh #483

Closed
apoelstra opened this issue Oct 27, 2022 · 2 comments · Fixed by #488
Closed

We should not be running pkh logic through rawpkh #483

apoelstra opened this issue Oct 27, 2022 · 2 comments · Fixed by #488

Comments

@apoelstra
Copy link
Member

See for example https://github.com/rust-bitcoin/rust-miniscript/blob/master/src/miniscript/satisfy.rs#L937 (not sure if there are any other examples). Here we have a public key, convert it to a hash, then look up the key again from that.

In #481 (the "planning module" PR) I am suggesting that we work with DescriptorPublicKeys rather than DefinitePublicKeys, which means that we may be working with keys where we don't know their hash160 and can't do this lookup. So in addition to being unnecessary, this roundtrip through a hash may become wrong.
@sanket1729
Copy link
Member

So in addition to being unnecessary, this roundtrip through a hash may become wrong.

I agree this is unnecessary, but why can it be wrong?

@apoelstra
Copy link
Member Author

@sanket1729 imagine we had a satisfier that simply did not have a map from hashes to public keys (because it assumes we have no raw pkhs), so the hash lookup would always return None.

@sanket1729 sanket1729 mentioned this issue Nov 5, 2022
Merged
apoelstra added a commit that referenced this issue Nov 14, 2022
@apoelstra
Merge #488: Release 9.0.0
a7bfff4 
a113e35 Release 9.0.0 (sanket1729)
63ceb5b Add psbt example for sign and finalize (sanket1729)
8f86992 Don't run rawpkh logic through pkh (sanket1729)
815fd1c Bug fix: pkh->pk lookup API (sanket1729)
43abc43 Use expr_raw_pk_h for Terminal::RawPkH (sanket1729)

Pull request description:

  - Cleanup some pkh code in satisfaction
  - Fixes #483
  - Fixes another bug dealing with dissatisfaction of thresh inside pkh
  - Adds example test vector from discussions
  - release 9.0.0 with above fixes

ACKs for top commit:
  apoelstra:
    ACK a113e35

Tree-SHA512: 373d80c5bc03032635ce0dfe6426b17970f1590012482ad3cc76b37337fe2f53e51b32e1fe11c449ec38bf4d4a0ba62d38f7316a148773e2b1854b8f9f1a877c
@jsarenik jsarenik mentioned this issue Jul 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Release 9.0.0 sanket1729/rust-miniscript
2 participants
@apoelstra @sanket1729