Transaction Queue Integration #607
Conversation
tomusdrw
added
the
A3-inprogress ⏳
Conflicts: sync/src/transaction_queue.rs
This reverts commit d330f0b. Conflicts: sync/src/transaction_queue.rs
tomusdrw
added
A0-pleasereview 🤓
tomusdrw
added
A0-pleasereview 🤓
NikVolf
added
A8-looksgood 🦄
arkpar
added
A4-gotissues 💥
|
See comments from https://github.com/ethcore/parity/pull/609 |
| for tx in &txs { | ||
| let _sender = tx.sender(); | ||
| } | ||
| let mut transaction_queue = self.transaction_queue.lock().unwrap(); |
There was a problem hiding this comment.
@arkpar This happens in multiple threads using rayon - note par_iter.
| trace!(target: "sync", "{} -> Transactions ({} entries)", peer_id, item_count); | ||
| let fetch_latest_nonce = |a : &Address| chain.nonce(a); | ||
|
|
||
| let mut transaction_queue = self.transaction_queue.lock().unwrap(); |
tomusdrw
added
A0-pleasereview 🤓
|
code quality good. tests good. haven't yet done deep review of logic. might be worth doing a little additional documentation? |
|
What kind of documentation would you like? Doctests or just module level description of transaction queue? or specific part of integration? |
| if let SyncMessage::BlockVerified = *message { | ||
| self.sync.write().unwrap().chain_blocks_verified(&mut NetSyncIo::new(io, self.chain.deref())); | ||
| match *message { | ||
| SyncMessage::BlockVerified => { |
There was a problem hiding this comment.
sync should not care about BlockVerified event at all. Please merge chain_blocks_verified function into chain_new_blocks. These two should be called for the same event and chain_blocks_verified is an incorrect name.
arkpar
added
A8-looksgood 🦄
|
looks good to me |
NikVolf
added
A8-looksgood 🦄
| closed_block.drain() | ||
| .commit(header.number(), &header.hash(), ancient) | ||
| .expect("State DB commit failed."); | ||
|
|
||
| // And update the chain |
There was a problem hiding this comment.
why is this moved down here?
There was a problem hiding this comment.
It was introduced here: ethcore/parity@bcaed67
to avoid race conditions when chain contains uncommitted hashes. Will add a comment explaining the order.
|
several questions. looks good otherwise. |
gavofyork
added
A6-mustntgrumble 💦
Transaction Queue Integration
No description provided.