Issue #106 #107
Issue #106 #107
Conversation
two containers: - ganache-cli - circles-contracts
| container_name: ganache-cli | ||
| ports: | ||
| - "8145:8545" | ||
| #command: ["--accounts=100", "--hostname=0.0.0.0", "--gasLimit=100000000", "--defaultBalanceEther=1000000", "--mnemonic=xxx", "--networkId=99999"] |
There was a problem hiding this comment.
I'm fine with adding docker to this repo, but can you clean up these unused lines please?
| circles-contracts: | ||
| image: node:12 | ||
| container_name: circles-contracts | ||
| #environment: |
| @@ -1,6 +1,10 @@ | |||
| # Dependency directory | |||
| node_modules | |||
|
|
|||
| package-lock.json | |||
|
|
|||
| build | |||
There was a problem hiding this comment.
We actually commit the build dir on purpose, because the rest of our stack pulls this repo in from npm and gets the abis from them
| @@ -325,6 +325,32 @@ contract('UBI', ([_, owner, recipient, attacker, systemOwner]) => { // eslint-di | |||
| bal.should.bignumber.satisfy(() => near(bal, goal, startingIssuance)); | |||
| }); | |||
|
|
|||
| it('should show no payable ubi if timeout is shorter than period', async () => { | |||
There was a problem hiding this comment.
Can this be moved into its own describe block, and follows the pattern of deploying a hub in the before each like the rest of this file?
Would suggest to run a version of all the same tests we use on other hub parameters (and probably mostly use the same code):
- should return that it is stopped when it has been manually stopped
- should not payout ubi if manually stopped
- should show no payable ubi if expired
- should return that it is stopped when it has expired
- should not payout ubi if expired
expired here meaning timeout has passed, and manually stopped meaning the stop method has been called
|
|
||
| const time = period.mul(bn(2)); | ||
| await increase(time.toNumber() + 500); | ||
| await token.stop({ from: owner }); |
There was a problem hiding this comment.
Manually calling stop here means we're not actually letting the timeout come into effect
|
Thanks @glesaint and sorry for taking so long to review! We were swamped after the launch, and seeing as how ... the contracts will (hopefully) take awhile before they get updated, this got put on the backburner. I'm not sure if you're still interested in contributing, because it's been so long, but I'll leave this open for awhile in case you are! |
PR contents:
@ana0 what other tests would you like to see for timeout<period?