Skip to content
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

feat: bsc p2p network #8061

Merged
merged 6 commits into from May 22, 2024
Merged

feat: bsc p2p network #8061

merged 6 commits into from May 22, 2024

Conversation

Quertyy
Copy link
Contributor

@Quertyy Quertyy commented May 2, 2024

Fixes #7738
I had to add a method to set ForkId manually because of different bsc forks, let me know if there's a better way to do it

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.

nice, this is pretty cool!

a few nits

Comment on lines 300 to 304
/// Set the current fork id
pub fn set_fork_id(&mut self, fork_id: ForkId) {
self.cache.fork_id = fork_id;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we're using this, right?

setting the forkid on network alone is sufficient, I suppose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using it to set it manually because of the various forks the bsc has, I couldn't make a valid forkhash without

Copy link
Collaborator

Choose a reason for hiding this comment

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

rename to chainspec.rs

.chain_spec(bsc_chain_spec())
.set_head(head())
.listener_addr(local_addr)
.build(NoopProvider::default());
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is

pub fn build_with_noop_provider(

@mattsse mattsse marked this pull request as ready for review May 2, 2024 20:17
@mattsse mattsse requested a review from gakonst as a code owner May 2, 2024 20:17
@mattsse mattsse added the C-enhancement New feature or request label May 2, 2024
onbjerg
onbjerg previously requested changes May 5, 2024
Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

please also add the example to examples/readme.md

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.

this is great!

this results in a lot of disconnects due to useless peer disconnect message, unclear why, but we're able to connect at least.
So I'm merging this and maybe we can figure out why we get a ton of useless peer disconnects

@mattsse mattsse enabled auto-merge May 22, 2024 11:23
@Quertyy
Copy link
Contributor Author

Quertyy commented May 22, 2024

this is great!

this results in a lot of disconnects due to useless peer disconnect message, unclear why, but we're able to connect at least. So I'm merging this and maybe we can figure out why we get a ton of useless peer disconnects

Thank you! I will try to figure out why

@mattsse mattsse added this pull request to the merge queue May 22, 2024
Merged via the queue into paradigmxyz:main with commit cd039d3 May 22, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add example for BSC p2p
3 participants