From 2157e69857770094ce9123e31fc42b2666e10798 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 25 Oct 2022 08:55:20 +1100 Subject: [PATCH] Document the `all` module The `all` module enables usage of a wildcard import statement without muddying the scope with any other types defined in `opcodes`, in other words if one wants to use the `All` type `opcodes::All` is the most clear way to use it, however usage of naked `OP_FOO` types is perfectly clear. Add documentation stating that we guarantee to never put anything else in the `all` module so folks are confident using a wildcard import will not bring any rubbish into scope. Expected usage in downstream applications that need types in `opcodes` as well as the opcodes: ``` use bitcoin::opcodes::all::*; use bitcoin::opcodes; ``` Also, we do no implement `Ord` or `PartialOrd`, document this including HTML tags hiding an example bug from Bitcoin Core that shows why not. --- bitcoin/src/blockdata/opcodes.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/bitcoin/src/blockdata/opcodes.rs b/bitcoin/src/blockdata/opcodes.rs index 7b37d663..0dc8d41b 100644 --- a/bitcoin/src/blockdata/opcodes.rs +++ b/bitcoin/src/blockdata/opcodes.rs @@ -16,11 +16,19 @@ use core::{fmt, convert::From}; use bitcoin_internals::debug_from_display; -// Note: I am deliberately not implementing PartialOrd or Ord on the -// opcode enum. If you want to check ranges of opcodes, etc., -// write an #[inline] helper function which casts to u8s. - /// A script Opcode. +/// +/// 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 +/// types of opcodes. +/// +///
+/// Example of Core bug caused by assuming ordering +/// +/// Bitcoin Core's `IsPushOnly` considers `OP_RESERVED` to be a "push code", allowing this opcode +/// in contexts where only pushes are supposed to be allowed. +///
#[derive(Copy, Clone, PartialEq, Eq)] pub struct All { code: u8, @@ -29,10 +37,14 @@ pub struct All { // private import so we don't have to use `all::OP_FOO` in this file. use self::all::*; +/// 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`). +/// +/// This module is guaranteed to never contain anything except opcode constants and all opcode +/// constants are guaranteed to begin with OP_. pub mod all { - //! Constants associated with All type use super::All; /// Push an empty array onto the stack.