Delayed transactions #3865
Delayed transactions #3865
Conversation
arkpar
added
A0-pleasereview 🤓
| @@ -324,7 +329,8 @@ mod tests { | |||
| fn should_deserialize_modification() { | |||
| // given | |||
| let s1 = r#"{ | |||
| "gasPrice":"0xba43b7400" | |||
| "gasPrice":"0xba43b7400", | |||
| "minBlock":42 | |||
There was a problem hiding this comment.
I think we encode block numbers as hex almost everywhere, would be good to keep consistency.
| @@ -41,6 +41,9 @@ pub struct TransactionRequest { | |||
| pub data: Option<Bytes>, | |||
| /// Transaction's nonce | |||
| pub nonce: Option<U256>, | |||
| /// Delay until this block if specified. | |||
| #[serde(rename="minBlock")] | |||
| pub min_block: Option<u64>, | |||
There was a problem hiding this comment.
Consistency for QUANTITY encoding for block number.
| let mut delayed = HashSet::new(); | ||
| for t in self.current.by_priority.iter() { | ||
| let tx = self.by_hash.get(&t.hash).expect("All transactions in `current` and `future` are always included in `by_hash`"); | ||
| let sender = tx.transaction.sender().expect("Queue only contains transactions with valid sender"); |
There was a problem hiding this comment.
Just use tx.sender() it doesn't require the proof.
| /// Returns local transactions (some of them might not be part of the queue anymore). | ||
| pub fn local_transactions(&self) -> &LinkedHashMap<H256, LocalTransactionStatus> { | ||
| self.local_transactions.all_transactions() | ||
| fn collect_pending_transaction<F>(&self, best_block: BlockNumber, mut f: F) |
There was a problem hiding this comment.
Wouldn't it be better to filter_pending_transactions?
| @@ -390,6 +390,34 @@ impl Deref for LocalizedTransaction { | |||
| } | |||
| } | |||
|
|
|||
| /// Queued information with additional information. | |||
There was a problem hiding this comment.
"Queued information" -> "Queued transaction"
| @@ -390,6 +390,34 @@ impl Deref for LocalizedTransaction { | |||
| } | |||
| } | |||
|
|
|||
| /// Queued information with additional information. | |||
| #[derive(Debug, Clone, PartialEq, Eq, Binary)] | |||
| pub struct PendingTransaction { | |||
There was a problem hiding this comment.
You could implement Deref<Target=SignedTransaction> to get rid of some tx.transaction.<functioncall> statements.
There was a problem hiding this comment.
I'd prefer it to be explicit. Makes the code easier to follow
|
Changes Unknown when pulling 060cc79 on tx-block into ** on master**. |
|
Changes Unknown when pulling 060cc79 on tx-block into ** on master**. |
| /// Signed transaction data. | ||
| pub transaction: SignedTransaction, | ||
| /// Gas price. | ||
| pub min_block: Option<BlockNumber>, |
rphmeier
added
A6-mustntgrumble 💦
|
Changes Unknown when pulling 060cc79 on tx-block into ** on master**. |
| @@ -1917,10 +1917,10 @@ impl ChainSync { | |||
| return 0; | |||
| } | |||
|
|
|||
| let all_transactions_hashes = transactions.iter().map(|tx| tx.hash()).collect::<HashSet<H256>>(); | |||
| let all_transactions_hashes = transactions.iter().map(|tx| tx.transaction.hash()).collect::<HashSet<H256>>(); | |||
There was a problem hiding this comment.
does this send out pending transactions, too?
There was a problem hiding this comment.
only those that can go into the next block
| /// Get a list of all transactions. | ||
| fn all_transactions(&self) -> Vec<SignedTransaction>; | ||
| /// Get a list of all ready transactions in the queue. | ||
| fn all_transactions(&self) -> Vec<PendingTransaction>; |
There was a problem hiding this comment.
rename to ready_transactions to make the intent clear?
|
looks ok, assuming that the pending transactions are indeed not sent out onto the network. would be nice to have one or more test demonstrating the functionality though... |
arkpar
added
A3-inprogress ⏳
|
Changes Unknown when pulling 6c9de9e on tx-block into ** on master**. |
arkpar
added
A0-pleasereview 🤓
|
Changes Unknown when pulling 65f07e5 on tx-block into ** on master**. |
gavofyork
added
A8-looksgood 🦄
Closes #3594