Add reputation staking in gas cost test #179
Conversation
|
Change all |
|
Typo fixed, thanks for the note. I added the measure for the hash submission process. |
elenadimitrova
changed the title
|
There seems to be an issue with gas cost reports for |
|
@lsenta can you have a look at the suggested fix in issue above and see if you can coerce |
Before we wouldn't get gas costs for calls on the ReputationMiningCycle contract. Workaround is: - give it its own file - require it before the tests starts (in migrations/) related issue: cgewecke/eth-gas-reporter#64
|
@elenadimitrova I implemented the following fixes and it's working:
The last one was a "just get it done" fix, let me know if we want to refactor the smart contract instead (I'll open a ticket). |
contracts/ColonyNetworkStaking.sol
Outdated
|
|
||
|
|
||
| } | ||
| // TODO: Can we handle a dispute regarding the very first hash that should be set? |
There was a problem hiding this comment.
This comment is in ReputationMiningCycle contract already. Does it need to be here too?
There was a problem hiding this comment.
Bad copy and paste, I'll put it at the top of the network staking contract since it's feature related.
migrations/2_deploy_contracts.js
Outdated
| // preparation. We need this for the eth-gas-reporter. | ||
| // See https://github.com/cgewecke/eth-gas-reporter/issues/64 | ||
| // This log prevents "variable not used" warnings. | ||
| console.log(ReputationMiningCycle); |
There was a problem hiding this comment.
Can we disable "variable not used" here instead of logging the contract. That will be a very messy console output.
There was a problem hiding this comment.
Nice catch, we don't even need a variable... force of habit!
| @@ -12,7 +12,8 @@ fs.readdirSync("./contracts/").forEach(contractName => { | |||
| "EtherRouter.sol", | |||
| "Migrations.sol", | |||
| "Resolver.sol", | |||
| "Token.sol" | |||
| "Token.sol", | |||
| "ReputationMiningCycle.sol" | |||
There was a problem hiding this comment.
No need to include it here as this is not an EternalStorage-based contract
There was a problem hiding this comment.
The build broke without it:
https://circleci.com/gh/JoinColony/colonyNetwork/1119
From what I understand:
We want to avoid state defined in contract that inherit from ColonyNetworkStorage. That's a way to centralize all the state in the ColonyNetworkStorage definition,
right?
If that's the case we might need to check the inheritance hierarchy in the check-storage.js script.
There was a problem hiding this comment.
Sorry the diff was hiding the part that this is an exclusion list. You've correctly excluded it, my bad.
Closes #173