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

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

Closed
wants to merge 3 commits into from

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Jul 8, 2016

Prerequisite PRs, merge these first to get rid of duplicate commits:


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.

@robotally
Copy link

robotally commented Jul 8, 2016

Vote Count Reviewers
👍 0
👎 0

Updated: Sat Jul 16 10:09:57 UTC 2016

@julian1
Copy link

julian1 commented Jul 10, 2016

It also helps full nodes to split the network between two forks and avoid cross network noise.

I am not sure why it is desirable to split at a p2p level as well as the chain level, and it could even be construed as interference with block relaying.

Also this only affects light-clients at the time of the fork correct? Since - presumably after the fork - a patch could be issued with hard-coded checkpoints to ensure they are on the right chain.

@karalabe
Copy link
Member Author

@julian1 The reason we need to split the network at a P2P level is because post fork the nodes from the two camps will be incompatible with each other. If they remain connected, they'll just be relaying "junk" form each others' perspective, which would put an extra burden on realizing nodes are incompatible and dropping them. (PS This is implemented in #2795).

The reason for the specific extra-data is indeed required during the transition period where light clients and fast sync wouldn't be able to know which chain is correct (post fork) at the moment of initial syncing.

Indeed it could be done with a hard coded block header, though there are mixed feelings towards those directions. We'll probably try to figure out a solution that can help us for future forks too, but the checkpointing is definitely a possibility.

@x-ETHeREAL-x
Copy link

Can an attacking miner node just mine non-fork blocks and include that extra-data and then confuse light client/fast syncers?

@julian1
Copy link

julian1 commented Jul 11, 2016

@karalabe I wonder if light-clients that do not validate protocol should expect to be able to follow their way through a protocol fork.

Flagging information about protocol in headers - adds complexity, creates potential for inconsistency, and is open (maybe incentivizes) to nodes lying.

Possible simpler alternatives,

  • light-clients turn on (partial) protocol validation at fork time, to establish which side of the fork they want
  • more simply - a post-fork patch could be issued after the fork specifying the chain checkpoints

@karalabe
Copy link
Member Author

@x-ETHeREAL-x They could, but full nodes won't accept it and hence you won't be able to create a heavy enough chain for light nodes to want to sync with you.

@julian1 It's not just light clients, it's fast-sync too. An alternative solution would be a multi-phse fast sync which switches to full sync around every fork point and then back to fast sync. Doable and elegant, albeit very error prone for this time frame. And it's hard to rationalize the need when post fork this complicated logic can indeed be thrown out for checkpoints as you mentioned.

@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants