Merge rust-bitcoin/rust-bitcoin#1559: Deprecate `script::read_uint`

a7117bf8f1 Document source of logic fro read_scriptint (Tobin C. Harding)
2eb2420b40 Add comment on rountripping read/write scripint (Tobin C. Harding)
657dd51e8b Use OP_0 to better mimic bitcoin core code (Tobin C. Harding)
31d254a6a8 Fix push operators URL (Tobin C. Harding)
84cd4ca964 Deprecate script::read_uint (Tobin C. Harding)

Pull request description:

  Patch one does the deprecation, the rest of the PR is made up of tiny improvements to the code around reading/writing 'scriptint's (conceptually `CScriptNum`s). I did all this while trying to decipher the discussion on #1547.

  ### Note Please

  There are many more changes in the pipeline for all this read/write "script int" stuff. This PR was done ages ago and I believe it stall adds value.

  I re-did the whole PR manually because of the recent `script` module changes. I hope no one else has to do that - if you do please feel free to holla and I'll "rebase" your PR for you.

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

Tree-SHA512: 5e8ee7fa8d1393a1a50e4241dd947b837cc0ddd15ff1239a49e4839489459fb95d184d6773f73633d55c436310bfab0c73f806d492ed4a4215f924c6c0993936

bitcoin/src/blockdata/script/builder.rs

@@ -44,7 +44,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) }

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`]: <https://github.com/bitcoin/bitcoin/blob/8ae2808a4354e8dcc697f76bacc5e2f2befe9220/src/script/script.h#L345>
 pub fn write_scriptint(out: &mut [u8; 8], n: i64) -> usize {
     let mut len = 0;
     if n == 0 { return len; }
@@ -110,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<i64, Error> {
     let len = v.len();
     if len > 4 { return Err(Error::NumericOverflow); }
@@ -167,6 +174,7 @@ pub fn read_scriptbool(v: &[u8]) -> bool {
 /// Note that this does **not** return an error for `size` between `core::size_of::<usize>()`
 /// 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<usize, Error> {
     read_uint_iter(&mut data.iter(), size).map_err(Into::into)
 }
@@ -212,7 +220,7 @@ fn opcode_to_verify(opcode: Option<opcodes::All>) -> Option<opcodes::All> {
 #[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`
+    /// <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,