Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Allow mining to be completely disabled on startup #248

Open
davidmurdoch opened this issue Dec 5, 2018 · 20 comments
Open

Allow mining to be completely disabled on startup #248

davidmurdoch opened this issue Dec 5, 2018 · 20 comments

Comments

@davidmurdoch
Copy link
Member

For complete determinism in our tests, like where we want to be able to compare generated blocks for strict equality, we currently need to set a high blockTime value and then use evm_mine with our desired timestamp. It'd be nice if we could just specify a blockTime of -1 to disable mining completely to always ensure determinism without having to choose an arbitrary blockTime value.

@marcelomorgado
Copy link

Will be possible with this new feature that new transactions only be mined when evm_mine be called?

@davidmurdoch
Copy link
Member Author

Yes, that would be the intent.

@marcelomorgado
Copy link

Hey, any news about this feature? Thanks!

@pcowgill
Copy link

pcowgill commented May 3, 2019

Because ganache-cli is frequently used in tests run during CI and determinism is needed there, I propose this issue gets a higher priority than low.

@davidmurdoch
Copy link
Member Author

We may enable this in the future, but it would be under a new option, something like miner.enabled, where the default is true. @seesemichaelj, thoughts on this for the Filecoin implementation?

@davidmurdoch davidmurdoch added this to the 7.1.0 milestone Feb 17, 2021
@mikeseese
Copy link
Contributor

mikeseese commented Feb 17, 2021

@davidmurdoch fairly easy to add; I've considered adding it recently for a certain test scenario. I believe this was the intention @tcoulter originally had with miner.automine, but I guess we just misinterpreted since auto and insta are easily confusable.

I can add it in the Filecoin implementation 👍

@davidmurdoch
Copy link
Member Author

@tcoulter was under the impression that you already did this via blockTime === -1.

@mikeseese
Copy link
Contributor

Oh, would you look at that:

A negative blockTime will require mining by manually calling Ganache.MineTipset.

I must be thinking of a different variable, or my internal time machine is warped by all the different changes I've done 😅 I double checked and the code should work that way; I'll need to update that test. Would you like to keep it as is then or would you prefer that blockTime gets set to 0 if it's negative and require a new variable miner.enabled to be set?

@mikeseese
Copy link
Contributor

For anyone following this thread, the above message about a negative block time is only currently available on our upcoming Filecoin-flavored Ganache. As mentioned by @davidmurdoch, this isn't currently supported in the default Ethereum-flavored Ganache

@davidmurdoch
Copy link
Member Author

Would you like to keep it as is then or would you prefer that blockTime gets set to 0 if it's negative and require a new variable miner.enabled to be set?

I suggested adding the blockTime -1 flag to ethereum ganache in an engineering meeting today, and the consensus there was that it weird, that it removes the ability to set an actual blocktime, and this it really is just a different feature and they shouldn't be joined. Those arguments make sense to me.

Since we are going forward with a miner.enabled solution on the Ethereum side, Filecoin probably should, too. Note: it could be called something else still, I'll try to discuss more internally.

@mikeseese
Copy link
Contributor

sounds good, i'll make the changes in @ganache/filecoin

@davidmurdoch davidmurdoch changed the title Allow mining to be completely disabled Allow mining to be completely disabled on startup Feb 18, 2021
@davidmurdoch
Copy link
Member Author

@seesemichaelj looks like miner.mine (default true) is the winner

@mikeseese
Copy link
Contributor

Ok..what is the accompanying help text/description that's going to make it clear to the user what it does?

@davidmurdoch
Copy link
Member Author

davidmurdoch commented Feb 18, 2021

I think it'll be different depending on the chain. For CLI docs, I'll probably go with something like

Enable mining. Set to `false` to pause the miner; call `miner_start` to resume.

@mikeseese
Copy link
Contributor

mikeseese commented Feb 18, 2021

Ok, I'll add a Ganache.EnableMiner method then?

Also does this mean that Ganache.MineTipset will not mine a tipset when miner.mine = false? The description you provided gives me that impression.

@davidmurdoch
Copy link
Member Author

It would still mine it. evm_mine still works after a call to miner_stop.

@mikeseese
Copy link
Contributor

Do you want one method Ganache.EnableMiner that has a boolean parameter to enable/disable, or two methods with no parameters Ganache.EnableMiner and Ganache.DisableMiner

Also do you want those to be StartMiner and StopMiner (or MinerStart/MinerStop) instead?

@davidmurdoch
Copy link
Member Author

Ganache.EnableMiner and Ganache.DisableMiner sound good

@ko3en
Copy link

ko3en commented Mar 18, 2022

How can this be done in the latest cli version?

@davidmurdoch
Copy link
Member Author

You need to stop the miner via miner_stop after start up.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Backlog
Development

No branches or pull requests

5 participants