Upgrade to bitcoin v0.32.0
#679
Conversation
tcharding
force-pushed
the
05-01-release-12.0.0
branch
from
de70b14
to
74385ba
Compare
|
CI is fixed in #678 |
tcharding
force-pushed
the
05-01-release-12.0.0
branch
4 times, most recently
from
33f3209
to
7c90cad
Compare
src/descriptor/segwitv0.rs
Outdated
| @@ -374,15 +374,23 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> { | |||
| impl<Pk: MiniscriptKey + ToPublicKey> Wpkh<Pk> { | |||
| /// Obtains the corresponding script pubkey for this descriptor. | |||
| pub fn script_pubkey(&self) -> ScriptBuf { | |||
| let addr = Address::p2wpkh(&self.pk.to_public_key(), Network::Bitcoin) | |||
| use core::convert::TryFrom; | |||
There was a problem hiding this comment.
Can't we move this up to the imports? It also repeated down a little bit in L388.
There was a problem hiding this comment.
Yes, definitely. I must have thrown this up just roughly hacked together, I tend to put use statements in functions when I'm doing quick and dirty work. I see from the PR description that I did not indicate that was the case at all, so apologies for stealing your review time.
| }); | ||
| let mut sigser = sig.serialize_der().to_vec(); | ||
| let mut sigser = signature.serialize_der().to_vec(); |
There was a problem hiding this comment.
Can't we do serialize trick here to avoid one heap allocation?
Also aren't we intending to deprecate to_vec anyways?
There was a problem hiding this comment.
I only see one allocation.
And yes, we are going to deprecate to_vec, but today it is not deprecated and there is no replacement :)
| sig: schnorr_sig, | ||
| hash_ty: bitcoin::sighash::TapSighashType::Default, | ||
| signature: schnorr_sig, | ||
| sighash_type: bitcoin::sighash::TapSighashType::Default, | ||
| }; | ||
| ser_schnorr_sigs.push(schnorr_sig.to_vec()); |
There was a problem hiding this comment.
Can't we do serialize trick here to avoid one heap allocation?
Also aren't we intending to deprecate to_vec anyways?
There was a problem hiding this comment.
Ah, yep, this serialization is unnecessary (though it's unrelated to this PR I think and can be done separately).
src/psbt/finalizer.rs
Outdated
| let addr = bitcoin::Address::p2wpkh(&pk, bitcoin::Network::Bitcoin) | ||
| .expect("Address corresponding to valid pubkey"); | ||
| *script_pubkey == addr.script_pubkey() | ||
| use core::convert::TryFrom; |
There was a problem hiding this comment.
Can't we move this up to the imports? It also repeated down a little bit in L252.
| ); | ||
| *redeem_script == addr.script_pubkey() | ||
| } | ||
| Err(_) => false, |
There was a problem hiding this comment.
Is this acceptable? I may mask some deeper problem by indicating that the key wasn't found and not indicating that there was a key but it was uncompressed?
There was a problem hiding this comment.
I also don't have good feelings about this.
What if you've matched on a specific Error?
|
Cleanup up, just needs @RCasatta to cut the new |
|
Bitcoind released |
We just released a new version of `bitcoin`, lets use it. Includes upgrading the version of: - `bitcoind` to `v0.36.0` - `secp256k1` to `0.29.0`
tcharding
force-pushed
the
05-01-release-12.0.0
branch
from
7c90cad
to
cf8c1c8
Compare
We just released a new version of
bitcoin, lets use it.Includes upgrading the version of:
bitcoindtov0.36.0secp256k1to0.29.0