ConsenSysMesh / Stow-Smart-Contracts

Formerly Linnia-Smart-Contracts
https://stow-protocol.com/
MIT License
56 stars 39 forks source link

LP-83 first set of changes for make Pausable. Adding openzepplin pausable. #43

Closed godfreyhobbs closed 6 years ago

codecov-io commented 6 years ago

Codecov Report

Merging #43 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #43   +/-   ##
=======================================
  Coverage   96.52%   96.52%           
=======================================
  Files           4        4           
  Lines         115      115           
  Branches       25       25           
=======================================
  Hits          111      111           
  Misses          4        4
Impacted Files Coverage Δ
contracts/LinniaRecords.sol 98.27% <100%> (ø) :arrow_up:
contracts/LinniaUsers.sol 94.73% <100%> (ø) :arrow_up:
contracts/LinniaPermissions.sol 95.23% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 41ce401...dfa1997. Read the comment docs.

libertylocked commented 6 years ago

Hub functions are only callable by owner, so there's no need to pause them. Maybe add pausable to other contracts, like users (register), records (upload, attest, etc), permissions (share)

I'm not sure if it's necessary to replace expect throw with assert revert. It may be a nice to have thing. Maybe in another PR?

godfreyhobbs commented 6 years ago

Will update other contracts in follow-up PR.

godfreyhobbs commented 6 years ago

I am going to merge this blocked PR in the morning, as I feel that I have addressed all requested changes and this is a pretty simple PR and it is now blocking other work.

The abi.encodePacked is actually part of the already approved PR #29 to 0.4.24. It only ended up here as the other PR took 11 days to get approval

libertylocked commented 6 years ago

29 wasn't a priority and I did not approve it.

Unfortunately I did not see that you are packing a single argument. The correct way is to covert it to bytes