Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

evm_revert does not reset time adjustment #390

Closed
recmo opened this issue Sep 13, 2017 · 14 comments
Closed

evm_revert does not reset time adjustment #390

recmo opened this issue Sep 13, 2017 · 14 comments

Comments

@recmo
Copy link

recmo commented Sep 13, 2017

Expected Behavior

(Deploy contracts)
evm_snapshot
(Pass first half of test A)
evm_increaseTime
(Pass second half of test A)
evm_revert
(Pass test B)

Current Behavior

(Deploy contracts)
evm_snapshot
(Pass first half of test A)
evm_increaseTime
(Pass second half of test A)
evm_revert
(Fail test B)

Cause: evm_revert did not reset the time adjustment. Test B is now run in a different time than when the snapshot was taken, and fails.

Possible Solutions

  • Store time adjustment as part of snapshot.
  • Allow negative values in increateTime.
  • Allow setting time adjustment directly.
  • Allow setting time directly (added advantage of making tests more deterministic).

Context

I'm trying to make my unit tests run faster by re-using setup transactions using snapshots.

Your Environment

  • Version used: v4.1.1
  • Environment name and version (e.g. PHP 5.4 on nginx 1.9.1): node v8.4.0
  • Server type and version: github.com
  • Operating System and version: Linux 4.10.0-33-generic #37-Ubuntu SMP x86_64 GNU/Linux
  • Link to your project: https://github.com/Neufund/ico-contracts
@satyamakgec
Copy link

I am having same problem , How do i resolve it .
Environment
ubuntu 16.04.LTS
testrpc v4.0.1
truffle v3.4.5

@scnale
Copy link

scnale commented Sep 22, 2017

You may wish to take a look at the pull request I created here. The fix is quite simple but I'm not sure it's the best way to do it. You can try that out in the meantime though.

@jaddison
Copy link

@scnale How does one go about building testrpc using your fix, exactly? Thanks!

@scnale
Copy link

scnale commented Oct 16, 2017

I haven't been using it in the last few weeks so the setup may need a few tweaks but here's what I did to test it:

  1. Get a copy of the working tree of the master branch of ganache-core with the fix commit included in it (at this time there may be new commits in master that are probably worth merging too). This can be easily achieved cloning both the pull request branch and master branch into a single local repository and merging master into the pull request branch.
  2. Get a copy of the working tree of the master branch of testrpc. Edit the devDependencies in the package.json(*) and replace the ganache-core version number with the path to the working tree of the ganache-core copy prepared in the previous step.
  3. To launch testrpc you can execute node <path to testrpc working tree>/cli.js [list of testrpc arguments that you want to use].

(*) I think there is a cleaner way to do this with npm link. It should be able to create a symbolic link in the node_modules subfolder to whichever path you choose. But I haven't really tested that out because I didn't know about npm link at the time. You could also try manually creating the symbolic link you need if you prefer.

Creating a symbolic link or alias in your terminal to launch the custom testrpc may be a good idea if you're planning on using it frequently.

Edit: Satisfying the other dependencies in both packages is necessary for this to work and prone to errors in my limited experience using npm. Use whatever means you deem adequate to satisfy them.

@jaddison
Copy link

Thanks, @scnale - got it 'working'. some dependencies failed to install, but it still works and all test cases work now. Cheers!

davidyuk added a commit to aeternity/aepp-response that referenced this issue Oct 17, 2017
Increased time cannot be reseted according to trufflesuite/ganache-cli-archive#390
davidyuk added a commit to aeternity/aepp-response that referenced this issue Oct 17, 2017
Increased time cannot be reseted according to trufflesuite/ganache-cli-archive#390
davidyuk added a commit to aeternity/aepp-response that referenced this issue Oct 17, 2017
Increased time cannot be reseted according to trufflesuite/ganache-cli-archive#390
@Zolmeister
Copy link

patch file

patch node_modules/ethereumjs-testrpc/build/cli.node.js testrpc-time.patch
83416c83416,83417
<       blockNumber: blockNumber
---
>       blockNumber: blockNumber,
> 			timeAdjustment: self.blockchain.timeAdjustment
83438a83440
> 	var timeAdjustment = this.snapshots[snapshot_id].timeAdjustment;
83465a83468,83469
> 		// The time adjustment is restored to its prior state
>  		self.blockchain.timeAdjustment = timeAdjustment;

davidyuk added a commit to davidyuk/education-hackathon-2017 that referenced this issue Nov 8, 2017
Increased time cannot be reseted according to trufflesuite/ganache-cli-archive#390
davidyuk added a commit to aeternity/aepp-response-contracts that referenced this issue Nov 30, 2017
Increased time cannot be reseted according to trufflesuite/ganache-cli-archive#390
davidyuk added a commit to aeternity/aepp-response-contracts that referenced this issue Nov 30, 2017
Increased time cannot be reseted according to trufflesuite/ganache-cli-archive#390
@cedricwalter
Copy link

status? will it be in soon?

@benjamincburns
Copy link
Contributor

benjamincburns commented Jan 16, 2018

There's an open PR on this trufflesuite/ganache#2. Last I knew the changes @scnale submitted there had problems that needed fixing (see this comment). I'll double check, but I don't think that's been done yet. If someone wants to submit another PR that finishes the work, I'd be all for it.

@benjamincburns
Copy link
Contributor

@scnale seems to have ghosted, and the changes he submitted were incomplete. If anyone else wants to take this on, I'd be much obliged.

@roderik
Copy link

roderik commented Feb 27, 2018

Funny, just looking for a solution to this issue as well :)
Anyone knows a workaround?

@PhABC
Copy link

PhABC commented Apr 2, 2018

Just made a very simple PR that helped me solve the problem for certains tests ; trufflesuite/ganache#95

@PhABC
Copy link

PhABC commented Apr 2, 2018

In addition, the PR mentioned above works as expected (see comment #377928638).

@benjamincburns
Copy link
Contributor

Fixed in current develop - thanks for following up on this, @PhABC!

@adamsoffer
Copy link

I'm also running into this issue. Which version of ganach-cli has the fix?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants