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

Commit

Permalink
Merge branch 'js' into jg-dapp-signaturereg
Browse files Browse the repository at this point in the history
* js:
  user defaults (#2014)
  Fixing jit feature compilation (#2310)
  Lenient bytes deserialization (#2036)
  Fixing tests
  saturating add
  Remove crufty code
  saturating not overflowing
  Peek transaction queue via RPC (#2270)
  Avoid penalizing legit transactions
  Penalize transactions with gas above gas limit
  Improving txqueue logs
  • Loading branch information
jacogr committed Sep 26, 2016
2 parents efe4491 + 4bde28a commit 3b80451
Show file tree
Hide file tree
Showing 23 changed files with 535 additions and 255 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ json-ipc-server = { git = "https://github.com/ethcore/json-ipc-server.git" }
ethcore-dapps = { path = "dapps", optional = true }
clippy = { version = "0.0.90", optional = true}
ethcore-stratum = { path = "stratum" }
serde = "0.8.0"
serde_json = "0.8.0"

[target.'cfg(windows)'.dependencies]
winapi = "0.2"
Expand All @@ -61,6 +63,7 @@ ui = ["dapps", "ethcore-signer/ui"]
use-precompiled-js = ["ethcore-dapps/use-precompiled-js", "ethcore-signer/use-precompiled-js"]
dapps = ["ethcore-dapps"]
ipc = ["ethcore/ipc"]
jit = ["ethcore/jit"]
dev = ["clippy", "ethcore/dev", "ethcore-util/dev", "ethsync/dev", "ethcore-rpc/dev", "ethcore-dapps/dev", "ethcore-signer/dev"]
json-tests = ["ethcore/json-tests"]
stratum = ["ipc"]
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl Client {

let db = Arc::new(try!(Database::open(&db_config, &path.to_str().unwrap()).map_err(ClientError::Database)));
let chain = Arc::new(BlockChain::new(config.blockchain.clone(), &gb, db.clone()));
let tracedb = RwLock::new(try!(TraceDB::new(config.tracing.clone(), db.clone(), chain.clone())));
let tracedb = RwLock::new(TraceDB::new(config.tracing.clone(), db.clone(), chain.clone()));

let mut state_db = journaldb::new(db.clone(), config.pruning, ::db::COL_STATE);
if state_db.is_empty() && try!(spec.ensure_db_good(state_db.as_hashdb_mut())) {
Expand Down Expand Up @@ -687,7 +687,7 @@ impl snapshot::DatabaseRestore for Client {

*state_db = journaldb::new(db.clone(), self.pruning, ::db::COL_STATE);
*chain = Arc::new(BlockChain::new(self.config.blockchain.clone(), &[], db.clone()));
*tracedb = try!(TraceDB::new(self.config.tracing.clone(), db.clone(), chain.clone()).map_err(ClientError::from));
*tracedb = TraceDB::new(self.config.tracing.clone(), db.clone(), chain.clone());
Ok(())
}
}
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::str::FromStr;
pub use std::time::Duration;
pub use block_queue::BlockQueueConfig;
pub use blockchain::Config as BlockChainConfig;
pub use trace::{Config as TraceConfig, Switch};
pub use trace::Config as TraceConfig;
pub use evm::VMType;
pub use verification::VerifierType;
use util::{journaldb, CompactionProfile};
Expand Down Expand Up @@ -102,7 +102,7 @@ pub struct ClientConfig {
/// State db compaction profile
pub db_compaction: DatabaseCompactionProfile,
/// Should db have WAL enabled?
pub db_wal: bool,
pub db_wal: bool,
/// Operating mode
pub mode: Mode,
/// Type of block verifier used by client.
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ mod trace;
mod client;

pub use self::client::*;
pub use self::config::{Mode, ClientConfig, DatabaseCompactionProfile, BlockQueueConfig, BlockChainConfig, Switch, VMType};
pub use self::config::{Mode, ClientConfig, DatabaseCompactionProfile, BlockQueueConfig, BlockChainConfig, VMType};
pub use self::error::Error;
pub use types::ids::*;
pub use self::test_client::{TestBlockChainClient, EachBlockWith};
Expand Down
19 changes: 15 additions & 4 deletions ethcore/src/evm/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use common::*;
use evmjit;
use evm::{self, GasLeft};
use types::executed::CallType;

/// Should be used to convert jit types to ethcore
trait FromJit<T>: Sized {
Expand Down Expand Up @@ -77,10 +78,11 @@ impl IntoJit<evmjit::I256> for U256 {
impl IntoJit<evmjit::I256> for H256 {
fn into_jit(self) -> evmjit::I256 {
let mut ret = [0; 4];
for i in 0..self.bytes().len() {
let rev = self.bytes().len() - 1 - i;
let len = self.len();
for i in 0..len {
let rev = len - 1 - i;
let pos = rev / 8;
ret[pos] += (self.bytes()[i] as u64) << ((rev % 8) * 8);
ret[pos] += (self[i] as u64) << ((rev % 8) * 8);
}
evmjit::I256 { words: ret }
}
Expand Down Expand Up @@ -206,6 +208,7 @@ impl<'a> evmjit::Ext for ExtAdapter<'a> {
let sender_address = unsafe { Address::from_jit(&*sender_address) };
let receive_address = unsafe { Address::from_jit(&*receive_address) };
let code_address = unsafe { Address::from_jit(&*code_address) };
// TODO Is it always safe in case of DELEGATE_CALL?
let transfer_value = unsafe { U256::from_jit(&*transfer_value) };
let value = Some(transfer_value);

Expand Down Expand Up @@ -239,14 +242,22 @@ impl<'a> evmjit::Ext for ExtAdapter<'a> {
}
}

// TODO [ToDr] Any way to detect DelegateCall?
let call_type = match is_callcode {
true => CallType::CallCode,
false => CallType::Call,
};

match self.ext.call(
&call_gas,
&sender_address,
&receive_address,
value,
unsafe { slice::from_raw_parts(in_beg, in_size as usize) },
&code_address,
unsafe { slice::from_raw_parts_mut(out_beg, out_size as usize) }) {
unsafe { slice::from_raw_parts_mut(out_beg, out_size as usize) },
call_type,
) {
evm::MessageCallResult::Success(gas_left) => unsafe {
*io_gas = (gas + gas_left).low_u64();
true
Expand Down
10 changes: 10 additions & 0 deletions ethcore/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,13 +292,20 @@ impl Miner {
};

let mut invalid_transactions = HashSet::new();
let mut transactions_to_penalize = HashSet::new();
let block_number = open_block.block().fields().header.number();
// TODO: push new uncles, too.
for tx in transactions {
let hash = tx.hash();
match open_block.push_transaction(tx, None) {
Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, gas })) => {
debug!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?} (limit: {:?}, used: {:?}, gas: {:?})", hash, gas_limit, gas_used, gas);

// Penalize transaction if it's above current gas limit
if gas > gas_limit {
transactions_to_penalize.insert(hash);
}

// Exit early if gas left is smaller then min_tx_gas
let min_tx_gas: U256 = 21000.into(); // TODO: figure this out properly.
if gas_limit - gas_used < min_tx_gas {
Expand Down Expand Up @@ -334,6 +341,9 @@ impl Miner {
for hash in invalid_transactions.into_iter() {
queue.remove_invalid(&hash, &fetch_account);
}
for hash in transactions_to_penalize {
queue.penalize(&hash);
}
}
(block, original_work_hash)
}
Expand Down
101 changes: 97 additions & 4 deletions ethcore/src/miner/transaction_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ struct TransactionOrder {
hash: H256,
/// Origin of the transaction
origin: TransactionOrigin,
/// Penalties
penalties: usize,
}


Expand All @@ -144,13 +146,19 @@ impl TransactionOrder {
gas_price: tx.transaction.gas_price,
hash: tx.hash(),
origin: tx.origin,
penalties: 0,
}
}

fn update_height(mut self, nonce: U256, base_nonce: U256) -> Self {
self.nonce_height = nonce - base_nonce;
self
}

fn penalize(mut self) -> Self {
self.penalties = self.penalties.saturating_add(1);
self
}
}

impl Eq for TransactionOrder {}
Expand All @@ -167,6 +175,11 @@ impl PartialOrd for TransactionOrder {

impl Ord for TransactionOrder {
fn cmp(&self, b: &TransactionOrder) -> Ordering {
// First check number of penalties
if self.penalties != b.penalties {
return self.penalties.cmp(&b.penalties);
}

// First check nonce_height
if self.nonce_height != b.nonce_height {
return self.nonce_height.cmp(&b.nonce_height);
Expand Down Expand Up @@ -387,7 +400,7 @@ pub struct AccountDetails {
}

/// Transactions with `gas > (gas_limit + gas_limit * Factor(in percents))` are not imported to the queue.
const GAS_LIMIT_HYSTERESIS: usize = 10; // %
const GAS_LIMIT_HYSTERESIS: usize = 10; // (100/GAS_LIMIT_HYSTERESIS) %

/// `TransactionQueue` implementation
pub struct TransactionQueue {
Expand Down Expand Up @@ -506,8 +519,6 @@ impl TransactionQueue {
pub fn add<T>(&mut self, tx: SignedTransaction, fetch_account: &T, origin: TransactionOrigin) -> Result<TransactionImportResult, Error>
where T: Fn(&Address) -> AccountDetails {

trace!(target: "txqueue", "Importing: {:?}", tx.hash());

if tx.gas_price < self.minimal_gas_price && origin != TransactionOrigin::Local {
trace!(target: "txqueue",
"Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})",
Expand Down Expand Up @@ -593,6 +604,39 @@ impl TransactionQueue {
assert_eq!(self.future.by_priority.len() + self.current.by_priority.len(), self.by_hash.len());
}

/// Penalize transactions from sender of transaction with given hash.
/// I.e. it should change the priority of the transaction in the queue.
///
/// NOTE: We need to penalize all transactions from particular sender
/// to avoid breaking invariants in queue (ordered by nonces).
/// Consecutive transactions from this sender would fail otherwise (because of invalid nonce).
pub fn penalize(&mut self, transaction_hash: &H256) {
let transaction = match self.by_hash.get(transaction_hash) {
None => return,
Some(t) => t,
};
let sender = transaction.sender();

// Penalize all transactions from this sender
let nonces_from_sender = match self.current.by_address.row(&sender) {
Some(row_map) => row_map.keys().cloned().collect::<Vec<U256>>(),
None => vec![],
};
for k in nonces_from_sender {
let order = self.current.drop(&sender, &k).unwrap();
self.current.insert(sender, k, order.penalize());
}
// Same thing for future
let nonces_from_sender = match self.future.by_address.row(&sender) {
Some(row_map) => row_map.keys().cloned().collect::<Vec<U256>>(),
None => vec![],
};
for k in nonces_from_sender {
let order = self.future.drop(&sender, &k).unwrap();
self.current.insert(sender, k, order.penalize());
}
}

/// Removes invalid transaction identified by hash from queue.
/// Assumption is that this transaction nonce is not related to client nonce,
/// so transactions left in queue are processed according to client nonce.
Expand Down Expand Up @@ -764,6 +808,7 @@ impl TransactionQueue {

let address = tx.sender();
let nonce = tx.nonce();
let hash = tx.hash();

let next_nonce = self.last_nonces
.get(&address)
Expand All @@ -785,6 +830,9 @@ impl TransactionQueue {
try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, &mut self.future, &mut self.by_hash)));
// Return an error if this transaction is not imported because of limit.
try!(check_if_removed(&address, &nonce, self.future.enforce_limit(&mut self.by_hash)));

debug!(target: "txqueue", "Importing transaction to future: {:?}", hash);
debug!(target: "txqueue", "status: {:?}", self.status());
return Ok(TransactionImportResult::Future);
}
try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash)));
Expand All @@ -811,7 +859,8 @@ impl TransactionQueue {
// Trigger error if the transaction we are importing was removed.
try!(check_if_removed(&address, &nonce, removed));

trace!(target: "txqueue", "status: {:?}", self.status());
debug!(target: "txqueue", "Imported transaction to current: {:?}", hash);
debug!(target: "txqueue", "status: {:?}", self.status());
Ok(TransactionImportResult::Current)
}

Expand Down Expand Up @@ -945,6 +994,17 @@ mod test {
(tx1.sign(secret), tx2.sign(secret))
}

/// Returns two consecutive transactions, both with increased gas price
fn new_tx_pair_with_gas_price_increment(gas_price_increment: U256) -> (SignedTransaction, SignedTransaction) {
let gas = default_gas_price() + gas_price_increment;
let tx1 = new_unsigned_tx(default_nonce(), gas);
let tx2 = new_unsigned_tx(default_nonce() + 1.into(), gas);

let keypair = Random.generate().unwrap();
let secret = &keypair.secret();
(tx1.sign(secret), tx2.sign(secret))
}

fn new_tx_pair_default(nonce_increment: U256, gas_price_increment: U256) -> (SignedTransaction, SignedTransaction) {
new_tx_pair(default_nonce(), default_gas_price(), nonce_increment, gas_price_increment)
}
Expand Down Expand Up @@ -1332,6 +1392,39 @@ mod test {
assert_eq!(top.len(), 2);
}

#[test]
fn should_penalize_transactions_from_sender() {
// given
let mut txq = TransactionQueue::new();
// txa, txb - slightly bigger gas price to have consistent ordering
let (txa, txb) = new_tx_pair_default(1.into(), 0.into());
let (tx1, tx2) = new_tx_pair_with_gas_price_increment(3.into());

// insert everything
txq.add(txa.clone(), &default_account_details, TransactionOrigin::External).unwrap();
txq.add(txb.clone(), &default_account_details, TransactionOrigin::External).unwrap();
txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();

let top = txq.top_transactions();
assert_eq!(top[0], tx1);
assert_eq!(top[1], txa);
assert_eq!(top[2], tx2);
assert_eq!(top[3], txb);
assert_eq!(top.len(), 4);

// when
txq.penalize(&tx1.hash());

// then
let top = txq.top_transactions();
assert_eq!(top[0], txa);
assert_eq!(top[1], txb);
assert_eq!(top[2], tx1);
assert_eq!(top[3], tx2);
assert_eq!(top.len(), 4);
}

#[test]
fn should_return_pending_hashes() {
// given
Expand Down
Loading

0 comments on commit 3b80451

Please sign in to comment.