[Merged by Bors] - Shift networking configuration #4426

armaganyildirak · 2023-06-22T11:49:06Z

Issue Addressed

Addresses #4401

Proposed Changes

Shift some constants into ChainSpec and remove the constant values from code space.

Additional Info

I mostly used MainnetEthSpec::default_spec() for getting ChainSpec. I wonder Did I make a mistake about that.

CLAassistant · 2023-06-22T11:49:13Z

All committers have signed the CLA.

beacon_node/beacon_chain/src/light_client_finality_update_verification.rs

divagant-martian · 2023-06-22T17:29:43Z

In most of these places (if not all, haven't check) you probably want to access the chain spec we already have. Left an example above, but @pawanjay176 prob has better pointers (and more time to put into the review)

pawanjay176

Initial review. Looking good, need to change a couple occurrences of default_spec to using the actual ChainSpec object.

beacon_node/beacon_chain/src/attestation_verification.rs

beacon_node/beacon_chain/src/block_verification.rs

beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs

beacon_node/beacon_chain/src/sync_committee_verification.rs

beacon_node/lighthouse_network/src/config.rs

beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs

beacon_node/lighthouse_network/src/rpc/handler.rs

consensus/types/src/chain_spec.rs

divagant-martian

Haven't finished the review, but found some time to give this a check

.vscode/settings.json

beacon_node/beacon_chain/src/attestation_verification.rs

beacon_node/beacon_chain/src/sync_committee_verification.rs

beacon_node/lighthouse_network/src/config.rs

beacon_node/lighthouse_network/src/rpc/protocol.rs

beacon_node/lighthouse_network/src/rpc/handler.rs

consensus/types/src/chain_spec.rs

…ming the ChainSpec

divagant-martian

@armaganyildirak has addressed the last bug I found and the pr is self-consistent now. LGTM

AgeManning

Ok. Lets get this in.

AgeManning · 2023-08-01T23:59:17Z

bors r+

## Issue Addressed Addresses [#4401](#4401) ## Proposed Changes Shift some constants into ```ChainSpec``` and remove the constant values from code space. ## Additional Info I mostly used ```MainnetEthSpec::default_spec()``` for getting ```ChainSpec```. I wonder Did I make a mistake about that. Co-authored-by: armaganyildirak <armaganyildirak@gmail.com> Co-authored-by: Paul Hauner <paul@paulhauner.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: Diva M <divma@protonmail.com>

bors · 2023-08-02T00:18:13Z

Build failed:

release-tests-windows

divagant-martian · 2023-08-02T00:57:54Z

weird that this failed in bors but not in the last commit. @paulhauner is this some kind of random failure? it looks kinda deterministic? or any ideas in general?
context

paulhauner · 2023-08-02T07:15:32Z

Hmm, a bunch of slashing protection tests started failing all with the same error:

---- attestation_tests::valid_empty_history stdout ----
thread 'attestation_tests::valid_empty_history' panicked at 'called `Result::unwrap()` on an `Err` value: SQLError("Unable to open database: Error(None)")', validator_client/slashing_protection/src/test_utils.rs:80:71

There's some conflicts to fix.. Maybe we can see how CI goes after they're fixed?

paulhauner · 2023-08-02T23:11:34Z

bors r+

## Issue Addressed Addresses [#4401](#4401) ## Proposed Changes Shift some constants into ```ChainSpec``` and remove the constant values from code space. ## Additional Info I mostly used ```MainnetEthSpec::default_spec()``` for getting ```ChainSpec```. I wonder Did I make a mistake about that. Co-authored-by: armaganyildirak <armaganyildirak@gmail.com> Co-authored-by: Paul Hauner <paul@paulhauner.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: Diva M <divma@protonmail.com>

bors · 2023-08-02T23:23:21Z

Build failed (retrying...):

debug-tests-ubuntu

## Issue Addressed Addresses [#4401](#4401) ## Proposed Changes Shift some constants into ```ChainSpec``` and remove the constant values from code space. ## Additional Info I mostly used ```MainnetEthSpec::default_spec()``` for getting ```ChainSpec```. I wonder Did I make a mistake about that. Co-authored-by: armaganyildirak <armaganyildirak@gmail.com> Co-authored-by: Paul Hauner <paul@paulhauner.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: Diva M <divma@protonmail.com>

bors · 2023-08-02T23:50:34Z

Build failed (retrying...):

release-tests-windows

## Issue Addressed Addresses [#4401](#4401) ## Proposed Changes Shift some constants into ```ChainSpec``` and remove the constant values from code space. ## Additional Info I mostly used ```MainnetEthSpec::default_spec()``` for getting ```ChainSpec```. I wonder Did I make a mistake about that. Co-authored-by: armaganyildirak <armaganyildirak@gmail.com> Co-authored-by: Paul Hauner <paul@paulhauner.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: Diva M <divma@protonmail.com>

bors · 2023-08-03T01:16:29Z

Build failed:

eth1-simulator-ubuntu

AgeManning · 2023-08-03T01:18:53Z

bors has chosen violence today.

bors retry

## Issue Addressed Addresses [#4401](#4401) ## Proposed Changes Shift some constants into ```ChainSpec``` and remove the constant values from code space. ## Additional Info I mostly used ```MainnetEthSpec::default_spec()``` for getting ```ChainSpec```. I wonder Did I make a mistake about that. Co-authored-by: armaganyildirak <armaganyildirak@gmail.com> Co-authored-by: Paul Hauner <paul@paulhauner.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: Diva M <divma@protonmail.com>

bors · 2023-08-03T02:40:50Z

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

Addresses [sigp#4401](sigp#4401) Shift some constants into ```ChainSpec``` and remove the constant values from code space. I mostly used ```MainnetEthSpec::default_spec()``` for getting ```ChainSpec```. I wonder Did I make a mistake about that. Co-authored-by: armaganyildirak <armaganyildirak@gmail.com> Co-authored-by: Paul Hauner <paul@paulhauner.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: Diva M <divma@protonmail.com>

armaganyildirak added 10 commits June 21, 2023 10:47

Adding networking configurations into ChainSpec

b97fe50

Finishing gossip_max_size

18c02d2

Finishing max_request_blocks?

3c770a7

Finishing max_chunk_size

e01ab7b

Finishing ttfb_timeout

e8f86cd

Finishing resp_timeout

2cccf6a

Finishing message_domain_valid_snappy

72f92af

Finishing maximum_gossip_clock_disparity?

489558c

Finishing maximum_gossip_clock_disparity

5e28c2e

Finishing for the PR

563ce17

Fixing conflicts

c0ac9dc

divagant-martian reviewed Jun 22, 2023

View reviewed changes

beacon_node/beacon_chain/src/light_client_finality_update_verification.rs Outdated Show resolved Hide resolved

armaganyildirak added 2 commits June 22, 2023 13:54

use chain instead of calling default_spec() function

47d1ae7

use chain instead of default_spec()

0724ae0

pawanjay176 requested changes Jun 23, 2023

View reviewed changes

armaganyildirak added 8 commits June 23, 2023 18:44

Adding gossip_max_size function ChainSpec object

a0f3b39

Adding ChainSpec object to other parts

816ebec

Adding ChainSpec object to other parts

b7e009c

Add the networking constants to Config object

d3d9b8f

Add ChainSpec object to max_rpc_size function

7090979

remove the needless borrows

eec5813

Fixing some test errors

6ca24e9

Fix the other tests

8f3b9a4

divagant-martian suggested changes Jun 28, 2023

View reviewed changes

armaganyildirak added 2 commits June 29, 2023 10:34

Resolve the some reviews

e003f20

fmt

49074a3

divagant-martian reviewed Jun 29, 2023

View reviewed changes

consensus/types/src/chain_spec.rs Outdated Show resolved Hide resolved

change self instead of &self for removing clone and unnecessary consu…

ba59277

…ming the ChainSpec

armaganyildirak added 2 commits August 1, 2023 11:48

change after review

165a1c2

change after review

a5368b0

divagant-martian approved these changes Aug 1, 2023

View reviewed changes

divagant-martian requested a review from AgeManning August 1, 2023 16:40

AgeManning approved these changes Aug 1, 2023

View reviewed changes

AgeManning added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Aug 1, 2023

AgeManning mentioned this pull request Aug 2, 2023

Remove the MaxRequestBlocks type #4562

Closed

armaganyildirak added 2 commits August 2, 2023 10:42

fix the confilicts

e9e1ff9

fmt

7fa9a17

bors bot changed the title ~~Shift networking configuration~~ [Merged by Bors] - Shift networking configuration Aug 3, 2023

bors bot closed this Aug 3, 2023

realbigsean mentioned this pull request Oct 12, 2023

Deneb #4054

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Shift networking configuration #4426

[Merged by Bors] - Shift networking configuration #4426

armaganyildirak commented Jun 22, 2023

CLAassistant commented Jun 22, 2023 •

edited

Loading

divagant-martian commented Jun 22, 2023 •

edited

Loading

pawanjay176 left a comment

divagant-martian left a comment

divagant-martian left a comment

AgeManning left a comment

AgeManning commented Aug 1, 2023

bors bot commented Aug 2, 2023

divagant-martian commented Aug 2, 2023 •

edited

Loading

paulhauner commented Aug 2, 2023 •

edited

Loading

paulhauner commented Aug 2, 2023

bors bot commented Aug 2, 2023

bors bot commented Aug 2, 2023

bors bot commented Aug 3, 2023

AgeManning commented Aug 3, 2023

bors bot commented Aug 3, 2023

[Merged by Bors] - Shift networking configuration #4426

[Merged by Bors] - Shift networking configuration #4426

Conversation

armaganyildirak commented Jun 22, 2023

Issue Addressed

Proposed Changes

Additional Info

CLAassistant commented Jun 22, 2023 • edited Loading

divagant-martian commented Jun 22, 2023 • edited Loading

pawanjay176 left a comment

Choose a reason for hiding this comment

divagant-martian left a comment

Choose a reason for hiding this comment

divagant-martian left a comment

Choose a reason for hiding this comment

AgeManning left a comment

Choose a reason for hiding this comment

AgeManning commented Aug 1, 2023

bors bot commented Aug 2, 2023

divagant-martian commented Aug 2, 2023 • edited Loading

paulhauner commented Aug 2, 2023 • edited Loading

paulhauner commented Aug 2, 2023

bors bot commented Aug 2, 2023

bors bot commented Aug 2, 2023

bors bot commented Aug 3, 2023

AgeManning commented Aug 3, 2023

bors bot commented Aug 3, 2023

CLAassistant commented Jun 22, 2023 •

edited

Loading

divagant-martian commented Jun 22, 2023 •

edited

Loading

divagant-martian commented Aug 2, 2023 •

edited

Loading

paulhauner commented Aug 2, 2023 •

edited

Loading