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

Transaction timestamp condition #4419

Merged
merged 4 commits into from
Feb 3, 2017
Merged

Transaction timestamp condition #4419

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a7d795
Transaction timestamp condtiion
arkpar Feb 3, 2017
bea8c36
Updated docs
arkpar Feb 3, 2017
b706d18
Updated docs
arkpar Feb 3, 2017
f307402
Check agains last block timestamp
arkpar Feb 3, 2017
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
9 changes: 7 additions & 2 deletions ethcore/light/src/client/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use ethcore::client::ClientReport;
use ethcore::ids::BlockId;
use ethcore::header::Header;
use ethcore::verification::queue::{self, HeaderQueue};
use ethcore::transaction::PendingTransaction;
use ethcore::transaction::{PendingTransaction, Condition as TransactionCondition};
use ethcore::blockchain_info::BlockChainInfo;
use ethcore::spec::Spec;
use ethcore::service::ClientIoMessage;
Expand All @@ -34,6 +34,7 @@ use util::{Bytes, Mutex, RwLock};

use provider::Provider;
use request;
use time;

use self::header_chain::HeaderChain;

Expand Down Expand Up @@ -110,7 +111,11 @@ impl Client {
let best_num = self.chain.best_block().number;
self.tx_pool.lock()
.values()
.filter(|t| t.min_block.as_ref().map_or(true, |x| x <= &best_num))
.filter(|t| match t.condition {
Some(TransactionCondition::Number(ref x)) => x <= &best_num,
Some(TransactionCondition::Timestamp(ref x)) => *x <= time::get_time().sec as u64,
Copy link
Contributor

Choose a reason for hiding this comment

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

this condition check needs updating as well. probably would be best done after changing HeaderChain to use ethcore::encoded (done locally).

Copy link
Contributor

Choose a reason for hiding this comment

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

given that it's "dead" code I don't mind merging this PR without it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I'll leave it to you then :)

None => true,
})
.cloned()
.collect()
}
Expand Down
8 changes: 4 additions & 4 deletions ethcore/src/miner/miner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use client::TransactionImportResult;
use executive::contract_address;
use block::{ClosedBlock, IsBlock, Block};
use error::*;
use transaction::{Action, UnverifiedTransaction, PendingTransaction, SignedTransaction};
use transaction::{Action, UnverifiedTransaction, PendingTransaction, SignedTransaction, Condition as TransactionCondition};
use receipt::{Receipt, RichReceipt};
use spec::Spec;
use engines::{Engine, Seal};
Expand Down Expand Up @@ -597,7 +597,7 @@ impl Miner {
client: &MiningBlockChainClient,
transactions: Vec<UnverifiedTransaction>,
default_origin: TransactionOrigin,
min_block: Option<BlockNumber>,
condition: Option<TransactionCondition>,
transaction_queue: &mut BanningTransactionQueue,
) -> Vec<Result<TransactionImportResult, Error>> {
let accounts = self.accounts.as_ref()
Expand Down Expand Up @@ -635,7 +635,7 @@ impl Miner {
let details_provider = TransactionDetailsProvider::new(client, &self.service_transaction_action);
match origin {
TransactionOrigin::Local | TransactionOrigin::RetractedBlock => {
transaction_queue.add(transaction, origin, insertion_time, min_block, &details_provider)
transaction_queue.add(transaction, origin, insertion_time, condition.clone(), &details_provider)
},
TransactionOrigin::External => {
transaction_queue.add_with_banlist(transaction, insertion_time, &details_provider)
Expand Down Expand Up @@ -892,7 +892,7 @@ impl MinerService for Miner {
let mut transaction_queue = self.transaction_queue.lock();
// We need to re-validate transactions
let import = self.add_transactions_to_queue(
chain, vec![pending.transaction.into()], TransactionOrigin::Local, pending.min_block, &mut transaction_queue
chain, vec![pending.transaction.into()], TransactionOrigin::Local, pending.condition, &mut transaction_queue
).pop().expect("one result returned per added transaction; one added => one result; qed");

match import {
Expand Down
32 changes: 19 additions & 13 deletions ethcore/src/miner/transaction_queue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ use std::ops::Deref;
use std::cmp::Ordering;
use std::cmp;
use std::collections::{HashSet, HashMap, BTreeSet, BTreeMap};
use time;
use linked_hash_map::LinkedHashMap;
use util::{Address, H256, Uint, U256};
use util::table::Table;
Expand Down Expand Up @@ -277,16 +278,16 @@ struct VerifiedTransaction {
/// Insertion time
insertion_time: QueuingInstant,
/// Delay until specifid block.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment not updated

min_block: Option<BlockNumber>,
condition: Option<Condition>,
}

impl VerifiedTransaction {
fn new(transaction: SignedTransaction, origin: TransactionOrigin, time: QueuingInstant, min_block: Option<BlockNumber>) -> Self {
fn new(transaction: SignedTransaction, origin: TransactionOrigin, time: QueuingInstant, condition: Option<Condition>) -> Self {
VerifiedTransaction {
transaction: transaction,
origin: origin,
insertion_time: time,
min_block: min_block,
condition: condition,
}
}

Expand Down Expand Up @@ -666,14 +667,14 @@ impl TransactionQueue {
tx: SignedTransaction,
origin: TransactionOrigin,
time: QueuingInstant,
min_block: Option<BlockNumber>,
condition: Option<Condition>,
details_provider: &TransactionDetailsProvider,
) -> Result<TransactionImportResult, Error> {
if origin == TransactionOrigin::Local {
let hash = tx.hash();
let cloned_tx = tx.clone();

let result = self.add_internal(tx, origin, time, min_block, details_provider);
let result = self.add_internal(tx, origin, time, condition, details_provider);
match result {
Ok(TransactionImportResult::Current) => {
self.local_transactions.mark_pending(hash);
Expand All @@ -694,7 +695,7 @@ impl TransactionQueue {
}
result
} else {
self.add_internal(tx, origin, time, min_block, details_provider)
self.add_internal(tx, origin, time, condition, details_provider)
}
}

Expand All @@ -704,7 +705,7 @@ impl TransactionQueue {
tx: SignedTransaction,
origin: TransactionOrigin,
time: QueuingInstant,
min_block: Option<BlockNumber>,
condition: Option<Condition>,
details_provider: &TransactionDetailsProvider,
) -> Result<TransactionImportResult, Error> {
if origin != TransactionOrigin::Local && tx.gas_price < self.minimal_gas_price {
Expand Down Expand Up @@ -815,7 +816,7 @@ impl TransactionQueue {
}
tx.check_low_s()?;
// No invalid transactions beyond this point.
let vtx = VerifiedTransaction::new(tx, origin, time, min_block);
let vtx = VerifiedTransaction::new(tx, origin, time, condition);
let r = self.import_tx(vtx, client_account.nonce).map_err(Error::Transaction);
assert_eq!(self.future.by_priority.len() + self.current.by_priority.len(), self.by_hash.len());
r
Expand Down Expand Up @@ -1082,7 +1083,12 @@ impl TransactionQueue {
if delayed.contains(&sender) {
continue;
}
if tx.min_block.unwrap_or(0) > best_block {
let delay = match tx.condition {
Some(Condition::Number(n)) => n > best_block,
Some(Condition::Timestamp(t)) => t > time::get_time().sec as u64,
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be based off of the best block's timestamp rather than current time, since that's all that contracts can inspect. It's additionally often set into the future and not consistent with "real" time, but can be agreed upon unanimously just like block numbers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Waiting for the block timestamp would result in transactions included no in the first available block, but in the next one. But at least the transaction is guaranteed not to fail, so I guess that's reasonable

None => false,
};
if delay {
delayed.insert(sender);
continue;
}
Expand All @@ -1100,7 +1106,7 @@ impl TransactionQueue {
/// Return all ready transactions.
pub fn pending_transactions(&self, best_block: BlockNumber) -> Vec<PendingTransaction> {
let mut r = Vec::new();
self.filter_pending_transaction(best_block, |tx| r.push(PendingTransaction::new(tx.transaction.clone(), tx.min_block)));
self.filter_pending_transaction(best_block, |tx| r.push(PendingTransaction::new(tx.transaction.clone(), tx.condition.clone())));
r
}

Expand All @@ -1109,7 +1115,7 @@ impl TransactionQueue {
self.future.by_priority
.iter()
.map(|t| self.by_hash.get(&t.hash).expect("All transactions in `current` and `future` are always included in `by_hash`"))
.map(|t| PendingTransaction { transaction: t.transaction.clone(), min_block: t.min_block })
.map(|t| PendingTransaction { transaction: t.transaction.clone(), condition: t.condition.clone() })
.collect()
}

Expand Down Expand Up @@ -1382,7 +1388,7 @@ pub mod test {
use super::{TransactionSet, TransactionOrder, VerifiedTransaction};
use miner::local_transactions::LocalTransactionsList;
use client::TransactionImportResult;
use transaction::{SignedTransaction, Transaction, Action};
use transaction::{SignedTransaction, Transaction, Action, Condition};

pub struct DummyTransactionDetailsProvider {
account_details: AccountDetails,
Expand Down Expand Up @@ -2178,7 +2184,7 @@ pub mod test {
let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());

// when
let res1 = txq.add(tx.clone(), TransactionOrigin::External, 0, Some(1), &default_tx_provider()).unwrap();
let res1 = txq.add(tx.clone(), TransactionOrigin::External, 0, Some(Condition::Number(1)), &default_tx_provider()).unwrap();
let res2 = txq.add(tx2.clone(), TransactionOrigin::External, 0, None, &default_tx_provider()).unwrap();

// then
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/tests/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use spec::Spec;
use views::BlockView;
use util::stats::Histogram;
use ethkey::{KeyPair, Secret};
use transaction::{PendingTransaction, Transaction, Action};
use transaction::{PendingTransaction, Transaction, Action, Condition};
use miner::MinerService;

#[test]
Expand Down Expand Up @@ -299,7 +299,7 @@ fn does_not_propagate_delayed_transactions() {
action: Action::Call(Address::default()),
value: 0.into(),
data: Vec::new(),
}.sign(secret, None), Some(2));
}.sign(secret, None), Some(Condition::Number(2)));
let tx1 = PendingTransaction::new(Transaction {
nonce: 1.into(),
gas_price: 0.into(),
Expand Down
20 changes: 15 additions & 5 deletions ethcore/src/types/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ impl Decodable for Action {
}
}

/// Transaction activation condition.
#[derive(Debug, Clone, PartialEq, Eq)]
#[cfg_attr(feature = "ipc", binary)]
pub enum Condition {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: doc over attrs

/// Valid at this block number or later.
Number(BlockNumber),
/// Valid at this unix time or later.
Timestamp(u64),
}

/// A set of information describing an externally-originating message call
/// or contract creation operation.
#[derive(Default, Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -448,16 +458,16 @@ impl Deref for LocalizedTransaction {
pub struct PendingTransaction {
/// Signed transaction data.
pub transaction: SignedTransaction,
/// To be activated at this block. `None` for immediately.
pub min_block: Option<BlockNumber>,
/// To be activated at this condition. `None` for immediately.
pub condition: Option<Condition>,
}

impl PendingTransaction {
/// Create a new pending transaction from signed transaction.
pub fn new(signed: SignedTransaction, min_block: Option<BlockNumber>) -> Self {
pub fn new(signed: SignedTransaction, condition: Option<Condition>) -> Self {
PendingTransaction {
transaction: signed,
min_block: min_block,
condition: condition,
}
}
}
Expand All @@ -466,7 +476,7 @@ impl From<SignedTransaction> for PendingTransaction {
fn from(t: SignedTransaction) -> Self {
PendingTransaction {
transaction: t,
min_block: None,
condition: None,
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions rpc/src/v1/helpers/dispatch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ pub fn sign_and_dispatch<C, M>(client: &C, miner: &M, accounts: &AccountProvider
{

let network_id = client.signing_network_id();
let min_block = filled.min_block.clone();
let condition = filled.condition.clone();
let signed_transaction = sign_no_dispatch(client, miner, accounts, filled, password)?;

let (signed_transaction, token) = match signed_transaction {
Expand All @@ -201,7 +201,7 @@ pub fn sign_and_dispatch<C, M>(client: &C, miner: &M, accounts: &AccountProvider
};

trace!(target: "miner", "send_transaction: dispatching tx: {} for network ID {:?}", rlp::encode(&signed_transaction).to_vec().pretty(), network_id);
let pending_transaction = PendingTransaction::new(signed_transaction, min_block);
let pending_transaction = PendingTransaction::new(signed_transaction, condition.map(Into::into));
dispatch_transaction(&*client, &*miner, pending_transaction).map(|hash| {
match token {
Some(ref token) => WithToken::Yes(hash, token.clone()),
Expand All @@ -222,7 +222,7 @@ pub fn fill_optional_fields<C, M>(request: TransactionRequest, default_sender: A
gas: request.gas.unwrap_or_else(|| miner.sensible_gas_limit()),
value: request.value.unwrap_or_else(|| 0.into()),
data: request.data.unwrap_or_else(Vec::new),
min_block: request.min_block,
condition: request.condition,
}
}

Expand Down
11 changes: 6 additions & 5 deletions rpc/src/v1/helpers/requests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

use util::{Address, U256, Bytes};
use v1::types::TransactionCondition;

/// Transaction request coming from RPC
#[derive(Debug, Clone, Default, Eq, PartialEq, Hash)]
Expand All @@ -33,8 +34,8 @@ pub struct TransactionRequest {
pub data: Option<Bytes>,
/// Transaction's nonce
pub nonce: Option<U256>,
/// Delay until this block if specified.
pub min_block: Option<u64>,
/// Delay until this condition is met.
pub condition: Option<TransactionCondition>,
}

/// Transaction request coming from RPC with default values filled in.
Expand All @@ -56,8 +57,8 @@ pub struct FilledTransactionRequest {
pub data: Bytes,
/// Transaction's nonce
pub nonce: Option<U256>,
/// Delay until this block if specified.
pub min_block: Option<u64>,
/// Delay until this condition is met.
pub condition: Option<TransactionCondition>,
}

impl From<FilledTransactionRequest> for TransactionRequest {
Expand All @@ -70,7 +71,7 @@ impl From<FilledTransactionRequest> for TransactionRequest {
value: Some(r.value),
data: Some(r.data),
nonce: r.nonce,
min_block: r.min_block,
condition: r.condition,
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion rpc/src/v1/helpers/signing_queue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ mod test {
value: 10_000_000.into(),
data: vec![],
nonce: None,
min_block: None,
condition: None,
})
}

Expand Down
6 changes: 3 additions & 3 deletions rpc/src/v1/impls/signer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ impl<C: 'static, M: 'static> SignerClient<C, M> where C: MiningBlockChainClient,
if let Some(gas) = modification.gas {
request.gas = gas.into();
}
if let Some(ref min_block) = modification.min_block {
request.min_block = min_block.as_ref().and_then(|b| b.to_min_block_num());
if let Some(ref condition) = modification.condition {
request.condition = condition.clone().map(Into::into);
}
}
let result = f(&*client, &*miner, &*accounts, payload);
Expand Down Expand Up @@ -160,7 +160,7 @@ impl<C: 'static, M: 'static> Signer for SignerClient<C, M> where C: MiningBlockC

// Dispatch if everything is ok
if sender_matches && data_matches && value_matches && nonce_matches {
let pending_transaction = PendingTransaction::new(signed_transaction, request.min_block);
let pending_transaction = PendingTransaction::new(signed_transaction, request.condition.map(Into::into));
dispatch_transaction(&*client, &*miner, pending_transaction)
.map(Into::into)
.map(ConfirmationResponse::SendTransaction)
Expand Down
5 changes: 2 additions & 3 deletions rpc/src/v1/tests/mocked/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ fn rpc_eth_pending_transaction_by_hash() {
tester.miner.pending_transactions.lock().insert(H256::zero(), tx);
}

let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":null,"creates":null,"from":"0x0f65fe9276bc9a24ae7083ae28e2660ef72df99e","gas":"0x5208","gasPrice":"0x1","hash":"0x41df922fd0d4766fcc02e161f8295ec28522f329ae487f14d811e4b64c8d6e31","input":"0x","minBlock":null,"networkId":null,"nonce":"0x0","publicKey":"0x7ae46da747962c2ee46825839c1ef9298e3bd2e70ca2938495c3693a485ec3eaa8f196327881090ff64cf4fbb0a48485d4f83098e189ed3b7a87d5941b59f789","r":"0x48b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353","raw":"0xf85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","s":"0xefffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","standardV":"0x0","to":"0x095e7baea6a6c7c4c2dfeb977efac326af552d87","transactionIndex":null,"v":"0x1b","value":"0xa"},"id":1}"#;
let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":null,"condition":null,"creates":null,"from":"0x0f65fe9276bc9a24ae7083ae28e2660ef72df99e","gas":"0x5208","gasPrice":"0x1","hash":"0x41df922fd0d4766fcc02e161f8295ec28522f329ae487f14d811e4b64c8d6e31","input":"0x","networkId":null,"nonce":"0x0","publicKey":"0x7ae46da747962c2ee46825839c1ef9298e3bd2e70ca2938495c3693a485ec3eaa8f196327881090ff64cf4fbb0a48485d4f83098e189ed3b7a87d5941b59f789","r":"0x48b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353","raw":"0xf85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","s":"0xefffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","standardV":"0x0","to":"0x095e7baea6a6c7c4c2dfeb977efac326af552d87","transactionIndex":null,"v":"0x1b","value":"0xa"},"id":1}"#;
let request = r#"{
"jsonrpc": "2.0",
"method": "eth_getTransactionByHash",
Expand Down Expand Up @@ -830,12 +830,11 @@ fn rpc_eth_sign_transaction() {
let response = r#"{"jsonrpc":"2.0","result":{"#.to_owned() +
r#""raw":"0x"# + &rlp.to_hex() + r#"","# +
r#""tx":{"# +
r#""blockHash":null,"blockNumber":null,"creates":null,"# +
r#""blockHash":null,"blockNumber":null,"condition":null,"creates":null,"# +
&format!("\"from\":\"0x{:?}\",", &address) +
r#""gas":"0x76c0","gasPrice":"0x9184e72a000","# +
&format!("\"hash\":\"0x{:?}\",", t.hash()) +
r#""input":"0x","# +
r#""minBlock":null,"# +
&format!("\"networkId\":{},", t.network_id().map_or("null".to_owned(), |n| format!("{}", n))) +
r#""nonce":"0x1","# +
&format!("\"publicKey\":\"0x{:?}\",", t.recover_public().unwrap()) +
Expand Down
Loading