Mark Miniscript with duplicate keys as insane #116
Conversation
bitcoin/script/miniscript.h
Outdated
| //! Returns true if the node contains at least one duplicate key. | ||
| bool ContainsDuplicateKey() const { | ||
| auto upfn = [](const Node& node, Span<std::set<Key>> subs) -> std::optional<std::set<Key>> { | ||
| size_t keys_count = node.keys.size(); |
There was a problem hiding this comment.
Perhaps add if (node.duplicate_key) return {};. That way no work is done in recomputing these sets all the time if we already know there is a duplicate.
There was a problem hiding this comment.
Hmm but the boolean would be uninitialized for the current node. Isn't its value undefined?
There was a problem hiding this comment.
Ah, yes, that's a bad idea. For the current node, we obviously don't know yet. But for any child nodes we do.
So what about if (&node != this && node.duplicate_key) return {};.
There was a problem hiding this comment.
So there isn't any point in actually computing the set for child, right? We just use the duplicate_key boolean: if one messed up with the keys after the Node was created they were really looking for troubles..
darosior
force-pushed
the
duplicate_keys
branch
2 times, most recently
from
6518ee3 to
d10a9ca
Compare
|
I don't follow.
It's because i didn't plug my brain before commenting, sorry about that.
I'm away and back on Sunday, i'll push your optimization then. (If you need it before i think you have access to push to my PRs.)
|
It's ok. I just made coffee without putting a cup in the machine.
No rush. |
darosior
force-pushed
the
duplicate_keys
branch
from
d10a9ca to
e983e05
Compare
As stated on the website, duplicate keys make it hard to reason about malleability as a single signature may unlock multiple paths.
darosior
force-pushed
the
duplicate_keys
branch
from
e983e05 to
13edbef
Compare
|
Rebased on #114 |
|
utACK 13edbef |
2 participants
Based off a rebased #114