b29ff9b715 Rename SchnorrSighashType::from_u8 -> from_consensus_u8 (Tobin C. Harding)
af16286679 Implement TryFrom sha256::Hash for TaprootMerkleBranch (Tobin C. Harding)
6b7b440cff Implement TryFrom<Key> for ProprietaryKey (Tobin C. Harding)
5c49fe775f Implement TryFrom<TaprootBuilder> for TapTree (Tobin C. Harding)
632a5db8d9 Implement TryFrom for WitnessVersion (Tobin C. Harding)

Pull request description:

  Audit the whole codebase checking for any method that is of the form `from_foo` where foo is not an interesting identifier (like 'consensus' and 'standard'). Implement `TryFrom` for any such methods, deprecating the original.

  Done as separate patches so any can be easily dropped if not liked.

ACKs for top commit:
  apoelstra:
    ACK b29ff9b715
  Kixunil:
    ACK b29ff9b715

Tree-SHA512: 40f1d96b505891080df1f7a9b3507979b0279a9e0f9d7cd32598bdc16c866785e6b13d5cb1face5ba50e3bc8484a5cd9c7f430d7abc86db9609962476dacd467
This commit is contained in:
Andrew Poelstra 2022-06-29 19:27:23 +00:00
commit 022ab0b069
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
10 changed files with 224 additions and 75 deletions

View File

@ -27,6 +27,7 @@ use crate::consensus::encode::MAX_VEC_SIZE;
use crate::prelude::*;
use crate::io;
use core::convert::TryFrom;
use core::{fmt, default::Default};
use core::ops::Index;
@ -498,7 +499,7 @@ impl Script {
/// Returns witness version of the script, if any, assuming the script is a `scriptPubkey`.
#[inline]
pub fn witness_version(&self) -> Option<WitnessVersion> {
self.0.get(0).and_then(|opcode| WitnessVersion::from_opcode(opcodes::All::from(*opcode)).ok())
self.0.get(0).and_then(|opcode| WitnessVersion::try_from(opcodes::All::from(*opcode)).ok())
}
/// Checks whether a script pubkey is a P2SH output.
@ -550,7 +551,7 @@ impl Script {
}
let ver_opcode = opcodes::All::from(self.0[0]); // Version 0 or PUSHNUM_1-PUSHNUM_16
let push_opbyte = self.0[1]; // Second byte push opcode 2-40 bytes
WitnessVersion::from_opcode(ver_opcode).is_ok()
WitnessVersion::try_from(ver_opcode).is_ok()
&& push_opbyte >= opcodes::all::OP_PUSHBYTES_2.to_u8()
&& push_opbyte <= opcodes::all::OP_PUSHBYTES_40.to_u8()
// Check that the rest of the script has the correct size

View File

@ -34,6 +34,7 @@
use crate::prelude::*;
use core::convert::TryFrom;
use core::fmt;
use core::num::ParseIntError;
use core::str::FromStr;
@ -243,8 +244,8 @@ impl FromStr for WitnessVersion {
type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let version = s.parse().map_err(Error::UnparsableWitnessVersion)?;
WitnessVersion::from_num(version)
let version: u8 = s.parse().map_err(Error::UnparsableWitnessVersion)?;
WitnessVersion::try_from(version)
}
}
@ -258,8 +259,9 @@ impl WitnessVersion {
/// # Errors
/// If the integer does not correspond to any witness version, errors with
/// [`Error::InvalidWitnessVersion`].
#[deprecated(since = "0.29.0", note = "use try_from instead")]
pub fn from_u5(value: ::bech32::u5) -> Result<Self, Error> {
WitnessVersion::from_num(value.to_u8())
Self::try_from(value)
}
/// Converts an 8-bit unsigned integer value into [`WitnessVersion`] variant.
@ -270,27 +272,9 @@ impl WitnessVersion {
/// # Errors
/// If the integer does not correspond to any witness version, errors with
/// [`Error::InvalidWitnessVersion`].
#[deprecated(since = "0.29.0", note = "use try_from instead")]
pub fn from_num(no: u8) -> Result<Self, Error> {
Ok(match no {
0 => WitnessVersion::V0,
1 => WitnessVersion::V1,
2 => WitnessVersion::V2,
3 => WitnessVersion::V3,
4 => WitnessVersion::V4,
5 => WitnessVersion::V5,
6 => WitnessVersion::V6,
7 => WitnessVersion::V7,
8 => WitnessVersion::V8,
9 => WitnessVersion::V9,
10 => WitnessVersion::V10,
11 => WitnessVersion::V11,
12 => WitnessVersion::V12,
13 => WitnessVersion::V13,
14 => WitnessVersion::V14,
15 => WitnessVersion::V15,
16 => WitnessVersion::V16,
wrong => return Err(Error::InvalidWitnessVersion(wrong)),
})
Self::try_from(no)
}
/// Converts bitcoin script opcode into [`WitnessVersion`] variant.
@ -301,13 +285,9 @@ impl WitnessVersion {
/// # Errors
/// If the opcode does not correspond to any witness version, errors with
/// [`Error::MalformedWitnessVersion`].
#[deprecated(since = "0.29.0", note = "use try_from instead")]
pub fn from_opcode(opcode: opcodes::All) -> Result<Self, Error> {
match opcode.to_u8() {
0 => Ok(WitnessVersion::V0),
version if version >= opcodes::all::OP_PUSHNUM_1.to_u8() && version <= opcodes::all::OP_PUSHNUM_16.to_u8() =>
WitnessVersion::from_num(version - opcodes::all::OP_PUSHNUM_1.to_u8() + 1),
_ => Err(Error::MalformedWitnessVersion)
}
Self::try_from(opcode)
}
/// Converts bitcoin script [`Instruction`] (parsed opcode) into [`WitnessVersion`] variant.
@ -319,12 +299,9 @@ impl WitnessVersion {
/// # Errors
/// If the opcode does not correspond to any witness version, errors with
/// [`Error::MalformedWitnessVersion`] for the rest of opcodes.
#[deprecated(since = "0.29.0", note = "use try_from instead")]
pub fn from_instruction(instruction: Instruction) -> Result<Self, Error> {
match instruction {
Instruction::Op(op) => WitnessVersion::from_opcode(op),
Instruction::PushBytes(bytes) if bytes.is_empty() => Ok(WitnessVersion::V0),
Instruction::PushBytes(_) => Err(Error::MalformedWitnessVersion),
}
Self::try_from(instruction)
}
/// Returns integer version number representation for a given [`WitnessVersion`] value.
@ -355,6 +332,102 @@ impl WitnessVersion {
}
}
impl TryFrom<bech32::u5> for WitnessVersion {
type Error = Error;
/// Converts 5-bit unsigned integer value matching single symbol from Bech32(m) address encoding
/// ([`bech32::u5`]) into [`WitnessVersion`] variant.
///
/// # Returns
/// Version of the Witness program.
///
/// # Errors
/// If the integer does not correspond to any witness version, errors with
/// [`Error::InvalidWitnessVersion`].
fn try_from(value: bech32::u5) -> Result<Self, Self::Error> {
Self::try_from(value.to_u8())
}
}
impl TryFrom<u8> for WitnessVersion {
type Error = Error;
/// Converts an 8-bit unsigned integer value into [`WitnessVersion`] variant.
///
/// # Returns
/// Version of the Witness program.
///
/// # Errors
/// If the integer does not correspond to any witness version, errors with
/// [`Error::InvalidWitnessVersion`].
fn try_from(no: u8) -> Result<Self, Self::Error> {
use WitnessVersion::*;
Ok(match no {
0 => V0,
1 => V1,
2 => V2,
3 => V3,
4 => V4,
5 => V5,
6 => V6,
7 => V7,
8 => V8,
9 => V9,
10 => V10,
11 => V11,
12 => V12,
13 => V13,
14 => V14,
15 => V15,
16 => V16,
wrong => return Err(Error::InvalidWitnessVersion(wrong)),
})
}
}
impl TryFrom<opcodes::All> for WitnessVersion {
type Error = Error;
/// Converts bitcoin script opcode into [`WitnessVersion`] variant.
///
/// # Returns
/// Version of the Witness program (for opcodes in range of `OP_0`..`OP_16`).
///
/// # Errors
/// If the opcode does not correspond to any witness version, errors with
/// [`Error::MalformedWitnessVersion`].
fn try_from(opcode: opcodes::All) -> Result<Self, Self::Error> {
match opcode.to_u8() {
0 => Ok(WitnessVersion::V0),
version if version >= opcodes::all::OP_PUSHNUM_1.to_u8() && version <= opcodes::all::OP_PUSHNUM_16.to_u8() =>
WitnessVersion::try_from(version - opcodes::all::OP_PUSHNUM_1.to_u8() + 1),
_ => Err(Error::MalformedWitnessVersion)
}
}
}
impl<'a> TryFrom<Instruction<'a>> for WitnessVersion {
type Error = Error;
/// Converts bitcoin script [`Instruction`] (parsed opcode) into [`WitnessVersion`] variant.
///
/// # Returns
/// Version of the Witness program for [`Instruction::Op`] and [`Instruction::PushBytes`] with
/// byte value within `1..=16` range.
///
/// # Errors
/// If the opcode does not correspond to any witness version, errors with
/// [`Error::MalformedWitnessVersion`] for the rest of opcodes.
fn try_from(instruction: Instruction) -> Result<Self, Self::Error> {
match instruction {
Instruction::Op(op) => WitnessVersion::try_from(op),
Instruction::PushBytes(bytes) if bytes.is_empty() => Ok(WitnessVersion::V0),
Instruction::PushBytes(_) => Err(Error::MalformedWitnessVersion),
}
}
}
impl From<WitnessVersion> for ::bech32::u5 {
/// Converts [`WitnessVersion`] instance into corresponding Bech32(m) u5-value ([`bech32::u5`]).
fn from(version: WitnessVersion) -> Self {
@ -405,7 +478,7 @@ impl Payload {
}
Payload::WitnessProgram {
version: WitnessVersion::from_opcode(opcodes::All::from(script[0]))?,
version: WitnessVersion::try_from(opcodes::All::from(script[0]))?,
program: script[2..].to_vec(),
}
} else {
@ -856,7 +929,7 @@ impl FromStr for Address {
// Get the script version and program (converted from 5-bit to 8-bit)
let (version, program): (WitnessVersion, Vec<u8>) = {
let (v, p5) = payload.split_at(1);
(WitnessVersion::from_u5(v[0])?, bech32::FromBase32::from_base32(p5)?)
(WitnessVersion::try_from(v[0])?, bech32::FromBase32::from_base32(p5)?)
};
if program.len() < 2 || program.len() > 40 {
@ -1277,7 +1350,7 @@ mod tests {
];
let segwit_payload = (0..=16).map(|version| {
Payload::WitnessProgram {
version: WitnessVersion::from_num(version).unwrap(),
version: WitnessVersion::try_from(version).unwrap(),
program: vec![]
}
}).collect::<Vec<_>>();

View File

@ -12,6 +12,8 @@
// If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
//
use core::convert::TryFrom;
use crate::prelude::*;
use crate::io::{self, Cursor, Read};
@ -191,7 +193,7 @@ impl PartiallySignedTransaction {
return Err(Error::InvalidKey(pair.key).into())
}
}
PSBT_GLOBAL_PROPRIETARY => match proprietary.entry(raw::ProprietaryKey::from_key(pair.key.clone())?) {
PSBT_GLOBAL_PROPRIETARY => match proprietary.entry(raw::ProprietaryKey::try_from(pair.key.clone())?) {
btree_map::Entry::Vacant(empty_key) => {
empty_key.insert(pair.value);
},

View File

@ -16,6 +16,7 @@ use crate::prelude::*;
use crate::io;
use core::fmt;
use core::str::FromStr;
use core::convert::TryFrom;
use secp256k1;
use crate::blockdata::script::Script;
@ -215,7 +216,7 @@ impl PsbtSighashType {
if self.inner > 0xffu32 {
Err(sighash::Error::InvalidSighashType(self.inner))
} else {
SchnorrSighashType::from_u8(self.inner as u8)
SchnorrSighashType::from_consensus_u8(self.inner as u8)
}
}
@ -356,7 +357,7 @@ impl Input {
}
}
PSBT_IN_PROPRIETARY => {
let key = raw::ProprietaryKey::from_key(raw_key.clone())?;
let key = raw::ProprietaryKey::try_from(raw_key.clone())?;
match self.proprietary.entry(key) {
btree_map::Entry::Vacant(empty_key) => {
empty_key.insert(raw_value);

View File

@ -14,6 +14,7 @@
use crate::prelude::*;
use core;
use core::convert::TryFrom;
use crate::io;
@ -158,16 +159,12 @@ impl TapTree {
/// # Returns
/// A [`TapTree`] iff the `builder` is complete, otherwise return [`IncompleteTapTree`]
/// error with the content of incomplete `builder` instance.
#[deprecated(since = "0.29.0", note = "use try_from instead")]
pub fn from_builder(builder: TaprootBuilder) -> Result<Self, IncompleteTapTree> {
if !builder.is_finalized() {
Err(IncompleteTapTree::NotFinalized(builder))
} else if builder.has_hidden_nodes() {
Err(IncompleteTapTree::HiddenParts(builder))
} else {
Ok(TapTree(builder))
}
Self::try_from(builder)
}
/// Converts self into builder [`TaprootBuilder`]. The builder is guaranteed to be finalized.
pub fn into_builder(self) -> TaprootBuilder {
self.0
@ -194,6 +191,25 @@ impl TapTree {
}
}
impl TryFrom<TaprootBuilder> for TapTree {
type Error = IncompleteTapTree;
/// Constructs [`TapTree`] from a [`TaprootBuilder`] if it is complete binary tree.
///
/// # Returns
/// A [`TapTree`] iff the `builder` is complete, otherwise return [`IncompleteTapTree`]
/// error with the content of incomplete `builder` instance.
fn try_from(builder: TaprootBuilder) -> Result<Self, Self::Error> {
if !builder.is_finalized() {
Err(IncompleteTapTree::NotFinalized(builder))
} else if builder.has_hidden_nodes() {
Err(IncompleteTapTree::HiddenParts(builder))
} else {
Ok(TapTree(builder))
}
}
}
/// Iterator for a taproot script tree, operating in DFS order over leaf depth and
/// leaf script pairs.
pub struct TapTreeIter<'tree> {
@ -233,7 +249,7 @@ impl Output {
}
}
PSBT_OUT_PROPRIETARY => {
let key = raw::ProprietaryKey::from_key(raw_key.clone())?;
let key = raw::ProprietaryKey::try_from(raw_key.clone())?;
match self.proprietary.entry(key) {
btree_map::Entry::Vacant(empty_key) => {
empty_key.insert(raw_value);

View File

@ -20,6 +20,7 @@
use crate::prelude::*;
use core::fmt;
use core::convert::TryFrom;
use crate::io;
use crate::consensus::encode::{self, ReadExt, WriteExt, Decodable, Encodable, VarInt, serialize, deserialize, MAX_VEC_SIZE};
@ -160,12 +161,9 @@ impl<Subtype> Decodable for ProprietaryKey<Subtype> where Subtype: Copy + From<u
impl<Subtype> ProprietaryKey<Subtype> where Subtype: Copy + From<u8> + Into<u8> {
/// Constructs [ProprietaryKey] from [Key]; returns
/// [Error::InvalidProprietaryKey] if `key` do not starts with 0xFC byte
#[deprecated(since = "0.29.0", note = "use try_from instead")]
pub fn from_key(key: Key) -> Result<Self, Error> {
if key.type_value != 0xFC {
return Err(Error::InvalidProprietaryKey)
}
Ok(deserialize(&key.key)?)
Self::try_from(key)
}
/// Constructs full [Key] corresponding to this proprietary key type
@ -176,3 +174,21 @@ impl<Subtype> ProprietaryKey<Subtype> where Subtype: Copy + From<u8> + Into<u8>
}
}
}
impl<Subtype> TryFrom<Key> for ProprietaryKey<Subtype>
where
Subtype:Copy + From<u8> + Into<u8> {
type Error = Error;
/// Constructs a [`ProprietaryKey`] from a [`Key`].
///
/// # Errors
/// Returns [`Error::InvalidProprietaryKey`] if `key` does not start with `0xFC` byte.
fn try_from(key: Key) -> Result<Self, Self::Error> {
if key.type_value != 0xFC {
return Err(Error::InvalidProprietaryKey)
}
Ok(deserialize(&key.key)?)
}
}

View File

@ -370,6 +370,8 @@ fn key_source_len(key_source: &KeySource) -> usize {
#[cfg(test)]
mod tests {
use core::convert::TryFrom;
use crate::hashes::hex::FromHex;
use super::*;
@ -393,14 +395,14 @@ mod tests {
let mut builder = compose_taproot_builder(0x51, &[2, 2, 2]);
builder = builder.add_leaf_with_ver(3, Script::from_hex("b9").unwrap(), LeafVersion::from_consensus(0xC2).unwrap()).unwrap();
builder = builder.add_hidden_node(3, sha256::Hash::default()).unwrap();
assert!(TapTree::from_builder(builder).is_err());
assert!(TapTree::try_from(builder).is_err());
}
#[test]
fn taptree_roundtrip() {
let mut builder = compose_taproot_builder(0x51, &[2, 2, 2, 3]);
builder = builder.add_leaf_with_ver(3, Script::from_hex("b9").unwrap(), LeafVersion::from_consensus(0xC2).unwrap()).unwrap();
let tree = TapTree::from_builder(builder).unwrap();
let tree = TapTree::try_from(builder).unwrap();
let tree_prime = TapTree::deserialize(&tree.serialize()).unwrap();
assert_eq!(tree, tree_prime);
}

View File

@ -18,6 +18,7 @@
//!
use core::fmt;
use crate::prelude::*;
use secp256k1::{self, Secp256k1, Verification, constants};
@ -238,7 +239,7 @@ impl SchnorrSig {
},
65 => {
let (hash_ty, sig) = sl.split_last().expect("Slice len checked == 65");
let hash_ty = SchnorrSighashType::from_u8(*hash_ty)
let hash_ty = SchnorrSighashType::from_consensus_u8(*hash_ty)
.map_err(|_| SchnorrSigError::InvalidSighashType(*hash_ty))?;
let sig = secp256k1::schnorr::Signature::from_slice(sig)
.map_err(SchnorrSigError::Secp256k1)?;

View File

@ -338,17 +338,25 @@ impl SchnorrSighashType {
}
/// Creates a [`SchnorrSighashType`] from raw `u8`.
#[deprecated(since = "0.29.0", note = "use from_consensus_u8 instead")]
pub fn from_u8(hash_ty: u8) -> Result<Self, Error> {
match hash_ty {
0x00 => Ok(SchnorrSighashType::Default),
0x01 => Ok(SchnorrSighashType::All),
0x02 => Ok(SchnorrSighashType::None),
0x03 => Ok(SchnorrSighashType::Single),
0x81 => Ok(SchnorrSighashType::AllPlusAnyoneCanPay),
0x82 => Ok(SchnorrSighashType::NonePlusAnyoneCanPay),
0x83 => Ok(SchnorrSighashType::SinglePlusAnyoneCanPay),
x => Err(Error::InvalidSighashType(x as u32)),
Self::from_consensus_u8(hash_ty)
}
/// Constructs a [`SchnorrSighashType`] from a raw `u8`.
pub fn from_consensus_u8(hash_ty: u8) -> Result<Self, Error> {
use SchnorrSighashType::*;
Ok(match hash_ty {
0x00 => Default,
0x01 => All,
0x02 => None,
0x03 => Single,
0x81 => AllPlusAnyoneCanPay,
0x82 => NonePlusAnyoneCanPay,
0x83 => SinglePlusAnyoneCanPay,
x => return Err(Error::InvalidSighashType(x as u32)),
})
}
}
@ -1113,7 +1121,7 @@ mod tests {
} else {
Some(hex_hash!(TapBranchHash, inp["given"]["merkleRoot"].as_str().unwrap()))
};
let hash_ty = SchnorrSighashType::from_u8(inp["given"]["hashType"].as_u64().unwrap() as u8).unwrap();
let hash_ty = SchnorrSighashType::from_consensus_u8(inp["given"]["hashType"].as_u64().unwrap() as u8).unwrap();
let expected_internal_pk = hex_hash!(XOnlyPublicKey, inp["intermediary"]["internalPubkey"].as_str().unwrap());
let expected_tweak = hex_hash!(TapTweakHash, inp["intermediary"]["tweak"].as_str().unwrap());
@ -1124,7 +1132,7 @@ mod tests {
let (expected_key_spend_sig, expected_hash_ty) = if sig_str.len() == 128 {
(secp256k1::schnorr::Signature::from_str(sig_str).unwrap(), SchnorrSighashType::Default)
} else {
let hash_ty = SchnorrSighashType::from_u8(Vec::<u8>::from_hex(&sig_str[128..]).unwrap()[0]).unwrap();
let hash_ty = SchnorrSighashType::from_consensus_u8(Vec::<u8>::from_hex(&sig_str[128..]).unwrap()[0]).unwrap();
(secp256k1::schnorr::Signature::from_str(&sig_str[..128]).unwrap(), hash_ty)
};

View File

@ -20,6 +20,7 @@ use crate::prelude::*;
use crate::io;
use secp256k1::{self, Secp256k1};
use core::convert::TryFrom;
use core::fmt;
use core::cmp::Reverse;
@ -672,6 +673,18 @@ impl TaprootMerkleBranch {
}
}
/// Creates a merkle proof from list of hashes.
///
/// # Errors
/// If inner proof length is more than [`TAPROOT_CONTROL_MAX_NODE_COUNT`] (128).
fn from_collection<T: AsRef<[sha256::Hash]> + Into<Vec<sha256::Hash>>>(collection: T) -> Result<Self, TaprootError> {
if collection.as_ref().len() > TAPROOT_CONTROL_MAX_NODE_COUNT {
Err(TaprootError::InvalidMerkleTreeDepth(collection.as_ref().len()))
} else {
Ok(TaprootMerkleBranch(collection.into()))
}
}
/// Serializes to a writer.
///
/// # Returns
@ -704,12 +717,9 @@ impl TaprootMerkleBranch {
/// # Errors
///
/// If inner proof length is more than [`TAPROOT_CONTROL_MAX_NODE_COUNT`] (128).
#[deprecated(since = "0.29.0", note = "use try_from instead")]
pub fn from_inner(inner: Vec<sha256::Hash>) -> Result<Self, TaprootError> {
if inner.len() > TAPROOT_CONTROL_MAX_NODE_COUNT {
Err(TaprootError::InvalidMerkleTreeDepth(inner.len()))
} else {
Ok(TaprootMerkleBranch(inner))
}
Self::try_from(inner)
}
/// Returns the inner list of hashes.
@ -718,6 +728,25 @@ impl TaprootMerkleBranch {
}
}
macro_rules! impl_try_from {
($from:ty) => {
impl TryFrom<$from> for TaprootMerkleBranch {
type Error = TaprootError;
/// Creates a merkle proof from list of hashes.
///
/// # Errors
/// If inner proof length is more than [`TAPROOT_CONTROL_MAX_NODE_COUNT`] (128).
fn try_from(v: $from) -> Result<Self, Self::Error> {
TaprootMerkleBranch::from_collection(v)
}
}
}
}
impl_try_from!(&[sha256::Hash]);
impl_try_from!(Vec<sha256::Hash>);
impl_try_from!(Box<[sha256::Hash]>);
/// Control block data structure used in Tapscript satisfaction.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]