-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Stop tracking sent transactions once they've been imported into the blockchain #5706
Conversation
4889e70
to
7462446
Compare
Remove transactions from EthereumCapability's list of sent transactions (which it uses to prevent sending the same transaction to peers more than once) once they've made it into the blockchain.
4e33711
to
0738b64
Compare
Codecov Report
@@ Coverage Diff @@
## master #5706 +/- ##
==========================================
+ Coverage 62.98% 62.99% +0.01%
==========================================
Files 351 350 -1
Lines 30014 30001 -13
Branches 3367 3363 -4
==========================================
- Hits 18903 18900 -3
+ Misses 9890 9882 -8
+ Partials 1221 1219 -2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, just two minor suggestions
libethereum/Client.cpp
Outdated
} | ||
auto h = m_host.lock(); | ||
if (h && goodTransactions.size()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (h && goodTransactions.size()) | |
if (h && !goodTransactions.empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move the check for empty vector inside removeSentTransactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, that way we get the benefit of the check for all callers.
Move empty check into EthereumCapability::removeSentTransactions (so that it's performed regardless of the caller) and use empty() to check for empty condition rather than !size().
Fix #5686