-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(mempool): add from_iter func to TransactionQueue #413
feat(mempool): add from_iter func to TransactionQueue #413
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
=======================================
Coverage 83.69% 83.69%
=======================================
Files 37 37
Lines 1717 1717
Branches 1717 1717
=======================================
Hits 1437 1437
Misses 203 203
Partials 77 77 ☔ View full report in Codecov by Sentry. |
dbecd1e
to
068f596
Compare
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.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware and @elintul)
crates/mempool/src/mempool_test.rs
line 33 at r2 (raw file):
for tx in txs { queue.insert(TransactionReference::new(&tx)); }
Suggestion:
txs.iter().for_each(|tx| queue.insert(TransactionReference::new(tx)));
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.
Reviewed all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ayeletstarkware)
crates/mempool/src/mempool_test.rs
line 30 at r2 (raw file):
impl FromIterator<ThinTransaction> for TransactionQueue { fn from_iter<T: IntoIterator<Item = ThinTransaction>>(txs: T) -> TransactionQueue { let mut queue = TransactionQueue::default();
Suggestion:
Self
255c53e
to
90acb6e
Compare
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.
Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ayeletstarkware)
crates/mempool/src/mempool_test.rs
line 28 at r2 (raw file):
// TODO(Ayelet): Consider using an iterator of references to ThinTransaction to avoid cloning. impl FromIterator<ThinTransaction> for TransactionQueue {
Suggestion:
TransactionReference
068f596
to
282dcf3
Compare
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.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ayeletstarkware)
This change is