Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding error messages to contracts #1182

Merged
merged 11 commits into from
Dec 23, 2019

Conversation

mrsmkl
Copy link
Contributor

@mrsmkl mrsmkl commented Oct 2, 2019

Description

Added error messages, otherwise the behaviour shouldn't change.

Tested

Deployment cost of Validators contract grew to 8258626 gas from 7315248 gas.

Other changes

Also removed some compiler warnings.

Related issues

Backwards compatibility

@codecov
Copy link

codecov bot commented Oct 2, 2019

Codecov Report

Merging #1182 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1182   +/-   ##
=======================================
  Coverage   74.67%   74.67%           
=======================================
  Files         274      274           
  Lines        7731     7731           
  Branches      984      984           
=======================================
  Hits         5773     5773           
  Misses       1846     1846           
  Partials      112      112
Flag Coverage Δ
#mobile 74.67% <ø> (ø) ⬆️

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 1fba584...56ce76c. Read the comment docs.

@asaj asaj self-assigned this Oct 11, 2019
Copy link
Contributor

@asaj asaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo! Thanks for doing this, will make debugging life much easier

packages/protocol/contracts/governance/Governance.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Governance.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Governance.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Governance.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Governance.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Validators.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Validators.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Validators.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Validators.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/governance/Validators.sol Outdated Show resolved Hide resolved
@asaj asaj assigned mrsmkl and unassigned asaj Oct 14, 2019
@asaj asaj added the audit label Dec 20, 2019
Copy link
Contributor

@mcortesi mcortesi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this!!!

@mcortesi mcortesi added the automerge Have PR merge automatically when checks pass label Dec 23, 2019
@celo-ci-bot-user celo-ci-bot-user merged commit 2ecb7b6 into celo-org:master Dec 23, 2019
aaronmgdr added a commit that referenced this pull request Jan 2, 2020
* master: (26 commits)
  Added new lint rule (#2349)
  [Slashing] Slash locked gold (#2317)
  [Slashing] Allow voting gold to be slashable (#2302)
  1666 precompiles assembly rewrite (#2324)
  Small fixes to deploy web (#2343)
  Governance, downtime and double signing slasher contracts (#2267)
  Added daily limit for reserve spending (#2303)
  Fixing multisig tests (#2342)
  [Wallet] Implement new send & import flow (#2332)
  Limechain/16xx (#1982)
  Add infinite pagination to Leaderboard (#2339)
  cli: Add rewards:show (#2160)
  Correct broken link formatting
  Celotool command for deploying hotfixes (#2142)
  Governance ContractKit + CLI changes (#2139)
  Complete codependent slashing precompile PRs (#2333)
  Add new and modified precompiles to UsingPrecompiles.sol (#2318)
  Adding error messages to contracts (#1182)
  Upgrade i18next and react-i18next to latest across monorepo (#2311)
  [Wallet] Fix exchange input on iOS and feed item display (#2319)
  ...

# Conflicts:
#	yarn.lock
aaronmgdr added a commit that referenced this pull request Jan 2, 2020
* master: (35 commits)
  Remove rep sentence from brand kit page (#2350)
  Added new lint rule (#2349)
  [Slashing] Slash locked gold (#2317)
  [Slashing] Allow voting gold to be slashable (#2302)
  1666 precompiles assembly rewrite (#2324)
  Small fixes to deploy web (#2343)
  Governance, downtime and double signing slasher contracts (#2267)
  Added daily limit for reserve spending (#2303)
  Fixing multisig tests (#2342)
  [Wallet] Implement new send & import flow (#2332)
  Limechain/16xx (#1982)
  Add infinite pagination to Leaderboard (#2339)
  cli: Add rewards:show (#2160)
  Correct broken link formatting
  Celotool command for deploying hotfixes (#2142)
  Governance ContractKit + CLI changes (#2139)
  Complete codependent slashing precompile PRs (#2333)
  Add new and modified precompiles to UsingPrecompiles.sol (#2318)
  Adding error messages to contracts (#1182)
  Upgrade i18next and react-i18next to latest across monorepo (#2311)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Users SBAT see explanations of failed validation checks on requires
5 participants