Skip to content

refactor: various cleanups#1833

Merged
mattsse merged 4 commits intoparadigmxyz:mainfrom
huitseeker:warts2
Mar 18, 2023
Merged

refactor: various cleanups#1833
mattsse merged 4 commits intoparadigmxyz:mainfrom
huitseeker:warts2

Conversation

@huitseeker
Copy link
Contributor

Opinion on those refactors loosely held, happy to accomodate. But some of the same was recently merged in #1810, so I thought this rounds things up a bit.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

tysm!

lgtm

@mattsse mattsse added the C-debt A clean up/refactor of existing code label Mar 18, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2023

Codecov Report

Merging #1833 (3111d9d) into main (661a242) will increase coverage by 0.00%.
The diff coverage is 64.70%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #1833   +/-   ##
=======================================
  Coverage   73.50%   73.51%           
=======================================
  Files         410      410           
  Lines       50510    50486   -24     
=======================================
- Hits        37129    37115   -14     
+ Misses      13381    13371   -10     
Flag Coverage Δ
integration-tests 19.71% <37.25%> (+<0.01%) ⬆️
unit-tests 67.85% <56.86%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
crates/net/discv4/src/test_utils.rs 89.09% <0.00%> (+0.40%) ⬆️
crates/net/network/src/eth_requests.rs 76.69% <0.00%> (+0.73%) ⬆️
crates/primitives/src/block.rs 77.56% <0.00%> (+0.36%) ⬆️
crates/revm/revm-inspectors/src/tracing/types.rs 0.00% <0.00%> (ø)
crates/rpc/rpc/src/result.rs 48.88% <50.00%> (-0.03%) ⬇️
crates/rpc/rpc-engine-api/src/engine_api.rs 92.28% <57.14%> (+0.08%) ⬆️
crates/net/eth-wire/src/p2pstream.rs 79.65% <66.66%> (-0.04%) ⬇️
crates/net/nat/src/lib.rs 45.32% <66.66%> (+0.32%) ⬆️
crates/executor/src/blockchain_tree/chain.rs 85.13% <100.00%> (-0.06%) ⬇️
crates/executor/src/blockchain_tree/mod.rs 90.89% <100.00%> (+0.13%) ⬆️
... and 8 more

... and 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@onbjerg onbjerg changed the title Cleanups while going through some of the library refactor: various cleanups Mar 18, 2023
@mattsse mattsse merged commit 075544e into paradigmxyz:main Mar 18, 2023
};
let Some(head) = self.client.header(&head_block_hash)? else {
// Block is not known, nothing to do
return Ok(ForkchoiceUpdated::from_status(PayloadStatusEnum::Syncing)) };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you indent this? Seems weird that no linter caught it

return Ok(ForkchoiceUpdated::from_status(PayloadStatusEnum::Invalid {
let Some(head_td) = self.client.header_td(&head_block_hash)? else {
// internal error - we have the head block but not the total difficulty
return Ok(ForkchoiceUpdated::from_status(PayloadStatusEnum::Invalid {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

@mattsse
Copy link
Collaborator

mattsse commented Mar 18, 2023

Oops, sorry for merging this prematurely @onbjerg .

looks like ruftmt has problems formatting let else?

@onbjerg
Copy link
Collaborator

onbjerg commented Mar 18, 2023

Looks like it. A bit annoying

onbjerg added a commit that referenced this pull request Mar 18, 2023
onbjerg added a commit that referenced this pull request Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-debt A clean up/refactor of existing code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants