feat(beacon_chain): Added Box to some fields in BeaconChainError

[PoulavBhowmick03]

Issue Addressed

Fixes #7473

• Box Hash256 fields in variants like InconsistentPayloadReconstructed, BadPreState, RevertedFinalizedEpoch, etc.
• Kept frequently accessed small fields (like Slot) unboxed for performance

[PoulavBhowmick03]

@eserilev can you PTAL and let me know if there is more to be done in this

[eserilev]

Hi @PoulavBhowmick03 In order to address this issue you'll need to figure out which variants inside BeaconChainError are too large and box those specific variants. Right now your changes wont pass clippy, you can run make lint locally to see the errors. Here's an example error message

error: the `Err`-variant returned from this function is very large
    --> beacon_node/beacon_chain/src/test_utils.rs:3096:10
     |
3096 |     ) -> Result<(), SyncCommitteeError> {
     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: beacon_node/beacon_chain/src/sync_committee_verification.rs:192:5
     |
192  |     BeaconChainError(BeaconChainError),
     |     ---------------------------------- the largest variant contains at least 128 bytes
     |
     = help: try reducing the size of `sync_committee_verification::Error`, for example by boxing large elements or replacing it with `Box<sync_committee_verification::Error>`
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err

[PoulavBhowmick03]

Hi @PoulavBhowmick03 In order to address this issue you'll need to figure out which variants inside BeaconChainError are too large and box those specific variants. Right now your changes wont pass clippy, you can run make lint locally to see the errors. Here's an example error message

error: the `Err`-variant returned from this function is very large
    --> beacon_node/beacon_chain/src/test_utils.rs:3096:10
     |
3096 |     ) -> Result<(), SyncCommitteeError> {
     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: beacon_node/beacon_chain/src/sync_committee_verification.rs:192:5
     |
192  |     BeaconChainError(BeaconChainError),
     |     ---------------------------------- the largest variant contains at least 128 bytes
     |
     = help: try reducing the size of `sync_committee_verification::Error`, for example by boxing large elements or replacing it with `Box<sync_committee_verification::Error>`
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err

Got it, looking into it

[PoulavBhowmick03]

@eserilev

PTAL, it passes clippy now, do let me know if i need to make any more changes

[eserilev]

Thanks for your changes @PoulavBhowmick03 . It does pass Clippy now, but I'm not sure the Hash256 values need Box. For example I just removed Box from the Hash256 inside the MissingBeaconBlock error variant and it passes clippy. The goal here is to minimize Box usage as much as we can so you might need to dig a bit deeper to find which variants absolutely need Box and which don't.

[eserilev]

see requested changes above

[PoulavBhowmick03]

@eserilev Removed Box from String, Hash256 and Checkpoint. Passes clippy as well, PTAL now.

[PoulavBhowmick03]

I had to box this since it was throwing error. Do i need to try something else?

[PoulavBhowmick03]

And this

[PoulavBhowmick03]

@eserilev is the test failing something to do with my changes? or a fault on the infra side? or some other logic

[PoulavBhowmick03]

hey @eserilev any updates on this, any changes for me to make?

[mergify]

Hi @PoulavBhowmick03, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time.