fix: do not wrap Sender with Arc

[hanabi1224]

Summary of changes

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

• I have performed a self-review of my own code,
• I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
• I have added tests that prove my fix is effective or that my feature works (if possible),
• I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

• Refactor
  • Simplified internal message pool trigger handling by removing unnecessary abstraction layers, improving code maintainability without impacting user-facing functionality.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Removes Arc wrapping from the republish trigger sender in the message pool module. The repub_trigger type changes from Arc<flume::Sender<()>> to flume::Sender<()>, with corresponding updates across the module's public API, internal implementations, and test setups.

Changes

Cohort / File(s)	Summary
Message pool core refactoring src/message_pool/msgpool/mod.rs	Updated head_change() function signature and MessagePool::new() constructor to use flume::Sender<()> instead of Arc<flume::Sender<()>>. Removed Arc import. Updated all constructor calls to use Default::default() in place of Arc::default().
Message pool implementation src/message_pool/msgpool/msg_pool.rs	Changed repub_trigger cloning from Arc-wrapped to direct clone within the head-change handling block.
Test code src/message_pool/msgpool/selection.rs	Updated test setup to initialize repub_trigger with direct clone instead of Arc::new() wrapping.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• The changes follow a consistent, mechanical refactoring pattern across all files
• Type simplification (removing Arc layer) with straightforward call site updates
• Verify that flume::Sender supports direct cloning without Arc and that no other code paths depend on Arc-wrapped behavior

Possibly related PRs

• chore: refactor cur_tipset in MessagePool to use RwLock instead Mutex #5901: Concurrently modifies the same head_change() codepath in src/message_pool/msgpool/mod.rs, replacing cur_tipset's Mutex with RwLock while this PR removes Arc wrapping from repub_trigger.

Suggested reviewers

• elmattic
• akaladarshi
• LesnyRumcajs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: removing Arc wrapping from Sender types across multiple files in the message pool module.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch hm/no-arc-around-sender

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 522e65a and 5c8e722.

📒 Files selected for processing (3)

• src/message_pool/msgpool/mod.rs (8 hunks)
• src/message_pool/msgpool/msg_pool.rs (1 hunks)
• src/message_pool/msgpool/selection.rs (2 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:461-487
Timestamp: 2025-08-08T12:11:55.266Z
Learning: Forest (src/ipld/util.rs, Rust): In UnorderedChainStream::poll_next, dropping `extract_sender` (when no more tipsets and the extract queue is empty) is the intended shutdown signal for workers. Any subsequent attempt to enqueue work after this drop is a logic error and should be treated as an error; do not change `send()` to ignore a missing sender.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6074
File: src/rpc/methods/chain.rs:55-56
Timestamp: 2025-09-17T11:32:44.185Z
Learning: In the Forest codebase, hanabi1224 prefers that CodeRabbit should not warn about potential compilation issues (such as Send bounds, async/await compatibility, etc.) since they are already guarded by CI tests. Focus should be on higher-level design and logic issues instead.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.

📚 Learning: 2025-08-08T12:11:55.266Z

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:461-487
Timestamp: 2025-08-08T12:11:55.266Z
Learning: Forest (src/ipld/util.rs, Rust): In UnorderedChainStream::poll_next, dropping `extract_sender` (when no more tipsets and the extract queue is empty) is the intended shutdown signal for workers. Any subsequent attempt to enqueue work after this drop is a logic error and should be treated as an error; do not change `send()` to ignore a missing sender.

Applied to files:

• src/message_pool/msgpool/msg_pool.rs
• src/message_pool/msgpool/mod.rs

🧬 Code graph analysis (1)

src/message_pool/msgpool/mod.rs (1)

src/message_pool/msgpool/msg_pool.rs (3)

• flume (490-490)
• new (72-77)
• new (465-583)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Build forest binaries on Linux AMD64
• GitHub Check: tests
• GitHub Check: tests-release
• GitHub Check: cargo-publish-dry-run
• GitHub Check: All lint checks
• GitHub Check: Build MacOS
• GitHub Check: Build Ubuntu

🔇 Additional comments (6)

src/message_pool/msgpool/selection.rs (1)

908-908: LGTM: Simplified repub_trigger cloning.

The direct clone of repub_trigger correctly reflects the type change from Arc<flume::Sender<()>> to flume::Sender<()>. Since flume::Sender is already internally reference-counted and cheaply cloneable, the Arc wrapper was redundant.

Also applies to: 946-946

src/message_pool/msgpool/msg_pool.rs (1)

516-516: LGTM: Correctly updated for simplified repub_trigger type.

The direct clone is appropriate now that repub_trigger is a flume::Sender<()> rather than Arc<flume::Sender<()>>. This maintains the same functionality while removing an unnecessary layer of indirection.

src/message_pool/msgpool/mod.rs (4)

12-12: LGTM: Import cleanup.

Correctly removed the unused Arc import since repub_trigger no longer requires Arc wrapping.

---

216-216: LGTM: Simplified function signature.

The head_change function signature correctly updated to accept flume::Sender<()> directly instead of Arc<flume::Sender<()>>. This is a good simplification since flume::Sender is already designed to be cheaply cloneable and thread-safe.

---

359-366: LGTM: Test code consistently updated.

All test instantiations of MessagePool::new are correctly updated to work with the new parameter types using Default::default(). The changes are consistent across all test cases.

Also applies to: 445-452, 512-519, 611-618

---

473-473: LGTM: Direct cloning is appropriate.

The direct clone of mpool.repub_trigger is correct and consistent with the simplified type. This maintains the same functionality while removing unnecessary Arc indirection.

Also applies to: 540-540

---

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