Skip to content

Conversation

danielrachi1
Copy link
Contributor

Issue Addressed

#3938

Proposed Changes

  • network::Processor is deleted and all it's logic is moved to network::Router.
  • The network::Router module is moved to a single file.
  • The following functions are deleted: on_disconnect send_status on_status_response on_blocks_by_root_request on_lightclient_bootstrap on_blocks_by_range_request on_block_gossip on_unaggregated_attestation_gossip on_aggregated_attestation_gossip on_voluntary_exit_gossip on_proposer_slashing_gossip on_attester_slashing_gossip on_sync_committee_signature_gossip on_sync_committee_contribution_gossip on_light_client_finality_update_gossip on_light_client_optimistic_update_gossip. This deletions are possible because the updated Router allows the underlying methods to be called directly.

@divagant-martian divagant-martian self-requested a review February 21, 2023 19:16
@paulhauner paulhauner added the ready-for-review The code is ready for review label Feb 22, 2023
@divagant-martian
Copy link
Contributor

Ignore the conflicts for now, they should be resolved after #4019

Copy link
Contributor

@divagant-martian divagant-martian left a comment

Choose a reason for hiding this comment

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

This is looking good! I left some suggestions regarding the merge of the structs, but most comments are simply docs and comment updates that we can take advantage and tackle right now. Also, as per previous comment please ignore the conflicts to unstable for now

@divagant-martian divagant-martian added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Feb 22, 2023
@divagant-martian divagant-martian added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Feb 26, 2023
Copy link
Contributor

@divagant-martian divagant-martian left a comment

Choose a reason for hiding this comment

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

Yeah this is a good and needed change. I missed a little typo, otherwise looks perfect. Thanks for the help!

@divagant-martian divagant-martian added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Feb 27, 2023
@divagant-martian divagant-martian removed the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Feb 27, 2023
@divagant-martian
Copy link
Contributor

@danielrachi #4019 has been merged so now conflicts should be more manageable, could you please merge unstable into your branch to handle those?

@divagant-martian divagant-martian added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Mar 1, 2023
Copy link
Contributor

@divagant-martian divagant-martian left a comment

Choose a reason for hiding this comment

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

LGTM. I still want to run this on our infra before merging

@divagant-martian divagant-martian added under-review A reviewer has only partially completed a review. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Mar 6, 2023
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for this :)

@divagant-martian divagant-martian added ready-for-merge This PR is ready to merge. and removed under-review A reviewer has only partially completed a review. labels Mar 10, 2023
@AgeManning AgeManning added the v4.0.0 Mainnet Capella release expected late March 2023 label Mar 14, 2023
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Mar 14, 2023
## Issue Addressed

#3938 

## Proposed Changes

- `network::Processor` is deleted and all it's logic is moved to `network::Router`.
- The `network::Router` module is moved to a single file.
- The following functions are deleted: `on_disconnect` `send_status` `on_status_response` `on_blocks_by_root_request` `on_lightclient_bootstrap` `on_blocks_by_range_request` `on_block_gossip` `on_unaggregated_attestation_gossip` `on_aggregated_attestation_gossip` `on_voluntary_exit_gossip` `on_proposer_slashing_gossip` `on_attester_slashing_gossip` `on_sync_committee_signature_gossip` `on_sync_committee_contribution_gossip` `on_light_client_finality_update_gossip` `on_light_client_optimistic_update_gossip`. This deletions are possible because the updated `Router` allows the underlying methods to be called directly.
@bors
Copy link

bors bot commented Mar 14, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Mar 14, 2023
## Issue Addressed

#3938 

## Proposed Changes

- `network::Processor` is deleted and all it's logic is moved to `network::Router`.
- The `network::Router` module is moved to a single file.
- The following functions are deleted: `on_disconnect` `send_status` `on_status_response` `on_blocks_by_root_request` `on_lightclient_bootstrap` `on_blocks_by_range_request` `on_block_gossip` `on_unaggregated_attestation_gossip` `on_aggregated_attestation_gossip` `on_voluntary_exit_gossip` `on_proposer_slashing_gossip` `on_attester_slashing_gossip` `on_sync_committee_signature_gossip` `on_sync_committee_contribution_gossip` `on_light_client_finality_update_gossip` `on_light_client_optimistic_update_gossip`. This deletions are possible because the updated `Router` allows the underlying methods to be called directly.
@bors
Copy link

bors bot commented Mar 14, 2023

Build failed:

@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Mar 15, 2023
## Issue Addressed

#3938 

## Proposed Changes

- `network::Processor` is deleted and all it's logic is moved to `network::Router`.
- The `network::Router` module is moved to a single file.
- The following functions are deleted: `on_disconnect` `send_status` `on_status_response` `on_blocks_by_root_request` `on_lightclient_bootstrap` `on_blocks_by_range_request` `on_block_gossip` `on_unaggregated_attestation_gossip` `on_aggregated_attestation_gossip` `on_voluntary_exit_gossip` `on_proposer_slashing_gossip` `on_attester_slashing_gossip` `on_sync_committee_signature_gossip` `on_sync_committee_contribution_gossip` `on_light_client_finality_update_gossip` `on_light_client_optimistic_update_gossip`. This deletions are possible because the updated `Router` allows the underlying methods to be called directly.
@bors bors bot changed the title Remove Router/Processor Code [Merged by Bors] - Remove Router/Processor Code Mar 15, 2023
@bors bors bot closed this Mar 15, 2023
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
- `network::Processor` is deleted and all it's logic is moved to `network::Router`.
- The `network::Router` module is moved to a single file.
- The following functions are deleted: `on_disconnect` `send_status` `on_status_response` `on_blocks_by_root_request` `on_lightclient_bootstrap` `on_blocks_by_range_request` `on_block_gossip` `on_unaggregated_attestation_gossip` `on_aggregated_attestation_gossip` `on_voluntary_exit_gossip` `on_proposer_slashing_gossip` `on_attester_slashing_gossip` `on_sync_committee_signature_gossip` `on_sync_committee_contribution_gossip` `on_light_client_finality_update_gossip` `on_light_client_optimistic_update_gossip`. This deletions are possible because the updated `Router` allows the underlying methods to be called directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. v4.0.0 Mainnet Capella release expected late March 2023
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants