Skip to content

fix: set priority mempool logger#496

Merged
beer-1 merged 1 commit into
mainfrom
fix/priority-mempool-logger
Apr 3, 2026
Merged

fix: set priority mempool logger#496
beer-1 merged 1 commit into
mainfrom
fix/priority-mempool-logger

Conversation

@beer-1

@beer-1 beer-1 commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • set the PriorityMempool logger field in NewPriorityMempool
  • preserve the existing constructor nil-check while ensuring cleanup error paths can log safely

Why

PriorityMempool validated that a logger was provided but never stored it on the struct. When mempool cleanup hit an error path and called p.logger.Debug(...), the nil logger field could panic the worker.

Validation

  • go test ./abcipp

@beer-1 beer-1 requested a review from a team as a code owner April 3, 2026 15:25
@github-actions

github-actions Bot commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

The latest Buf updates on your PR. Results from workflow Protobuf / buf-build (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed❌ failed (4)Apr 3, 2026, 3:26 PM

@coderabbitai

coderabbitai Bot commented Apr 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 868dbd14-17c0-4b21-ae0d-48d8f3b6d992

📥 Commits

Reviewing files that changed from the base of the PR and between 7a827b1 and 5fa9ad4.

📒 Files selected for processing (1)
  • abcipp/mempool.go
📝 Walkthrough

Walkthrough

The NewPriorityMempool constructor in abcipp/mempool.go now properly initializes the logger struct field with the provided logger parameter during instance creation, fixing an unset field that previously remained uninitialized.

Changes

Cohort / File(s) Summary
Logger Initialization Fix
abcipp/mempool.go		 Added logger field assignment in NewPriorityMempool constructor to properly initialize the logger when creating a new PriorityMempool instance.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A whisker-twitch of joy today,
The logger now has found its way,
No longer lost in memory's mist,
One line of code, but oh, not missed!
The mempool sings with proper light, 🔦✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: assigning the logger to the PriorityMempool struct field in the constructor.
Description check ✅ Passed The description clearly explains what was changed, why it was necessary (preventing nil pointer panic), and provides validation evidence (test command).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/priority-mempool-logger

Comment @coderabbitai help to get the list of available commands and usage tips.

@beer-1 beer-1 force-pushed the fix/priority-mempool-logger branch from b515a82 to 5fa9ad4 Compare April 3, 2026 15:26
@github-actions

github-actions Bot commented Apr 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 5fa9ad4.
Ensure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@codecov

codecov Bot commented Apr 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.04%. Comparing base (7a827b1) to head (5fa9ad4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #496      +/-   ##
==========================================
+ Coverage   38.03%   38.04%   +0.01%     
==========================================
  Files         325      325              
  Lines       30765    30766       +1     
==========================================
+ Hits        11701    11705       +4     
+ Misses      17177    17175       -2     
+ Partials     1887     1886       -1
Files with missing lines Coverage Δ
abcipp/mempool.go 80.91% <100.00%> (+0.07%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@beer-1 beer-1 merged commit e805b08 into main Apr 3, 2026
13 checks passed
@beer-1 beer-1 deleted the fix/priority-mempool-logger branch April 3, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@beer-1