Cache transactions before adding them to pool #1398
Conversation
# Conflicts: # components/merkledb/src/db.rs
| ValidatorId::zero(), | ||
| Height(height), | ||
| txs, | ||
| &mut BTreeMap::new(), |
There was a problem hiding this comment.
| &mut BTreeMap::new(), | |
| None, |
I suppose BTreeMap::new is not free operation.
There was a problem hiding this comment.
https://github.com/rust-lang/rust/blob/master/src/liballoc/collections/btree/node.rs#L127
It seems that BtreeMap::new is cheap operation.
There was a problem hiding this comment.
Yes, cheap but not like Option.
There was a problem hiding this comment.
This code with BtreeMap::new is used only in benchmarks or tests. It doesn't affect the production code. But adding Option will result in unnecessary unwraps.
Codecov Report
@@ Coverage Diff @@
## master #1398 +/- ##
==========================================
+ Coverage 93.74% 93.79% +0.04%
==========================================
Files 165 165
Lines 25653 25884 +231
==========================================
+ Hits 24049 24277 +228
- Misses 1604 1607 +3
Continue to review full report at Codecov.
|
| pub(crate) fn commit_transaction(&mut self, hash: &Hash) { | ||
| self.transactions_pool().remove(hash); | ||
| pub(crate) fn commit_transaction(&mut self, hash: &Hash, tx: Signed<RawTransaction>) { | ||
| if !self.transactions().contains(hash) { |
There was a problem hiding this comment.
Not a very clear case, what means if we are trying to commit a transaction that is not in self.transactions()?
There was a problem hiding this comment.
Because now we have transactions that stored only in cache during the round and added to storage after commit(or before the node is stopped).
exonum/src/blockchain/schema.rs
Outdated
|
|
||
| /// Return transaction from persistent pool. If transaction is not present in pool, try | ||
| /// to return it from transactions cache. | ||
| pub fn get_tx<T: IndexAccess>( |
There was a problem hiding this comment.
Why this helper function is public?
There was a problem hiding this comment.
No reason, I will change this.
exonum/src/blockchain/tests.rs
Outdated
| let db = create_database(path); | ||
| let service_keypair = gen_keypair(); | ||
| let api_channel = mpsc::unbounded(); | ||
| let api_channel = mpsc::channel(1); |
There was a problem hiding this comment.
channel(0) is more useful, in my opinion.
| blockchain.create_patch( | ||
| ValidatorId::zero(), | ||
| Height(height), | ||
| txs, |
There was a problem hiding this comment.
So I think that we have to unify this to arguments by the single trait TransactionsSource with the good interface in future.
Overview
Added tx_cache to store incoming transactions.
Definition of Done