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, consensus, core, miner: instatx clique for --dev #15323

Merged
merged 3 commits into from
Oct 24, 2017

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Oct 17, 2017

Since forever we've had --dev in order to spin up a developer chain. Unfortunately it had quite a few issues:

  • Being based on ethash, it was wasting an entire CPU core to keep mining non-stop
  • Mining non-stop made the block time converge to 15 seconds, annoying to test
  • Running a start/stop script to only mine on transactions is cumbersome

This PR attempts to fix the --dev flag by:

  • Replace the ethash consensus engine with clique
    • This allows blocks to be mined as fast as the CPU and memory can handle, no PoW nonsense
  • Unless a datadir is explicitly needed, databases are ephemeral in-memory
    • This permits fast test runs without being bound by disk
  • Permit 0 block times in the clique consensus engine
    • This ensures that blocks can come as fast as the miner pleases
  • Forbid empty blocks in clique for 0 periods
    • Since there's no block/uncle rewards, insta-mining empty blocks in pointless
  • For 0-period clique miners, commit new mining work whenever a transaction arrives
    • This allows each transaction to be instantly included (this may need tuning though).
  • Add --dev.period to control the block period time and not always default to 0
    • This allows developers to also tests dapps with real-ish block times

Note: the PR also supports the --datadir flag for persistent storage (emphemeral by default).

@MicahZoltu
Copy link
Contributor

Possibly worth delaying until a future PR, but it would be nice to be able to configure dev mode to mine blocks on a fixed schedule as well. Depending on the type of test I'm running, I sometimes want to mine blocks every time a transaction comes in, and other times I want to have regular blocks (e.g., 1 per second).

The primary reason for having regular block intervals in testing is to ensure that the dApp/scripts work in an environment where there may be a delay between transaction submission and transaction mining (receipt available).

Instant seal on the other hand is really useful for running unit tests, where you aren't testing the dApp->Ethereum Node interaction, but instead testing Solidity contracts or general features and you don't want to sit around waiting forever (second) for blocks to arrive to burn through your test suite.

From the PR description, it sounds like this fulfills one of the two needs (instant blocks on transaction), but still requires mining and 15 second block times for the latter.

Perhaps fixed block time is already possible by running a single-node clique chain? If so, then that half of the problem is already solved and this PR finishes it. :)

@karalabe
Copy link
Member Author

@MicahZoltu Clique does support arbitrary block times, so supporting your request would entail overriding the default 0 block time for --dev with one provided from a flag. Should be doable, I'll add it to this PR.

@karalabe
Copy link
Member Author

@MicahZoltu Added dev.period to control the block time in seconds. The default 0 is insta-mining (if there are txs). Setting it to anything higher will result in the usual mode of operation.

@karalabe karalabe merged commit 6d6a5a9 into ethereum:master Oct 24, 2017
vincentserpoul pushed a commit to vincentserpoul/go-ethereum that referenced this pull request Nov 22, 2017
* cmd, consensus, core, miner: instatx clique for --dev

* cmd, consensus, clique: support configurable --dev block times

* cmd, core: allow --dev to use persistent storage too
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.

3 participants