diff --git a/chain/src/types.rs b/chain/src/types.rs index 754e49eee..47ca289d2 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -16,7 +16,6 @@ use std::io; -use secp; use secp::pedersen::Commitment; use grin_store as store; diff --git a/core/src/consensus.rs b/core/src/consensus.rs index fabeb8bfb..c502bc322 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -26,6 +26,11 @@ use core::target::Difficulty; /// The block subsidy amount pub const REWARD: u64 = 1_000_000_000; +/// Actual block reward for a given total fee amount +pub fn reward(fee: u64) -> u64 { + REWARD + fee / 2 +} + /// Number of blocks before a coinbase matures and can be spent pub const COINBASE_MATURITY: u64 = 1_000; diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 5b1fb67f1..fef3450ad 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -20,7 +20,7 @@ use std::collections::HashSet; use core::Committed; use core::{Input, Output, Proof, TxKernel, Transaction, COINBASE_KERNEL, COINBASE_OUTPUT}; -use consensus::REWARD; +use consensus::{REWARD, reward}; use consensus::MINIMUM_DIFFICULTY; use core::hash::{Hash, Hashed, ZERO_HASH}; use core::target::Difficulty; @@ -42,6 +42,10 @@ pub enum Error { /// The sum of output minus input commitments does not match the sum of /// kernel commitments KernelSumMismatch, + /// Same as above but for the coinbase part of a block, including reward + CoinbaseSumMismatch, + /// Kernel fee can't be odd, due to half fee burning + OddKernelFee, /// Underlying Secp256k1 error (signature validation or invalid public /// key typically) Secp(secp::Error), @@ -237,7 +241,7 @@ impl Committed for Block { &self.outputs } fn overage(&self) -> i64 { - (self.total_fees() as i64) - (REWARD as i64) + ((self.total_fees() / 2) as i64) - (REWARD as i64) } } @@ -264,7 +268,8 @@ impl Block { pubkey: keychain::Identifier, ) -> Result { - let (reward_out, reward_proof) = Block::reward_output(keychain, pubkey)?; + let fees = txs.iter().map(|tx| tx.fee).sum(); + let (reward_out, reward_proof) = Block::reward_output(keychain, pubkey, fees)?; let block = Block::with_reward(prev, txs, reward_out, reward_proof)?; Ok(block) } @@ -278,6 +283,7 @@ impl Block { 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 let secp = Secp256k1::with_caps(secp::ContextFlag::Commit); @@ -420,13 +426,18 @@ impl Block { /// trees, reward, etc. pub fn validate(&self, secp: &Secp256k1) -> Result<(), Error> { self.verify_coinbase(secp)?; - self.verify_kernels(secp)?; + self.verify_kernels(secp, false)?; Ok(()) } /// Validate the sum of input/output commitments match the sum in kernels /// and that all kernel signatures are valid. - pub fn verify_kernels(&self, secp: &Secp256k1) -> Result<(), Error> { + fn verify_kernels(&self, secp: &Secp256k1, skip_sig: bool) -> Result<(), Error> { + for k in &self.kernels { + if k.fee & 1 != 0 { + return Err(Error::OddKernelFee); + } + } // sum all inputs and outs commitments let io_sum = self.sum_commitments(secp)?; @@ -440,8 +451,10 @@ impl Block { } // verify all signatures with the commitment as pk - for proof in &self.kernels { - proof.verify(secp)?; + if !skip_sig { + for proof in &self.kernels { + proof.verify(secp)?; + } } Ok(()) } @@ -452,25 +465,21 @@ impl Block { // * That the sum of blinding factors for all coinbase-marked outputs match // the coinbase-marked kernels. fn verify_coinbase(&self, secp: &Secp256k1) -> Result<(), Error> { - let cb_outs = self.outputs - .iter() - .filter(|out| out.features.contains(COINBASE_OUTPUT)) - .map(|o| o.clone()) - .collect::>(); - let cb_kerns = self.kernels - .iter() - .filter(|k| k.features.contains(COINBASE_KERNEL)) - .map(|k| k.clone()) - .collect::>(); + let cb_outs = filter_map_vec!(self.outputs, |out| { + if out.features.contains(COINBASE_OUTPUT) { Some(out.commitment()) } else { None } + }); + let cb_kerns = filter_map_vec!(self.kernels, |k| { + if k.features.contains(COINBASE_KERNEL) { Some(k.excess) } else { None } + }); - // verifying the kernels on a block composed of just the coinbase outputs - // and kernels checks all we need - Block { - header: BlockHeader::default(), - inputs: vec![], - outputs: cb_outs, - kernels: cb_kerns, - }.verify_kernels(secp) + let over_commit = secp.commit_value(reward(self.total_fees()))?; + let out_adjust_sum = secp.commit_sum(cb_outs, vec![over_commit])?; + + let kerns_sum = secp.commit_sum(cb_kerns, vec![])?; + if kerns_sum != out_adjust_sum { + return Err(Error::CoinbaseSumMismatch); + } + Ok(()) } /// Builds the blinded output and related signature proof for the block @@ -478,16 +487,15 @@ impl Block { pub fn reward_output( keychain: &keychain::Keychain, pubkey: keychain::Identifier, + fees: u64, ) -> Result<(Output, TxKernel), keychain::Error> { + let secp = keychain.secp(); - let msg = secp::Message::from_slice(&[0; secp::constants::MESSAGE_SIZE])?; - let sig = keychain.sign(&msg, &pubkey)?; - let commit = keychain.commit(REWARD, &pubkey)?; - let msg = secp::pedersen::ProofMessage::empty(); + let commit = keychain.commit(reward(fees), &pubkey)?; // let switch_commit = keychain.switch_commit(pubkey)?; - - let rproof = keychain.range_proof(REWARD, &pubkey, commit, msg)?; + let msg = secp::pedersen::ProofMessage::empty(); + let rproof = keychain.range_proof(reward(fees), &pubkey, commit, msg)?; let output = Output { features: COINBASE_OUTPUT, @@ -495,9 +503,12 @@ impl Block { proof: rproof, }; - let over_commit = try!(secp.commit_value(REWARD as u64)); + let over_commit = secp.commit_value(reward(fees))?; let out_commit = output.commitment(); - let excess = try!(secp.commit_sum(vec![out_commit], vec![over_commit])); + let excess = secp.commit_sum(vec![out_commit], vec![over_commit])?; + + let msg = secp::Message::from_slice(&[0; secp::constants::MESSAGE_SIZE])?; + let sig = keychain.sign(&msg, &pubkey)?; let proof = TxKernel { features: COINBASE_KERNEL, @@ -529,7 +540,7 @@ mod test { // utility producing a transaction that spends an output with the provided // value and blinding key fn txspend1i1o(v: u64, keychain: &Keychain, pk1: Identifier, pk2: Identifier) -> Transaction { - build::transaction(vec![input(v, pk1), output(3, pk2), with_fee(1)], &keychain) + build::transaction(vec![input(v, pk1), output(3, pk2), with_fee(2)], &keychain) .map(|(tx, _)| tx) .unwrap() } @@ -544,13 +555,13 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(5, pk1), output(4, pk2.clone()), with_fee(1)], + vec![input(7, pk1), output(5, pk2.clone()), with_fee(2)], &keychain, ).unwrap(); // spending tx2 - reuse pk2 - let mut btx3 = txspend1i1o(4, &keychain, pk2.clone(), pk3); + let mut btx3 = txspend1i1o(5, &keychain, pk2.clone(), pk3); let b = new_block(vec![&mut btx1, &mut btx2, &mut btx3], &keychain); // block should have been automatically compacted (including reward @@ -572,12 +583,12 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(5, pk1), output(4, pk2.clone()), with_fee(1)], + vec![input(7, pk1), output(5, pk2.clone()), with_fee(2)], &keychain, ).unwrap(); // spending tx2 - reuse pk2 - let mut btx3 = txspend1i1o(4, &keychain, pk2.clone(), pk3); + let mut btx3 = txspend1i1o(5, &keychain, pk2.clone(), pk3); let b1 = new_block(vec![&mut btx1, &mut btx2], &keychain); b1.validate(&keychain.secp()).unwrap(); @@ -632,13 +643,13 @@ mod test { assert_eq!( b.verify_coinbase(&keychain.secp()), - Err(Error::KernelSumMismatch) + Err(Error::CoinbaseSumMismatch) ); - assert_eq!(b.verify_kernels(&keychain.secp()), Ok(())); + assert_eq!(b.verify_kernels(&keychain.secp(), false), Ok(())); assert_eq!( b.validate(&keychain.secp()), - Err(Error::KernelSumMismatch) + Err(Error::CoinbaseSumMismatch) ); } @@ -656,7 +667,7 @@ mod test { b.verify_coinbase(&keychain.secp()), Err(Error::Secp(secp::Error::IncorrectCommitSum)) ); - assert_eq!(b.verify_kernels(&keychain.secp()), Ok(())); + assert_eq!(b.verify_kernels(&keychain.secp(), true), Ok(())); assert_eq!( b.validate(&keychain.secp()), diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index 0216a884d..877b2b4a6 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -219,7 +219,7 @@ mod test { let mut vec = Vec::new(); ser::serialize(&mut vec, &tx).expect("serialization failed"); let dtx: Transaction = ser::deserialize(&mut &vec[..]).unwrap(); - assert_eq!(dtx.fee, 1); + assert_eq!(dtx.fee, 2); assert_eq!(dtx.inputs.len(), 2); assert_eq!(dtx.outputs.len(), 1); assert_eq!(tx.hash(), dtx.hash()); @@ -305,7 +305,7 @@ mod test { // Alice builds her transaction, with change, which also produces the sum // of blinding factors before they're obscured. let (tx, sum) = build::transaction( - vec![in1, in2, output(1, pk3), with_fee(1)], + vec![in1, in2, output(1, pk3), with_fee(2)], &keychain, ).unwrap(); tx_alice = tx; @@ -317,7 +317,7 @@ mod test { // ready for broadcast. let (tx_final, _) = build::transaction( - vec![initial_tx(tx_alice), with_excess(blind_sum), output(5, pk4)], + vec![initial_tx(tx_alice), with_excess(blind_sum), output(4, pk4)], &keychain, ).unwrap(); @@ -386,7 +386,7 @@ mod test { let pk3 = keychain.derive_pubkey(3).unwrap(); build::transaction( - vec![input(10, pk1), input(11, pk2), output(20, pk3), with_fee(1)], + vec![input(10, pk1), input(11, pk2), output(19, pk3), with_fee(2)], &keychain, ).map(|(tx, _)| tx).unwrap() } @@ -398,7 +398,7 @@ mod test { let pk2 = keychain.derive_pubkey(2).unwrap(); build::transaction( - vec![input(5, pk1), output(4, pk2), with_fee(1)], + vec![input(5, pk1), output(3, pk2), with_fee(2)], &keychain, ).map(|(tx, _)| tx).unwrap() } diff --git a/core/src/core/pmmr.rs b/core/src/core/pmmr.rs index 505313c18..2339a24b4 100644 --- a/core/src/core/pmmr.rs +++ b/core/src/core/pmmr.rs @@ -181,6 +181,10 @@ where /// implementation. fn append(&mut self, position: u64, data: Vec>) -> Result<(), String>; + /// Rewind the backend state to a previous position, as if all append + /// operations after that had been canceled. Expects a position in the PMMR + /// to rewind to as well as the consumer-provided index of when the change + /// occurred (see remove). fn rewind(&mut self, position: u64, index: u32) -> Result<(), String>; /// Get a HashSum by insertion position @@ -382,6 +386,7 @@ pub struct VecBackend where T: Summable + Clone, { + /// Backend elements pub elems: Vec>>, } @@ -397,6 +402,7 @@ where fn get(&self, position: u64) -> Option> { self.elems[(position - 1) as usize].clone() } + #[allow(unused_variables)] fn remove(&mut self, positions: Vec, index: u32) -> Result<(), String> { for n in positions { self.elems[(n - 1) as usize] = None @@ -971,7 +977,7 @@ mod test { { let mut pmmr = PMMR::at(&mut ba, sz); for n in 1..16 { - pmmr.prune(n, 0); + let _ = pmmr.prune(n, 0); } assert_eq!(orig_root, pmmr.root()); } diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index fce651a85..bdb1a8816 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -34,6 +34,22 @@ bitflags! { } } +/// Errors thrown by Block validation +#[derive(Debug, PartialEq)] +pub enum Error { + /// Transaction fee can't be odd, due to half fee burning + OddFee, + /// Underlying Secp256k1 error (signature validation or invalid public + /// key typically) + Secp(secp::Error), +} + +impl From for Error { + fn from(e: secp::Error) -> Error { + Error::Secp(e) + } +} + /// A proof that a transaction sums to zero. Includes both the transaction's /// Pedersen commitment and the signature, that guarantees that the commitments /// amount to zero. The signature signs the fee, which is retained for @@ -245,11 +261,14 @@ impl Transaction { /// 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) + self.verify_sig(secp).map_err(&From::from) } } diff --git a/grin/src/miner.rs b/grin/src/miner.rs index 35c2a3ed4..a14166dd4 100644 --- a/grin/src/miner.rs +++ b/grin/src/miner.rs @@ -29,7 +29,7 @@ use core::core; use core::core::Proof; use pow::cuckoo; use core::core::target::Difficulty; -use core::core::{Block, BlockHeader}; +use core::core::{Block, BlockHeader, Transaction}; use core::core::hash::{Hash, Hashed}; use pow::MiningWorker; use pow::types::MinerConfig; @@ -43,7 +43,7 @@ use secp; use pool; use util; use keychain::Keychain; -use wallet::{CbAmount, WalletReceiveRequest, CbData}; +use wallet::{BlockFees, WalletReceiveRequest, CbData}; use pow::plugin::PluginMiner; @@ -428,13 +428,16 @@ impl Miner { )); } - let mut coinbase = self.get_coinbase(); + // to prevent the wallet from generating a new HD key derivation for each + // iteration, we keep the returned derivation to provide it back when + // nothing has changed + let mut derivation = 0; loop { // get the latest chain state and build a block on top of it let head = self.chain.head_header().unwrap(); let mut latest_hash = self.chain.head().unwrap().last_block_h; - let mut b = self.build_block(&head, coinbase.clone()); + let (mut b, deriv) = self.build_block(&head, derivation); let mut sol = None; let mut use_async = false; @@ -496,34 +499,37 @@ impl Miner { self.debug_output_id, e ); - } else { - coinbase = self.get_coinbase(); } + derivation = 0; + } else { + derivation = deriv; } } } /// Builds a new block with the chain head as previous and eligible /// transactions from the pool. - fn build_block( - &self, - head: &core::BlockHeader, - coinbase: (core::Output, core::TxKernel), - ) -> core::Block { + fn build_block(&self, head: &core::BlockHeader, deriv: u32) -> (core::Block, u32) { + // prepare the block header timestamp let mut now_sec = time::get_time().sec; let head_sec = head.timestamp.to_timespec().sec; if now_sec == head_sec { now_sec += 1; } + // get the difficulty our block should be at let diff_iter = self.chain.difficulty_iter(); let difficulty = consensus::next_difficulty(diff_iter).unwrap(); + // extract current transaction from the pool let txs_box = self.tx_pool.read().unwrap().prepare_mineable_transactions( MAX_TX, ); - let txs = txs_box.iter().map(|tx| tx.as_ref()).collect(); - let (output, kernel) = coinbase; + let txs: Vec<&Transaction> = txs_box.iter().map(|tx| tx.as_ref()).collect(); + + // build the coinbase and the block itself + let fees = txs.iter().map(|tx| tx.fee).sum(); + let (output, kernel, deriv) = self.get_coinbase(fees, deriv); let mut b = core::Block::with_reward(head, txs, output, kernel).unwrap(); debug!( "(Server ID: {}) Built new block with {} inputs and {} outputs, difficulty: {}", @@ -544,20 +550,21 @@ impl Miner { self.chain.set_sumtree_roots(&mut b).expect( "Error setting sum tree roots", ); - b + (b, deriv) } - fn get_coinbase(&self) -> (core::Output, core::TxKernel) { + fn get_coinbase(&self, fees: u64, derivation: u32) -> (core::Output, core::TxKernel, u32) { if self.config.burn_reward { let keychain = Keychain::from_random_seed().unwrap(); let pubkey = keychain.derive_pubkey(1).unwrap(); - core::Block::reward_output(&keychain, pubkey).unwrap() + let (out, kern) = core::Block::reward_output(&keychain, pubkey, fees).unwrap(); + (out, kern, 0) } else { let url = format!( "{}/v1/receive/coinbase", self.config.wallet_receiver_url.as_str() ); - let request = WalletReceiveRequest::Coinbase(CbAmount { amount: consensus::REWARD }); + let request = WalletReceiveRequest::Coinbase(BlockFees { fees: fees, derivation: derivation }); let res: CbData = api::client::post(url.as_str(), &request).expect( format!( "(Server ID: {}) Wallet receiver unreachable, could not claim reward. Is it running?", @@ -570,7 +577,7 @@ impl Miner { let output = ser::deserialize(&mut &out_bin[..]).unwrap(); let kernel = ser::deserialize(&mut &kern_bin[..]).unwrap(); - (output, kernel) + (output, kernel, res.derivation) } } } diff --git a/wallet/src/lib.rs b/wallet/src/lib.rs index fc1c1e65d..a931d4236 100644 --- a/wallet/src/lib.rs +++ b/wallet/src/lib.rs @@ -39,4 +39,4 @@ mod types; pub use info::show_info; pub use receiver::{WalletReceiver, receive_json_tx}; pub use sender::issue_send_tx; -pub use types::{WalletConfig, WalletReceiveRequest, CbAmount, CbData}; +pub use types::{WalletConfig, WalletReceiveRequest, BlockFees, CbData}; diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index d09f0c5b9..63dc82dae 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -49,7 +49,7 @@ //! double-exchange will be required as soon as we support Schnorr signatures. //! So we may as well have it in place already. - +use core::consensus::reward; use core::core::{Block, Transaction, TxKernel, Output, build}; use core::ser; use api::{self, ApiEndpoint, Operation, ApiResult}; @@ -106,16 +106,14 @@ impl ApiEndpoint for WalletReceiver { match op.as_str() { "coinbase" => { match input { - WalletReceiveRequest::Coinbase(cb_amount) => { - debug!("Operation {} with amount {}", op, cb_amount.amount); - if cb_amount.amount == 0 { - return Err(api::Error::Argument(format!("Zero amount not allowed."))); - } - let (out, kern) = + WalletReceiveRequest::Coinbase(cb_fees) => { + debug!("Operation {} with amount {}", op, cb_fees.fees); + let (out, kern, derivation) = receive_coinbase( &self.config, &self.keychain, - cb_amount.amount, + cb_fees.fees, + cb_fees.derivation, ).map_err(|e| { api::Error::Internal(format!("Error building coinbase: {:?}", e)) })?; @@ -130,6 +128,7 @@ impl ApiEndpoint for WalletReceiver { Ok(CbData { output: util::to_hex(out_bin), kernel: util::to_hex(kern_bin), + derivation: derivation, }) } _ => Err(api::Error::Argument( @@ -149,6 +148,7 @@ impl ApiEndpoint for WalletReceiver { Ok(CbData { output: String::from(""), kernel: String::from(""), + derivation: 0, }) } _ => Err(api::Error::Argument( @@ -165,20 +165,24 @@ impl ApiEndpoint for WalletReceiver { fn receive_coinbase( config: &WalletConfig, keychain: &Keychain, - amount: u64, -) -> Result<(Output, TxKernel), Error> { + fees: u64, + mut derivation: u32, +) -> Result<(Output, TxKernel, u32), Error> { + let fingerprint = keychain.clone().fingerprint(); // operate within a lock on wallet data WalletData::with_wallet(&config.data_file_dir, |wallet_data| { - let derivation = wallet_data.next_child(fingerprint.clone()); + if derivation == 0 { + derivation = wallet_data.next_child(fingerprint.clone()); + } let pubkey = keychain.derive_pubkey(derivation)?; // track the new output and return the stuff needed for reward wallet_data.append_output(OutputData { fingerprint: fingerprint.clone(), n_child: derivation, - value: amount, + value: reward(fees), status: OutputStatus::Unconfirmed, height: 0, lock_height: 0, @@ -186,8 +190,8 @@ fn receive_coinbase( debug!("Received coinbase and built output - {}, {}, {}", fingerprint.clone(), pubkey.fingerprint(), derivation); - let result = Block::reward_output(&keychain, pubkey)?; - Ok(result) + let (out, kern) = Block::reward_output(&keychain, pubkey, fees)?; + Ok((out, kern, derivation)) })? } @@ -199,6 +203,7 @@ fn receive_transaction( blinding: BlindingFactor, partial: Transaction, ) -> Result { + let fingerprint = keychain.clone().fingerprint(); // operate within a lock on wallet data diff --git a/wallet/src/types.rs b/wallet/src/types.rs index 61bf73a4f..621c8de54 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -23,7 +23,7 @@ use serde_json; use secp; use api; -use core::core::Transaction; +use core::core::{Transaction, transaction}; use core::ser; use keychain; use util; @@ -36,6 +36,7 @@ const LOCK_FILE: &'static str = "wallet.lock"; pub enum Error { NotEnoughFunds(u64), Keychain(keychain::Error), + Transaction(transaction::Error), Secp(secp::Error), WalletData(String), /// An error in the format of the JSON structures exchanged by the wallet @@ -52,6 +53,10 @@ impl From for Error { fn from(e: secp::Error) -> Error { Error::Secp(e) } } +impl From for Error { + fn from(e: transaction::Error) -> Error { Error::Transaction(e) } +} + impl From for Error { fn from(e: serde_json::Error) -> Error { Error::Format(e.to_string()) } } @@ -268,8 +273,7 @@ impl WalletData { } /// Select a subset of unspent outputs to spend in a transaction - /// transferring - /// the provided amount. + /// transferring the provided amount. pub fn select( &self, fingerprint: keychain::Fingerprint, @@ -356,15 +360,16 @@ pub fn partial_tx_from_json( /// Amount in request to build a coinbase output. #[derive(Serialize, Deserialize, Debug, Clone)] pub enum WalletReceiveRequest { - Coinbase(CbAmount), + Coinbase(BlockFees), PartialTransaction(String), Finalize(String), } -/// Amount in request to build a coinbase output. +/// Fees in block to use for coinbase amount calculation #[derive(Serialize, Deserialize, Debug, Clone)] -pub struct CbAmount { - pub amount: u64, +pub struct BlockFees { + pub fees: u64, + pub derivation: u32, } /// Response to build a coinbase output. @@ -372,4 +377,5 @@ pub struct CbAmount { pub struct CbData { pub output: String, pub kernel: String, + pub derivation: u32, }