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

core: gracefully handle missing homestead block config #2790

Closed
wants to merge 1 commit into from

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Jul 8, 2016

Our nodes didn't handle an interesting corner case until now: having a chain configuration (in the genesis.json), but not having a homesteadBlock set in it (i.e. {..., "config": {}}). Since our configs until now only featured the homestead block number, either it (block number) was present or the entire config section was missing (resulting the the defaults being used).

With the DAO fork being worked on and requiring an additional config section, it becomes entirely possible to have the DAO fork number set, but the homestead number not set. This PR ensures that it the homestead block number is missing, it's assumed not forked.

Note, there's almost no end user use case to set the DAO hard fork in a private network (so skipping the entire config section or only setting the homestead block is still entirely workable), but it is needed to properly test it n automated ways so better to fix it up properly now.

@robotally
Copy link

robotally commented Jul 8, 2016

Vote Count Reviewers
👍 1 @fjl
👎 0

Updated: Sat Jul 16 10:10:01 UTC 2016

@fjl
Copy link
Contributor

fjl commented Jul 12, 2016

👍

@@ -38,7 +38,7 @@ type ChainConfig struct {

// IsHomestead returns whether num is either equal to the homestead block or greater.
func (c *ChainConfig) IsHomestead(num *big.Int) bool {
if num == nil {
if c.HomesteadBlock == nil || num == nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

Make it true if nil

Copy link
Contributor

Choose a reason for hiding this comment

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

Or not. It might be easier to just leave the behaviour as is.

@karalabe
Copy link
Member Author

Superseeded and merged in #2814.

@karalabe karalabe closed this Jul 16, 2016
@obscuren obscuren removed the review label Jul 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants