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: add Parlia genesis config for BSC #740

Merged
merged 3 commits into from
May 15, 2024

Conversation

forcodedancing
Copy link
Contributor

@forcodedancing forcodedancing commented May 13, 2024

Motivation

BSC uses Parlia consensus engine. To support BSC genesis, Parlia config is added.

Solution

Update genesis config to support Parlia.

Copy link
Member

@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.

supportive, pedantic doc nit

crates/genesis/src/lib.rs Show resolved Hide resolved
@prestwich
Copy link
Member

given that the config props are identical to clique config, should we reuse the struct?

@forcodedancing
Copy link
Contributor Author

given that the config props are identical to clique config, should we reuse the struct?

You are right. The fields are the same. However, the meanings of the fields are different for clique and parlia.

I am ok if you think reuse is better. thanks a lot.

@prestwich
Copy link
Member

You are right. The fields are the same. However, the meanings of the fields are different for clique and parlia.

Ya, I understand that in Parlia the fields are used differently than in clique. But given that the names and comments are the same it seems safe to reuse. I don't have a strong opinion here, so I'm fine leaving it as 2 identical structs

@@ -541,6 +545,21 @@ pub struct CliqueConfig {
pub epoch: Option<u64>,
}

/// Consensus configuration for Parlia.
/// Parlia is the consensus engine for BNB Smart Chain.
/// For the general introduction: https://docs.bnbchain.org/docs/learn/consensus/
Copy link
Member

Choose a reason for hiding this comment

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

doc nit again:
for links you should follow recommendation here:

https://doc.rust-lang.org/rustdoc/lints.html?highlight=http%3A%2F%2F#bare_urls

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, thanks a lot

Copy link
Member

@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 doesn't have any impact on how genesis is used for eth and alike so I'm fine with adding this field out of convenience

@mattsse mattsse merged commit 5fbf57b into alloy-rs:main May 15, 2024
24 checks passed
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
* feat: add Parlia genesis config for BSC

* chore: update comments based on review suggestion

* fix review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants