From 84cd4ca96437263ff39be1142b1cdc5d8f96e534 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 10 Feb 2023 11:58:25 +1100 Subject: [PATCH 1/5] Deprecate script::read_uint There is no current usage for reading an unsigned script integer, seems like this is kruft from days gone past. --- bitcoin/src/blockdata/script/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index 199dfb58..5c17c6ee 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -167,6 +167,7 @@ pub fn read_scriptbool(v: &[u8]) -> bool { /// Note that this does **not** return an error for `size` between `core::size_of::()` /// and `u16::max_value / 8` if there's no overflow. #[inline] +#[deprecated(since = "0.30.0", note = "bitcoin integers are signed 32 bits, use read_scriptint")] pub fn read_uint(data: &[u8], size: usize) -> Result { read_uint_iter(&mut data.iter(), size).map_err(Into::into) } From 31d254a6a85c7f96f4bdabf9735737c72abb34da Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 10 Feb 2023 11:59:37 +1100 Subject: [PATCH 2/5] Fix push operators URL The URL is wrong (section `#Push_operators` should be `#push-operators`), also should use angle brackets not back ticks. --- bitcoin/src/blockdata/script/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index 5c17c6ee..c4e027fa 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -213,7 +213,7 @@ fn opcode_to_verify(opcode: Option) -> Option { #[non_exhaustive] pub enum Error { /// Something did a non-minimal push; for more information see - /// `https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki#Push_operators` + /// NonMinimalPush, /// Some opcode expected a parameter but it was missing or truncated. EarlyEndOfScript, From 657dd51e8bfeb0606c53889d6d4a10ed38446a5d Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 10 Feb 2023 12:01:23 +1100 Subject: [PATCH 3/5] Use OP_0 to better mimic bitcoin core code Our `Builder::push_int` method is the same as Bitcoin Core `CScript` `push_int64` method. We currently use `OP_FALSE` (equivalent to `OP_0`) but recently we added `OP_0`, lets use it to make our code better mimic Core (also saves devs checking that `OP_FALSE` is the same as `OP_0`). --- bitcoin/src/blockdata/script/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitcoin/src/blockdata/script/builder.rs b/bitcoin/src/blockdata/script/builder.rs index a69b19eb..158b7e28 100644 --- a/bitcoin/src/blockdata/script/builder.rs +++ b/bitcoin/src/blockdata/script/builder.rs @@ -42,7 +42,7 @@ impl Builder { } // We can also special-case zero else if data == 0 { - self.push_opcode(opcodes::OP_FALSE) + self.push_opcode(opcodes::OP_0) } // Otherwise encode it as data else { self.push_int_non_minimal(data) } From 2eb2420b4043134bfe1323e376bc94c826f92c45 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 10 Feb 2023 12:02:00 +1100 Subject: [PATCH 4/5] Add comment on rountripping read/write scripint We only support reads of upto 4 bytes where as Bitcoin Core allows reading a `CScriptNum` with more bytes than that. Add a rustdoc comment (incl. link to Bitcoin Core) mentioning that. --- bitcoin/src/blockdata/script/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index c4e027fa..5e00b1a5 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -66,6 +66,11 @@ pub use self::types::*; /// Encodes an integer in script(minimal CScriptNum) format. /// /// Writes bytes into the buffer and returns the number of bytes written. +/// +/// Note that `write_scriptint`/`read_scriptint` do not roundtrip if the value written requires +/// more than 4 bytes, this is in line with Bitcoin Core (see [`CScriptNum::serialize`]). +/// +/// [`CScriptNum::serialize`]: pub fn write_scriptint(out: &mut [u8; 8], n: i64) -> usize { let mut len = 0; if n == 0 { return len; } From a7117bf8f1ca295258c25c7d2eb5646bf78da078 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 10 Feb 2023 12:02:32 +1100 Subject: [PATCH 5/5] Document source of logic fro read_scriptint Our `script::read_scriptint` function is based on the constructor code (incl. call to `set_vch`) code from Bitcoin Core. Add rustdoc comment saying so, emit a link because there are already multiple links to `script.h` in this file (one just right below the added comment). --- bitcoin/src/blockdata/script/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index 5e00b1a5..b843ac82 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -115,6 +115,8 @@ pub fn write_scriptint(out: &mut [u8; 8], n: i64) -> usize { /// don't fit in 64 bits (for efficiency on modern processors) so we /// simply say, anything in excess of 32 bits is no longer a number. /// This is basically a ranged type implementation. +/// +/// This code is based on the `CScriptNum` constructor in Bitcoin Core (see `script.h`). pub fn read_scriptint(v: &[u8]) -> Result { let len = v.len(); if len > 4 { return Err(Error::NumericOverflow); }