hardhat icon indicating copy to clipboard operation
hardhat copied to clipboard

feature request: `hardhat_mine` should allow zero intervals

Open cruzdanilo opened this issue 2 years ago • 20 comments

hardhat_mine gives advanced control over mining, block count and timestamp interval, but it's still not possible to include/mine a transaction without increasing the timestamp. this is relevant when testing fee calculations, for example.

provider.send("hardhat_mine", ["0x2", "0x0"]) works, but there is always a timestamp increase:

https://github.com/NomicFoundation/hardhat/blob/4d43cbbebdb2ced843a29fa12b895d4fe5f01e83/packages/hardhat-core/src/internal/hardhat-network/provider/node.ts#L455-L462

https://github.com/NomicFoundation/hardhat/blob/4d43cbbebdb2ced843a29fa12b895d4fe5f01e83/packages/hardhat-core/src/internal/hardhat-network/provider/node.ts#L519-L520

another option would be something similar to evm_setAutomine to disable auto timestamp increments (dapptools/foundry default behavior).

cruzdanilo avatar Mar 17 '22 15:03 cruzdanilo

This issue is also being tracked on Linear.

We use Linear to manage our development process, but we keep the conversations on Github.

LINEAR-ID: 0e6d54e9-3fda-4b1a-bfa4-b9010fcb64c0

github-actions[bot] avatar Mar 17 '22 15:03 github-actions[bot]

I think it's part of the consensus rules that each block must have a timestamp that is different from the previous block's. That's why we enforce this.

That being said, hardhat_mine already does things that are not technically valid. For example, the mined blocks don't have a valid parentHash (to do that, we would need to create all blocks, and that wouldn't be possible in constant time). And, of course, we have things like hardhat_setStorageAt, etc.

So I don't think it's unreasonable to consider adding support for this. But it would be interesting to first understand better why you would need that behavior. Can you expand on this:

this is relevant when testing fee calculations, for example

?

fvictorio avatar Mar 18 '22 13:03 fvictorio

this is an example of an real test (flaky). currently we're using provider.send("evm_setAutomine", [false]), provider.send("evm_setNextBlockTimestamp", [timestamp]) and then provider.send("hardhat_mine", ["0x2", "0x0"]) to include new transactions, but the small timestamp difference sometimes causes a difference in the calculated fees

1) [redacted]
      [redacted] Early Withdrawal / Early Repayment
        GIVEN a borrowMP of 10000 (500 fees owed by user)
          WHEN an early repayment of 5250
            AND WHEN an early repayment of 5250 with a spFeeRate of 10%
              THEN the actualRepayAmount returned is 5025 = 5250 - 250 - 25 (10% spFeeRate):

    AssertionError: Expected "5025001041666666666666" to be equal 5025000000000000000000
    + expected - actual

      {
    -  "_hex": "0x011067e23c68b1e40000"
    +  "_hex": "0x011067e5efcc9d881aaa"
        "_isBigNumber": true
      }

besides mining with zero interval, it would also be nice to allow evm_setNextBlockTimestamp with the same timestamp from the previous block.

cruzdanilo avatar Mar 18 '22 15:03 cruzdanilo

Oh, so what you want is "Mine 1000 blocks so that the latest block has timestamp X"? You can do that by using (X - now) / 1000 as the interval, but I understand that that can be annoying.

I'll think about this, thanks for the explanation.

fvictorio avatar Mar 21 '22 14:03 fvictorio

@fvictorio not exactly. what i want is to evm_setNextBlockTimestamp(x), then send+mine some transactions and at the end, block.timestamp is still x that i set initially.

cruzdanilo avatar Mar 21 '22 14:03 cruzdanilo

Oh, I see. Can you give me a higher-level description of the test, to understand better why this is necessary? Or a simplified example where this would be needed?

fvictorio avatar Mar 21 '22 15:03 fvictorio

This issue was marked as stale because it didn't have any activity in the last 30 days. If you think it's still relevant, please leave a comment indicating so. Otherwise, it will be closed in 7 days.

github-actions[bot] avatar Apr 27 '22 20:04 github-actions[bot]

It means many TXs in the next block, but process all of TXs will have a time delay, so if have some control can "freeze an EVM block have same time" until all the tx have been done, it lets all TXs have the same timebase, prevent some sensitive tests go Error

weretyczx avatar Apr 28 '22 09:04 weretyczx

It means many TXs in the next block, but process all of TXs will have a time delay,

If what you want is to mine several transactions in the same block, you can disable automining and manually trigger a mine after sending the transactions.

fvictorio avatar Apr 28 '22 12:04 fvictorio

This issue was marked as stale because it didn't have any activity in the last 30 days. If you think it's still relevant, please leave a comment indicating so. Otherwise, it will be closed in 7 days.

github-actions[bot] avatar May 28 '22 13:05 github-actions[bot]

Not stale.

fvictorio avatar May 30 '22 14:05 fvictorio

In our case, we have a bunch of bootstrapping to do to initialize our local environment with a bunch of contracts. Currently this takes ages since we have automine turned OFF with a second interval. These contracts have various dependencies so we essentially deploy each contract one by one to simplify things. With automine turned ON, this bootstrap takes like 3 seconds. With it off, it takes like a minute+.

The issue is, with automine turned ON and the timestamp constantly increased, the time quickly becomes a few minutes in the future, which causes all further development after bootstrapping to go wonky since the timestamps are desynchronized.

Ideally, we could bootstrap our environment, then set the timestamp back to the current time. Or manually set the time interval to be 0 seconds during bootstrapping and then re-enable it.

Perhaps I'm missing another simple solution, but this makes iteration quite a pain when setting up a local environment.

impguard avatar Jul 17 '22 23:07 impguard

@impguard that's really strange. Even if you have interval mining enabled, all your transactions should be sent to the mempool and then after a second all of them should be mined.

My guess is that you have a script that does something like this:

const foo = await Foo.deploy()
await foo.deployed()

const tx = await foo.someMethod()
await tx.wait()

In that case, each .deployed/.wait call will wait until the transaction is mined, and therefore each deployment/transaction will take up to one second.

In that case, I think you have two options:

  • Write your script so that it doesn't wait for each tx to be mined
  • Enable automining during your setup and then disable it after everythin is ready. Notice that you can do this programmatically.

Another alternative is to manually mine a new block after sending each transaction. You can do it with the network helper (recommended) or with the low-level JSON-RPC hardhat_mine method.

fvictorio avatar Jul 18 '22 09:07 fvictorio

@fvictorio you're right about what we're doing. The problem is we require our contracts to be mined and deployed since the contracts have dependencies on one another. so parallelization of their deployment can only be done to a shallow extent.

Your two options won't work. (1) Writing the script so deployments aren't awaited won't work for the above reason. (2) Enabling automining produces the timestamp issue described in this Issue. About 200+ or so requests are sent out during our bootstrapping and the timestamp gets pushed out into the future during automining. Waiting for the timestamp to sync back properly essentially defeats the point of automining.

Manually mining doesn't really help either. What we really would like is automining and a resetting of the timestamp.

impguard avatar Jul 18 '22 16:07 impguard

Oh, I see. And sorry for asking so many questions, but why the timestamp moving 200 seconds (a couple of minutes) in the future is a problem? I'm not trying to minimize the need for this feature, since a lot of people are asking for it, but I would like to have a better understanding of the testing scenario.

(For the record, I'm pretty sure we'll implement this, although I can't give an ETA.)

fvictorio avatar Jul 19 '22 09:07 fvictorio

We rely on the timestamp in some of our contracts. Let's say for sake of example, we wait until the contract timestamp passes real time to allow the user to execute an action. A 200 second in the future timestamp means we have to wait 200 seconds after initial setup for us to develop. Adding essentially a 3 minute bootup time for local development iteration.

Not to mention certain test suites that constantly tear down and reinitialize an environment take essentially forever to finish testing. At the moment we essentially skip local testing for real time based tests since it's hard to manipulate the timestamps to where we want it, and just deal with an automine bootstrap time.

impguard avatar Jul 20 '22 07:07 impguard

@impguard I see, thanks for the explanation. That makes a lot of sense.

I wonder if, as a temporary workaround, setting an initialDate in the past could help, but that might not work if you are using forking. For example, this sets the first block one hour in the past:

    hardhat: {
      initialDate: new Date(Date.now() - 3600000).toUTCString()
    },

fvictorio avatar Jul 20 '22 09:07 fvictorio

humm, that's a great idea, I didn't realize one could do that. Let me test that out. Will work as an intermediary for sure.

impguard avatar Jul 22 '22 03:07 impguard

Just for future interest, this did resolve my issue. It's obviously not perfect, but setting the start time to some crazy time in the past and then resetting to modern time after completion of all bootstrapping tasks ensures the timestamp is current and not in the future. Of course this comes at the cost of any bootstrapped work basically far off in the past than perhaps is reasonable but for my use case it's fine.

impguard avatar Jul 31 '22 07:07 impguard

@fvictorio As an update to the update. This works generally at startup. It does NOT work permanently since now one must reset their node completely every time they want to update some contracts. During development we might redeploy contracts as we tweak them and since time setting only happens at node startup time (and once forwarded to modern time will not be able to be rewound), you end up with the same problem after the first load.

Not a killer, we just now have to keep restarting the hardhat node everytime we make a contract change, which does suck. Better than waiting 5 minutes to bootstrap though.

impguard avatar Aug 01 '22 02:08 impguard

Hey @cruzdanilo @impguard, we've decided that we'll implement this, although I can't give an estimate of when. I created #3045 to track the effort, so I'm going to close this issue in favor of that one.

Anyone interested in this: please leave a :+1: on that new issue to signal support for it. That will help us properly prioritize it.

fvictorio avatar Aug 16 '22 08:08 fvictorio