Skip to content
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

chore(conductor): send boxed objects over channels #1865

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ethanoroshiba
Copy link
Contributor

@ethanoroshiba ethanoroshiba commented Dec 6, 2024

Summary

Changed firm block channel to use boxed ReconstructedBlock.

Background

A large error variant lint was triggered when running clippy with Rust 1.83.0 in #1857 due to send errors returning the whole block. Large enum variants should be avoided because enums are only as small as their largest variant: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant. Changing the channel to consist of a boxed block instead solves this problem at its source.

Changes

  • Changed firm block channel to use boxed ReconstructedBlock instead of it being unboxed.

Testing

Passing all tests.

Changelogs

No updates needed.

Breaking Changes

Overridden code freeze since this is a very small, non breaking change that shouldn't have any bearing since our previous audit.

Related Issues

closes #1858

@ethanoroshiba ethanoroshiba added conductor pertaining to the astria-conductor crate code-quality override-freeze labels Dec 6, 2024
@ethanoroshiba ethanoroshiba marked this pull request as ready for review December 6, 2024 21:12
@ethanoroshiba ethanoroshiba requested a review from a team as a code owner December 6, 2024 21:12
@SuperFluffy
Copy link
Member

Interesting that this showed up now - I remember clippy flagging this at some point, and I was almost certain that I did add an exception attribute, but maybe I am hallucinating.

Instead of boxing the problematic error variant, can you change the mpsc channel to be Box<T> instead? You'd then fix the error variants "for free" so to say.

@SuperFluffy
Copy link
Member

Neither PR text nor PR title match what the changes.

@ethanoroshiba ethanoroshiba changed the title chore(conductor): box large enum variants chore(conductor): use boxed block in firm channel Dec 10, 2024
@SuperFluffy SuperFluffy changed the title chore(conductor): use boxed block in firm channel chore(conductor): send boxed objects over channels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality conductor pertaining to the astria-conductor crate override-freeze
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore(conductor): avoid large enum variants
2 participants