cmd, core, eth, miner, params, tests: finalize the DAO fork #2814

Merged
merged 9 commits into from Jul 16, 2016

Projects

None yet

6 participants

@karalabe
Member
karalabe commented Jul 14, 2016 edited

Note: This PR contains all the pieces of the DAO hard-fork, each detailed below. The remaining PRs tagged dao-fork-part may be disregarded, they were meant to aid review.

Note 2: After this PR is merged, we need to prep a merge into the release/1.4 branch and inherently master, which needs a tiny amount of work due to the databse reogs already on develop.


cmd, core, eth, params: implement flags to control dao fork blocks #2788

This PR implements setting the --support-dao-fork and --oppose-dao-fork flags. As of now they only modify a single database entry specifying whether the current default behavior should change.

  • If no dao fork number is set, --support-dao-fork will set it to either mainnet/testnet number
  • If a dao fork number is set, --support-dao-fork will leave it at its present value
  • If a dao fork number is set, --oppose-dao-fork will remove it

Caveat: setting the dao number from a custom genesis file, removing it via --oppose-dao-fork and reenabling it via --support-dao-fork will result in the mainnet/testnet number being set, the original private number being lost. Don't use this willy nilly on private networks; rather use geth init genesis.json for those scenarios.

This PR further drops support to the deprecated --genesis flag. This is important because the --support-dao-fork and --oppose-dao-fork flags need to know the previous state/number in the database to ensure it's not accidentally overwritten! We do not know this with the --genesis flag as the user might supply something we can only interpret much later in the code. Note, for main and test network we know in advance what their genesis file would contain, so that's not an issue.

core: gracefully handle missing homestead block config #2790

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.

cmd/geth, miner, params: special extradata for DAO fork start #2791

This PR modifies the miner so that if the DAO hard-fork is enabled, the fork block and 10 consecutive blocks all have 0x64616f2d686172642d666f726b (hex for dao-hard-fork) as their extra-data.

This is important to allow header-only mode clients (fast sync algo, light client) to verify whether a header is indeed on the correct chain and not get duped into syncing a live alternative fork. It also helps full nodes to split the network between two forks and avoid cross network noise.

Consecutive blocks are needed to prevent any malicious miner from minting a non-forked block with the fork extra-data. Unless almost all non-forking miners are malicious, there will be a non-forking block in the first 10 that does not have the fork extra-data set, separating that chain from the forked chain.

This PR does not contain unit tests as out miner module wasn't written with unit tests in mind and I'd like to avoid a too invasive rewrite in such a critical time. However, this feature is covered by hive end to end tests with full mining.

cmd, core, miner: add extradata validation to consensus rules #2794

This PR makes the necessary consensus modifications to header validation rules so that block header extraData fields are enforced to be either 0x64616f2d686172642d666f726b (hex for dao-hard-fork) for pro-forkers or everything but that for non-forkers. These rules are upheld for 10 blocks straight to protect both forkers and non-forkers against malicious actors of the opposite side.

Since non-forkers also need to know the actual DAO hard-fork block number to enforce themselves onto the opposite side of the network (i.e. a non-forker fast syncing needs to be aware of the headers it needs to check), this PR makes a further modification to the previous chain configurations so not only a single ChainConfig.DAOForkNumber is maintained (previously implicitly signalling pro-fork), but rather an explicit ChainConfig.DAOForkSupport boolean flag is added too. This way both camps can maintain the block numbers whilst also allowing both decisions.

core, eth: enforce network split post DAO hard-fork #2795

This PR is meant to enforce the network split after passing the DAO hard-fork block number. It is done by requesting the DAO fork-block's header immediately after an eth handshake completes. If the DAO challenge isn't completed within 15 seconds, the connection is dropped.

To complete the DAO challenge, the remote peer must reply with the requested header (every peer will do so, it's how the eth protocol works). A few things can happen:

  • The returned header's extra-data conforms to the side we're on. Challenge completed.
  • The returned header's extra-data doesn't conform to our side. Challenge failed, peer dropped.
  • No header is returned, this complicates things:
    • We ourselves haven't reached the fork point, assume friendly (no way to challenge)
    • We've already reached the fork point, so we already have our fork block:
      • If peer's TD is smaller than this, we cannot check it's side, assume friendly
      • If peer's TD is higher than this, assume the packet is a reply to something else, wait further
core, params, tests: add DAO hard-fork balance moves #2800

This PR implements the last component of the DAO hard-fork, namely defines the list of DAO addresses the balances of which should be moved and executes the actual state transformation when the DAO hard-fork block arrives.

The PR also adds the DAO consensus tests from the upstream https://github.com/ethereum/tests repo, although as far as I know those tests are still under development, and hence are subject to change.

Note, the fork parameters (block number and refund account) are just placeholders, whereas the DAO drain list may or may not change.

cmd, core, eth, miner, params, tests: finalize the DAO fork #2814

The last commit for the DAO hard-fork. This finalizes the DAO child list, the refund contract address, the testnet and mainnet transition blocks as well as sets the default behavior of Geth to be pro-fork as per internal discussions.

@robotally
robotally commented Jul 14, 2016 edited
Vote Count Reviewers
👍 0
👎 0

Updated: Fri Jul 15 06:52:33 UTC 2016

@karalabe karalabe changed the title from Dao hard finalcombo to cmd, core, eth, miner, params, tests: finalize the DAO fork Jul 14, 2016
@ebuchman
Contributor

Is there a spec for the hard fork independent of the PRs that you could link to?

Is there no plan to prevent transaction replay across forks?

@jarradh
Contributor
jarradh commented Jul 14, 2016 edited

sets the default behavior of Geth to be pro-fork as per internal discussions
...

this line of reasoning was sound

@julian1
julian1 commented Jul 14, 2016 edited

There are 35 files changed, including modifications to the configuration/genesis, p2p networking, and EVM protocol. Given the inherent complexity - I wonder if the default should be the option with less technical risk.

@julian1
julian1 commented Jul 15, 2016 edited

as well as sets the default behavior of Geth to be pro-fork as per internal discussions.

The application could randomize this boolean on first-start when there is no explicit flag to support or oppose the fork.

This would help shield developers from accusations of bias and protect Geth neutrality.

Was this possibility considered?

Edit. Alternatively sidestep the issue of default behavior entirely and make it mandatory for the user to choose. Same as mist ethereum/mist#970.

@jarradh
Contributor
jarradh commented Jul 15, 2016

The explicit flag for or against makes far more sense to me

@karalabe
Member

@julian1 Most of the change files are tests (22/35). The actual changes aren't that huge, but I agree that it's a non trivial change. Regarding the defaults, I believe @obscuren is prepping a post about it.

karalabe added some commits Jul 7, 2016
@karalabe karalabe cmd, core, eth, params: implement flags to control dao fork blocks 6060e09
@karalabe karalabe core: gracefully handle missing homestead block config 9e56811
@karalabe karalabe cmd/geth, miner, params: special extradata for DAO fork start 1e24c2e
@karalabe karalabe cmd, core, miner: add extradata validation to consensus rules a87089f
@karalabe karalabe core, eth: enforce network split post DAO hard-fork 7f00e8c
@karalabe karalabe core, params, tests: add DAO hard-fork balance moves 461cdb5
@karalabe karalabe cmd, core, eth, miner, params, tests: finalize the DAO fork
2c2e389
@karalabe karalabe accounts, core, eth: pass chain config for chain maker to test DAO 3291235
@karalabe karalabe cmd/utils, eth: display the user's current fork, minor text tweak
993b412
@obscuren obscuren merged commit a4c4125 into ethereum:develop Jul 16, 2016

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
commit-message-check/gitcop All commit messages are valid
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@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