Transaction timestamp condition #4419
Conversation
arkpar
added
A0-pleasereview 🤓
arkpar
changed the title
ethcore/src/types/transaction.rs
Outdated
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| #[cfg_attr(feature = "ipc", binary)] | ||
| /// Transaction activation condition. | ||
| pub enum Condition { |
| use ethcore; | ||
|
|
||
| /// Represents rpc api block number param. | ||
| #[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)] |
| pub min_block: Option<BlockNumber>, | ||
| /// Delay until this block condition. | ||
| #[serde(rename="condition")] | ||
| pub condition: Option<TransactionCondition>, |
There was a problem hiding this comment.
rename no longer necessary
| @@ -277,16 +278,16 @@ struct VerifiedTransaction { | |||
| /// Insertion time | |||
| insertion_time: QueuingInstant, | |||
| /// Delay until specifid block. | |||
rpc/src/v1/types/confirmations.rs
Outdated
| @@ -197,8 +197,8 @@ pub struct TransactionModification { | |||
| /// Modified gas | |||
| pub gas: Option<U256>, | |||
| /// Modified min block | |||
There was a problem hiding this comment.
Comment & rename not needed.
rphmeier
added
A8-looksgood 🦄
| 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, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
rphmeier
added
A5-grumble 🔥
arkpar
added
A0-pleasereview 🤓
| .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, |
There was a problem hiding this comment.
this condition check needs updating as well. probably would be best done after changing HeaderChain to use ethcore::encoded (done locally).
There was a problem hiding this comment.
given that it's "dead" code I don't mind merging this PR without it
There was a problem hiding this comment.
Ok, I'll leave it to you then :)
rphmeier
added
A8-looksgood 🦄
No description provided.