From a33f00621be9898892d43965f7656d572aec962c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 26 Mar 2018 10:25:33 -0400 Subject: [PATCH] Move witness inside of TxIn. This is a rather large breaking API change, but is significantly more sensible. In the "do not allow internal representation to represent an invalid state" category, this ensures that witness cannot have an length other than the number of inputs. Further, it reduces vec propagation, which may help performance in some cases by reducing allocs. Fianlly, this just makes more sense (tm). Witness are a per-input field like the scriptSig, placing them outside of the TxIn is just where they are serialized, not where they logically belong. --- src/blockdata/constants.rs | 4 +-- src/blockdata/transaction.rs | 66 +++++++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/blockdata/constants.rs b/src/blockdata/constants.rs index 5e6cfef..468211f 100644 --- a/src/blockdata/constants.rs +++ b/src/blockdata/constants.rs @@ -61,7 +61,6 @@ fn bitcoin_genesis_tx() -> Transaction { lock_time: 0, input: vec![], output: vec![], - witness: vec![] }; // Inputs @@ -73,7 +72,8 @@ fn bitcoin_genesis_tx() -> Transaction { prev_hash: Default::default(), prev_index: 0xFFFFFFFF, script_sig: in_script, - sequence: MAX_SEQUENCE + sequence: MAX_SEQUENCE, + witness: vec![], }); // Outputs diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs index 4fd4d12..f107791 100644 --- a/src/blockdata/transaction.rs +++ b/src/blockdata/transaction.rs @@ -66,8 +66,14 @@ pub struct TxIn { /// to ignore this feature. This is generally never used since /// the miner behaviour cannot be enforced. pub sequence: u32, + /// Witness data: an array of byte-arrays. + /// Note that this field is *not* (de)serialized with the rest of the TxIn in + /// ConsensusEncodable/ConsennsusDecodable, as it is (de)serialized at the end of the full + /// Transaction. It *is* (de)serialized with the rest of the TxIn in other (de)serializationn + /// routines. + pub witness: Vec> } -serde_struct_impl!(TxIn, prev_hash, prev_index, script_sig, sequence); +serde_struct_impl!(TxIn, prev_hash, prev_index, script_sig, sequence, witness); /// A transaction output, which defines new coins to be created from old ones. #[derive(Clone, PartialEq, Eq, Debug, Hash)] @@ -98,10 +104,8 @@ pub struct Transaction { pub input: Vec, /// List of outputs pub output: Vec, - /// Witness data: for each txin, an array of byte-arrays - pub witness: Vec>> } -serde_struct_impl!(Transaction, version, lock_time, input, output, witness); +serde_struct_impl!(Transaction, version, lock_time, input, output); impl Transaction { /// Computes a "normalized TXID" which does not include any signatures. @@ -111,9 +115,8 @@ impl Transaction { let cloned_tx = Transaction { version: self.version, lock_time: self.lock_time, - input: self.input.iter().map(|txin| TxIn { script_sig: Script::new(), .. *txin }).collect(), + input: self.input.iter().map(|txin| TxIn { script_sig: Script::new(), witness: vec![], .. *txin }).collect(), output: self.output.clone(), - witness: vec![] }; cloned_tx.bitcoin_hash() } @@ -165,7 +168,6 @@ impl Transaction { lock_time: self.lock_time, input: vec![], output: vec![], - witness: vec![] }; // Add all inputs necessary.. if anyone_can_pay { @@ -173,7 +175,8 @@ impl Transaction { prev_hash: self.input[input_index].prev_hash, script_sig: script_pubkey.clone(), prev_index: self.input[input_index].prev_index, - sequence: self.input[input_index].sequence + sequence: self.input[input_index].sequence, + witness: vec![], }]; } else { tx.input = Vec::with_capacity(self.input.len()); @@ -182,7 +185,8 @@ impl Transaction { prev_hash: input.prev_hash, prev_index: input.prev_index, script_sig: if n == input_index { script_pubkey.clone() } else { Script::new() }, - sequence: if n != input_index && (sighash == SigHashType::Single || sighash == SigHashType::None) { 0 } else { input.sequence } + sequence: if n != input_index && (sighash == SigHashType::Single || sighash == SigHashType::None) { 0 } else { input.sequence }, + witness: vec![], }); } } @@ -238,13 +242,39 @@ impl BitcoinHash for Transaction { } } -impl_consensus_encoding!(TxIn, prev_hash, prev_index, script_sig, sequence); impl_consensus_encoding!(TxOut, value, script_pubkey); +impl ConsensusEncodable for TxIn { + fn consensus_encode(&self, s: &mut S) -> Result <(), S::Error> { + try!(self.prev_hash.consensus_encode(s)); + try!(self.prev_index.consensus_encode(s)); + try!(self.script_sig.consensus_encode(s)); + self.sequence.consensus_encode(s) + } +} +impl ConsensusDecodable for TxIn { + fn consensus_decode(d: &mut D) -> Result { + Ok(TxIn { + prev_hash: try!(ConsensusDecodable::consensus_decode(d)), + prev_index: try!(ConsensusDecodable::consensus_decode(d)), + script_sig: try!(ConsensusDecodable::consensus_decode(d)), + sequence: try!(ConsensusDecodable::consensus_decode(d)), + witness: vec![], + }) + } +} + impl ConsensusEncodable for Transaction { fn consensus_encode(&self, s: &mut S) -> Result <(), S::Error> { try!(self.version.consensus_encode(s)); - if self.witness.is_empty() { + let mut have_witness = false; + for input in &self.input { + if !input.witness.is_empty() { + have_witness = true; + break; + } + } + if !have_witness { try!(self.input.consensus_encode(s)); try!(self.output.consensus_encode(s)); } else { @@ -252,8 +282,8 @@ impl ConsensusEncodable for Transaction { try!(1u8.consensus_encode(s)); try!(self.input.consensus_encode(s)); try!(self.output.consensus_encode(s)); - for witness in &self.witness { - try!(witness.consensus_encode(s)); + for input in &self.input { + try!(input.witness.consensus_encode(s)); } } self.lock_time.consensus_encode(s) @@ -275,22 +305,19 @@ impl ConsensusDecodable for Transaction { input: input, output: vec![], lock_time: try!(ConsensusDecodable::consensus_decode(d)), - witness: vec![] }) } // BIP144 input witnesses 1 => { - let input: Vec = try!(ConsensusDecodable::consensus_decode(d)); + let mut input: Vec = try!(ConsensusDecodable::consensus_decode(d)); let output: Vec = try!(ConsensusDecodable::consensus_decode(d)); - let mut witness: Vec>> = Vec::with_capacity(input.len()); - for _ in 0..input.len() { - witness.push(try!(ConsensusDecodable::consensus_decode(d))); + for txin in input.iter_mut() { + txin.witness = try!(ConsensusDecodable::consensus_decode(d)); } Ok(Transaction { version: version, input: input, output: output, - witness: witness, lock_time: try!(ConsensusDecodable::consensus_decode(d)) }) } @@ -306,7 +333,6 @@ impl ConsensusDecodable for Transaction { input: input, output: try!(ConsensusDecodable::consensus_decode(d)), lock_time: try!(ConsensusDecodable::consensus_decode(d)), - witness: vec![] }) } }