-
Notifications
You must be signed in to change notification settings - Fork 879
feat(beacon_chain): Added Box
to some fields in BeaconChainError
#7623
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
base: unstable
Are you sure you want to change the base?
feat(beacon_chain): Added Box
to some fields in BeaconChainError
#7623
Conversation
@eserilev can you PTAL and let me know if there is more to be done in this |
Hi @PoulavBhowmick03 In order to address this issue you'll need to figure out which variants inside
|
Got it, looking into it |
PTAL, it passes clippy now, do let me know if i need to make any more changes |
Thanks for your changes @PoulavBhowmick03 . It does pass Clippy now, but I'm not sure the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see requested changes above
@eserilev Removed Box from String, Hash256 and Checkpoint. Passes clippy as well, PTAL now. |
@@ -507,17 +506,20 @@ fn build_gossip_verified_blobs<T: BeaconChainTypes>( | |||
fn publish_blob_sidecars<T: BeaconChainTypes>( | |||
sender_clone: &UnboundedSender<NetworkMessage<T::EthSpec>>, | |||
blob: &GossipVerifiedBlob<T>, | |||
) -> Result<(), BlockError> { | |||
) -> Result<(), Box<BlockError>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to box this since it was throwing error. Do i need to try something else?
crate::publish_pubsub_message(sender_clone, pubsub_message) | ||
.map_err(|_| BlockError::BeaconChainError(Box::new(BeaconChainError::UnableToPublish))) | ||
crate::publish_pubsub_message(sender_clone, pubsub_message).map_err(|_| { | ||
Box::new(BlockError::BeaconChainError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this
0fa7ca5
to
16a348b
Compare
Issue Addressed
Fixes #7473
Box
Hash256 fields in variants like InconsistentPayloadReconstructed, BadPreState, RevertedFinalizedEpoch, etc.