From 7d6687451a5b49c7d779b9441cc88a138d104fea Mon Sep 17 00:00:00 2001
From: Riccardo Casatta <RCasatta@users.noreply.github.com>
Date: Fri, 9 Aug 2019 17:03:12 +0200
Subject: [PATCH] use BTreeMap instead of HashMap to always serialize the same
 (#310)

* use BTreeMap instead of HashMap to always serialize the same

* fix rust 1.22 error

* psbt fuzz roundtrip

* psbt fuzz roundtrip on our ser
---
 a                                     |  0
 fuzz/fuzz_targets/deserialize_psbt.rs | 11 ++++++++++-
 src/util/psbt/map/global.rs           |  6 +++---
 src/util/psbt/map/input.rs            |  8 ++++----
 src/util/psbt/map/output.rs           |  6 +++---
 src/util/psbt/mod.rs                  | 19 +++++++++++++------
 src/util/psbt/raw.rs                  |  2 +-
 7 files changed, 34 insertions(+), 18 deletions(-)
 create mode 100644 a

diff --git a/a b/a
new file mode 100644
index 00000000..e69de29b
diff --git a/fuzz/fuzz_targets/deserialize_psbt.rs b/fuzz/fuzz_targets/deserialize_psbt.rs
index 9021fc3b..d19563b1 100644
--- a/fuzz/fuzz_targets/deserialize_psbt.rs
+++ b/fuzz/fuzz_targets/deserialize_psbt.rs
@@ -1,7 +1,16 @@
 extern crate bitcoin;
 
 fn do_test(data: &[u8]) {
-    let _: Result<bitcoin::util::psbt::PartiallySignedTransaction, _> = bitcoin::consensus::encode::deserialize(data);
+    let psbt: Result<bitcoin::util::psbt::PartiallySignedTransaction, _> = bitcoin::consensus::encode::deserialize(data);
+    match psbt {
+        Err(_) => {},
+        Ok(psbt) => {
+            let ser = bitcoin::consensus::encode::serialize(&psbt);
+            let deser: bitcoin::util::psbt::PartiallySignedTransaction  = bitcoin::consensus::encode::deserialize(&ser).unwrap();
+            // Since the fuzz data could order psbt fields differently, we compare to our deser/ser instead of data
+            assert_eq!(ser, bitcoin::consensus::encode::serialize(&deser));
+        }
+    }
 }
 
 #[cfg(feature = "afl")]
diff --git a/src/util/psbt/map/global.rs b/src/util/psbt/map/global.rs
index 81fc3f57..b34b4114 100644
--- a/src/util/psbt/map/global.rs
+++ b/src/util/psbt/map/global.rs
@@ -12,7 +12,7 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::io::{self, Cursor};
 
 use blockdata::transaction::Transaction;
@@ -29,7 +29,7 @@ pub struct Global {
     /// empty.
     pub unsigned_tx: Transaction,
     /// Unknown global key-value pairs.
-    pub unknown: HashMap<raw::Key, Vec<u8>>,
+    pub unknown: BTreeMap<raw::Key, Vec<u8>>,
 }
 
 impl Global {
@@ -124,7 +124,7 @@ impl Decodable for Global {
     fn consensus_decode<D: io::Read>(mut d: D) -> Result<Self, encode::Error> {
 
         let mut tx: Option<Transaction> = None;
-        let mut unknowns: HashMap<raw::Key, Vec<u8>> = Default::default();
+        let mut unknowns: BTreeMap<raw::Key, Vec<u8>> = Default::default();
 
         loop {
             match raw::Pair::consensus_decode(&mut d) {
diff --git a/src/util/psbt/map/input.rs b/src/util/psbt/map/input.rs
index b685ff73..32f19bc5 100644
--- a/src/util/psbt/map/input.rs
+++ b/src/util/psbt/map/input.rs
@@ -12,7 +12,7 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 
 use blockdata::script::Script;
 use blockdata::transaction::{SigHashType, Transaction, TxOut};
@@ -38,7 +38,7 @@ pub struct Input {
     pub witness_utxo: Option<TxOut>,
     /// A map from public keys to their corresponding signature as would be
     /// pushed to the stack from a scriptSig or witness.
-    pub partial_sigs: HashMap<PublicKey, Vec<u8>>,
+    pub partial_sigs: BTreeMap<PublicKey, Vec<u8>>,
     /// The sighash type to be used for this input. Signatures for this input
     /// must use the sighash type.
     pub sighash_type: Option<SigHashType>,
@@ -48,7 +48,7 @@ pub struct Input {
     pub witness_script: Option<Script>,
     /// A map from public keys needed to sign this input to their corresponding
     /// master key fingerprints and derivation paths.
-    pub hd_keypaths: HashMap<PublicKey, (Fingerprint, DerivationPath)>,
+    pub hd_keypaths: BTreeMap<PublicKey, (Fingerprint, DerivationPath)>,
     /// The finalized, fully-constructed scriptSig with signatures and any other
     /// scripts necessary for this input to pass validation.
     pub final_script_sig: Option<Script>,
@@ -56,7 +56,7 @@ pub struct Input {
     /// other scripts necessary for this input to pass validation.
     pub final_script_witness: Option<Vec<Vec<u8>>>,
     /// Unknown key-value pairs for this input.
-    pub unknown: HashMap<raw::Key, Vec<u8>>,
+    pub unknown: BTreeMap<raw::Key, Vec<u8>>,
 }
 
 impl Map for Input {
diff --git a/src/util/psbt/map/output.rs b/src/util/psbt/map/output.rs
index f5398fb0..8219e812 100644
--- a/src/util/psbt/map/output.rs
+++ b/src/util/psbt/map/output.rs
@@ -12,7 +12,7 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 
 use blockdata::script::Script;
 use consensus::encode;
@@ -33,9 +33,9 @@ pub struct Output {
     pub witness_script: Option<Script>,
     /// A map from public keys needed to spend this output to their
     /// corresponding master key fingerprints and derivation paths.
-    pub hd_keypaths: HashMap<PublicKey, (Fingerprint, DerivationPath)>,
+    pub hd_keypaths: BTreeMap<PublicKey, (Fingerprint, DerivationPath)>,
     /// Unknown key-value pairs for this output.
-    pub unknown: HashMap<raw::Key, Vec<u8>>,
+    pub unknown: BTreeMap<raw::Key, Vec<u8>>,
 }
 
 impl Map for Output {
diff --git a/src/util/psbt/mod.rs b/src/util/psbt/mod.rs
index 53b8508f..1fc33355 100644
--- a/src/util/psbt/mod.rs
+++ b/src/util/psbt/mod.rs
@@ -165,7 +165,7 @@ mod tests {
     use bitcoin_hashes::hex::FromHex;
     use bitcoin_hashes::sha256d;
 
-    use std::collections::HashMap;
+    use std::collections::BTreeMap;
 
     use hex::decode as hex_decode;
 
@@ -192,7 +192,7 @@ mod tests {
                     input: vec![],
                     output: vec![],
                 },
-                unknown: HashMap::new(),
+                unknown: BTreeMap::new(),
             },
             inputs: vec![],
             outputs: vec![],
@@ -208,7 +208,7 @@ mod tests {
         let secp = &Secp256k1::new();
         let seed = hex_decode("000102030405060708090a0b0c0d0e0f").unwrap();
 
-        let mut hd_keypaths: HashMap<PublicKey, (Fingerprint, DerivationPath)> = Default::default();
+        let mut hd_keypaths: BTreeMap<PublicKey, (Fingerprint, DerivationPath)> = Default::default();
 
         let mut sk: ExtendedPrivKey = ExtendedPrivKey::new_master(Bitcoin, &seed).unwrap();
 
@@ -302,8 +302,15 @@ mod tests {
         assert_eq!(expected, actual);
     }
 
+    #[test]
+    fn deserialize_and_serialize_psbt_with_two_partial_sigs() {
+        let hex = "70736274ff0100890200000001207ae985d787dfe6143d5c58fad79cc7105e0e799fcf033b7f2ba17e62d7b3200000000000ffffffff02563d03000000000022002019899534b9a011043c0dd57c3ff9a381c3522c5f27c6a42319085b56ca543a1d6adc020000000000220020618b47a07ebecca4e156edb1b9ea7c24bdee0139fc049237965ffdaf56d5ee73000000000001012b801a0600000000002200201148e93e9315e37dbed2121be5239257af35adc03ffdfc5d914b083afa44dab82202025fe7371376d53cf8a2783917c28bf30bd690b0a4d4a207690093ca2b920ee076473044022007e06b362e89912abd4661f47945430739b006a85d1b2a16c01dc1a4bd07acab022061576d7aa834988b7ab94ef21d8eebd996ea59ea20529a19b15f0c9cebe3d8ac01220202b3fe93530020a8294f0e527e33fbdff184f047eb6b5a1558a352f62c29972f8a473044022002787f926d6817504431ee281183b8119b6845bfaa6befae45e13b6d430c9d2f02202859f149a6cd26ae2f03a107e7f33c7d91730dade305fe077bae677b5d44952a01010547522102b3fe93530020a8294f0e527e33fbdff184f047eb6b5a1558a352f62c29972f8a21025fe7371376d53cf8a2783917c28bf30bd690b0a4d4a207690093ca2b920ee07652ae0001014752210283ef76537f2d58ae3aa3a4bd8ae41c3f230ccadffb1a0bd3ca504d871cff05e7210353d79cc0cb1396f4ce278d005f16d948e02a6aec9ed1109f13747ecb1507b37b52ae00010147522102b3937241777b6665e0d694e52f9c1b188433641df852da6fc42187b5d8a368a321034cdd474f01cc5aa7ff834ad8bcc882a87e854affc775486bc2a9f62e8f49bd7852ae00";
+        let psbt: PartiallySignedTransaction = hex_psbt!(hex).unwrap();
+        assert_eq!(hex, serialize_hex(&psbt));
+    }
+
     mod bip_vectors {
-        use std::collections::HashMap;
+        use std::collections::BTreeMap;
 
         use hex::decode as hex_decode;
 
@@ -376,7 +383,7 @@ mod tests {
                             },
                         ],
                     },
-                    unknown: HashMap::new(),
+                    unknown: BTreeMap::new(),
                 },
                 inputs: vec![Input {
                     non_witness_utxo: Some(Transaction {
@@ -546,7 +553,7 @@ mod tests {
                 ).unwrap()
             );
 
-            let mut unknown: HashMap<raw::Key, Vec<u8>> = HashMap::new();
+            let mut unknown: BTreeMap<raw::Key, Vec<u8>> = BTreeMap::new();
             let key: raw::Key = raw::Key {
                 type_value: 0x0fu8,
                 key: hex_decode("010203040506070809").unwrap(),
diff --git a/src/util/psbt/raw.rs b/src/util/psbt/raw.rs
index 42c52e44..2568f333 100644
--- a/src/util/psbt/raw.rs
+++ b/src/util/psbt/raw.rs
@@ -23,7 +23,7 @@ use consensus::encode::{self, Decodable, Encodable, VarInt, MAX_VEC_SIZE};
 use util::psbt::Error;
 
 /// A PSBT key in its raw byte form.
-#[derive(Debug, PartialEq, Hash, Eq, Clone)]
+#[derive(Debug, PartialEq, Hash, Eq, Clone, Ord, PartialOrd)]
 pub struct Key {
     /// The type of this PSBT key.
     pub type_value: u8,