diff --git a/solidity/ecdsa/contracts/WalletRegistry.sol b/solidity/ecdsa/contracts/WalletRegistry.sol index d3f3ab00f5..7eec6d7f96 100644 --- a/solidity/ecdsa/contracts/WalletRegistry.sol +++ b/solidity/ecdsa/contracts/WalletRegistry.sol @@ -783,6 +783,60 @@ contract WalletRegistry is ); } + /// @notice Allows the wallet owner to add all signing group members of the + /// wallet with the given ID to the slashing queue of the staking . + /// contract. The notifier will receive reward per each group member + /// from the staking contract notifiers treasury. The reward is + /// scaled by the `rewardMultiplier` provided as a parameter. + /// @param amount Amount of tokens to seize from each signing group member + /// @param rewardMultiplier Fraction of the staking contract notifiers + /// reward the notifier should receive; should be between [0, 100] + /// @param notifier Address of the misbehavior notifier + /// @param walletID ID of the wallet + /// @param walletMembersIDs Identifiers of the wallet signing group members + /// @dev Requirements: + /// - The expression `keccak256(abi.encode(walletMembersIDs))` must + /// be exactly the same as the hash stored under `membersIdsHash` + /// for the given `walletID`. Those IDs are not directly stored + /// in the contract for gas efficiency purposes but they can be + /// read from appropriate `DkgResultSubmitted` and `DkgResultApproved` + /// events. + /// - `rewardMultiplier` must be between [0, 100]. + /// - This function does revert if staking contract call reverts. + /// The calling code needs to handle the potential revert. + function seize( + uint96 amount, + uint256 rewardMultiplier, + address notifier, + bytes32 walletID, + uint32[] calldata walletMembersIDs + ) external onlyWalletOwner { + bytes32 memberIdsHash = wallets.getWalletMembersIdsHash(walletID); + require( + memberIdsHash == keccak256(abi.encode(walletMembersIDs)), + "Invalid wallet members identifiers" + ); + + address[] memory groupMembersAddresses = sortitionPool.getIDOperators( + walletMembersIDs + ); + address[] memory stakingProvidersAddresses = new address[]( + walletMembersIDs.length + ); + for (uint256 i = 0; i < groupMembersAddresses.length; i++) { + stakingProvidersAddresses[i] = operatorToStakingProvider( + groupMembersAddresses[i] + ); + } + + staking.seize( + amount, + rewardMultiplier, + notifier, + stakingProvidersAddresses + ); + } + /// @notice Checks if DKG result is valid for the current DKG. /// @param result DKG result. /// @return True if the result is valid. If the result is invalid it returns diff --git a/solidity/ecdsa/contracts/api/IWalletRegistry.sol b/solidity/ecdsa/contracts/api/IWalletRegistry.sol index 424e3afc0d..677769a1b9 100644 --- a/solidity/ecdsa/contracts/api/IWalletRegistry.sol +++ b/solidity/ecdsa/contracts/api/IWalletRegistry.sol @@ -18,14 +18,44 @@ import "../libraries/EcdsaDkg.sol"; interface IWalletRegistry { /// @notice Requests a new wallet creation. - /// @dev Only a Wallet Owner can call this function. + /// @dev Only the Wallet Owner can call this function. function requestNewWallet() external; /// @notice Closes an existing wallet. /// @param walletID ID of the wallet. - /// @dev Only a Wallet Owner can call this function. + /// @dev Only the Wallet Owner can call this function. function closeWallet(bytes32 walletID) external; + /// @notice Adds all signing group members of the wallet with the given ID + /// to the slashing queue of the staking contract. The notifier will + /// receive reward per each group member from the staking contract + /// notifiers treasury. The reward is scaled by the + /// `rewardMultiplier` provided as a parameter. + /// @param amount Amount of tokens to seize from each signing group member + /// @param rewardMultiplier Fraction of the staking contract notifiers + /// reward the notifier should receive; should be between [0, 100] + /// @param notifier Address of the misbehavior notifier + /// @param walletID ID of the wallet + /// @param walletMembersIDs Identifiers of the wallet signing group members + /// @dev Only the Wallet Owner can call this function. + /// Requirements: + /// - The expression `keccak256(abi.encode(walletMembersIDs))` must + /// be exactly the same as the hash stored under `membersIdsHash` + /// for the given `walletID`. Those IDs are not directly stored + /// in the contract for gas efficiency purposes but they can be + /// read from appropriate `DkgResultSubmitted` and `DkgResultApproved` + /// events. + /// - `rewardMultiplier` must be between [0, 100]. + /// - This function does revert if staking contract call reverts. + /// The calling code needs to handle the potential revert. + function seize( + uint96 amount, + uint256 rewardMultiplier, + address notifier, + bytes32 walletID, + uint32[] calldata walletMembersIDs + ) external; + /// @notice Gets public key of a wallet with a given wallet ID. /// The public key is returned in an uncompressed format as a 64-byte /// concatenation of X and Y coordinates. diff --git a/solidity/ecdsa/package.json b/solidity/ecdsa/package.json index a34d3558c7..9b2789ce10 100644 --- a/solidity/ecdsa/package.json +++ b/solidity/ecdsa/package.json @@ -65,7 +65,7 @@ "@keep-network/random-beacon": "development", "@keep-network/sortition-pools": "^2.0.0-pre.7", "@openzeppelin/contracts": "^4.4.1", - "@threshold-network/solidity-contracts": ">1.1.0-dev <1.1.0-ropsten" + "@threshold-network/solidity-contracts": ">1.2.0-dev <1.2.0-ropsten" }, "engines": { "node": ">= 14.0.0" diff --git a/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts b/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts index 5a70330e9c..4be721899b 100644 --- a/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts +++ b/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts @@ -1473,12 +1473,7 @@ describe("WalletRegistry - Authorization", () => { ) const slashingTo = minimumAuthorization.sub(1) - // Note that we slash from the entire staked amount given that the - // initially authorized amount is less than staked amount and it is - // another application slashing. To go below the minimum stake, we need - // to start slashing from the entire staked amount, not just the - // one authorized for WalletRegistry. - const slashedAmount = stakedAmount.sub(slashingTo) + const slashedAmount = authorizedAmount.sub(slashingTo) await staking .connect(slasher.wallet) @@ -2232,12 +2227,7 @@ describe("WalletRegistry - Authorization", () => { ) const slashingTo = minimumAuthorization.sub(1) - // Note that we slash from the entire staked amount given that the - // initially authorized amount is less than staked amount and it is - // another application slashing. To go below the minimum stake, we need - // to start slashing from the entire staked amount, not just the - // one authorized for WalletRegistry. - const slashedAmount = stakedAmount.sub(slashingTo) + const slashedAmount = authorizedAmount.sub(slashingTo) await staking .connect(slasher.wallet) @@ -2782,21 +2772,15 @@ describe("WalletRegistry - Authorization", () => { await createSnapshot() slashingTo = minimumAuthorization.sub(1) - // Note that we slash from the entire staked amount given that the - // initially authorized amount is less than staked amount and it is - // another application slashing. To go below the minimum stake, we need - // to start slashing from the entire staked amount, not just the - // one authorized for WalletRegistry. - const slashedAmount = stakedAmount.sub(slashingTo) + const slashedAmount = initialIncrease.sub(slashingTo) await staking .connect(slasher.wallet) .slash(slashedAmount, [stakingProvider.address]) await staking.connect(thirdParty).processSlashing(1) - // Given that we slashed from the entire staked amount, we need to give - // the stake owner some more T and let them top-up the stake before they - // increase the authorization again. + // Give the stake owner some more T and let them top-up the stake before + // they increase the authorization again. secondIncrease = to1e18(10000) await t.connect(deployer).mint(owner.address, secondIncrease) await t.connect(owner).approve(staking.address, secondIncrease) @@ -2851,10 +2835,7 @@ describe("WalletRegistry - Authorization", () => { .updateOperatorStatus(operator.address) slashingTo = initialIncrease.sub(to1e18(100)) - // Note that we slash from the entire staked amount given that the - // initially authorized amount is less than staked amount and it is - // another application slashing. - const slashedAmount = stakedAmount.sub(slashingTo) + const slashedAmount = initialIncrease.sub(slashingTo) await staking .connect(slasher.wallet) @@ -2898,10 +2879,7 @@ describe("WalletRegistry - Authorization", () => { .updateOperatorStatus(operator.address) slashingTo = initialIncrease.sub(to1e18(100)) - // Note that we slash from the entire staked amount given that the - // initially authorized amount is less than staked amount and it is - // another application slashing. - const slashedAmount = stakedAmount.sub(slashingTo) + const slashedAmount = initialIncrease.sub(slashingTo) await staking .connect(slasher.wallet) @@ -2955,10 +2933,9 @@ describe("WalletRegistry - Authorization", () => { ) slashingTo = initialIncrease.sub(to1e18(2500)) - // Note that we slash from the entire staked amount given that the - // initially authorized amount is less than staked amount and it is - // another application slashing. - const slashedAmount = stakedAmount.sub(slashingTo) + const slashedAmount = initialIncrease + .sub(decreasedAmount) + .sub(slashingTo) await staking .connect(slasher.wallet) diff --git a/solidity/ecdsa/test/WalletRegistry.Slashing.test.ts b/solidity/ecdsa/test/WalletRegistry.Slashing.test.ts new file mode 100644 index 0000000000..3549ebe0a0 --- /dev/null +++ b/solidity/ecdsa/test/WalletRegistry.Slashing.test.ts @@ -0,0 +1,157 @@ +/* eslint-disable no-await-in-loop */ +import { helpers } from "hardhat" +import { expect } from "chai" +import { to1e18 } from "@keep-network/hardhat-helpers/dist/src/number" + +import ecdsaData from "./data/ecdsa" +import { constants, walletRegistryFixture } from "./fixtures" +import { createNewWallet } from "./utils/wallets" + +import type { + WalletRegistry, + IWalletOwner, + TokenStaking, + T, +} from "../typechain" +import type { FakeContract } from "@defi-wonderland/smock" +import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers" +import type { Operator, OperatorID } from "./utils/operators" + +const { createSnapshot, restoreSnapshot } = helpers.snapshot + +describe("WalletRegistry - Slashing", () => { + let walletRegistry: WalletRegistry + let walletOwner: FakeContract + let thirdParty: SignerWithAddress + let staking: TokenStaking + let tToken: T + + let members: Operator[] + let membersIDs: OperatorID[] + let membersAddresses: string[] + let walletID: string + + const walletPublicKey: string = ecdsaData.group1.publicKey + const amountToSlash = to1e18(1000) + const rewardMultiplier = 30 + + before(async () => { + // eslint-disable-next-line @typescript-eslint/no-extra-semi + ;({ walletRegistry, walletOwner, thirdParty, staking, tToken } = + await walletRegistryFixture()) + ;({ walletID, members } = await createNewWallet( + walletRegistry, + walletOwner.wallet, + walletPublicKey + )) + + membersIDs = members.map((member) => member.id) + membersAddresses = members.map((member) => member.signer.address) + }) + + describe("seize", () => { + context("when called not by the wallet owner", () => { + it("should revert", async () => { + await expect( + walletRegistry + .connect(thirdParty) + .seize( + amountToSlash, + rewardMultiplier, + thirdParty.address, + walletID, + membersIDs + ) + ).to.be.revertedWith("Caller is not the Wallet Owner") + }) + }) + + context("when called by the wallet owner", () => { + context("when the passed wallet members identifiers are invalid", () => { + it("should revert", async () => { + const corruptedMembersIDs = membersIDs.slice().reverse() + await expect( + walletRegistry + .connect(walletOwner.wallet) + .seize( + amountToSlash, + rewardMultiplier, + thirdParty.address, + walletID, + corruptedMembersIDs + ) + ).to.be.revertedWith("Invalid wallet members identifiers") + }) + }) + + context("when the passed wallet members identifiers are valid", () => { + let notifierBalanceBefore + let notifierBalanceAfter + + before(async () => { + await createSnapshot() + + notifierBalanceBefore = await tToken.balanceOf(thirdParty.address) + await walletRegistry + .connect(walletOwner.wallet) + .seize( + amountToSlash, + rewardMultiplier, + thirdParty.address, + walletID, + membersIDs + ) + notifierBalanceAfter = await tToken.balanceOf(thirdParty.address) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should slash all group members", async () => { + expect(await staking.getSlashingQueueLength()).to.equal( + constants.groupSize + ) + }) + + it("should slash with correct amounts", async () => { + for (let i = 0; i < constants.groupSize; i++) { + const slashing = await staking.slashingQueue(i) + expect(slashing.amount).to.equal(amountToSlash) + } + }) + + it("should slash correct staking providers", async () => { + for (let i = 0; i < constants.groupSize; i++) { + const slashing = await staking.slashingQueue(i) + const expectedStakingProvider = + await walletRegistry.operatorToStakingProvider( + membersAddresses[i] + ) + + expect(slashing.stakingProvider).to.equal(expectedStakingProvider) + } + }) + + it("should send correct reward to notifier", async () => { + // reward multiplier is in % so we first multiply and then divide by + // 100 to get the actual number + const perMemberReward = constants.tokenStakingNotificationReward + .mul(rewardMultiplier) + .div(100) + + const receivedReward = notifierBalanceAfter.sub(notifierBalanceBefore) + + expect(receivedReward).to.equal( + perMemberReward.mul(constants.groupSize) + ) + }) + }) + + // TODO: Add a unit test ensuring `seize` call reverts if the staking + // contract `seize` call reverts. + // Currently blocked by https://github.com/defi-wonderland/smock/issues/101 + // See https://github.com/keep-network/keep-core/issues/2870 + }) + }) +}) diff --git a/solidity/ecdsa/test/fixtures/index.ts b/solidity/ecdsa/test/fixtures/index.ts index 2f89d0f4cd..5f701c686d 100644 --- a/solidity/ecdsa/test/fixtures/index.ts +++ b/solidity/ecdsa/test/fixtures/index.ts @@ -127,7 +127,9 @@ async function updateTokenStakingParams( staking: TokenStaking, deployer: SignerWithAddress ) { - const initialNotifierTreasury = to1e18(100000) // 100k T + const initialNotifierTreasury = constants.tokenStakingNotificationReward.mul( + constants.groupSize + ) await tToken .connect(deployer) .approve(staking.address, initialNotifierTreasury) diff --git a/solidity/ecdsa/yarn.lock b/solidity/ecdsa/yarn.lock index 1a8a3ac28b..25a990e9ab 100644 --- a/solidity/ecdsa/yarn.lock +++ b/solidity/ecdsa/yarn.lock @@ -685,10 +685,10 @@ "@types/sinon-chai" "^3.2.3" "@types/web3" "1.0.19" -"@openzeppelin/contracts-upgradeable@^4.4": - version "4.5.1" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.5.1.tgz#dc354082460eb34f5833afdecfab46538b208c4f" - integrity sha512-xcKycsSyFauIGMhSeeTJW/Jzz9jZUJdiFNP9Wo/9VhMhw8t5X0M92RY6x176VfcIWsxURMHFWOJVTlFA78HI/w== +"@openzeppelin/contracts-upgradeable@^4.5": + version "4.5.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.5.2.tgz#90d9e47bacfd8693bfad0ac8a394645575528d05" + integrity sha512-xgWZYaPlrEOQo3cBj97Ufiuv79SPd8Brh4GcFYhPgb6WvAq4ppz8dWKL6h+jLAK01rUqMRp/TS9AdXgAeNvCLA== "@openzeppelin/contracts@^4.1.0", "@openzeppelin/contracts@^4.4.2": version "4.5.0" @@ -941,14 +941,14 @@ dependencies: "@openzeppelin/contracts" "^4.1.0" -"@threshold-network/solidity-contracts@>1.1.0-dev <1.1.0-ropsten": - version "1.1.0-dev.8" - resolved "https://registry.yarnpkg.com/@threshold-network/solidity-contracts/-/solidity-contracts-1.1.0-dev.8.tgz#5c8ed6e9dab26823f25c319a5ade167c6bb8efa3" - integrity sha512-7xsjMIO3jtVDOB3X+tY6rEy9qViGHLwxyNEdgYH85BzpOdXvL3tlmwVnAn0k1fQyRhGLrUOtS56Z6fsDCxyRRg== +"@threshold-network/solidity-contracts@>1.2.0-dev <1.2.0-ropsten": + version "1.2.0-dev.4" + resolved "https://registry.yarnpkg.com/@threshold-network/solidity-contracts/-/solidity-contracts-1.2.0-dev.4.tgz#087d88f03a2d1bd07b64724eaf314f30cf9fa444" + integrity sha512-ud90FCblZPTz/62BWUusMZWIIC4dAuHGDbubyY7EmwEoC77YoVgG37X0qCuFE2tIkAImQOW6rcobA7JUNbz9bw== dependencies: "@keep-network/keep-core" ">1.8.0-dev <1.8.0-pre" - "@openzeppelin/contracts" "^4.4" - "@openzeppelin/contracts-upgradeable" "^4.4" + "@openzeppelin/contracts" "^4.5" + "@openzeppelin/contracts-upgradeable" "^4.5" "@thesis/solidity-contracts" "github:thesis/solidity-contracts#4985bcf" "@tsconfig/node10@^1.0.7":