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 slashing multiplier to validators.sol and reward calculation #2239

Merged
merged 12 commits into from
Dec 17, 2019

Conversation

lucasege
Copy link
Contributor

@lucasege lucasege commented Dec 13, 2019

Description

Added setSlashingMultiplierResetPeriod, resetSlashingMultiplier, getValidatorGroupSlashingMultiplier and halveSlashingMultiplier to Validators.sol, as well as adding record keeping to faciliate this: GroupSlashInfo struct which is stored within the ValidatorGroup struct and maintains the group's slashingModifier (M in the docs) and the lastSlashedTimestamp which tracks the last time a group was slashed to control when the multiplier can be reset yet.

Tested

Added unit tests for each fn in Validators contract.

Other changes

Fixed a bug in Validators.sol where we were calling signerToAccount when we should have been calling validatorSignerToAccount. Updated this in most functions that called signerToAccount with a few exceptions: _updateValidatorScoreFromSigner, _distributeEpochPaymentsFromSigner, getValidatorBlsPublicKeyFromSigner, and getMembershipInLastEpochFromSigner.

Changed Validators.sol initialize fn signature, IValidators contract, and the MockValidators contract. I also changed the gasLimit within truffle config and the ganache config in runTests since it was running out of gas when I changed Validator.

Related issues

Backwards compatibility

No, changes to initialize fn in Validator

packages/protocol/contracts/governance/Election.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/test/governance/validators.ts Outdated Show resolved Hide resolved
packages/protocol/test/governance/validators.ts Outdated Show resolved Hide resolved
packages/protocol/test/governance/validators.ts Outdated Show resolved Hide resolved
packages/protocol/test/governance/validators.ts Outdated Show resolved Hide resolved
@asaj asaj assigned lucasege and unassigned asaj Dec 13, 2019
@lucasege lucasege force-pushed the lucasege/slashing-multiplier branch 2 times, most recently from 260b8c4 to 24e8ab5 Compare December 14, 2019 00:11
@lucasege lucasege assigned asaj and unassigned lucasege Dec 14, 2019
packages/protocol/contracts/governance/Validators.sol Outdated Show resolved Hide resolved
* @notice Halves the group's slashing multiplier.
* @param account The group being slashed.
*/
function halveSlashingMultiplier(address account)
Copy link
Contributor

Choose a reason for hiding this comment

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

Been thinking about this more, and wondering if it makes sense for different slashers to set their own multiplier between 0 and 1, let's discuss more offline

packages/protocol/migrationsConfig.js Outdated Show resolved Hide resolved
packages/protocol/test/governance/validators.ts Outdated Show resolved Hide resolved
@asaj asaj assigned lucasege and unassigned asaj Dec 17, 2019
@codecov
Copy link

codecov bot commented Dec 17, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #2239    +/-   ##
========================================
  Coverage   74.89%   74.89%            
========================================
  Files         274      274            
  Lines        7803     7803            
  Branches      701      985   +284     
========================================
  Hits         5844     5844            
  Misses       1846     1846            
  Partials      113      113
Flag Coverage Δ
#mobile 74.89% <ø> (ø) ⬆️

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 c82468d...ae3ef3a. Read the comment docs.

@celo-ci-bot-user celo-ci-bot-user merged commit 3fbb017 into master Dec 17, 2019
@celo-ci-bot-user celo-ci-bot-user deleted the lucasege/slashing-multiplier branch December 17, 2019 19:06
aaronmgdr added a commit that referenced this pull request Dec 17, 2019
* master:
  Adding slashing multiplier to validators.sol and reward calculation (#2239)
  Update genesis value, show name in election:list, fix assertRevert (#2216)
  Audit fixes for Reserve (#1816)
  Adapting the script to the last documentation changes (#2241)
  Bump excon from 0.65.0 to 0.71.0 in /packages/mobile (#2274)
  Fix wrong type for comment key (#2250)
  Fix/block gas limit exceeded #2 (#2263)
  Stop running broken end-to-end mobile test (#2096)
  [Wallet] Fix non integer wei issue in exchange (#2277)
  Fix validatorgroup:list bug by not fetching unneeded info (#2257)
  [Wallet] New exchange trade screen and fix ts build in celotool (#2167)
  Update Role on About Page (#2271)

# Conflicts:
#	packages/web/src/about/team/team-list.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add slashing multiplier to Validators contract, read when distributing rewards
3 participants