fix(mpool): fix flaky eth_newPendingTransactionFilter test#6946
fix(mpool): fix flaky eth_newPendingTransactionFilter test#6946LesnyRumcajs merged 5 commits intomainfrom
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/message_pool/msgpool/msg_pool.rs`:
- Around line 593-595: The loop currently calls self.pending_for(&addr) for each
addr from the cloned pending snapshot, which can return None due to a race and
silently drop messages; modify the loop in pending() to iterate the cloned
pending snapshot directly and extend out with the messages already captured in
that snapshot (use the values from the pending variable rather than calling
pending_for), ensuring None is handled explicitly (e.g., treat as empty or log)
so messages aren't silently omitted; update the loop around pending, out,
pending_for and pending() accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6bb3cd8f-313d-4d2f-9387-e57328345033
📒 Files selected for processing (1)
src/message_pool/msgpool/msg_pool.rs
f291282 to
1ba5f99
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 16 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/message_pool/msgpool/mod.rs (1)
618-619: Strengthen this regression assertion.
assert_eq!(p.len(), 3)will still pass if the wrong three messages come back. Please assert the expected sequences or CIDs as well so the test guards the actual revert behavior.🧪 Suggested assertion tightening
let (p, _) = mpool.pending(); - assert_eq!(p.len(), 3); + let sequences = p.into_iter().map(|m| m.sequence()).collect::<Vec<_>>(); + assert_eq!(sequences, vec![1, 2, 3]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/message_pool/msgpool/mod.rs` around lines 618 - 619, The test currently only checks count with let (p, _) = mpool.pending(); assert_eq!(p.len(), 3); — strengthen it by asserting the actual messages/CIDs and order returned by mpool.pending(). For example, collect the returned entries from p (e.g., p.iter().map(|e| e.key.cid.clone()) or p.iter().map(|e| e.message.sequence())) and compare to the expected vector of CIDs or sequence numbers (the known CIDs/sequence values created earlier in this test) so the assertion verifies the exact three messages and their order that should remain after the revert.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/message_pool/msgpool/mod.rs`:
- Around line 618-619: The test currently only checks count with let (p, _) =
mpool.pending(); assert_eq!(p.len(), 3); — strengthen it by asserting the actual
messages/CIDs and order returned by mpool.pending(). For example, collect the
returned entries from p (e.g., p.iter().map(|e| e.key.cid.clone()) or
p.iter().map(|e| e.message.sequence())) and compare to the expected vector of
CIDs or sequence numbers (the known CIDs/sequence values created earlier in this
test) so the assertion verifies the exact three messages and their order that
should remain after the revert.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a70768f1-8fa1-4e7e-a90b-06f52f78ff9d
📒 Files selected for processing (4)
src/message_pool/msgpool/mod.rssrc/message_pool/msgpool/msg_pool.rssrc/rpc/methods/eth.rssrc/rpc/methods/mpool.rs
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit