use Script::new_p2pkh for creating P2PKH scriptPubKey #8
Conversation
|
Thanks for the PR, I'll review it shortly |
theStack
force-pushed
the
use-new_p2pkh
branch
from
7c5d8f0
to
b8a827c
Compare
|
Reviewed and tested. Looks good to me. However please change the commit message. Use the body to explain what and why vs. how. I suggest something like "Replaced verbose code with call to rust-bitcoin" |
theStack
force-pushed
the
use-new_p2pkh
branch
from
b8a827c
to
eca8e65
Compare
|
Thanks for reviewing. Forced-pushed with updated commit body. |
|
I mean you should remove the text |
For creating the P2PKH output script, the following methods from rust-bitcoin are used (both available since version 0.25.1): - `bitcoin::blockdata::script::Script::new_p2kh()` - `bitcoin::util::key::PublicKey::pubkey_hash()`
theStack
force-pushed
the
use-new_p2pkh
branch
from
eca8e65
to
dccc75e
Compare
|
Ah, got it, I was reading sloppily and thought the commit message body was not detailled enough. Force-pushed a new version. Note that I also considered to use the imperative mood in the subject line (i.e. "replace" instead of "replaced") ;) |
|
Ah yes that's right, I forgot about the imperative mood. |
This is a simple refactor PR that takes advantage of
Script::new_p2pkh()from the bitcoin crate (see https://docs.rs/bitcoin/0.25.2/bitcoin/blockdata/script/struct.Script.html#method.new_p2pkh, avilable since 0.25.1) for creating a P2PKH output script rather than crafting it by hand. Also, for calculating the hash from the pubkey, it'spubkey_hash()method (https://docs.rs/bitcoin/0.25.2/bitcoin/util/key/struct.PublicKey.html#method.pubkey_hash -- available since 0.25.1) is used instead ofhashes::hash160.The implementation of
Script::new_p2pkh()actually looks quite similar to what is replaced:https://github.com/rust-bitcoin/rust-bitcoin/blob/b48f374c2cd6127aaa5e8b2ffbed80c3aa03245c/src/blockdata/script.rs#L238-L247
Note that I'm a total Rust noob, but at least the silent output of
cargo buildgives me some confidence that the change works 馃槑