Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Validating minimal required gas for a transaction #2937

Merged
merged 4 commits into from
Oct 28, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use receipt::LocalizedReceipt;
use trace::{TraceDB, ImportRequest as TraceImportRequest, LocalizedTrace, Database as TraceDatabase};
use trace;
use trace::FlatTransactionTraces;
use evm::Factory as EvmFactory;
use evm::{Factory as EvmFactory, Schedule};
use miner::{Miner, MinerService};
use snapshot::{self, io as snapshot_io};
use factory::Factories;
Expand Down Expand Up @@ -1141,6 +1141,23 @@ impl BlockChainClient for Client {
}

impl MiningBlockChainClient for Client {

fn latest_schedule(&self) -> Schedule {
let header_data = self.best_block_header();
let view = HeaderView::new(&header_data);

let env_info = EnvInfo {
number: view.number(),
author: view.author(),
timestamp: view.timestamp(),
difficulty: view.difficulty(),
last_hashes: self.build_last_hashes(view.hash()),
gas_used: U256::default(),
gas_limit: view.gas_limit(),
};
self.engine.schedule(&env_info)
}

fn prepare_open_block(&self, author: Address, gas_range_target: (U256, U256), extra_data: Bytes) -> OpenBlock {
let engine = &*self.engine;
let chain = self.chain.read();
Expand Down
8 changes: 6 additions & 2 deletions ethcore/src/client/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use log_entry::LocalizedLogEntry;
use receipt::{Receipt, LocalizedReceipt};
use blockchain::extras::BlockReceipts;
use error::{ImportResult};
use evm::{Factory as EvmFactory, VMType};
use evm::{Factory as EvmFactory, VMType, Schedule};
use miner::{Miner, MinerService, TransactionImportResult};
use spec::Spec;

Expand Down Expand Up @@ -137,7 +137,7 @@ impl TestBlockChainClient {
client.genesis_hash = client.last_hash.read().clone();
client
}

/// Set the transaction receipt result
pub fn set_transaction_receipt(&self, id: TransactionID, receipt: LocalizedReceipt) {
self.receipts.write().insert(id, receipt);
Expand Down Expand Up @@ -306,6 +306,10 @@ pub fn get_temp_state_db() -> GuardedTempResult<StateDB> {
}

impl MiningBlockChainClient for TestBlockChainClient {
fn latest_schedule(&self) -> Schedule {
Schedule::new_homestead_gas_fix()
}

fn prepare_open_block(&self, author: Address, gas_range_target: (U256, U256), extra_data: Bytes) -> OpenBlock {
let engine = &*self.spec.engine;
let genesis_header = self.spec.genesis_header();
Expand Down
5 changes: 4 additions & 1 deletion ethcore/src/client/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use views::{BlockView};
use error::{ImportResult, CallError};
use receipt::LocalizedReceipt;
use trace::LocalizedTrace;
use evm::Factory as EvmFactory;
use evm::{Factory as EvmFactory, Schedule};
use types::ids::*;
use types::trace_filter::Filter as TraceFilter;
use executive::Executed;
Expand Down Expand Up @@ -236,6 +236,9 @@ pub trait MiningBlockChainClient : BlockChainClient {

/// Import sealed block. Skips all verifications.
fn import_sealed_block(&self, block: SealedBlock) -> ImportResult;

/// Returns latest schedule.
fn latest_schedule(&self) -> Schedule;
}

impl IpcConfig for BlockChainClient { }
9 changes: 9 additions & 0 deletions ethcore/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ pub enum TransactionError {
/// Transaction gas price
got: U256,
},
/// Transaction's gas is below currently set minimal gas requirement.
InsufficientGas {
/// Minimal expected gas
minimal: U256,
/// Transaction gas
got: U256,
},
/// Sender doesn't have enough funds to pay for this transaction
InsufficientBalance {
/// Senders balance
Expand Down Expand Up @@ -81,6 +88,8 @@ impl fmt::Display for TransactionError {
LimitReached => "Transaction limit reached".into(),
InsufficientGasPrice { minimal, got } =>
format!("Insufficient gas price. Min={}, Given={}", minimal, got),
InsufficientGas { minimal, got } =>
format!("Insufficient gas. Min={}, Given={}", minimal, got),
InsufficientBalance { balance, cost } =>
format!("Insufficient balance for transaction. Balance={}, Cost={}",
balance, cost),
Expand Down
28 changes: 18 additions & 10 deletions ethcore/src/miner/banning_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,15 @@ impl BanningTransactionQueue {

/// Add to the queue taking bans into consideration.
/// May reject transaction because of the banlist.
pub fn add_with_banlist<F>(
pub fn add_with_banlist<F, G>(
&mut self,
transaction: SignedTransaction,
account_details: &F,
) -> Result<TransactionImportResult, Error> where F: Fn(&Address) -> AccountDetails {
gas_estimator: &G,
) -> Result<TransactionImportResult, Error>
where F: Fn(&Address) -> AccountDetails,
G: Fn(&SignedTransaction) -> U256,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no spaces mixing in with the tabs!!

{
if let Threshold::BanAfter(threshold) = self.ban_threshold {
// NOTE In all checks use direct query to avoid increasing ban timeout.

Expand Down Expand Up @@ -111,7 +115,7 @@ impl BanningTransactionQueue {
}
}
}
self.queue.add(transaction, account_details, TransactionOrigin::External)
self.queue.add(transaction, TransactionOrigin::External, account_details, gas_estimator)
}

/// Ban transaction with given hash.
Expand Down Expand Up @@ -228,6 +232,10 @@ mod tests {
}
}

fn gas_required(_tx: &SignedTransaction) -> U256 {
0.into()
}

fn transaction(action: Action) -> SignedTransaction {
let keypair = Random.generate().unwrap();
Transaction {
Expand Down Expand Up @@ -255,7 +263,7 @@ mod tests {
let mut txq = queue();

// when
txq.queue().add(tx, &default_account_details, TransactionOrigin::External).unwrap();
txq.queue().add(tx, TransactionOrigin::External, &default_account_details, &gas_required).unwrap();

// then
// should also deref to queue
Expand All @@ -271,12 +279,12 @@ mod tests {
let banlist1 = txq.ban_sender(tx.sender().unwrap());
assert!(!banlist1, "Threshold not reached yet.");
// Insert once
let import1 = txq.add_with_banlist(tx.clone(), &default_account_details).unwrap();
let import1 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required).unwrap();
assert_eq!(import1, TransactionImportResult::Current);

// when
let banlist2 = txq.ban_sender(tx.sender().unwrap());
let import2 = txq.add_with_banlist(tx.clone(), &default_account_details);
let import2 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required);

// then
assert!(banlist2, "Threshold should be reached - banned.");
Expand All @@ -295,12 +303,12 @@ mod tests {
let banlist1 = txq.ban_recipient(recipient);
assert!(!banlist1, "Threshold not reached yet.");
// Insert once
let import1 = txq.add_with_banlist(tx.clone(), &default_account_details).unwrap();
let import1 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required).unwrap();
assert_eq!(import1, TransactionImportResult::Current);

// when
let banlist2 = txq.ban_recipient(recipient);
let import2 = txq.add_with_banlist(tx.clone(), &default_account_details);
let import2 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required);

// then
assert!(banlist2, "Threshold should be reached - banned.");
Expand All @@ -317,12 +325,12 @@ mod tests {
let banlist1 = txq.ban_codehash(codehash);
assert!(!banlist1, "Threshold not reached yet.");
// Insert once
let import1 = txq.add_with_banlist(tx.clone(), &default_account_details).unwrap();
let import1 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required).unwrap();
assert_eq!(import1, TransactionImportResult::Current);

// when
let banlist2 = txq.ban_codehash(codehash);
let import2 = txq.add_with_banlist(tx.clone(), &default_account_details);
let import2 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required);

// then
assert!(banlist2, "Threshold should be reached - banned.");
Expand Down
6 changes: 4 additions & 2 deletions ethcore/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,13 +562,15 @@ impl Miner {
balance: chain.latest_balance(a),
};

let schedule = chain.latest_schedule();
let gas_required = |tx: &SignedTransaction| tx.gas_required(&schedule).into();
transactions.into_iter()
.map(|tx| match origin {
TransactionOrigin::Local | TransactionOrigin::RetractedBlock => {
transaction_queue.add(tx, &fetch_account, origin)
transaction_queue.add(tx, origin, &fetch_account, &gas_required)
},
TransactionOrigin::External => {
transaction_queue.add_with_banlist(tx, &fetch_account)
transaction_queue.add_with_banlist(tx, &fetch_account, &gas_required)
}
})
.collect()
Expand Down
Loading