From ad07866275648efef9dd29fe4ca09db29133120b Mon Sep 17 00:00:00 2001 From: AntiochP <30642645+antiochp@users.noreply.github.com> Date: Fri, 3 Nov 2017 14:17:09 -0400 Subject: [PATCH] iterate once over txs in a block (#233) Split out verify_sig and build_kernel to make kernel explicit. --- core/src/core/block.rs | 64 +++++++++++++++++++++++++----------- core/src/core/transaction.rs | 40 ++++++++++------------ 2 files changed, 62 insertions(+), 42 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index e059fa87e..b0c520477 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -25,6 +25,7 @@ use core::{Input, Output, Proof, SwitchCommitHash, Transaction, TxKernel, COINBA use consensus::{exceeds_weight, reward, MINIMUM_DIFFICULTY, REWARD}; use core::hash::{Hash, Hashed, ZERO_HASH}; use core::target::Difficulty; +use core::transaction; use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer}; use util::LOGGER; use global; @@ -47,8 +48,18 @@ pub enum Error { /// The lock_height causing this validation error lock_height: u64, }, + /// Underlying tx related error + Transaction(transaction::Error), /// Underlying Secp256k1 error (signature validation or invalid public key typically) Secp(secp::Error), + /// Underlying keychain related error + Keychain(keychain::Error), +} + +impl From for Error { + fn from(e: transaction::Error) -> Error { + Error::Transaction(e) + } } impl From for Error { @@ -57,6 +68,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: keychain::Error) -> Error { + Error::Keychain(e) + } +} + /// Block header, fairly standard compared to other blockchains. #[derive(Clone, Debug, PartialEq)] pub struct BlockHeader { @@ -266,7 +283,7 @@ impl Block { txs: Vec<&Transaction>, keychain: &keychain::Keychain, key_id: &keychain::Identifier, - ) -> Result { + ) -> Result { let fees = txs.iter().map(|tx| tx.fee).sum(); let (reward_out, reward_proof) = Block::reward_output(keychain, key_id, fees)?; let block = Block::with_reward(prev, txs, reward_out, reward_proof)?; @@ -281,30 +298,37 @@ impl Block { txs: Vec<&Transaction>, reward_out: Output, reward_kern: TxKernel, - ) -> Result { - // note: the following reads easily but may not be the most efficient due to - // repeated iterations, revisit if a problem + ) -> Result { let secp = Secp256k1::with_caps(secp::ContextFlag::Commit); - // validate each transaction and gather their kernels - let mut kernels = try_map_vec!(txs, |tx| tx.verify_sig(&secp)); - kernels.push(reward_kern); + let mut kernels = vec![]; + let mut inputs = vec![]; + let mut outputs = vec![]; - // build vectors with all inputs and all outputs, ordering them by hash - // needs to be a fold so we don't end up with a vector of vectors and we - // want to fully own the refs (not just a pointer like flat_map). - let mut inputs = txs.iter().fold(vec![], |mut acc, ref tx| { - let mut inputs = tx.inputs.clone(); - acc.append(&mut inputs); - acc - }); - let mut outputs = txs.iter().fold(vec![], |mut acc, ref tx| { - let mut outputs = tx.outputs.clone(); - acc.append(&mut outputs); - acc - }); + // iterate over the all the txs + // build the kernel for each + // and collect all the kernels, inputs and outputs + // to build the block (which we can sort of think of as one big tx?) + for tx in txs { + // validate each transaction and gather their kernels + let excess = tx.validate(&secp)?; + let kernel = tx.build_kernel(excess); + kernels.push(kernel); + + for input in tx.inputs.clone() { + inputs.push(input); + } + + for output in tx.outputs.clone() { + outputs.push(output); + } + } + + // also include the reward kernel and output + kernels.push(reward_kern); outputs.push(reward_out); + // now sort everything to the block is built deterministically inputs.sort(); outputs.sort(); kernels.sort(); diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index bcf5c6452..57b21f39f 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -26,7 +26,6 @@ use core::hash::Hashed; use core::pmmr::Summable; use keychain::{Identifier, Keychain}; use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer}; -use util::LOGGER; /// The size to use for the stored blake2 hash of a switch_commitment pub const SWITCH_COMMIT_HASH_SIZE: usize = 20; @@ -65,7 +64,7 @@ macro_rules! hashable_ord { } /// Errors thrown by Block validation -#[derive(Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum Error { /// Transaction fee can't be odd, due to half fee burning OddFee, @@ -305,49 +304,46 @@ impl Transaction { /// sum to zero as they should in r.G + v.H then only k.G the excess /// of the sum of r.G should be left. And r.G is the definition of a /// public key generated using r as a private key. - pub fn verify_sig(&self, secp: &Secp256k1) -> Result { + pub fn verify_sig(&self, secp: &Secp256k1) -> Result { let rsum = self.sum_commitments(secp)?; let msg = Message::from_slice(&kernel_sig_msg(self.fee, self.lock_height))?; let sig = Signature::from_der(secp, &self.excess_sig)?; // pretend the sum is a public key (which it is, being of the form r.G) and - // verify the transaction sig with it - // - // we originally converted the commitment to a key_id here (commitment to zero) - // and then passed the key_id to secp.verify() - // the secp api no longer allows us to do this so we have wrapped the complexity - // of generating a public key from a commitment behind verify_from_commit + // verify the transaction sig with it + // + // we originally converted the commitment to a key_id here (commitment to zero) + // and then passed the key_id to secp.verify() + // the secp api no longer allows us to do this so we have wrapped the complexity + // of generating a public key from a commitment behind verify_from_commit secp.verify_from_commit(&msg, &sig, &rsum)?; - let kernel = TxKernel { + Ok(rsum) + } + + pub fn build_kernel(&self, excess: Commitment) -> TxKernel { + TxKernel { features: DEFAULT_KERNEL, - excess: rsum, + excess: excess, excess_sig: self.excess_sig.clone(), fee: self.fee, lock_height: self.lock_height, - }; - debug!( - LOGGER, - "tx verify_sig: fee - {}, lock_height - {}", - kernel.fee, - kernel.lock_height - ); - - Ok(kernel) + } } /// Validates all relevant parts of a fully built transaction. Checks the /// excess value against the signature as well as range proofs for each /// output. - pub fn validate(&self, secp: &Secp256k1) -> Result { + pub fn validate(&self, secp: &Secp256k1) -> Result { if self.fee & 1 != 0 { return Err(Error::OddFee); } for out in &self.outputs { out.verify_proof(secp)?; } - self.verify_sig(secp).map_err(&From::from) + let excess = self.verify_sig(secp)?; + Ok(excess) } }