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

Common custom chain bugs #2448

Merged
merged 3 commits into from
Dec 13, 2022
Merged

Common custom chain bugs #2448

merged 3 commits into from
Dec 13, 2022

Conversation

acolytec3
Copy link
Contributor

While researching #2436, I encountered what I think are a couple of minor logic bugs:

  • When the miner starts in the client, if your common instance's consensus is set to pow, it errors trying to access the PoA consensus algorithm's subproperty of period since the consensusConfig object in the common._chainParams is undefined for PoW (this would presumably result in the same error for PoS). This resolve by just returning an empty object {} if no configuration parameters are defined.
  • I think we had some misplaced parentheses in cli in lines 588 - 591. The error should occur in any scenario where we provide custom genesis parameters and also pass in either network or networkId since those tell the common constructor to pull in predefined network genesis parameters.

gabrocheleau
gabrocheleau previously approved these changes Dec 12, 2022
@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Merging #2448 (e2b8d2c) into master (0d398cb) will decrease coverage by 0.52%.
The diff coverage is 80.00%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 89.51% <ø> (ø)
blockchain 90.03% <ø> (ø)
client 87.45% <ø> (ø)
common 98.08% <80.00%> (-0.09%) ⬇️
devp2p 91.78% <ø> (ø)
ethash ∅ <ø> (∅)
evm 79.73% <ø> (ø)
rlp ?
statemanager 89.61% <ø> (ø)
trie 90.66% <ø> (ø)
tx ?
util 84.59% <ø> (ø)
vm 85.67% <ø> (ø)

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

@acolytec3 acolytec3 merged commit f34d237 into master Dec 13, 2022
@acolytec3 acolytec3 deleted the common-customChain-bugs branch December 13, 2022 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants