Various improvements of Taproot signing example #34
Conversation
The Taproot signing example had some parts that were unidiomatic, weirdly worded or just plain wrong. This change fixes some of them though not all. There's still things missing such as using `witness_mut` method on `SighashCache` and possibly things I missed because I was only looking for obvious things.
| @@ -157,7 +151,7 @@ Now we are ready for our main function that will sign a transaction that spends | |||
| # } | |||
| # | |||
| # fn receivers_address() -> Address { | |||
| # Address::from_str("bc1p0dq0tzg2r780hldthn5mrznmpxsxc0jux5f20fwj0z3wqxxk6fpqm7q0va") | |||
| # "bc1p0dq0tzg2r780hldthn5mrznmpxsxc0jux5f20fwj0z3wqxxk6fpqm7q0va".parse::<Address<_>>() | |||
There was a problem hiding this comment.
Is it possible to do a straight parse::<Address<Network::Bitcoin>>? That would be less code and more ergonomic in my opinion.
There was a problem hiding this comment.
Address isn't paramatized by the network but rather by NetworkValidation. (The underscore here is inferred to be NetworkUnchecked.)
| Note that `bc1p0dq0tzg2r780hldthn5mrznmpxsxc0jux5f20fwj0z3wqxxk6fpqm7q0va` is a [Bech32](https://bitcoinops.org/en/topics/bech32/) address. | ||
| This is an arbitrary, however valid, Bitcoin mainnet address. | ||
| Hence we use the `require_network` method to ensure that the address is valid for mainnet. | ||
| Bitcoin applications are usually configured with specific Bitcoin network at the start and use that. | ||
| To prevent mistakes related to people sending satoshis to a wrong network we need to call the `require_network` method to ensure that the address is valid for the network, in our case mainnet. |
There was a problem hiding this comment.
| To prevent mistakes related to people sending satoshis to a wrong network we need to call the `require_network` method to ensure that the address is valid for the network, in our case mainnet. | |
| To prevent mistakes related to people sending satoshis to the wrong network we need to call the `require_network` method to ensure that the address is valid for the network, in our case mainnet. |
There was a problem hiding this comment.
I believe my version is correct. There are more networks than 2 so if one is correct in the context then there are multiple incorrect ones and you're only sending to one of them and it's not known which of them you're sending to so "a" makes sense. "the" would imply there's a specific wrong network but there isn't.
There was a problem hiding this comment.
Technically I think Kix is right, but I think both articles are acceptable in idiomatic English. (When learning mathematics native English speakers have to explicitly learn that "the" is an explicit claim of uniqueness and has a bunch of semantic content.)
There was a problem hiding this comment.
"a wrong network" does actually sound a little weird. I don't know if there's a clear rule for when it "sounds weird" to use an indefinite article, but it is what it is.
But Kix's text is correct so we should keep it.
There was a problem hiding this comment.
Thanks, you've convinced me its correct as is.
|
As for "has the X type" vs "is a X", I think either one is fine. Types are (approximately) sets so the relationship is that objects are members of a type. But the existing "is a X type" is definitely wrong. |
Yes, my reasoning is that |
|
All good to merge @apoelstra! |
|
Not sure what the ACK policy on this repo is, but FWIW @tcharding's doesn't count because he didn't provide any commit ID. But @storopoli did so I'll take his instead (and Tobin's soft "go ahead and merge" and Kix's implicit "I am the author" ACK). |
|
I also don't know if there's some publish step that we're supposed to take after merging stuff here. |
|
Oh woops, I could have just merged myself (after ack'ed correctly). In my mind at least its a one ack policy here. |
We have auto publish magic in this repo :) |
The Taproot signing example had some parts that were unidiomatic, weirdly worded or just plain wrong. This change fixes some of them though not all. There's still things missing such as using
witness_mutmethod onSighashCacheand possibly things I missed because I was only looking for obvious things.