Clean up the examples #387
Conversation
In an effort to make the example more clear refactor the main function into two separate functions. This is an example not a test; remove the assertion on creating the address with xpubs passed in the opposite order, instead include a comment explaining as such.
In an effort to make the example more clear; refactor the `verify_tx` example, improving docs and adding separation between setup, and each example section of the main function. Refactor only, no logic changes.
In an effort to make the example more clear; refactor the `sign_multisig` example. Refactor only, no logic changes.
In an effort to make the example more clear; refactor the `parse` example. Refactor only, no logic changes.
In an effort to make the example more clear; refactor the `htlc` example. Refactor only, no logic changes.
| // `bitcoin::PublicKey` from the `MiniscriptKey` which can be supplied by | ||
| // the `to_pk_ctx` parameter. For example, when calculating the script | ||
| // pubkey of a descriptor with xpubs, the secp context and child information | ||
| // maybe required. |
There was a problem hiding this comment.
Ouch, we no longer have to_pk_ctx in this codebase. This comment should have been removed. Not a part of this PR.
| // Or they contain a combination of timelock and heightlock. | ||
| // Check whether the descriptor is safe. This checks whether all spend paths are accessible in | ||
| // the Bitcoin network. It may be possible that some of the spend paths require more than 100 | ||
| // elements in Wsh scripts or they contain a combination of timelock and heightlock. | ||
| assert!(htlc_descriptor.sanity_check().is_ok()); |
There was a problem hiding this comment.
I would really like to get rid of these sanity checks. I think we lose of using rust if users have to call sanity check to double-check things. This feels like CPubKey.isVliad() type stuff
There was a problem hiding this comment.
Yeah, I think they don't belong in the example. They'd be useful for users who are manually constructing the Descriptor object I think.
There was a problem hiding this comment.
Should we be doing the sanity checks in all the constructors (incl. FromStr) instead?
There was a problem hiding this comment.
@tcharding yes, but we can't do it in the "implicit" constructor where the user just constructs the enum type themselves.
In an effort to make the examples more digestible to newcomers do various clean ups and refactorings. Each example is done as a separate patch to assist review.
The diffs get smaller as you go down the list :)