Merge #288: Introduce bitcoin-consensus-encoding 1.0 and implement traits for Transaction and subtypes

5eb4f8d add fuzztest for equivalence between new and old transaction decoding (Andrew Poelstra)
faf6ab5 implement Encode/Decode for Transaction (Andrew Poelstra)
b7b5886 implement Encode/Decode for TxInWitness (Andrew Poelstra)
d72094a implement Encode/Decode for TxIn (Andrew Poelstra)
02ab70d implement Encode/Decode for TxOutWitness (Andrew Poelstra)
28a3d6d implement Encode/Decode for TxOut (Andrew Poelstra)
a61cbab transaction: introduce PeginWitness type (Andrew Poelstra)
0b8fa43 transaction: strongly type script_witness (Andrew Poelstra)
9f82b4b serde: remove impls for PSET and Transaction (Andrew Poelstra)
c13d392 rename transaction to transaction/mod.rs (Andrew Poelstra)
c7e16cd confidential: implement Encode and Decode for rangeproof/surjectionproof (Andrew Poelstra)
aacd081 confidential: implement Encode/Decode for confidential commitments (Andrew Poelstra)
6baad08 hashes: remove deprecated macro (Andrew Poelstra)
dca9bb0 update bitcoin dependency to 0.32.102, hashes to 1.1 (Andrew Poelstra)

Pull request description:

  This is a big PR. Please forgive me.

  The goal is to replace the `Encodable` and `Decodable` traits from rust-elements with the new [`Encode` and `Decode` traits from bitcoin-consensus-encoding](https://docs.rs/bitcoin-consensus-encoding/latest/bitcoin_consensus_encoding/index.html). There are many benefits to this:

  * Unlike the old rust-bitcoin traits, these ones are designed to be used by the general public for wire encoding formats, so we don't need to create our own duplicate crate and add a bunch of shim implementations; we can also write generic code that works between Bitcoin and Elements...
  * ...but we largely won't need to, because `bitcoin-consensus-encoding` ships with functions for decoding/encoding to `std::io` types, unbuffered `std::io` types, Vecs/slices, and hex strings, while `bitcoin-hashes` ships with functions for encoding into hash engines, and `bitcoin-io` with functions for decoding/encoding into `bitcoin::io` types.
  * These traits use the "sans-IO paradigm", meaning that they can be used in no-std environments without the need for shim crates like `core2` (deleted from crates.io) or `bitcoin-io`, whose types tend to pollute every codebase that touches them.
  * The traits also require no allocators: it is easy to encode even complex objects like `Transaction` without needing to copy any data into auxliary buffers, and some for decoding. If an object does not itself hold a `Box` or `Vec` it should be possible to encode/decode it completely allocator-free. Again, without shim crates like `ArrayVec` or whatever. Coupled with allocator-free encoders like `rust-bech32`, it is now possible to write arbitrary error correcting codes and encode bitcoin/elements types into them without needing any auxiliary buffers
  * Because these traits are defined by a stable 1.0 crate, after *years* of the rust-bitcoin ecosystem fighting over API breakage and promising not to 2.0 basic stuff, they should remain stable across rust-elements versions. Since rust-elements is far from stable, this should make upgrades *much* easier since it will be increasingly possible to interoperate multiple versions of it.
  * It paves the way for us to replace `Sequence` and `LockTime` with `bitcoin-units 1.0` types; this may invite type confusion between "bitcoin locktimes" and "elements locktimes" but in exchange we get stable types that have had years of bugfixes and API improvements. (And locktimes in Bitcoin, and therefore Elements, are notoriously fickle and hand to use correctly. Every API improvement we shipped found bugs in rust-miniscript.)
  * Along the way, I made many API improvements to rust-elements that aren't directly related to encoding, but without which it was quite difficult to implement. These increase the size of this PR but they're all changes that need to be made eventually.

ACKs for top commit:
  delta1:
    ACK 5eb4f8d; tested locally

Tree-SHA512: 2796f84295db4bf784e45a672eb108ece4406982fd897cd3dfc218d93d4b345ce6c742a7e9b6fef1e3ecaf48ec64f21fb5a10c20eeed9dc744a66d088330220e

Author: delta1

Cargo-recent.lock

@@ -22,12 +22,11 @@ checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50"
 
 [[package]]
 name = "base58ck"
-version = "0.1.0"
+version = "0.1.101"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2c8d66485a3a2ea485c1913c4572ce0256067a5377ac8c75c4960e1cda98605f"
+checksum = "365c0acd5b2e8dd0111a46c4faea83fb3cfb6e39a49a7c73a06e090db7b2eff0"
 dependencies = [
- "bitcoin-internals 0.3.0",
- "bitcoin_hashes 0.14.0",
+ "bitcoin_hashes 0.14.101",
 ]
 
 [[package]]
@@ -59,52 +58,48 @@ dependencies = [
 
 [[package]]
 name = "bitcoin"
-version = "0.32.6"
+version = "0.32.102"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ad8929a18b8e33ea6b3c09297b687baaa71fb1b97353243a3f1029fad5c59c5b"
+checksum = "bb0ce8bd5baaa0d303a19915a6d93afed161f528654e42da2a7a97d05c59499a"
 dependencies = [
  "base58ck",
  "base64 0.21.7",
  "bech32",
- "bitcoin-internals 0.3.0",
+ "bitcoin-consensus-encoding",
  "bitcoin-io",
  "bitcoin-units",
- "bitcoin_hashes 0.14.0",
- "hex-conservative 0.2.1",
+ "bitcoin_hashes 0.14.101",
+ "hex-conservative 0.2.2",
  "hex_lit",
  "secp256k1",
  "serde",
 ]
 
 [[package]]
 name = "bitcoin-consensus-encoding"
-version = "1.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b2d6094e2a1ba3c93b5a596fe5a10d1a10c3c6e06785cde89f693a044c01aa40"
-dependencies = [
- "bitcoin-internals 0.5.0",
-]
-
-[[package]]
-name = "bitcoin-internals"
-version = "0.3.0"
+version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "30bdbe14aa07b06e6cfeffc529a1f099e5fbe249524f8125358604df99a4bed2"
+checksum = "207311705279250ba465076a1bac4b1ac982855fff73fc5f67e22158ac58cdc9"
 dependencies = [
+ "bitcoin-internals",
+ "hex-conservative 1.1.0",
  "serde",
 ]
 
 [[package]]
 name = "bitcoin-internals"
-version = "0.5.0"
+version = "0.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a30a22d1f112dde8e16be7b45c63645dc165cef254f835b3e1e9553e485cfa64"
+checksum = "d573f4cf32996a8dce612e4348cece65a241f1882ed594047c9ba348e8869fa5"
 
 [[package]]
 name = "bitcoin-io"
-version = "0.1.3"
+version = "0.1.101"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0b47c4ab7a93edb0c7198c5535ed9b52b63095f4e9b45279c6736cec4b856baf"
+checksum = "bb5de036369d1ac59d3c1819ebc4d850f89466f5401c571a285b6ed564a4cb78"
+dependencies = [
+ "bitcoin-consensus-encoding",
+]
 
 [[package]]
 name = "bitcoin-private"
@@ -114,33 +109,33 @@ checksum = "73290177011694f38ec25e165d0387ab7ea749a4b81cd4c80dae5988229f7a57"
 
 [[package]]
 name = "bitcoin-units"
-version = "0.1.2"
+version = "0.1.101"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5285c8bcaa25876d07f37e3d30c303f2609179716e11d688f51e8f1fe70063e2"
+checksum = "9cb95693f371d089a4b5b6fc41c6f3ea6e01ee8c15388335dfac8ea685173b51"
 dependencies = [
- "bitcoin-internals 0.3.0",
+ "bitcoin-consensus-encoding",
  "serde",
 ]
 
 [[package]]
 name = "bitcoin_hashes"
-version = "0.14.0"
+version = "0.14.101"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bb18c03d0db0247e147a21a6faafd5a7eb851c743db062de72018b6b7e8e4d16"
+checksum = "bca4c7abb40c8817d77403c880988cfd484f23ab2365726afb2f798363e2c4a2"
 dependencies = [
  "bitcoin-io",
- "hex-conservative 0.2.1",
+ "hex-conservative 0.2.2",
  "serde",
 ]
 
 [[package]]
 name = "bitcoin_hashes"
-version = "1.0.0"
+version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8f70c29ac06e7effa19682e91318deae86bdb46c4fd1bbd0f12fd196ff427ab0"
+checksum = "a67800fcf7f3ca52f7796d4466948a424326b225950c79d1e76d0458760bb25d"
 dependencies = [
  "bitcoin-consensus-encoding",
- "bitcoin-internals 0.5.0",
+ "bitcoin-internals",
  "hex-conservative 1.1.0",
  "serde",
 ]
@@ -230,8 +225,9 @@ dependencies = [
  "bech32",
  "bincode",
  "bitcoin",
- "bitcoin-internals 0.5.0",
- "bitcoin_hashes 1.0.0",
+ "bitcoin-consensus-encoding",
+ "bitcoin-internals",
+ "bitcoin_hashes 1.1.0",
  "getrandom 0.2.16",
  "hex-conservative 1.1.0",
  "rand",
@@ -313,9 +309,9 @@ dependencies = [
 
 [[package]]
 name = "hex-conservative"
-version = "0.2.1"
+version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5313b072ce3c597065a808dbf612c4c8e8590bdbf8b579508bf7a762c5eae6cd"
+checksum = "fda06d18ac606267c40c04e41b9947729bf8b9efe74bd4e82b61a5f26a510b9f"
 dependencies = [
  "arrayvec",
 ]
@@ -512,7 +508,7 @@ version = "0.29.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9465315bc9d4566e1724f0fffcbcc446268cb522e60f9a27bcded6b19c108113"
 dependencies = [
- "bitcoin_hashes 0.14.0",
+ "bitcoin_hashes 0.14.101",
  "rand",
  "secp256k1-sys",
  "serde",

Cargo.toml

@@ -29,9 +29,10 @@ base64 = ["bitcoin/base64"]
 
 [dependencies]
 bech32 = "0.11.0"
-bitcoin = "0.32.2"
-hashes = { package = "bitcoin_hashes", version = "1.0", features = [ "hex" ] }
-internals = { package = "bitcoin-internals", version = "0.5" }
+bitcoin = { version = "0.32.102", default-features = false, features = [ "encoding", "std" ] }
+encoding = { package = "bitcoin-consensus-encoding", version = "1.1.0", default-features = false, features = [ "hex", "std" ] }
+hashes = { package = "bitcoin_hashes", version = "1.1", features = [ "hex" ] }
+internals = { package = "bitcoin-internals", version = "0.6" }
 secp256k1-zkp = { version = "0.11.0", features = ["global-context", "hashes"] }
 
 # Used for ContractHash::from_json_contract.
@@ -73,6 +74,7 @@ match_bool = "allow" # Adds extra indentation and LOC.
 match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other.
 must_use_candidate = "allow" # Useful for audit but many false positives.
 similar_names = "allow" # Too many (subjectively) false positives.
+struct_field_names = "allow" # Dumb
 # Exhaustive list of pedantic clippy lints
 assigning_clones = "warn"
 bool_to_int_with_if = "warn"
@@ -167,7 +169,6 @@ stable_sort_primitive = "warn"
 str_split_at_newline = "warn"
 string_add_assign = "warn"
 struct_excessive_bools = "warn"
-struct_field_names = "warn"
 too_many_lines = "allow" # FIXME 14 triggers for this lint; probably most should be fixed
 transmute_ptr_to_ptr = "warn"
 trivially_copy_pass_by_ref = "warn"
@@ -191,7 +192,6 @@ zero_sized_map_values = "warn"
 [package.metadata.rbmt.lint]
 allowed_duplicates = [
     "bitcoin_hashes",
-    "bitcoin-internals",
     "hex-conservative",
 ]
 

elementsd-tests/src/taproot.rs

@@ -223,7 +223,7 @@ fn taproot_spend_test(
             hash_ty: sighash_ty,
         };
 
-        tx.input[0].witness.script_witness = vec![schnorr_sig.to_vec()];
+        tx.input[0].witness.script_witness.push(schnorr_sig.to_vec());
     } else {
         // script spend
         // try spending using leaf1
@@ -250,11 +250,9 @@ fn taproot_spend_test(
             hash_ty: sighash_ty,
         };
 
-        tx.input[0].witness.script_witness = vec![
-            schnorr_sig.to_vec(), // witness
-            script_ver.0.into_bytes(), // leaf script
-            ctrl_block.serialize(), // control block
-        ];
+        tx.input[0].witness.script_witness.push(schnorr_sig.to_vec()); // witness
+        tx.input[0].witness.script_witness.push(script_ver.0.into_bytes()); // leaf script
+        tx.input[0].witness.script_witness.push(ctrl_block.serialize()); // control block
     }
 
     let tx_hex = serialize_hex(&tx);

examples/raw_blind.rs

@@ -305,15 +305,13 @@ fn main() {
 
     // Finalize(TODO in miniscript)
     pset.inputs_mut()[0].partial_sigs.clear();
-    pset.inputs_mut()[0].final_script_witness = Some(vec![
-        inp0_sig,
-        inp0_pk.to_bytes(),
-    ]);
+    let wit = pset.inputs_mut()[0].final_script_witness.insert(elements::Witness::new());
+    wit.push(inp0_sig);
+    wit.push(inp0_pk.to_bytes());
     pset.inputs_mut()[1].partial_sigs.clear();
-    pset.inputs_mut()[1].final_script_witness = Some(vec![
-        inp1_sig,
-        inp1_pk.to_bytes(),
-    ]);
+    let wit = pset.inputs_mut()[1].final_script_witness.insert(elements::Witness::new());
+    wit.push(inp1_sig);
+    wit.push(inp1_pk.to_bytes());
     assert_eq!(pset, deser_pset(&tests["finalized"]));
 
     // Extracted tx

fuzz/Cargo.toml

@@ -33,6 +33,13 @@ allowed_duplicates = [
     "wasi",
 ]
 
+[[bin]]
+name = "decode_equiv_transaction"
+path = "fuzz_targets/decode_equiv_transaction.rs"
+test = false
+doc = false
+bench = false
+
 [[bin]]
 name = "deserialize_block"
 path = "fuzz_targets/deserialize_block.rs"

fuzz/fuzz_targets/decode_equiv_transaction.rs

@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+//! Fuzz test for equivalent decoding between the legacy and consensus-encoding decoding schemes.
+
+#![cfg_attr(fuzzing, no_main)]
+#![cfg_attr(not(fuzzing), allow(unused))]
+use libfuzzer_sys::fuzz_target;
+
+use elements::bitcoin::hex::DisplayHex as _;
+
+type Target = elements::Transaction;
+
+#[cfg(not(fuzzing))]
+fn main() {}
+
+struct DisplayError<'e>(&'e dyn std::error::Error);
+
+impl core::fmt::Display for DisplayError<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        writeln!(f, "{}", self.0)?;
+        let mut e = self.0;
+        while let Some(source) = e.source() {
+            writeln!(f, "caused by {}", source)?;
+            e = source;
+        }
+        Ok(())
+    }
+}
+
+fn do_test(data: &[u8]) {
+    let old: Result<Target, _> = elements::encode::deserialize(data);
+    let new: Result<Target, _> = elements::encoding::decode_from_slice(data);
+
+    match (old, new) {
+        (Err(_), Err(_)) => {},
+        (Err(e), Ok(new)) => {
+            let e = DisplayError(&e);
+            panic!("Decodable trait failed with {e}; Decode parsed {:?}", new);
+        }
+        (Ok(old), Err(e)) => {
+            let e = DisplayError(&e);
+            panic!("Decode trait failed with {e}; Decodable parsed {:?}", old);
+        }
+        (Ok(old), Ok(new)) => {
+            assert_eq!(
+                old, new,
+                "Decodable (left) did not match Decode (right)",
+            );
+
+            let reser_old = elements::encode::serialize(&old);
+            let reser_new = elements::encoding::encode_to_vec(&new);
+            assert_eq!(
+                reser_old, reser_new,
+                "Encodable (left) did not match Encode (right)\nOld hex: {}\nNew hex: {}\nTransaction: {:?}",
+                reser_old.as_hex(),
+                reser_new.as_hex(),
+                old,
+            );
+        }
+    }
+}
+
+fuzz_target!(|data: &[u8]| {
+    do_test(data);
+});
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn duplicate_crash() {
+        let v = elements::hex::decode_to_vec("abcd").unwrap();
+        super::do_test(&v);
+    }
+}

rustfmt.toml

@@ -2,6 +2,9 @@ ignore = [
     "/",
     "!/src/lib.rs",
     "!/src/confidential/*.rs",
+    "!/src/transaction/decoders.rs",
+    "!/src/transaction/encoders.rs",
+    "!/src/transaction/pegin_witness.rs"
 ]
 hard_tabs = false
 tab_spaces = 4

src/block.rs

@@ -360,7 +360,6 @@ pub struct Block {
     /// Complete list of transaction in the block
     pub txdata: Vec<Transaction>,
 }
-serde_struct_impl!(Block, header, txdata);
 impl_consensus_encoding!(Block, header, txdata);
 
 impl Block {