Skip to content

chore: reduce clones#6903

Merged
LesnyRumcajs merged 1 commit intomainfrom
reduce-clones
Apr 14, 2026
Merged

chore: reduce clones#6903
LesnyRumcajs merged 1 commit intomainfrom
reduce-clones

Conversation

@LesnyRumcajs
Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs commented Apr 14, 2026

Summary of changes

Changes introduced in this pull request:

  • reduce the amount of some random unnecessary clones
  • I also forgot to label the release PR with Release so needed an excuse to do it somewhere 😁

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.

Outside contributions

  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

Release Notes

  • Performance

    • Optimized block payload decoding to reduce unnecessary memory allocation and copying during bitswap operations.
  • Refactor

    • Simplified internal event handling and data access patterns across multiple modules for improved code efficiency.
    • Streamlined type imports and method signatures for cleaner implementation.

@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner April 14, 2026 17:13
@LesnyRumcajs LesnyRumcajs requested review from akaladarshi and hanabi1224 and removed request for a team April 14, 2026 17:13
@LesnyRumcajs LesnyRumcajs added RPC requires calibnet RPC checks to run on CI Release PR which marks a new Release labels Apr 14, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 14, 2026

Walkthrough

This PR refactors event accessor APIs across the codebase by removing intermediate accessors (like StampedEvent::event() and ActorEvent::entries()) and introducing direct retrieval methods (StampedEvent::entries() and StampedEvent::into_entries()). All call sites are updated accordingly, and unnecessary memory allocations are eliminated where applicable.

Changes

Cohort / File(s) Summary
Event Accessor Refactoring
src/shim/executor.rs
Removed ActorEvent::entries() and StampedEvent::event() methods; added StampedEvent::entries() and StampedEvent::into_entries() for direct entry access without intermediate conversions.
Event API Call Sites
src/rpc/types/mod.rs, src/rpc/methods/eth/filter/mod.rs
Updated to use new direct entry accessors; replaced event.event().entries() with into_entries() or entries() patterns.
Memory Optimization
src/libp2p_bitswap/internals/codec.rs
Removed to_vec() allocation in BitswapRequestResponseCodec::read_request by passing payload data directly.
Type Import Cleanup
src/db/parity_db.rs
Added use bytes::Bytes import and updated subscribe_write_ops return type and all instantiation sites to use Bytes directly instead of fully-qualified bytes::Bytes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • hanabi1224
  • sudo-shashank
  • akaladarshi
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: reduce clones' is fully related to the main change in the changeset, which focuses on removing unnecessary clone operations across multiple files.

✏️ 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 reduce-clones
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch reduce-clones

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/shim/executor.rs`:
- Around line 243-251: The public methods entries and into_entries on
StampedEvent are undocumented; add Rustdoc comments above pub fn entries(&self)
and pub fn into_entries(self) explaining what each does, their return types,
ownership semantics (entries returns a Vec<Entry> cloned from the inner event;
into_entries consumes the StampedEvent and moves out the inner entries), any
differences between V3/V4 variants if relevant, and include examples or
panics/behavior notes if applicable so the public API is properly documented.
🪄 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: 0aee1614-66a8-49d4-b605-e99f0f8776b8

📥 Commits

Reviewing files that changed from the base of the PR and between e55a0a3 and 8249110.

📒 Files selected for processing (5)
  • src/db/parity_db.rs
  • src/libp2p_bitswap/internals/codec.rs
  • src/rpc/methods/eth/filter/mod.rs
  • src/rpc/types/mod.rs
  • src/shim/executor.rs

Comment thread src/shim/executor.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.97%. Comparing base (e55a0a3) to head (8249110).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/shim/executor.rs 71.42% 1 Missing and 1 partial ⚠️
src/db/parity_db.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/libp2p_bitswap/internals/codec.rs 100.00% <ø> (ø)
src/rpc/methods/eth/filter/mod.rs 88.54% <100.00%> (ø)
src/rpc/types/mod.rs 93.87% <100.00%> (-0.36%) ⬇️
src/db/parity_db.rs 69.00% <75.00%> (ø)
src/shim/executor.rs 85.94% <71.42%> (-0.40%) ⬇️

... and 10 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e55a0a3...8249110. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Apr 14, 2026
@LesnyRumcajs
Copy link
Copy Markdown
Member Author

All green

Merged via the queue into main with commit d9746f2 Apr 14, 2026
47 of 71 checks passed
@LesnyRumcajs LesnyRumcajs deleted the reduce-clones branch April 14, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release PR which marks a new Release RPC requires calibnet RPC checks to run on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants