Merge rust-bitcoin/rust-bitcoin#1955: Rename `opcodes::All` to `Opcode`

63d0fa0164 Rename All to Opcode (Tobin C. Harding)
881d1b1a9d Move opcode aliases to the all module (Tobin C. Harding)
f3e8dbc392 Remove floating code comment (Tobin C. Harding)
dfd9384d14 opcodes: Fix whitespace (Tobin C. Harding)

Pull request description:

  I may be missing something about the `All` type but it seems it can be re-named to `Opcode` with no loss of clarity.

  The first few patches do some other clean ups, the last patch is the meat and potatoes.

ACKs for top commit:
  apoelstra:
    ACK 63d0fa0164

Tree-SHA512: c1b8be9e0c090d015c2bbfcb7ed6abe998ccada5a1eb0f6966c2a7dc462f8bbc45e6b99bf1387c736d2fca21a993a2c7fd5844c18b97a2e37ec43da517566ab1

bitcoin/src/blockdata/opcodes.rs

@@ -22,7 +22,7 @@ use crate::prelude::*;
 ///
 /// We do not implement Ord on this type because there is no natural ordering on opcodes, but there
 /// may appear to be one (e.g. because all the push opcodes appear in a consecutive block) and we
-/// don't want to encourage subtly buggy code. Please use [`All::classify`] to distinguish different
+/// don't want to encourage subtly buggy code. Please use [`Opcode::classify`] to distinguish different
 /// types of opcodes.
 ///
 /// <details>
@@ -32,7 +32,7 @@ use crate::prelude::*;
 ///   in contexts where only pushes are supposed to be allowed.
 /// </details>
 #[derive(Copy, Clone, PartialEq, Eq)]
-pub struct All {
+pub struct Opcode {
     code: u8,
 }
 
@@ -40,24 +40,33 @@ use self::all::*;
 
 macro_rules! all_opcodes {
     ($($op:ident => $val:expr, $doc:expr);*) => {
-        // private import so we don't have to use `all::OP_FOO` in this file.
-
         /// Enables wildcard imports to bring into scope all opcodes and nothing else.
         ///
         /// The `all` module is provided so one can use a wildcard import `use bitcoin::opcodes::all::*` to
-        /// get all the `OP_FOO` opcodes without getting other types defined in `opcodes` (e.g. `All`, `Class`).
+        /// get all the `OP_FOO` opcodes without getting other types defined in `opcodes` (e.g. `Opcode`, `Class`).
         ///
         /// This module is guaranteed to never contain anything except opcode constants and all opcode
         /// constants are guaranteed to begin with OP_.
         pub mod all {
-            use super::All;
+            use super::Opcode;
-                $(
+            $(
-                    #[doc = $doc]
+                #[doc = $doc]
-                    pub const $op: All = All { code: $val};
+                pub const $op: Opcode = Opcode { code: $val};
-                )*
+            )*
         }
 
-        impl fmt::Display for All {
+        /// Push an empty array onto the stack.
+        pub static OP_0: Opcode = OP_PUSHBYTES_0;
+        /// Empty stack is also FALSE.
+        pub static OP_FALSE: Opcode = OP_PUSHBYTES_0;
+        /// Number 1 is also TRUE.
+        pub static OP_TRUE: Opcode = OP_PUSHNUM_1;
+        /// Previously called OP_NOP2.
+        pub static OP_NOP2: Opcode = OP_CLTV;
+        /// Previously called OP_NOP3.
+        pub static OP_NOP3: Opcode = OP_CSV;
+
+        impl fmt::Display for Opcode {
             fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                  match *self {
                    $(
@@ -66,7 +75,6 @@ macro_rules! all_opcodes {
                 }
             }
         }
-
     }
 }
 
@@ -344,7 +352,7 @@ pub enum ClassifyContext {
     Legacy,
 }
 
-impl All {
+impl Opcode {
     /// Classifies an Opcode into a broad class.
     #[inline]
     pub fn classify(self, ctx: ClassifyContext) -> Class {
@@ -408,11 +416,11 @@ impl All {
         }
     }
 
-    /// Encodes [`All`] as a byte.
+    /// Encodes [`Opcode`] as a byte.
     #[inline]
     pub const fn to_u8(self) -> u8 { self.code }
 
-    /// Encodes PUSHNUM [`All`] as a `u8` representing its number (1-16).
+    /// Encodes PUSHNUM [`Opcode`] as a `u8` representing its number (1-16).
     ///
     /// Does not convert `OP_FALSE` to 0. Only `1` to `OP_PUSHNUM_16` are covered.
     ///
@@ -430,15 +438,15 @@ impl All {
     }
 }
 
-impl From<u8> for All {
+impl From<u8> for Opcode {
     #[inline]
-    fn from(b: u8) -> All { All { code: b } }
+    fn from(b: u8) -> Opcode { Opcode { code: b } }
 }
 
-debug_from_display!(All);
+debug_from_display!(Opcode);
 
 #[cfg(feature = "serde")]
-impl serde::Serialize for All {
+impl serde::Serialize for Opcode {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
@@ -447,17 +455,6 @@ impl serde::Serialize for All {
     }
 }
 
-/// Push an empty array onto the stack.
-pub static OP_0: All = OP_PUSHBYTES_0;
-/// Empty stack is also FALSE.
-pub static OP_FALSE: All = OP_PUSHBYTES_0;
-/// Number 1 is also TRUE.
-pub static OP_TRUE: All = OP_PUSHNUM_1;
-/// Previously called OP_NOP2.
-pub static OP_NOP2: All = OP_CLTV;
-/// Previously called OP_NOP3.
-pub static OP_NOP3: All = OP_CSV;
-
 /// Broad categories of opcodes with similar behavior.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum Class {
@@ -495,15 +492,15 @@ macro_rules! ordinary_opcode {
         }
 
         impl Ordinary {
-            fn with(b: All) -> Self {
+            fn with(b: Opcode) -> Self {
                 match b {
                     $( $op => { Ordinary::$op } ),*
                     _ => unreachable!("construction of `Ordinary` type from non-ordinary opcode {}", b),
                 }
             }
 
-            /// Try to create from an All
+            /// Try to create a [`Ordinary`] from an [`Opcode`].
-            pub fn try_from_all(b: All) -> Option<Self> {
+            pub fn from_opcode(b: Opcode) -> Option<Self> {
                 match b {
                     $( $op => { Some(Ordinary::$op) } ),*
                     _ => None,
@@ -540,7 +537,7 @@ ordinary_opcode! {
 }
 
 impl Ordinary {
-    /// Encodes [`All`] as a byte.
+    /// Encodes [`Opcode`] as a byte.
     #[inline]
     pub fn to_u8(self) -> u8 { self as u8 }
 }
@@ -553,7 +550,7 @@ mod tests {
 
     macro_rules! roundtrip {
         ($unique:expr, $op:ident) => {
-            assert_eq!($op, All::from($op.to_u8()));
+            assert_eq!($op, Opcode::from($op.to_u8()));
 
             let s1 = format!("{}", $op);
             let s2 = format!("{:?}", $op);
@@ -582,7 +579,7 @@ mod tests {
                 0x51..=0x60 => Some(i - 0x50),
                 _ => None,
             };
-            assert_eq!(All::from(i).decode_pushnum(), expected);
+            assert_eq!(Opcode::from(i).decode_pushnum(), expected);
         }
 
         // Test the named opcode constants

bitcoin/src/blockdata/script/borrowed.rs

@@ -10,7 +10,7 @@ use hashes::Hash;
 use secp256k1::{Secp256k1, Verification};
 
 use crate::blockdata::opcodes::all::*;
-use crate::blockdata::opcodes::{self};
+use crate::blockdata::opcodes::{self, Opcode};
 use crate::blockdata::script::witness_version::WitnessVersion;
 use crate::blockdata::script::{
     bytes_to_asm_fmt, Builder, Instruction, InstructionIndices, Instructions, ScriptBuf,
@@ -166,7 +166,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.first().and_then(|opcode| WitnessVersion::try_from(opcodes::All::from(*opcode)).ok())
+        self.0.first().and_then(|opcode| WitnessVersion::try_from(Opcode::from(*opcode)).ok())
     }
 
     /// Checks whether a script pubkey is a P2SH output.
@@ -229,7 +229,7 @@ impl Script {
         if !(4..=42).contains(&script_len) {
             return false;
         }
-        let ver_opcode = opcodes::All::from(self.0[0]); // Version 0 or PUSHNUM_1-PUSHNUM_16
+        let ver_opcode = Opcode::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::try_from(ver_opcode).is_ok()
             && push_opbyte >= OP_PUSHBYTES_2.to_u8()
@@ -286,7 +286,7 @@ impl Script {
 
         match self.0.first() {
             Some(b) => {
-                let first = opcodes::All::from(*b);
+                let first = Opcode::from(*b);
                 let class = first.classify(opcodes::ClassifyContext::Legacy);
 
                 class == ReturnOp || class == IllegalOp
@@ -475,14 +475,14 @@ impl Script {
     pub fn to_hex_string(&self) -> String { self.as_bytes().to_lower_hex_string() }
 
     /// Returns the first opcode of the script (if there is any).
-    pub fn first_opcode(&self) -> Option<opcodes::All> {
+    pub fn first_opcode(&self) -> Option<Opcode> {
         self.as_bytes().first().copied().map(From::from)
     }
 
     /// Iterates the script to find the last opcode.
     ///
     /// Returns `None` is the instruction is data push or if the script is empty.
-    pub(in crate::blockdata::script) fn last_opcode(&self) -> Option<opcodes::All> {
+    pub(in crate::blockdata::script) fn last_opcode(&self) -> Option<Opcode> {
         match self.instructions().last() {
             Some(Ok(Instruction::Op(op))) => Some(op),
             _ => None,

bitcoin/src/blockdata/script/builder.rs

@@ -7,7 +7,7 @@ use secp256k1::XOnlyPublicKey;
 
 use crate::blockdata::locktime::absolute;
 use crate::blockdata::opcodes::all::*;
-use crate::blockdata::opcodes::{self};
+use crate::blockdata::opcodes::{self, Opcode};
 use crate::blockdata::script::{opcode_to_verify, write_scriptint, PushBytes, Script, ScriptBuf};
 use crate::blockdata::transaction::Sequence;
 use crate::key::PublicKey;
@@ -15,7 +15,7 @@ use crate::prelude::*;
 
 /// An Object which can be used to construct a script piece by piece.
 #[derive(PartialEq, Eq, Clone)]
-pub struct Builder(ScriptBuf, Option<opcodes::All>);
+pub struct Builder(ScriptBuf, Option<Opcode>);
 
 impl Builder {
     /// Creates a new empty script.
@@ -34,7 +34,7 @@ impl Builder {
     pub fn push_int(self, data: i64) -> Builder {
         // We can special-case -1, 1-16
         if data == -1 || (1..=16).contains(&data) {
-            let opcode = opcodes::All::from((data - 1 + opcodes::OP_TRUE.to_u8() as i64) as u8);
+            let opcode = Opcode::from((data - 1 + opcodes::OP_TRUE.to_u8() as i64) as u8);
             self.push_opcode(opcode)
         }
         // We can also special-case zero
@@ -78,7 +78,7 @@ impl Builder {
     }
 
     /// Adds a single opcode to the script.
-    pub fn push_opcode(mut self, data: opcodes::All) -> Builder {
+    pub fn push_opcode(mut self, data: Opcode) -> Builder {
         self.0.push_opcode(data);
         self.1 = Some(data);
         self

bitcoin/src/blockdata/script/instruction.rs

@@ -2,7 +2,7 @@
 
 use core::convert::TryInto;
 
-use crate::blockdata::opcodes;
+use crate::blockdata::opcodes::{self, Opcode};
 use crate::blockdata::script::{read_uint_iter, Error, PushBytes, Script, ScriptBuf, UintError};
 
 /// A "parsed opcode" which allows iterating over a [`Script`] in a more sensible way.
@@ -11,12 +11,12 @@ pub enum Instruction<'a> {
     /// Push a bunch of data.
     PushBytes(&'a PushBytes),
     /// Some non-push opcode.
-    Op(opcodes::All),
+    Op(Opcode),
 }
 
 impl<'a> Instruction<'a> {
     /// Returns the opcode if the instruction is not a data push.
-    pub fn opcode(&self) -> Option<opcodes::All> {
+    pub fn opcode(&self) -> Option<Opcode> {
         match self {
             Instruction::Op(op) => Some(*op),
             Instruction::PushBytes(_) => None,
@@ -124,7 +124,7 @@ impl<'a> Iterator for Instructions<'a> {
 
         // classify parameter does not really matter here since we are only using
         // it for pushes and nums
-        match opcodes::All::from(byte).classify(opcodes::ClassifyContext::Legacy) {
+        match Opcode::from(byte).classify(opcodes::ClassifyContext::Legacy) {
             opcodes::Class::PushBytes(n) => {
                 // make sure safety argument holds across refactorings
                 let n: u32 = n;
@@ -152,7 +152,7 @@ impl<'a> Iterator for Instructions<'a> {
             opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA4) =>
                 self.next_push_data_len(PushDataLenLen::Four, 0x10000),
             // Everything else we can push right through
-            _ => Some(Ok(Instruction::Op(opcodes::All::from(byte)))),
+            _ => Some(Ok(Instruction::Op(Opcode::from(byte)))),
         }
     }
 

bitcoin/src/blockdata/script/mod.rs

@@ -60,7 +60,7 @@ use hashes::{hash160, sha256};
 use serde;
 
 use crate::blockdata::opcodes::all::*;
-use crate::blockdata::opcodes::{self};
+use crate::blockdata::opcodes::{self, Opcode};
 use crate::consensus::{encode, Decodable, Encodable};
 use crate::prelude::*;
 use crate::{io, OutPoint};
@@ -236,7 +236,7 @@ fn read_uint_iter(data: &mut core::slice::Iter<'_, u8>, size: usize) -> Result<u
     }
 }
 
-fn opcode_to_verify(opcode: Option<opcodes::All>) -> Option<opcodes::All> {
+fn opcode_to_verify(opcode: Option<Opcode>) -> Option<Opcode> {
     opcode.and_then(|opcode| match opcode {
         OP_EQUAL => Some(OP_EQUALVERIFY),
         OP_NUMEQUAL => Some(OP_NUMEQUALVERIFY),
@@ -606,7 +606,7 @@ pub(super) fn bytes_to_asm_fmt(script: &[u8], f: &mut dyn fmt::Write) -> fmt::Re
     // `iter` needs to be borrowed in `read_push_data_len`, so we have to use `while let` instead
     // of `for`.
     while let Some(byte) = iter.next() {
-        let opcode = opcodes::All::from(*byte);
+        let opcode = Opcode::from(*byte);
 
         let data_len = if let opcodes::Class::PushBytes(n) =
             opcode.classify(opcodes::ClassifyContext::Legacy)

bitcoin/src/blockdata/script/owned.rs

@@ -6,7 +6,7 @@ use core::ops::Deref;
 use secp256k1::{Secp256k1, Verification};
 
 use crate::blockdata::opcodes::all::*;
-use crate::blockdata::opcodes::{self};
+use crate::blockdata::opcodes::{self, Opcode};
 use crate::blockdata::script::witness_program::WitnessProgram;
 use crate::blockdata::script::witness_version::WitnessVersion;
 use crate::blockdata::script::{
@@ -176,7 +176,7 @@ impl ScriptBuf {
     pub fn into_bytes(self) -> Vec<u8> { self.0 }
 
     /// Adds a single opcode to the script.
-    pub fn push_opcode(&mut self, data: opcodes::All) { self.0.push(data.to_u8()); }
+    pub fn push_opcode(&mut self, data: Opcode) { self.0.push(data.to_u8()); }
 
     /// Adds instructions to push some arbitrary data onto the stack.
     pub fn push_slice<T: AsRef<PushBytes>>(&mut self, data: T) {
@@ -266,7 +266,7 @@ impl ScriptBuf {
     /// alternative.
     ///
     /// See the public fn [`Self::scan_and_push_verify`] to learn more.
-    pub(in crate::blockdata::script) fn push_verify(&mut self, last_opcode: Option<opcodes::All>) {
+    pub(in crate::blockdata::script) fn push_verify(&mut self, last_opcode: Option<Opcode>) {
         match opcode_to_verify(last_opcode) {
             Some(opcode) => {
                 self.0.pop();

bitcoin/src/blockdata/script/witness_version.rs

@@ -13,8 +13,8 @@ use core::str::FromStr;
 
 use internals::write_err;
 
-use crate::blockdata::opcodes;
 use crate::blockdata::opcodes::all::*;
+use crate::blockdata::opcodes::Opcode;
 use crate::blockdata::script::Instruction;
 use crate::error::ParseIntError;
 
@@ -149,7 +149,7 @@ impl TryFrom<u8> for WitnessVersion {
     }
 }
 
-impl TryFrom<opcodes::All> for WitnessVersion {
+impl TryFrom<Opcode> for WitnessVersion {
     type Error = Error;
 
     /// Converts bitcoin script opcode into [`WitnessVersion`] variant.
@@ -162,7 +162,7 @@ impl TryFrom<opcodes::All> for WitnessVersion {
     ///
     /// If the opcode does not correspond to any witness version, errors with
     /// [`Error::Malformed`].
-    fn try_from(opcode: opcodes::All) -> Result<Self, Self::Error> {
+    fn try_from(opcode: Opcode) -> Result<Self, Self::Error> {
         match opcode.to_u8() {
             0 => Ok(WitnessVersion::V0),
             version if version >= OP_PUSHNUM_1.to_u8() && version <= OP_PUSHNUM_16.to_u8() =>
@@ -202,12 +202,12 @@ impl From<WitnessVersion> for bech32::u5 {
     }
 }
 
-impl From<WitnessVersion> for opcodes::All {
+impl From<WitnessVersion> for Opcode {
     /// Converts [`WitnessVersion`] instance into corresponding Bitcoin scriptopcode (`OP_0`..`OP_16`).
-    fn from(version: WitnessVersion) -> opcodes::All {
+    fn from(version: WitnessVersion) -> Opcode {
         match version {
             WitnessVersion::V0 => OP_PUSHBYTES_0,
-            no => opcodes::All::from(OP_PUSHNUM_1.to_u8() + no.to_num() - 1),
+            no => Opcode::from(OP_PUSHNUM_1.to_u8() + no.to_num() - 1),
         }
     }
 }