[Slashing] Slash locked gold #2317

lucasege · 2019-12-19T23:59:53Z

Description

Adds slash function to LockedGold to allow slashing contracts to slash an account's gold.
Adds isSlasher mapping to allow addition of registered slashing addresses for permissioned use of functions.

Tested

Added unit tests.

Related issues

Fixes Make Locked Gold slashable #2084

Driveby changes

Changed MockGovernance to be payable to allow testing of sending to community fund.
Changed Election interface and mock to allow LockedGold to test slashing voting gold.
Adds governance slasher as registered slasher in migrations (this will be expanded upon in PR #2347

packages/protocol/contracts/governance/LockedGold.sol

packages/protocol/test/governance/lockedgold.ts

asaj · 2019-12-20T00:20:14Z

packages/protocol/contracts/governance/LockedGold.sol

+   *                 has the most votes of any validator group.
+   * @param indices The indices of the i'th group in `account`'s voting list.
+   */
+  function slash(


Emit an event

should I emit one event AccountSlashed(account, penalty, reporter, reward) or two events to split account/penalty and reporter/reward. Or is the reporter/reward not as important

one event is ok imo

packages/protocol/test/governance/lockedgold.ts

mrsmkl · 2019-12-20T09:39:53Z

I'll try to add a test into test/common/integration.ts

packages/protocol/contracts/governance/LockedGold.sol

mrsmkl · 2019-12-20T09:48:52Z

packages/protocol/contracts/governance/LockedGold.sol

+      // If not enough nonvoting, revoke the difference
+      if (nonvotingBalance < penalty) {
+        difference = penalty.sub(nonvotingBalance);
+        require(


What should we do if there's not enough to slash anymore?

I believe we would want to just slash all of the account's gold, I'm assuming this is handled on the caller side (Slasher contract) by passing in for penalty max(normalSlashPenalty, totalGoldOwnedByAccount) where normalSlashPenalty is the default penalty and totalGoldOwnedByAccount is the total balance + voting balance for the account.

Now that the other PRs are in, we can update e.g. https://github.com/celo-org/celo-monorepo/blob/master/packages/protocol/contracts/governance/SlasherUtil.sol#L81 to do the max(normalSlashPenalty, totalGoldOwnedByAccount).

Perhaps it's easier to handle in LockedGold, because the needed balances are stored there.

Based on this I am changing LockedGold.slash to only slash min(totalLockedGold, penalty). A problematic implication is that if totalLockedGold is somehow less than the reward passed in to slash, the call will revert. However, without doing a bunch of wasteful calculations (adjusting reward based on its original proportion or something similar) slash doesn't have the right info to change reward properly. Just something to keep in mind

I think this is OK for now, but i would follow the discussion on this.

My concern is that Given the min() i will be considered "slashed" and thus can't be slashed again for the same thing, but i wouldn't be really paying the penalty that i'm due... i wonder if i can use that to shield me from penalties somehow.

But to analyze this we need to see the bigger picture...

mrsmkl · 2019-12-21T03:16:41Z

Did you solve the issue with sending to governance? I guess you just need to make MockGovernance able to receive gold like Governance (fallback function).

packages/protocol/test/governance/lockedgold.ts

codecov · 2019-12-30T21:21:33Z

Codecov Report

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

@@           Coverage Diff            @@
##           master    #2317    +/-   ##
========================================
  Coverage   74.55%   74.55%            
========================================
  Files         278      278            
  Lines        7777     7777            
  Branches      707      991   +284     
========================================
  Hits         5798     5798            
  Misses       1868     1868            
  Partials      111      111

Flag	Coverage Δ
#mobile	`74.55% <ø> (ø)`	⬆️

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 f7ca165...9fa1aa7. Read the comment docs.

mrsmkl · 2019-12-31T23:08:16Z

Looks like the integration test broke here. Should be fixed by using the migration from https://github.com/mrsmkl/celo-monorepo/blob/test-slashing2/packages/protocol/migrations/19_governance_slasher.ts

mrsmkl · 2020-01-01T12:46:06Z

Looks like there was some flakiness in the validators test, perhaps because the epoch number was calculated differently.

…ved unit tests

…nity fund.

mcortesi

left a comment, but approved!

packages/protocol/test/governance/lockedgold.ts

* 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

* 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) ...

lucasege added the do not merge label Dec 19, 2019

lucasege requested review from asaj and m-chrzan as code owners December 19, 2019 23:59

lucasege changed the title ~~Lucasege/slashing slash locked gold~~ [Slashing] Slash locked gold Dec 20, 2019

lucasege assigned asaj Dec 20, 2019

asaj reviewed Dec 20, 2019

View reviewed changes

asaj assigned lucasege and unassigned asaj Dec 20, 2019

asaj added the audit label Dec 20, 2019

lucasege mentioned this pull request Dec 20, 2019

[Slashing] Allow voting gold to be slashable #2302

Merged

lucasege force-pushed the lucasege/slashing-slash-locked-gold branch from 312bdd4 to 1aac117 Compare December 20, 2019 06:04

mrsmkl reviewed Dec 20, 2019

View reviewed changes

packages/protocol/contracts/governance/LockedGold.sol Outdated Show resolved Hide resolved

mrsmkl reviewed Dec 20, 2019

View reviewed changes

packages/protocol/contracts/governance/LockedGold.sol Outdated Show resolved Hide resolved

mrsmkl reviewed Dec 20, 2019

View reviewed changes

packages/protocol/contracts/governance/LockedGold.sol Outdated Show resolved Hide resolved

mrsmkl reviewed Dec 20, 2019

View reviewed changes

mrsmkl reviewed Dec 30, 2019

View reviewed changes

packages/protocol/test/governance/lockedgold.ts Outdated Show resolved Hide resolved

lucasege force-pushed the lucasege/slashing-slash-locked-gold branch from ec84451 to e923f34 Compare December 30, 2019 21:21

lucasege force-pushed the lucasege/slashing-slash-locked-gold branch from b0d3ee3 to 24c0a9e Compare December 31, 2019 20:39

lucasege removed the do not merge label Dec 31, 2019

lucasege force-pushed the lucasege/slashing-slash-locked-gold branch from 4df3962 to 29919e6 Compare January 1, 2020 14:19

lucasege removed their assignment Jan 1, 2020

lucasege mentioned this pull request Jan 1, 2020

[Slashing] Adding whitelist checks for onlySlasher to Validators.sol #2347

Merged

Lucas Ege added 3 commits January 2, 2020 08:37

Adding slashing fn and groundwork for testing if we want to add invol…

26e271b

…ved unit tests

Adding events, changing names, and starting to add more tests

6e412a3

Fixing lint

a992063

Lucas Ege and others added 14 commits January 2, 2020 08:37

Holding review changes

16721f1

Fixing name changes

fbcd917

Parking test additions, need to fix voting in lockedgold tests

cb79559

Fixing tests to add test of slashing voting gold and sending to commu…

5a99b80

…nity fund.

fixing Validators test epoch number

7fa4baf

lint

9f265c0

Slashing will only slash min(totalLockedGold, penalty)

b57534a

Comments

cec7646

WIP, working out bugs in election interface

4d44719

function complete, still need to add more tests and clean style

f814e66

Fixing tests

62b1a5d

Addressing review, adding more tests and adding registry constraint

8046174

Changing names and changing votes counter to public

6c984ec

Adding descriptions and minor opimitizations.

65b6d2b

lucasege force-pushed the lucasege/slashing-slash-locked-gold branch from b744f78 to 65b6d2b Compare January 2, 2020 14:38

removing comments

e050be2

lucasege assigned mcortesi Jan 2, 2020

mcortesi approved these changes Jan 2, 2020

View reviewed changes

packages/protocol/test/governance/lockedgold.ts Show resolved Hide resolved

Adding test of only slashing max account balance

9fa1aa7

lucasege added the automerge Have PR merge automatically when checks pass label Jan 2, 2020

celo-ci-bot-user merged commit 97f2f31 into master Jan 2, 2020

lucasege pushed a commit that referenced this pull request Jan 2, 2020

[Slashing] Slash locked gold (#2317)

9868012

aaronmgdr deleted the lucasege/slashing-slash-locked-gold branch December 16, 2021 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Slashing] Slash locked gold #2317

[Slashing] Slash locked gold #2317

lucasege commented Dec 19, 2019 •

edited

Loading

asaj Dec 20, 2019

lucasege Dec 20, 2019

mcortesi Jan 2, 2020

mrsmkl commented Dec 20, 2019

mrsmkl Dec 20, 2019

lucasege Dec 27, 2019 •

edited by jfoutts-celo

Loading

jfoutts-celo Dec 30, 2019 •

edited

Loading

mrsmkl Dec 30, 2019

lucasege Jan 1, 2020 •

edited

Loading

mcortesi Jan 2, 2020

mrsmkl commented Dec 21, 2019

codecov bot commented Dec 30, 2019 •

edited

Loading

mrsmkl commented Dec 31, 2019

mrsmkl commented Jan 1, 2020

mcortesi left a comment

[Slashing] Slash locked gold #2317

[Slashing] Slash locked gold #2317

Conversation

lucasege commented Dec 19, 2019 • edited Loading

Description

Tested

Related issues

Driveby changes

asaj Dec 20, 2019

Choose a reason for hiding this comment

lucasege Dec 20, 2019

Choose a reason for hiding this comment

mcortesi Jan 2, 2020

Choose a reason for hiding this comment

mrsmkl commented Dec 20, 2019

mrsmkl Dec 20, 2019

Choose a reason for hiding this comment

lucasege Dec 27, 2019 • edited by jfoutts-celo Loading

Choose a reason for hiding this comment

jfoutts-celo Dec 30, 2019 • edited Loading

Choose a reason for hiding this comment

mrsmkl Dec 30, 2019

Choose a reason for hiding this comment

lucasege Jan 1, 2020 • edited Loading

Choose a reason for hiding this comment

mcortesi Jan 2, 2020

Choose a reason for hiding this comment

mrsmkl commented Dec 21, 2019

codecov bot commented Dec 30, 2019 • edited Loading

Codecov Report

mrsmkl commented Dec 31, 2019

mrsmkl commented Jan 1, 2020

mcortesi left a comment

Choose a reason for hiding this comment

lucasege commented Dec 19, 2019 •

edited

Loading

lucasege Dec 27, 2019 •

edited by jfoutts-celo

Loading

jfoutts-celo Dec 30, 2019 •

edited

Loading

lucasege Jan 1, 2020 •

edited

Loading

codecov bot commented Dec 30, 2019 •

edited

Loading