From 5cbdaa19547334b09a2a8331ec641a2a6662671a Mon Sep 17 00:00:00 2001 From: lyj729047457 Date: Wed, 13 Nov 2019 18:52:15 +0800 Subject: [PATCH 01/13] fix: release node info lock before update node in status res --- core/src/sync/handler/status.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/core/src/sync/handler/status.rs b/core/src/sync/handler/status.rs index bee07f5c..e298dba9 100644 --- a/core/src/sync/handler/status.rs +++ b/core/src/sync/handler/status.rs @@ -133,18 +133,19 @@ pub fn receive_res( // Update node info // TODO: improve this - let mut node_info_write = node_info.write(); - if !node_info_write.contains_key(&hash) { - trace!(target: "sync", "new node info: hash:{}, bn:{}, bh:{}, td:{}", hash, best_block_num, best_hash, total_difficulty); + { + let mut node_info_write = node_info.write(); + if !node_info_write.contains_key(&hash) { + trace!(target: "sync", "new node info: hash:{}, bn:{}, bh:{}, td:{}", hash, best_block_num, best_hash, total_difficulty); + } + let info_lock = node_info_write + .entry(hash) + .or_insert(RwLock::new(NodeInfo::new())); + let mut info = info_lock.write(); + info.best_block_hash = best_hash; + info.best_block_number = best_block_num; + info.total_difficulty = total_difficulty; } - let info_lock = node_info_write - .entry(hash) - .or_insert(RwLock::new(NodeInfo::new())); - let mut info = info_lock.write(); - info.best_block_hash = best_hash; - info.best_block_number = best_block_num; - info.total_difficulty = total_difficulty; - drop(info); p2p.update_node(&hash); } else { From 7fdbbe09f304ef353784f470e0136d486f77fe1b Mon Sep 17 00:00:00 2001 From: fredericzha Date: Wed, 20 Nov 2019 20:47:14 +0800 Subject: [PATCH 02/13] update release file name --- release | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release b/release index 3eefcb9d..7dea76ed 100644 --- a/release +++ b/release @@ -1 +1 @@ -1.0.0 +1.0.1 From cff5a869519692fd665e7c91c67cd7c63d5617a5 Mon Sep 17 00:00:00 2001 From: fredericzha Date: Mon, 25 Nov 2019 19:07:38 +0800 Subject: [PATCH 03/13] fix: fix bug in backward syncing --- core/src/sync/handler/headers.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/core/src/sync/handler/headers.rs b/core/src/sync/handler/headers.rs index 137cf931..55aa8848 100644 --- a/core/src/sync/handler/headers.rs +++ b/core/src/sync/handler/headers.rs @@ -47,7 +47,7 @@ const NORMAL_REQUEST_SIZE: u32 = 24; const LARGE_REQUEST_SIZE: u32 = 44; const LIGHTNING_REQUEST_SIZE: u32 = 40; const REQUEST_COOLDOWN: u64 = 5000; -const BACKWARD_SYNC_STEP: u64 = NORMAL_REQUEST_SIZE as u64 * 6 - 1; +const DISTANCE_FLAG: u64 = NORMAL_REQUEST_SIZE as u64 * 6 - 1; const FAR_OVERLAPPING_BLOCKS: u64 = 3; const CLOSE_OVERLAPPING_BLOCKS: u64 = 15; const JUMP_SIZE: u64 = 200; @@ -107,12 +107,12 @@ fn prepare_send( match node_info.mode { Mode::Normal => { - if node_best_number >= local_best_number + BACKWARD_SYNC_STEP { + if node_best_number >= local_best_number + DISTANCE_FLAG { if local_best_number > FAR_OVERLAPPING_BLOCKS { from = local_best_number - FAR_OVERLAPPING_BLOCKS; } - } else if local_best_number <= BACKWARD_SYNC_STEP - || node_best_number >= local_best_number - BACKWARD_SYNC_STEP + } else if local_best_number <= DISTANCE_FLAG + || node_best_number >= local_best_number - DISTANCE_FLAG { if local_best_number > CLOSE_OVERLAPPING_BLOCKS { from = local_best_number - CLOSE_OVERLAPPING_BLOCKS; @@ -141,9 +141,10 @@ fn prepare_send( size = LIGHTNING_REQUEST_SIZE; } Mode::Backward => { - if sync_base_number > BACKWARD_SYNC_STEP { - from = sync_base_number - BACKWARD_SYNC_STEP; + if sync_base_number > LARGE_REQUEST_SIZE as u64 { + from = sync_base_number - LARGE_REQUEST_SIZE as u64; } + size = LARGE_REQUEST_SIZE; } Mode::Forward => { from = sync_base_number; From 09b22dc1130ae8ddb666eaa184fa79d9c1bf26bb Mon Sep 17 00:00:00 2001 From: fredericzha Date: Thu, 28 Nov 2019 14:30:22 +0800 Subject: [PATCH 04/13] fix: update transaction import path --- core/src/block.rs | 23 +++---- core/src/executive.rs | 12 ++-- core/src/state/mod.rs | 138 ++++++++++++++++++++++++++++++------------ vms/common/src/lib.rs | 1 + 4 files changed, 119 insertions(+), 55 deletions(-) diff --git a/core/src/block.rs b/core/src/block.rs index 3317d9fa..9dbc11c1 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -338,7 +338,7 @@ impl<'x> OpenBlock<'x> { &mut self, txs: &[SignedTransaction], h: Option, - check_gas: bool, + is_building_block: bool, ) -> Vec> { //TODO: deal with AVM parallelism @@ -357,7 +357,7 @@ impl<'x> OpenBlock<'x> { for apply_result in self.block .state - .apply_batch(&env_info, self.engine.machine(), txs, check_gas) + .apply_batch(&env_info, self.engine.machine(), txs, is_building_block) { let result = match apply_result { Ok(outcome) => { @@ -392,7 +392,7 @@ impl<'x> OpenBlock<'x> { &mut self, t: SignedTransaction, h: Option, - check_gas: bool, + is_building_block: bool, ) -> Result<&Receipt, Error> { if self.block.transactions_set.contains(&t.hash()) { @@ -415,22 +415,23 @@ impl<'x> OpenBlock<'x> { &env_info, self.engine.machine(), &[t.clone()], - check_gas, + is_building_block, )); } else { result.push(self.block.state.apply( &env_info, self.engine.machine(), &t, - check_gas, + is_building_block, )); } } else { - result.push( - self.block - .state - .apply(&env_info, self.engine.machine(), &t, check_gas), - ); + result.push(self.block.state.apply( + &env_info, + self.engine.machine(), + &t, + is_building_block, + )); } match result.pop().unwrap() { @@ -845,7 +846,7 @@ fn push_transactions( for t in transactions { let hash = t.hash(); let start = time::Instant::now(); - block.push_transaction(t.clone(), None)?; + block.push_transaction(t.clone(), None, false)?; let took = start.elapsed(); let took_ms = took.as_secs() * 1000 + took.subsec_nanos() as u64 / 1000000; if took > time::Duration::from_millis(slow_tx) { diff --git a/core/src/executive.rs b/core/src/executive.rs index 029c72c1..1aa581c4 100644 --- a/core/src/executive.rs +++ b/core/src/executive.rs @@ -182,7 +182,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { &'a mut self, txs: &[SignedTransaction], is_local_call: bool, - check_gas: bool, + is_building_block: bool, ) -> Vec> { let _vm_lock = AVM_LOCK.lock().unwrap(); @@ -205,7 +205,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { .add_balance(&sender, &(needed_balance), CleanupMode::NoEmpty); } debug!(target: "vm", "sender: {:?}, balance: {:?}", sender, self.state.balance(&sender).unwrap_or(0.into())); - } else if check_gas && self.info.gas_used + t.gas > self.info.gas_limit { + } else if is_building_block && self.info.gas_used + t.gas > self.info.gas_limit { // check gas limit return vec![Err(From::from(ExecutionError::BlockGasLimitReached { gas_limit: self.info.gas_limit, @@ -344,7 +344,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { t: &SignedTransaction, check_nonce: bool, is_local_call: bool, - check_gas_limit: bool, + is_building_block: bool, ) -> Result { let _vm_lock = VM_LOCK.lock().unwrap(); @@ -385,7 +385,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // Don't check max gas limit for local call. // Local node has the right (and is free) to execute "big" calls with its own resources. // NOTE check gas limit during mining, always try vm execution on import - if !is_local_call && check_gas_limit && t.gas > max_gas_limit { + if !is_local_call && t.gas > max_gas_limit { return Err(From::from(ExecutionError::ExceedMaxGasLimit { max: max_gas_limit, got: t.gas, @@ -393,7 +393,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { } // 2.3 Gas limit should not exceed the remaining gas limit of the current block - if check_gas_limit && self.info.gas_used + t.gas > self.info.gas_limit { + if is_building_block && self.info.gas_used + t.gas > self.info.gas_limit { return Err(From::from(ExecutionError::BlockGasLimitReached { gas_limit: self.info.gas_limit, gas_used: self.info.gas_used, @@ -859,7 +859,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { touched.insert(account); } - total_gas_used = total_gas_used + gas_used; if total_gas_used + self.info.gas_used > self.info.gas_limit { final_results.push(Err(ExecutionError::BlockGasLimitReached { gas_limit: self.info.gas_limit, @@ -867,6 +866,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { gas: t.gas, })); } else { + total_gas_used = total_gas_used + gas_used; final_results.push(Ok(Executed { exception: result.exception, gas: t.gas, diff --git a/core/src/state/mod.rs b/core/src/state/mod.rs index 3e98a889..0f200585 100644 --- a/core/src/state/mod.rs +++ b/core/src/state/mod.rs @@ -45,6 +45,7 @@ use db::StateDB; use transaction::SignedTransaction; use types::state::state_diff::StateDiff; use vms::EnvInfo; +use vms::AvmStatusCode; use aion_types::{Address, H256, U256}; use acore_bytes::Bytes; @@ -669,27 +670,53 @@ impl State { env_info: &EnvInfo, machine: &Machine, t: &SignedTransaction, - check_gas_limit: bool, + is_building_block: bool, ) -> ApplyResult { - let e = self.execute(env_info, machine, t, true, false, check_gas_limit)?; - - self.commit()?; - let state_root = self.root().clone(); - - let receipt = Receipt::new( - state_root, - e.gas_used, - e.transaction_fee, - e.logs, - e.output, - e.exception, - ); - trace!(target: "state", "Transaction receipt: {:?}", receipt); - - Ok(ApplyOutcome { - receipt, - }) + // Only fvm transactions (including precompiled) and balance transfers are executed in this function + let result = self.execute(env_info, machine, t, true, false, is_building_block); + match result { + // Include the transaction into block if the execution result if ok + Ok(e) => { + self.commit()?; + let state_root = self.root().clone(); + let receipt = Receipt::new( + state_root, + e.gas_used, + e.transaction_fee, + e.logs, + e.output, + e.exception, + ); + trace!(target: "state", "Transaction receipt: {:?}", receipt); + Ok(ApplyOutcome { + receipt, + }) + } + // For rejected transactions... + Err(x) => { + // If building local block, reject it + if is_building_block { + Err(From::from(x)) + } + // If importing external block, accept it but do not commit state + else { + let state_root = self.root().clone(); + let receipt = Receipt::new( + state_root, + U256::from(0u64), + U256::from(0u64), + vec![], + vec![], + String::default(), + ); + trace!(target: "state", "Transaction receipt: {:?}", receipt); + Ok(ApplyOutcome { + receipt, + }) + } + } + } } pub fn apply_batch( @@ -697,35 +724,70 @@ impl State { env_info: &EnvInfo, machine: &Machine, txs: &[SignedTransaction], - check_gas: bool, + is_building_block: bool, ) -> Vec { - let exec_results = self.execute_bulk(env_info, machine, txs, false, false, check_gas); + // Only avm transactions will go here + let exec_results = + self.execute_bulk(env_info, machine, txs, false, false, is_building_block); if !exec_results.is_empty() && !exec_results[0].is_ok() { return vec![Err(From::from(exec_results[0].clone().unwrap_err()))]; } let mut receipts = Vec::new(); + let mut index = 0; for result in exec_results { //self.commit_touched(result.clone().unwrap().touched); let outcome = match result { Ok(e) => { - let state_root = e.state_root.clone(); - let receipt = Receipt::new( - state_root, - e.gas_used, - e.transaction_fee, - e.logs, - e.output, - e.exception, - ); - Ok(ApplyOutcome { - receipt, - }) + // For avm rejected transactions, reject if building local blocks, or accept if importing external blocks + if e.exception == AvmStatusCode::Rejected.to_string() && is_building_block { + Err(From::from(ExecutionError::Internal( + "AVM rejected".to_string(), + ))) + } else { + if e.exception == AvmStatusCode::Rejected.to_string() { + println!("AAA"); + } + let state_root = e.state_root.clone(); + let receipt = Receipt::new( + state_root, + e.gas_used, + e.transaction_fee, + e.logs, + e.output, + e.exception, + ); + Ok(ApplyOutcome { + receipt, + }) + } + } + Err(x) => { + // If building local block, reject it + if is_building_block { + Err(From::from(x)) + } + // If importing external block, accept it but do not commit state + else { + let state_root = self.root().clone(); + let receipt = Receipt::new( + state_root, + txs[index].gas, + txs[index].gas * txs[index].gas_price, + vec![], + vec![], + String::default(), + ); + trace!(target: "state", "Transaction receipt: {:?}", receipt); + Ok(ApplyOutcome { + receipt, + }) + } } - Err(x) => Err(From::from(x)), }; receipts.push(outcome); + index += 1; } trace!(target: "state", "Transaction receipt: {:?}", receipts); @@ -740,14 +802,14 @@ impl State { txs: &[SignedTransaction], check_nonce: bool, virt: bool, - check_gas: bool, + is_building_block: bool, ) -> Vec> { let mut e = Executive::new(self, env_info, machine); match virt { true => e.transact_virtual_bulk(txs, check_nonce), - false => e.transact_bulk(txs, false, check_gas), + false => e.transact_bulk(txs, false, is_building_block), } } @@ -762,14 +824,14 @@ impl State { t: &SignedTransaction, check_nonce: bool, virt: bool, - check_gas_limit: bool, + is_building_block: bool, ) -> Result { let mut e = Executive::new(self, env_info, machine); match virt { true => e.transact_virtual(t, check_nonce), - false => e.transact(t, check_nonce, false, check_gas_limit), + false => e.transact(t, check_nonce, false, is_building_block), } } diff --git a/vms/common/src/lib.rs b/vms/common/src/lib.rs index cebdbf44..7bce99a2 100644 --- a/vms/common/src/lib.rs +++ b/vms/common/src/lib.rs @@ -34,3 +34,4 @@ mod test; mod fvm; pub use fvm::*; +pub use avm::*; From 84a4b499479bd83f4bfef0f82648955a902de5bd Mon Sep 17 00:00:00 2001 From: fredericzha Date: Thu, 28 Nov 2019 16:45:18 +0800 Subject: [PATCH 05/13] fix: handle rejected transaction when importing --- core/src/block.rs | 2 +- core/src/executive.rs | 2 +- core/src/state/mod.rs | 41 ++++++++++++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/core/src/block.rs b/core/src/block.rs index 9dbc11c1..e29c956b 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -369,13 +369,13 @@ impl<'x> OpenBlock<'x> { .header .add_transaction_fee(&outcome.receipt.transaction_fee); self.block.receipts.push(outcome.receipt.clone()); - idx += 1; Ok(outcome.receipt) } Err(x) => Err(From::from(x)), }; receipts_results.push(result); + idx += 1; } receipts_results diff --git a/core/src/executive.rs b/core/src/executive.rs index 1aa581c4..275aaa31 100644 --- a/core/src/executive.rs +++ b/core/src/executive.rs @@ -859,7 +859,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { touched.insert(account); } - if total_gas_used + self.info.gas_used > self.info.gas_limit { + if gas_used + total_gas_used + self.info.gas_used > self.info.gas_limit { final_results.push(Err(ExecutionError::BlockGasLimitReached { gas_limit: self.info.gas_limit, gas_used: self.info.gas_used + total_gas_used, diff --git a/core/src/state/mod.rs b/core/src/state/mod.rs index 0f200585..f4eaec28 100644 --- a/core/src/state/mod.rs +++ b/core/src/state/mod.rs @@ -42,7 +42,7 @@ use pod_account::*; use pod_state::{self, PodState}; use receipt::Receipt; use db::StateDB; -use transaction::SignedTransaction; +use transaction::{SignedTransaction, Action}; use types::state::state_diff::StateDiff; use vms::EnvInfo; use vms::AvmStatusCode; @@ -702,13 +702,39 @@ impl State { // If importing external block, accept it but do not commit state else { let state_root = self.root().clone(); + let gas_used = match x { + ExecutionError::InvalidNonce { + expected: _, + got: _, + } + | ExecutionError::NotEnoughCash { + required: _, + got: _, + } => t.gas, + ExecutionError::BlockGasLimitReached { + gas_used: _, + gas_limit: _, + gas: _, + } => { + if let Action::Call(address) = t.action { + match self.code(&address) { + Ok(Some(_)) => U256::from(0u64), + _ => t.gas, + } + } else { + U256::from(0u64) + } + } + _ => U256::from(0u64), + }; + let receipt = Receipt::new( state_root, - U256::from(0u64), - U256::from(0u64), + gas_used, + gas_used * t.gas_price, vec![], vec![], - String::default(), + x.to_string(), ); trace!(target: "state", "Transaction receipt: {:?}", receipt); Ok(ApplyOutcome { @@ -730,9 +756,6 @@ impl State { // Only avm transactions will go here let exec_results = self.execute_bulk(env_info, machine, txs, false, false, is_building_block); - if !exec_results.is_empty() && !exec_results[0].is_ok() { - return vec![Err(From::from(exec_results[0].clone().unwrap_err()))]; - } let mut receipts = Vec::new(); let mut index = 0; @@ -747,7 +770,7 @@ impl State { ))) } else { if e.exception == AvmStatusCode::Rejected.to_string() { - println!("AAA"); + println!("rejected avm transaction: {:?}", txs[index].hash()); } let state_root = e.state_root.clone(); let receipt = Receipt::new( @@ -777,7 +800,7 @@ impl State { txs[index].gas * txs[index].gas_price, vec![], vec![], - String::default(), + x.to_string(), ); trace!(target: "state", "Transaction receipt: {:?}", receipt); Ok(ApplyOutcome { From 126a4475586d81320c44edd0c63b343b3fed866b Mon Sep 17 00:00:00 2001 From: fredericzha Date: Thu, 28 Nov 2019 18:19:30 +0800 Subject: [PATCH 06/13] fix: remove unnecessary gas_left calculation for avm transactions --- core/src/executive.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/src/executive.rs b/core/src/executive.rs index 275aaa31..c0f101d3 100644 --- a/core/src/executive.rs +++ b/core/src/executive.rs @@ -844,11 +844,8 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { for address in &substate.suicides { self.state.kill_account(address); } - let gas_left = match result.status_code { - ExecStatus::Success | ExecStatus::Revert => result.gas_left, - _ => 0.into(), - }; - let gas_used = t.gas - gas_left; + + let gas_used = t.gas - result.gas_left; //TODO: check whether avm has already refunded //let refund_value = gas_left * t.gas_price; @@ -871,7 +868,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { exception: result.exception, gas: t.gas, gas_used: gas_used, - refunded: gas_left, + refunded: result.gas_left, cumulative_gas_used: self.info.gas_used + gas_used, logs: substate.logs, contracts_created: substate.contracts_created, From 85accf12fed6660655001fecd28b7e476ec5640b Mon Sep 17 00:00:00 2001 From: fredericzha Date: Thu, 28 Nov 2019 18:35:43 +0800 Subject: [PATCH 07/13] fix: fix a bug that the verification queue may get blocked when handling two identical blocks --- core/src/verification/queue/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/verification/queue/mod.rs b/core/src/verification/queue/mod.rs index 0db3f039..845c649f 100644 --- a/core/src/verification/queue/mod.rs +++ b/core/src/verification/queue/mod.rs @@ -380,7 +380,7 @@ impl VerificationQueue { let mut verifying = verification.verifying.lock(); let mut idx = None; for (i, e) in verifying.iter_mut().enumerate() { - if e.hash == hash { + if e.hash == hash && e.output.is_none() { idx = Some(i); verification.sizes.verifying.fetch_add( From cfff46d700e4417ca9bea6a87f835fa296187e62 Mon Sep 17 00:00:00 2001 From: Camus Qiu Date: Thu, 28 Nov 2019 18:54:56 +0800 Subject: [PATCH 08/13] add executive tests --- core/src/executive.rs | 2 +- core/src/state/mod.rs | 1 - core/src/tests/executive.rs | 69 +++++++++++++++++++++++++++++++++++-- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/core/src/executive.rs b/core/src/executive.rs index c0f101d3..22cb1e66 100644 --- a/core/src/executive.rs +++ b/core/src/executive.rs @@ -923,7 +923,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { self.state .export_kvdb() .write(batch) - .expect("GRAPH DB write failed"); + .expect("EXTRA DB write failed"); } return final_results; diff --git a/core/src/state/mod.rs b/core/src/state/mod.rs index f4eaec28..ab798486 100644 --- a/core/src/state/mod.rs +++ b/core/src/state/mod.rs @@ -760,7 +760,6 @@ impl State { let mut receipts = Vec::new(); let mut index = 0; for result in exec_results { - //self.commit_touched(result.clone().unwrap().touched); let outcome = match result { Ok(e) => { // For avm rejected transactions, reject if building local blocks, or accept if importing external blocks diff --git a/core/src/tests/executive.rs b/core/src/tests/executive.rs index 2b91f29c..b13fbeca 100644 --- a/core/src/tests/executive.rs +++ b/core/src/tests/executive.rs @@ -32,8 +32,8 @@ use vms::{ ReturnData }; use state::{Substate, CleanupMode}; -use transaction::{Action, Transaction, SignedTransaction, DEFAULT_TRANSACTION_TYPE}; -use types::error::ExecutionError; +use transaction::{Action, Transaction, SignedTransaction, DEFAULT_TRANSACTION_TYPE, AVM_TRANSACTION_TYPE}; +use types::error::{ExecutionError}; use executive::{Executive, contract_address}; use avm_abi::{AVMEncoder, AbiToken, ToBytes}; @@ -1971,6 +1971,71 @@ fn avm_balance_transfer() { assert_eq!(state.balance(¶ms2.address), Ok(99.into())); } +#[test] +fn avm_status_rejected() { + let mut file = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + // NOTE: tested with avm v1.3 + file.push("src/tests/avmjars/demo-0.2.0.jar"); + let file_str = file.to_str().expect("Failed to locate the demo.jar"); + let mut code = read_file(file_str).expect("unable to open avm dapp"); + let mut avm_code: Vec = (code.len() as u32).to_vm_bytes(); + println!("code of hello_avm = {:?}", code.len()); + avm_code.append(&mut code); + + let sender = Address::from_slice(b"cd1722f3947def4cf144679da39c4c32bdc35681"); + let machine = make_aion_machine(); + let mut info = EnvInfo::default(); + info.gas_limit = U256::from(3_000_000); + let mut state = get_temp_state(); + state + .add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty) + .unwrap(); + + // 1. Invalid gas limit + // 1.1 Transaction gas limit exceeds block gas limit + let transaction: Transaction = Transaction::new( + U256::one(), + U256::zero(), + U256::from(4_000_000), + Action::Create, + 0.into(), + avm_code.clone(), + DEFAULT_TRANSACTION_TYPE, + None, + ); + let signed_transaction: SignedTransaction = transaction.fake_sign(sender); + let errors = { + let mut ex = Executive::new(&mut state, &info, &machine); + ex.transact_bulk(&[signed_transaction.clone()], false, true) + }; + assert_eq!( + errors[0].clone().unwrap_err(), + ExecutionError::BlockGasLimitReached { + gas_limit: U256::from(3_000_000), + gas_used: U256::from(0), + gas: U256::from(4_000_000), + } + ); + + let transaction: Transaction = Transaction::new( + U256::zero(), + U256::zero(), + U256::from(2_000_000), + Action::Create, + 0.into(), + avm_code.clone(), + AVM_TRANSACTION_TYPE, + None, + ); + + let signed_transaction: SignedTransaction = transaction.fake_sign(sender); + let errors = { + let mut ex = Executive::new(&mut state, &info, &machine); + ex.transact_bulk(&[signed_transaction], false, true) + }; + assert!(errors[0].is_ok()); +} + #[test] fn contract_create2() { // test internal creation to address with balance From cddd601db82ebd011cbfb41078a0197ef5d746ff Mon Sep 17 00:00:00 2001 From: Camus Qiu Date: Fri, 29 Nov 2019 14:36:26 +0800 Subject: [PATCH 09/13] refine pre tx validator --- core/src/client/client.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 60b8d372..2bbc096d 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -401,6 +401,41 @@ impl Client { let last_hashes = self.build_last_hashes(header.parent_hash().clone()); let db = self.state_db.read().boxed_clone_canon(&parent_hash); + // check transaction nonce and type + match State::from_existing( + db.boxed_clone(), + parent.state_root().clone(), + engine.machine().account_start_nonce(parent.number() + 1), + self.factories.clone(), + self.db.read().clone(), + ) { + Ok(s) => { + let mut nonce_cache = HashMap::::new(); + for t in block.transactions.clone() { + match nonce_cache.clone().get(t.sender()) { + Some(n) => { + if *n != t.nonce { + warn!(target: "client", "Invalid transaction: Tx nonce {} != expected nonce {}\n{:?}", t.nonce, n, t); + return Err(()); + } else { + nonce_cache.insert(*t.sender(), *n + U256::from(1)); + } + } + None => { + nonce_cache.insert( + *t.sender(), + s.nonce(t.sender()).unwrap_or(U256::zero()) + U256::from(1), + ); + } + } + } + } + _ => { + error!(target: "client", "statedb fatal error"); + return Err(()); + } + } + let enact_result = enact_verified( block, engine, From 0cace8503dc4869852698b7c89e8eb16114f15b9 Mon Sep 17 00:00:00 2001 From: fredericzha Date: Fri, 29 Nov 2019 14:41:22 +0800 Subject: [PATCH 10/13] fix: unify transaction gas check --- core/src/transaction/transaction.rs | 45 ++++++++++++----------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/core/src/transaction/transaction.rs b/core/src/transaction/transaction.rs index e0551059..1adb112b 100644 --- a/core/src/transaction/transaction.rs +++ b/core/src/transaction/transaction.rs @@ -60,13 +60,6 @@ pub const AVM_TRANSACTION_TYPE: U256 = U256([2, 0, 0, 0]); pub const BEACON_HASH_EXTENSION: u8 = 1; -struct TransactionEnergyRule; -impl TransactionEnergyRule { - fn is_valid_gas_create(gas: U256) -> bool { (gas >= GAS_CREATE_MIN) && (gas <= GAS_CREATE_MAX) } - - fn is_valid_gas_call(gas: U256) -> bool { gas >= GAS_CALL_MIN && gas <= GAS_CALL_MAX } -} - /// Transaction action type. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Action { @@ -287,6 +280,19 @@ impl Transaction { ) } + /// Get the max gas limit depending on the type of the transaction + pub fn max_gas_limit(&self) -> U256 { + match self.action { + Action::Create => GAS_CREATE_MAX, + Action::Call(_) => GAS_CALL_MAX, + } + } + + /// Check if the gas limit of the transaction is valid + pub fn is_gas_limit_valid(&self) -> bool { + self.gas >= self.gas_required() && self.gas <= self.max_gas_limit() + } + /// Set beacon hash fn _set_beacon(&mut self, hash: H256) { self.beacon = Some(hash) } } @@ -509,25 +515,12 @@ impl UnverifiedTransaction { } // verify energy - match &self.unsigned.action { - Action::Create => { - if !TransactionEnergyRule::is_valid_gas_create(self.gas) { - return Err(error::Error::InvalidContractCreateGas { - minimal: GAS_CREATE_MIN, - maximal: GAS_CREATE_MAX, - got: self.gas, - }); - } - } - Action::Call(_) => { - if !TransactionEnergyRule::is_valid_gas_call(self.gas) { - return Err(error::Error::InvalidTransactionGas { - minimal: GAS_CALL_MIN, - maximal: GAS_CALL_MAX, - got: self.gas, - }); - } - } + if !self.is_gas_limit_valid() { + return Err(error::Error::InvalidTransactionGas { + minimal: self.gas_required(), + maximal: self.max_gas_limit(), + got: self.gas, + }); } // verify energy price From 8cd6ad3c679d83a82510dc72621a8e533a45eb99 Mon Sep 17 00:00:00 2001 From: fredericzha Date: Fri, 29 Nov 2019 15:13:27 +0800 Subject: [PATCH 11/13] fix: nonce check --- core/src/client/client.rs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 2bbc096d..126d83fe 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -412,22 +412,15 @@ impl Client { Ok(s) => { let mut nonce_cache = HashMap::::new(); for t in block.transactions.clone() { - match nonce_cache.clone().get(t.sender()) { - Some(n) => { - if *n != t.nonce { - warn!(target: "client", "Invalid transaction: Tx nonce {} != expected nonce {}\n{:?}", t.nonce, n, t); - return Err(()); - } else { - nonce_cache.insert(*t.sender(), *n + U256::from(1)); - } - } - None => { - nonce_cache.insert( - *t.sender(), - s.nonce(t.sender()).unwrap_or(U256::zero()) + U256::from(1), - ); - } + let expected_nonce: U256 = nonce_cache + .get(t.sender()) + .unwrap_or(&s.nonce(t.sender()).unwrap_or(U256::zero())) + .clone(); + if expected_nonce != t.nonce { + warn!(target: "client", "Stage 4 block verification failed for #{}. Invalid transaction {}: Tx nonce {} != expected nonce {}\n", header.number(), t.hash(), t.nonce, expected_nonce); + return Err(()); } + nonce_cache.insert(t.sender().clone(), t.nonce + U256::from(1u64)); } } _ => { From 576dfcd276140d9cd1b7039c6ff42224c328890f Mon Sep 17 00:00:00 2001 From: fredericzha Date: Fri, 29 Nov 2019 15:35:48 +0800 Subject: [PATCH 12/13] fix: fix unit tests --- core/src/transaction/transaction.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/core/src/transaction/transaction.rs b/core/src/transaction/transaction.rs index 1adb112b..509f17d9 100644 --- a/core/src/transaction/transaction.rs +++ b/core/src/transaction/transaction.rs @@ -722,7 +722,7 @@ mod tests { fn test_verify_basic_success() { let mut t: UnverifiedTransaction = rlp::decode(&::rustc_hex::FromHex::from_hex("f87c80800184646174618800057a9d04e38ebe83030d408398968001b860fdc74311a02604a1171e984d64363a5c6073b7dff9e063d1c2eee84f7364021bbea5d2484bc0adc48f4eff40d2d41ab142f38cc66c7df9051792f03196042dd4d8d200f595f5775bd66417d26ef79e2d2bd8ec6faed5f35f28422c9b3c36f700").unwrap()); // assert!(t.verify_basic(Some(0)).is_err()); - t.unsigned.gas = U256::from(221000); + t.unsigned.gas = U256::from(221256); assert!(t.verify_basic(Some(0)).is_ok()); } @@ -820,8 +820,8 @@ mod tests { assert_eq!( e, error::Error::InvalidTransactionGas { - minimal: GAS_CALL_MIN, - maximal: GAS_CALL_MAX, + minimal: U256::from(21256), + maximal: U256::from(2000000), got: t.gas, } ); @@ -858,8 +858,8 @@ mod tests { assert_eq!( e, error::Error::InvalidTransactionGas { - minimal: GAS_CALL_MIN, - maximal: GAS_CALL_MAX, + minimal: U256::from(21256), + maximal: U256::from(2000000), got: t.gas, } ); @@ -897,9 +897,9 @@ mod tests { let e = r.err().unwrap(); assert_eq!( e, - error::Error::InvalidContractCreateGas { - minimal: GAS_CREATE_MIN, - maximal: GAS_CREATE_MAX, + error::Error::InvalidTransactionGas { + minimal: U256::from(221256), + maximal: U256::from(5000000), got: t.gas, } ); @@ -937,9 +937,9 @@ mod tests { let e = r.err().unwrap(); assert_eq!( e, - error::Error::InvalidContractCreateGas { - minimal: GAS_CREATE_MIN, - maximal: GAS_CREATE_MAX, + error::Error::InvalidTransactionGas { + minimal: U256::from(221256), + maximal: U256::from(5000000), got: t.gas, } ); From 4699555d4e949048762ba8f5348392a9b23c52fa Mon Sep 17 00:00:00 2001 From: fredericzha Date: Fri, 29 Nov 2019 16:06:31 +0800 Subject: [PATCH 13/13] other: upgrade to 1.0.2 --- Cargo.lock | 10 +++++----- Cargo.toml | 2 +- release | 2 +- util/version/Cargo.toml | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa0bd625..97aa6f3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -111,7 +111,7 @@ dependencies = [ "acore 0.1.0", "acore-bytes 0.1.0", "aion-types 0.1.0", - "aion-version 1.0.1", + "aion-version 1.0.2", "ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)", "blake2b 0.1.0", "ethbloom 0.5.1", @@ -155,7 +155,7 @@ dependencies = [ [[package]] name = "aion-version" -version = "1.0.1" +version = "1.0.2" dependencies = [ "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "target_info 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -165,14 +165,14 @@ dependencies = [ [[package]] name = "aionr" -version = "1.0.1" +version = "1.0.2" dependencies = [ "acore 0.1.0", "acore-bytes 0.1.0", "acore-io 0.1.0", "aion-rpc 0.1.0", "aion-types 0.1.0", - "aion-version 1.0.1", + "aion-version 1.0.2", "ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)", "blake2b 0.1.0", "clap 2.32.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1395,7 +1395,7 @@ version = "0.1.0" dependencies = [ "acore-bytes 0.1.0", "aion-types 0.1.0", - "aion-version 1.0.1", + "aion-version 1.0.2", "bincode 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.7 (registry+https://github.com/rust-lang/crates.io-index)", "bytes 0.4.9 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index b0bfc196..b6c37513 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ description = "aion network rust implementation" name = "aionr" # NOTE Make sure to update util/version/Cargo.toml as well -version = "1.0.1" +version = "1.0.2" license = "GPL-3.0" authors = ["aion foundation "] diff --git a/release b/release index 7dea76ed..6d7de6e6 100644 --- a/release +++ b/release @@ -1 +1 @@ -1.0.1 +1.0.2 diff --git a/util/version/Cargo.toml b/util/version/Cargo.toml index 6552f91a..29b60493 100644 --- a/util/version/Cargo.toml +++ b/util/version/Cargo.toml @@ -3,7 +3,7 @@ [package] name = "aion-version" # NOTE: this value is used for Aion version string (via env CARGO_PKG_VERSION) -version = "1.0.1" +version = "1.0.2" authors = ["Aion Foundation "] build = "build.rs"