a8d50a5541 Remove Push enum (Tobin C. Harding)

Pull request description:

  The `Push` enum is only ever used to get access to one of its variants. Since it is a private type we can remove it entirely and just return `PushBytes` from the `last_pushdata` function.

  Needs careful review but I believe the function name is still correctly descriptive.

  This was discovered by of a new nightly clippy warning.

ACKs for top commit:
  apoelstra:
    ACK a8d50a5541 Looks good to me. The latest compiler complains about the currently-unused variants.
  sanket1729:
    ACK a8d50a5541.

Tree-SHA512: 7f96057b0f6f5673252578253ad4f1789793dbf6e917d3974274dedf942da27e6247946262a0669eb500d47987788fcca0e020ed16c0d672188e95ee31163242
This commit is contained in:
Andrew Poelstra 2024-01-08 20:26:53 +00:00
commit 8aab550e97
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
2 changed files with 5 additions and 29 deletions

View File

@ -593,22 +593,12 @@ impl Script {
/// Iterates the script to find the last pushdata. /// Iterates the script to find the last pushdata.
/// ///
/// Returns `None` if the instruction is an opcode or if the script is empty. /// Returns `None` if the instruction is an opcode or if the script is empty.
pub(crate) fn last_pushdata(&self) -> Option<Push> { pub(crate) fn last_pushdata(&self) -> Option<&PushBytes> {
match self.instructions().last() { match self.instructions().last() {
// Handles op codes up to (but excluding) OP_PUSHNUM_NEG. // Handles op codes up to (but excluding) OP_PUSHNUM_NEG.
Some(Ok(Instruction::PushBytes(bytes))) => Some(Push::Data(bytes)), Some(Ok(Instruction::PushBytes(bytes))) => Some(bytes),
// OP_16 (0x60) and lower are considered "pushes" by Bitcoin Core (excl. OP_RESERVED). // OP_16 (0x60) and lower are considered "pushes" by Bitcoin Core (excl. OP_RESERVED).
// By here we know that op is between OP_PUSHNUM_NEG AND OP_PUSHNUM_16 inclusive. // However we are only interested in the pushdata so we can ignore them.
Some(Ok(Instruction::Op(op))) if op.to_u8() <= 0x60 => {
if op == OP_PUSHNUM_NEG1 {
Some(Push::Num(-1))
} else if op == OP_RESERVED {
Some(Push::Reserved)
} else {
let num = (op.to_u8() - 0x50) as i8; // cast ok, num is [1, 16].
Some(Push::Num(num))
}
}
_ => None, _ => None,
} }
} }
@ -626,19 +616,6 @@ impl Script {
} }
} }
/// Data pushed by "push" opcodes.
///
/// "push" opcodes are defined by Bitcoin Core as OP_PUSHBYTES_, OP_PUSHDATA, OP_PUSHNUM_, and
/// OP_RESERVED i.e., everything less than OP_PUSHNUM_16 (0x60) . (TODO: Add link to core code).
pub(crate) enum Push<'a> {
/// All the OP_PUSHBYTES_ and OP_PUSHDATA_ opcodes.
Data(&'a PushBytes),
/// All the OP_PUSHNUM_ opcodes (-1, 1, 2, .., 16)
Num(i8),
/// OP_RESERVED
Reserved,
}
/// Iterator over bytes of a script /// Iterator over bytes of a script
pub struct Bytes<'a>(core::iter::Copied<core::slice::Iter<'a, u8>>); pub struct Bytes<'a>(core::iter::Copied<core::slice::Iter<'a, u8>>);

View File

@ -28,7 +28,6 @@ use crate::consensus::{encode, Decodable, Encodable};
use crate::internal_macros::{impl_consensus_encoding, impl_hashencode}; use crate::internal_macros::{impl_consensus_encoding, impl_hashencode};
use crate::parse::impl_parse_str_from_int_infallible; use crate::parse::impl_parse_str_from_int_infallible;
use crate::prelude::*; use crate::prelude::*;
use crate::script::Push;
#[cfg(doc)] #[cfg(doc)]
use crate::sighash::{EcdsaSighashType, TapSighashType}; use crate::sighash::{EcdsaSighashType, TapSighashType};
use crate::string::FromHexStr; use crate::string::FromHexStr;
@ -909,7 +908,7 @@ impl Transaction {
fn count_sigops(prevout: &TxOut, input: &TxIn) -> usize { fn count_sigops(prevout: &TxOut, input: &TxIn) -> usize {
let mut count: usize = 0; let mut count: usize = 0;
if prevout.script_pubkey.is_p2sh() { if prevout.script_pubkey.is_p2sh() {
if let Some(Push::Data(redeem)) = input.script_sig.last_pushdata() { if let Some(redeem) = input.script_sig.last_pushdata() {
count = count =
count.saturating_add(Script::from_bytes(redeem.as_bytes()).count_sigops()); count.saturating_add(Script::from_bytes(redeem.as_bytes()).count_sigops());
} }
@ -955,7 +954,7 @@ impl Transaction {
} else if prevout.script_pubkey.is_p2sh() && script_sig.is_push_only() { } else if prevout.script_pubkey.is_p2sh() && script_sig.is_push_only() {
// If prevout is P2SH and scriptSig is push only // If prevout is P2SH and scriptSig is push only
// then we wrap the last push (redeemScript) in a Script // then we wrap the last push (redeemScript) in a Script
if let Some(Push::Data(push_bytes)) = script_sig.last_pushdata() { if let Some(push_bytes) = script_sig.last_pushdata() {
Script::from_bytes(push_bytes.as_bytes()) Script::from_bytes(push_bytes.as_bytes())
} else { } else {
return 0; return 0;