From 0f38aab6ac35544814ee1841b8f4260e12b39bdb Mon Sep 17 00:00:00 2001 From: Brice Wang Date: Fri, 1 Nov 2019 13:02:40 -0700 Subject: [PATCH 01/71] Support protocol hotfixing (#613) --- .../src/test-utils/ganache.setup.ts | 2 +- .../contracts/governance/Governance.sol | 181 +++++++++-- .../contracts/governance/Proposals.sol | 71 ++++- .../governance/interfaces/IGovernance.sol | 11 + .../governance/test/GovernanceTest.sol | 21 ++ packages/protocol/migrationsConfig.js | 5 +- packages/protocol/runTests.js | 2 +- packages/protocol/scripts/devchain.ts | 2 +- .../protocol/test/governance/governance.ts | 292 ++++++++++++++++-- packages/protocol/truffle-config.js | 2 +- 10 files changed, 528 insertions(+), 61 deletions(-) create mode 100644 packages/protocol/contracts/governance/test/GovernanceTest.sol diff --git a/packages/contractkit/src/test-utils/ganache.setup.ts b/packages/contractkit/src/test-utils/ganache.setup.ts index 66f26bbe394..44cbb558393 100644 --- a/packages/contractkit/src/test-utils/ganache.setup.ts +++ b/packages/contractkit/src/test-utils/ganache.setup.ts @@ -43,7 +43,7 @@ export async function startGanache(datadir: string, opts: { verbose?: boolean } network_id: 1101, db_path: datadir, mnemonic: MNEMONIC, - gasLimit: 7000000, + gasLimit: 10000000, allowUnlimitedContractSize: true, }) diff --git a/packages/protocol/contracts/governance/Governance.sol b/packages/protocol/contracts/governance/Governance.sol index b006da40399..67aa4e78d34 100644 --- a/packages/protocol/contracts/governance/Governance.sol +++ b/packages/protocol/contracts/governance/Governance.sol @@ -10,18 +10,24 @@ import "./Proposals.sol"; import "../common/ExtractFunctionSignature.sol"; import "../common/Initializable.sol"; import "../common/FixidityLib.sol"; -import "../common/FractionUtil.sol"; import "../common/linkedlists/IntegerSortedLinkedList.sol"; import "../common/UsingRegistry.sol"; +import "../common/UsingPrecompiles.sol"; // TODO(asa): Hardcode minimum times for queueExpiry, etc. /** * @title A contract for making, passing, and executing on-chain governance proposals. */ -contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, UsingRegistry { +contract Governance is + IGovernance, + Ownable, + Initializable, + ReentrancyGuard, + UsingRegistry, + UsingPrecompiles +{ using Proposals for Proposals.Proposal; using FixidityLib for FixidityLib.Fraction; - using FractionUtil for FractionUtil.Fraction; using SafeMath for uint256; using IntegerSortedLinkedList for SortedLinkedList.List; using BytesLib for bytes; @@ -66,11 +72,17 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi struct ContractConstitution { FixidityLib.Fraction defaultThreshold; - // Maps a function ID to a corresponding passing function, overriding the - // default. + // Maps a function ID to a corresponding threshold, overriding the default. mapping(bytes4 => FixidityLib.Fraction) functionThresholds; } + struct HotfixRecord { + bool executed; + bool approved; + uint256 preparedEpoch; + mapping(address => bool) whitelisted; + } + // The baseline is updated as // max{floor, (1 - baselineUpdateFactor) * baseline + baselineUpdateFactor * participation} struct ParticipationParameters { @@ -96,6 +108,7 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi mapping(address => ContractConstitution) private constitution; mapping(uint256 => Proposals.Proposal) private proposals; mapping(address => Voter) private voters; + mapping(bytes32 => HotfixRecord) public hotfixes; SortedLinkedList.List private queue; uint256[] public dequeued; uint256[] public emptyIndices; @@ -199,6 +212,24 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi uint256 baselineQuorumFactor ); + event HotfixWhitelisted( + bytes32 indexed hash, + address whitelister + ); + + event HotfixApproved( + bytes32 indexed hash + ); + + event HotfixPrepared( + bytes32 indexed hash, + uint256 indexed epoch + ); + + event HotfixExecuted( + bytes32 indexed hash + ); + function() external payable {} // solhint-disable no-empty-blocks /** @@ -666,7 +697,74 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi } /** - * @notice Withdraws refunded Celo Gold commitments. + * @notice Whitelists the hash of a hotfix transaction(s). + * @param hash The abi encoded keccak256 hash of the hotfix transaction(s) to be whitelisted. + */ + function approveHotfix(bytes32 hash) external { + require(msg.sender == approver); + hotfixes[hash].approved = true; + emit HotfixApproved(hash); + } + + /** + * @notice Whitelists the hash of a hotfix transaction(s). + * @param hash The abi encoded keccak256 hash of the hotfix transaction(s) to be whitelisted. + */ + function whitelistHotfix(bytes32 hash) external { + hotfixes[hash].whitelisted[msg.sender] = true; + emit HotfixWhitelisted(hash, msg.sender); + } + + /** + * @notice Gives hotfix a prepared epoch for execution. + * @param hash The hash of the hotfix to be prepared. + */ + function prepareHotfix(bytes32 hash) external { + require(isHotfixPassing(hash), "hotfix not whitelisted by 2f+1 validators"); + uint256 epoch = getEpochNumber(); + require(hotfixes[hash].preparedEpoch < epoch, "hotfix already prepared for this epoch"); + hotfixes[hash].preparedEpoch = epoch; + emit HotfixPrepared(hash, epoch); + } + + /** + * @notice Executes a whitelisted proposal. + * @param values The values of Celo Gold to be sent in the proposed transactions. + * @param destinations The destination addresses of the proposed transactions. + * @param data The concatenated data to be included in the proposed transactions. + * @param dataLengths The lengths of each transaction's data. + * @dev Reverts if hotfix is already executed, not approved, or not prepared for current epoch. + */ + function executeHotfix( + uint256[] calldata values, + address[] calldata destinations, + bytes calldata data, + uint256[] calldata dataLengths + ) + external + { + bytes32 hash = keccak256(abi.encode(values, destinations, data, dataLengths)); + + (bool approved, bool executed, uint256 preparedEpoch) = getHotfixRecord(hash); + require(!executed, "hotfix already executed"); + require(approved, "hotfix not approved"); + require(preparedEpoch == getEpochNumber(), "hotfix must be prepared for this epoch"); + + Proposals.makeMem( + values, + destinations, + data, + dataLengths, + msg.sender, + 0 + ).executeMem(); + + hotfixes[hash].executed = true; + emit HotfixExecuted(hash); + } + + /** + * @notice Withdraws refunded Celo Gold deposits. * @return Whether or not the withdraw was successful. */ function withdraw() external nonReentrant returns (bool) { @@ -678,21 +776,8 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi } /** - * @notice Returns the participation parameters. - * @return The participation parameters. - */ - function getParticipationParameters() external view returns (uint256, uint256, uint256, uint256) { - return ( - participationParameters.baseline.unwrap(), - participationParameters.baselineFloor.unwrap(), - participationParameters.baselineUpdateFactor.unwrap(), - participationParameters.baselineQuorumFactor.unwrap() - ); - } - - /** - * @notice Returns the number of seconds proposals stay in the approval stage. - * @return The number of seconds proposals stay in the execution stage. + * @notice Returns the number of seconds proposals stay in approval stage. + * @return The number of seconds proposals stay in approval stage. */ function getApprovalStageDuration() external view returns (uint256) { return stageDurations.approval; @@ -700,7 +785,7 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi /** * @notice Returns the number of seconds proposals stay in the referendum stage. - * @return The number of seconds proposals stay in the execution stage. + * @return The number of seconds proposals stay in the referendum stage. */ function getReferendumStageDuration() external view returns (uint256) { return stageDurations.referendum; @@ -714,6 +799,19 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi return stageDurations.execution; } + /** + * @notice Returns the participation parameters. + * @return The participation parameters. + */ + function getParticipationParameters() external view returns (uint256, uint256, uint256, uint256) { + return ( + participationParameters.baseline.unwrap(), + participationParameters.baselineFloor.unwrap(), + participationParameters.baselineUpdateFactor.unwrap(), + participationParameters.baselineQuorumFactor.unwrap() + ); + } + /** * @notice Returns whether or not a proposal exists. * @param proposalId The ID of the proposal. @@ -838,6 +936,45 @@ contract Governance is IGovernance, Ownable, Initializable, ReentrancyGuard, Usi return voters[account].mostRecentReferendumProposal; } + /** + * @notice Checks if a byzantine quorum of validators has whitelisted the given hotfix. + * @param hash The abi encoded keccak256 hash of the hotfix transaction. + * @return Whether validator whitelist tally >= validator byztanine quorum (2f+1) + */ + function isHotfixPassing(bytes32 hash) public view returns (bool) { + uint256 tally = 0; + uint256 n = numberValidatorsInCurrentSet(); + for (uint256 idx = 0; idx < n; idx++) { + address validator = validatorAddressFromCurrentSet(idx); + if (hotfixes[hash].whitelisted[validator]) { + tally = tally.add(1); + } + } + + return tally >= byzantineQuorumValidatorsInCurrentSet(); + } + + /** + * @notice Computes byzantine quorum from current validator set size + * @return Byzantine quorum of validators. + */ + function byzantineQuorumValidatorsInCurrentSet() public view returns (uint256) { + return numberValidatorsInCurrentSet().mul(2).div(3).add(1); + } + + /** + * @notice Gets information about a hotfix. + * @param hash The abi encoded keccak256 hash of the hotfix transaction. + * @return Hotfix tuple of (approved, executed, preparedEpoch) + */ + function getHotfixRecord(bytes32 hash) public view returns (bool, bool, uint256) { + return ( + hotfixes[hash].approved, + hotfixes[hash].executed, + hotfixes[hash].preparedEpoch + ); + } + /** * @notice Removes the proposals with the most upvotes from the queue, moving them to the * approval stage. diff --git a/packages/protocol/contracts/governance/Proposals.sol b/packages/protocol/contracts/governance/Proposals.sol index c7ec5fcce6b..775a56528ca 100644 --- a/packages/protocol/contracts/governance/Proposals.sol +++ b/packages/protocol/contracts/governance/Proposals.sol @@ -90,12 +90,56 @@ library Proposals { proposal.timestamp = now; uint256 dataPosition = 0; + delete proposal.transactions; for (uint256 i = 0; i < transactionCount; i = i.add(1)) { - proposal.transactions.push( - Transaction(values[i], destinations[i], data.slice(dataPosition, dataLengths[i])) + proposal.transactions.push(Transaction( + values[i], + destinations[i], + data.slice(dataPosition, dataLengths[i]) + )); + dataPosition = dataPosition.add(dataLengths[i]); + } + } + + /** + * @notice Constructs a proposal for use in memory. + * @param values The values of Celo Gold to be sent in the proposed transactions. + * @param destinations The destination addresses of the proposed transactions. + * @param data The concatenated data to be included in the proposed transactions. + * @param dataLengths The lengths of each transaction's data. + * @param proposer The proposer. + * @param deposit The proposal deposit. + */ + function makeMem( + uint256[] memory values, + address[] memory destinations, + bytes memory data, + uint256[] memory dataLengths, + address proposer, + uint256 deposit + ) + internal view returns (Proposal memory) + { + require(values.length == destinations.length && destinations.length == dataLengths.length); + uint256 transactionCount = values.length; + + Proposal memory proposal; + proposal.proposer = proposer; + proposal.deposit = deposit; + // solhint-disable-next-line not-rely-on-time + proposal.timestamp = now; + + uint256 dataPosition = 0; + proposal.transactions = new Transaction[](transactionCount); + for (uint256 i = 0; i < transactionCount; i = i.add(1)) { + proposal.transactions[i] = Transaction( + values[i], + destinations[i], + data.slice(dataPosition, dataLengths[i]) ); dataPosition = dataPosition.add(dataLengths[i]); } + return proposal; } /** @@ -140,13 +184,26 @@ library Proposals { * @dev Reverts if any transaction fails. */ function execute(Proposal storage proposal) public { - for (uint256 i = 0; i < proposal.transactions.length; i = i.add(1)) { + executeTransactions(proposal.transactions); + } + + /** + * @notice Executes the proposal. + * @param proposal The proposal struct. + * @dev Reverts if any transaction fails. + */ + function executeMem(Proposal memory proposal) internal { + executeTransactions(proposal.transactions); + } + + function executeTransactions(Transaction[] memory transactions) internal { + for (uint256 i = 0; i < transactions.length; i = i.add(1)) { require( externalCall( - proposal.transactions[i].destination, - proposal.transactions[i].value, - proposal.transactions[i].data.length, - proposal.transactions[i].data + transactions[i].destination, + transactions[i].value, + transactions[i].data.length, + transactions[i].data ) ); } diff --git a/packages/protocol/contracts/governance/interfaces/IGovernance.sol b/packages/protocol/contracts/governance/interfaces/IGovernance.sol index 2db70134e22..21afee851b2 100644 --- a/packages/protocol/contracts/governance/interfaces/IGovernance.sol +++ b/packages/protocol/contracts/governance/interfaces/IGovernance.sol @@ -27,6 +27,17 @@ interface IGovernance { function revokeUpvote(uint256, uint256) external returns (bool); function approve(uint256, uint256) external returns (bool); function execute(uint256, uint256) external returns (bool); + + function whitelistHotfix(bytes32) external; + function prepareHotfix(bytes32) external; + function executeHotfix( + uint256[] calldata, + address[] calldata, + bytes calldata, + uint256[] calldata + ) external; + function isHotfixPassing(bytes32) external view returns (bool); + function withdraw() external returns (bool); function dequeueProposalsIfReady() external; function getParticipationParameters() external view returns (uint256, uint256, uint256, uint256); diff --git a/packages/protocol/contracts/governance/test/GovernanceTest.sol b/packages/protocol/contracts/governance/test/GovernanceTest.sol new file mode 100644 index 00000000000..869e6d4d086 --- /dev/null +++ b/packages/protocol/contracts/governance/test/GovernanceTest.sol @@ -0,0 +1,21 @@ +pragma solidity ^0.5.3; + +import "../Governance.sol"; + +contract GovernanceTest is Governance { + address[] validatorSet; + + // Minimally override core functions from UsingPrecompiles + function numberValidatorsInCurrentSet() public view returns (uint256) { + return validatorSet.length; + } + + function validatorAddressFromCurrentSet(uint256 index) public view returns (address) { + return validatorSet[index]; + } + + // Expose test utilities + function addValidator(address validator) external { + validatorSet.push(validator); + } +} \ No newline at end of file diff --git a/packages/protocol/migrationsConfig.js b/packages/protocol/migrationsConfig.js index 43c2d21f3fc..215bbcb1684 100644 --- a/packages/protocol/migrationsConfig.js +++ b/packages/protocol/migrationsConfig.js @@ -110,12 +110,13 @@ const linkedLibraries = { 'Exchange', 'GasPriceMinimum', 'Governance', + 'GovernanceTest', 'Proposals', 'SortedOracles', 'StableToken', 'Validators', ], - Proposals: ['Governance', 'ProposalsTest'], + Proposals: ['Governance', 'GovernanceTest', 'ProposalsTest'], LinkedList: ['AddressLinkedList', 'SortedLinkedList', 'LinkedListTest'], SortedLinkedList: [ 'AddressSortedLinkedList', @@ -125,7 +126,7 @@ const linkedLibraries = { SortedLinkedListWithMedian: ['AddressSortedLinkedListWithMedian'], AddressLinkedList: ['Validators', 'ValidatorsTest'], AddressSortedLinkedList: ['Election', 'ElectionTest'], - IntegerSortedLinkedList: ['Governance', 'IntegerSortedLinkedListTest'], + IntegerSortedLinkedList: ['Governance', 'GovernanceTest', 'IntegerSortedLinkedListTest'], AddressSortedLinkedListWithMedian: ['SortedOracles', 'AddressSortedLinkedListWithMedianTest'], Signatures: ['Accounts', 'TestAttestations', 'Attestations', 'LockedGold', 'Escrow'], } diff --git a/packages/protocol/runTests.js b/packages/protocol/runTests.js index 2704b7c37a1..ca608d819a0 100644 --- a/packages/protocol/runTests.js +++ b/packages/protocol/runTests.js @@ -17,7 +17,7 @@ async function startGanache() { network_id: network.network_id, mnemonic: network.mnemonic, gasPrice: network.gasPrice, - gasLimit: 8000000, + gasLimit: 10000000, allowUnlimitedContractSize: true, }) diff --git a/packages/protocol/scripts/devchain.ts b/packages/protocol/scripts/devchain.ts index 220c75430b7..10ee8e955f7 100644 --- a/packages/protocol/scripts/devchain.ts +++ b/packages/protocol/scripts/devchain.ts @@ -7,7 +7,7 @@ import * as yargs from 'yargs' const MNEMONIC = 'concert load couple harbor equip island argue ramp clarify fence smart topic' -const gasLimit = 8000000 +const gasLimit = 10000000 const ProtocolRoot = path.normalize(path.join(__dirname, '../')) diff --git a/packages/protocol/test/governance/governance.ts b/packages/protocol/test/governance/governance.ts index 79b5b9f92f1..5934e988c05 100644 --- a/packages/protocol/test/governance/governance.ts +++ b/packages/protocol/test/governance/governance.ts @@ -1,3 +1,18 @@ +import BigNumber from 'bignumber.js' +import { keccak256 } from 'ethereumjs-util' +import { + AccountsContract, + AccountsInstance, + GovernanceTestContract, + GovernanceTestInstance, + MockLockedGoldContract, + MockLockedGoldInstance, + RegistryContract, + RegistryInstance, + TestTransactionsContract, + TestTransactionsInstance, +} from 'types' + import { CeloContractName } from '@celo/protocol/lib/registry-utils' import { assertBalance, @@ -5,27 +20,15 @@ import { assertLogMatches2, assertRevert, matchAny, + mineBlocks, NULL_ADDRESS, stripHexEncoding, timeTravel, } from '@celo/protocol/lib/test-utils' import { fixed1, multiply, toFixed } from '@celo/utils/lib/fixidity' -import BigNumber from 'bignumber.js' -import { - AccountsContract, - AccountsInstance, - GovernanceContract, - GovernanceInstance, - MockLockedGoldContract, - MockLockedGoldInstance, - RegistryContract, - RegistryInstance, - TestTransactionsContract, - TestTransactionsInstance, -} from 'types' +const Governance: GovernanceTestContract = artifacts.require('GovernanceTest') const Accounts: AccountsContract = artifacts.require('Accounts') -const Governance: GovernanceContract = artifacts.require('Governance') const MockLockedGold: MockLockedGoldContract = artifacts.require('MockLockedGold') const Registry: RegistryContract = artifacts.require('Registry') const TestTransactions: TestTransactionsContract = artifacts.require('TestTransactions') @@ -58,11 +61,20 @@ enum VoteValue { Yes, } +interface Transaction { + value: number + destination: string + data: Buffer +} + +// hard coded in ganache +const EPOCH = 100 + // TODO(asa): Test dequeueProposalsIfReady // TODO(asa): Dequeue explicitly to make the gas cost of operations more clear contract('Governance', (accounts: string[]) => { + let governance: GovernanceTestInstance let accountsInstance: AccountsInstance - let governance: GovernanceInstance let mockLockedGold: MockLockedGoldInstance let testTransactions: TestTransactionsInstance let registry: RegistryInstance @@ -88,10 +100,11 @@ contract('Governance', (accounts: string[]) => { const expectedParticipationBaseline = multiply(baselineUpdateFactor, participation).plus( multiply(fixed1.minus(baselineUpdateFactor), participationBaseline) ) - - let transactionSuccess1 - let transactionSuccess2 - let transactionFail + let transactionSuccess1: Transaction + let transactionSuccess2: Transaction + let transactionFail: Transaction + let proposalHash: Buffer + let proposalHashStr: string beforeEach(async () => { accountsInstance = await Accounts.new() governance = await Governance.new() @@ -151,6 +164,18 @@ contract('Governance', (accounts: string[]) => { 'hex' ), } + proposalHash = keccak256( + web3.eth.abi.encodeParameters( + ['uint256[]', 'address[]', 'bytes', 'uint256[]'], + [ + [String(transactionSuccess1.value)], + [transactionSuccess1.destination.toString()], + transactionSuccess1.data, + [String(transactionSuccess1.data.length)], + ] + ) + ) as Buffer + proposalHashStr = '0x' + proposalHash.toString('hex') }) describe('#initialize()', () => { @@ -224,7 +249,7 @@ contract('Governance', (accounts: string[]) => { }) describe('#setApprover', () => { - const newApprover = accounts[1] + const newApprover = accounts[2] it('should set the approver', async () => { await governance.setApprover(newApprover) assert.equal(await governance.approver(), newApprover) @@ -545,9 +570,7 @@ contract('Governance', (accounts: string[]) => { it('should revert when called by anyone other than the owner', async () => { await assertRevert( - governance.setBaselineUpdateFactor(differentBaselineUpdateFactor, { - from: nonOwner, - }) + governance.setBaselineUpdateFactor(differentBaselineUpdateFactor, { from: nonOwner }) ) }) }) @@ -579,9 +602,7 @@ contract('Governance', (accounts: string[]) => { it('should revert when called by anyone other than the owner', async () => { await assertRevert( - governance.setBaselineQuorumFactor(differentBaselineQuorumFactor, { - from: nonOwner, - }) + governance.setBaselineQuorumFactor(differentBaselineQuorumFactor, { from: nonOwner }) ) }) }) @@ -694,6 +715,7 @@ contract('Governance', (accounts: string[]) => { const id = await governance.propose.call( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -706,6 +728,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -718,6 +741,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -765,6 +789,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -782,6 +807,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -801,6 +827,7 @@ contract('Governance', (accounts: string[]) => { const resp = await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -900,6 +927,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -912,6 +940,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -931,6 +960,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -979,6 +1009,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1002,6 +1033,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1045,6 +1077,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1080,6 +1113,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1138,6 +1172,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1243,6 +1278,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1278,6 +1314,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1325,6 +1362,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1343,6 +1381,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1362,6 +1401,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1407,6 +1447,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1468,6 +1509,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1595,6 +1637,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1659,6 +1702,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionFail.value], [transactionFail.destination], + // @ts-ignore bytes type transactionFail.data, [transactionFail.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1682,6 +1726,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [accounts[1]], + // @ts-ignore transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1824,6 +1869,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1880,6 +1926,197 @@ contract('Governance', (accounts: string[]) => { }) }) + describe('#approveHotfix()', () => { + it('should mark the hotfix record approved when called by approver', async () => { + await governance.approveHotfix(proposalHashStr, { from: approver }) + const [approved, ,] = await governance.getHotfixRecord.call(proposalHashStr) + assert.isTrue(approved) + }) + + it('should emit the HotfixApproved event', async () => { + const resp = await governance.approveHotfix(proposalHashStr, { from: approver }) + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'HotfixApproved', + args: { + hash: matchAny, + }, + }) + assert.isTrue(Buffer.from(stripHexEncoding(log.args.hash), 'hex').equals(proposalHash)) + }) + + it('should revert when called by non-approver', async () => { + await assertRevert(governance.approveHotfix(proposalHashStr, { from: accounts[2] })) + }) + }) + + describe('#whitelistHotfix()', () => { + beforeEach(async () => { + // from GovernanceTest + await governance.addValidator(accounts[2]) + await governance.addValidator(accounts[3]) + }) + + it('should emit the HotfixWhitelist event', async () => { + const resp = await governance.whitelistHotfix(proposalHashStr, { from: accounts[3] }) + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'HotfixWhitelisted', + args: { + hash: matchAny, + whitelister: accounts[3], + }, + }) + assert.isTrue(Buffer.from(stripHexEncoding(log.args.hash), 'hex').equals(proposalHash)) + }) + }) + + describe('#isHotfixPassing', () => { + beforeEach(async () => { + await governance.addValidator(accounts[2]) + await governance.addValidator(accounts[3]) + }) + + it('should return false when hotfix has not been whitelisted', async () => { + const passing = await governance.isHotfixPassing.call(proposalHashStr) + assert.isFalse(passing) + }) + + it('should return false when hotfix has been whitelisted but not by quorum', async () => { + await governance.whitelistHotfix(proposalHashStr, { from: accounts[2] }) + const passing = await governance.isHotfixPassing.call(proposalHashStr) + assert.isFalse(passing) + }) + + it('should return true when hotfix is whitelisted by quorum', async () => { + await governance.whitelistHotfix(proposalHashStr, { from: accounts[2] }) + await governance.whitelistHotfix(proposalHashStr, { from: accounts[3] }) + const passing = await governance.isHotfixPassing.call(proposalHashStr) + assert.isTrue(passing) + }) + }) + + describe('#prepareHotfix()', () => { + beforeEach(async () => { + await governance.addValidator(accounts[2]) + }) + + it('should revert when hotfix is not passing', async () => { + await assertRevert(governance.prepareHotfix(proposalHashStr)) + }) + + describe('when hotfix is passing', () => { + beforeEach(async () => { + await mineBlocks(EPOCH, web3) + await governance.whitelistHotfix(proposalHashStr, { from: accounts[2] }) + }) + + it('should mark the hotfix record prepared epoch', async () => { + await governance.prepareHotfix(proposalHashStr) + const [, , preparedEpoch] = await governance.getHotfixRecord.call(proposalHashStr) + const currEpoch = new BigNumber(await governance.getEpochNumber()) + assertEqualBN(preparedEpoch, currEpoch) + }) + + it('should emit the HotfixPrepared event', async () => { + const resp = await governance.prepareHotfix(proposalHashStr) + const currEpoch = new BigNumber(await governance.getEpochNumber()) + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'HotfixPrepared', + args: { + hash: matchAny, + epoch: currEpoch, + }, + }) + assert.isTrue(Buffer.from(stripHexEncoding(log.args.hash), 'hex').equals(proposalHash)) + }) + + it('should revert when epoch == preparedEpoch', async () => { + await governance.prepareHotfix(proposalHashStr) + await assertRevert(governance.prepareHotfix(proposalHashStr)) + }) + + it('should succeed for epoch != preparedEpoch', async () => { + await governance.prepareHotfix(proposalHashStr) + await mineBlocks(EPOCH, web3) + await governance.prepareHotfix(proposalHashStr) + }) + }) + }) + + describe('#executeHotfix()', () => { + const executeHotfixTx = () => + governance.executeHotfix( + [transactionSuccess1.value], + [transactionSuccess1.destination], + // @ts-ignore bytes type + transactionSuccess1.data, + [transactionSuccess1.data.length] + ) + + it('should revert when hotfix not approved', async () => { + await assertRevert(executeHotfixTx()) + }) + + it('should revert when hotfix not prepared for current epoch', async () => { + await mineBlocks(EPOCH, web3) + await governance.approveHotfix(proposalHashStr, { from: approver }) + await assertRevert(executeHotfixTx()) + }) + + it('should revert when hotfix prepared but not for current epoch', async () => { + await governance.approveHotfix(proposalHashStr, { from: approver }) + await governance.addValidator(accounts[2]) + await governance.whitelistHotfix(proposalHashStr, { from: accounts[2] }) + await governance.prepareHotfix(proposalHashStr, { from: accounts[2] }) + await mineBlocks(EPOCH, web3) + await assertRevert(executeHotfixTx()) + }) + + describe('when hotfix is approved and prepared for current epoch', () => { + beforeEach(async () => { + await governance.approveHotfix(proposalHashStr, { from: approver }) + await mineBlocks(EPOCH, web3) + await governance.addValidator(accounts[2]) + await governance.whitelistHotfix(proposalHashStr, { from: accounts[2] }) + await governance.prepareHotfix(proposalHashStr) + }) + + it('should execute the hotfix tx', async () => { + await executeHotfixTx() + assert.equal(await testTransactions.getValue(1).valueOf(), 1) + }) + + it('should mark the hotfix record as executed', async () => { + await executeHotfixTx() + const [, executed] = await governance.getHotfixRecord.call(proposalHashStr) + assert.isTrue(executed) + }) + + it('should emit the HotfixExecuted event', async () => { + const resp = await executeHotfixTx() + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'HotfixExecuted', + args: { + hash: matchAny, + }, + }) + assert.isTrue(Buffer.from(stripHexEncoding(log.args.hash), 'hex').equals(proposalHash)) + }) + + it('should not be executable again', async () => { + await executeHotfixTx() + await assertRevert(executeHotfixTx()) + }) + }) + }) + /* describe('#isVoting()', () => { describe('when the account has never acted on a proposal', () => { @@ -1895,6 +2132,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1936,6 +2174,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails @@ -1973,6 +2212,7 @@ contract('Governance', (accounts: string[]) => { await governance.propose( [transactionSuccess1.value], [transactionSuccess1.destination], + // @ts-ignore bytes type transactionSuccess1.data, [transactionSuccess1.data.length], // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails diff --git a/packages/protocol/truffle-config.js b/packages/protocol/truffle-config.js index 567d41513ed..a88b8757f54 100644 --- a/packages/protocol/truffle-config.js +++ b/packages/protocol/truffle-config.js @@ -19,7 +19,7 @@ const ALFAJORES_FROM = '0x456f41406B32c45D59E539e4BBA3D7898c3584dA' const PILOT_FROM = '0x387bCb16Bfcd37AccEcF5c9eB2938E30d3aB8BF2' const PILOTSTAGING_FROM = '0x545DEBe3030B570731EDab192640804AC8Cf65CA' -const gasLimit = 8000000 +const gasLimit = 10000000 const defaultConfig = { host: '127.0.0.1', From d8d97af82507e1753a181a48b2a4e22583591a94 Mon Sep 17 00:00:00 2001 From: "Victor \"Nate\" Graf" Date: Fri, 1 Nov 2019 14:37:31 -0700 Subject: [PATCH 02/71] Avoid re-encrypting key files with yarn keys:encrypt command (#1560) --- scripts/key_placer.sh | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/scripts/key_placer.sh b/scripts/key_placer.sh index ebd7df35e40..dd975e1a012 100755 --- a/scripts/key_placer.sh +++ b/scripts/key_placer.sh @@ -1,13 +1,8 @@ #!/usr/bin/env bash -# Note: Encryption includes changing the metadata with a timestamp of when -# the file was encrypted, so every time you run this it will produce a diff -# in all of the files. Please only check in the files you intend to change - -# files to be processed - echo "Processing encrypted files" +# Set list of secret files to encrypt and decrypt. files=( "packages/mobile/android/app/google-services.json" "packages/mobile/android/app/src/staging/google-services.json" @@ -34,34 +29,47 @@ files=( ".env.mnemonic.pilotstaging" ) -# this is to allow the script to be called from anywhere -DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -cd $DIR -cd .. - -if [[ $1 != "encrypt" ]] && [[ $1 != "decrypt" ]]; then +if [[ -z "$1" ]]; then + echo "Encrypt or decrypt secret files using GCP keystore." + echo "usage: $0 < encrypt | decrypt >" + exit 1 +elif [[ $1 != "encrypt" ]] && [[ $1 != "decrypt" ]]; then echo "invalid action $1. Choose 'encrypt' or 'decrypt'" + echo "usage: $0 < encrypt | decrypt >" exit 1 fi command -v gcloud > /dev/null 2>&1 - if [[ $? -eq 1 ]]; then echo "gcloud is not installed - skipping ${1}ion" exit 0 fi +# this is to allow the script to be called from anywhere +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +cd $DIR +cd .. + for file_path in "${files[@]}"; do encrypted_file_path="$file_path.enc" + # When decrypting ensure the encrypted file exists or skip. if [[ $1 == "decrypt" ]] && ! test -f "$encrypted_file_path"; then - echo "$encrypted_file_path does not exist, cannot decrypt - skipping file" - continue - elif [[ $1 == "encrypt" ]] && ! test -f "$file_path"; then - echo "$file_path does not exist, cannot encrypt - skipping file" + echo "$encrypted_file_path does not exist, cannot decrypt - skipping file" >&2 continue fi + # When encrypting ensure the plaintext file exists and the encrypted file does not. + if [[ $1 == "encrypt" ]]; then + if [[ -f "$encrypted_file_path" ]]; then + continue + elif [[ ! -f "$file_path" ]]; then + echo "$file_path does not exist, cannot encrypt - skipping file" >&2 + continue + fi + fi + + # Encrypt or decrypt this file. gcloud kms $1 --ciphertext-file=$encrypted_file_path --plaintext-file=$file_path --key=github-key --keyring=celo-keyring --location=global --project celo-testnet if [[ $? -eq 1 ]]; then echo "Only C Labs employees can $1 keys - skipping ${1}ion" From 82aac04333cea6b9261bbc73307483bc800f057c Mon Sep 17 00:00:00 2001 From: Martin Date: Sat, 2 Nov 2019 00:56:44 +0100 Subject: [PATCH 03/71] Allow a specified address to disable/enable the Exchange (#1467) --- .../protocol/contracts/baklava/Freezable.sol | 43 +++++++++ .../contracts/baklava/test/FreezableTest.sol | 20 ++++ .../protocol/contracts/stability/Exchange.sol | 11 ++- .../stability/interfaces/IExchange.sol | 1 + packages/protocol/migrations/09_exchange.ts | 5 +- packages/protocol/test/baklava/freezable.ts | 91 +++++++++++++++++++ packages/protocol/test/stability/exchange.ts | 13 +++ 7 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 packages/protocol/contracts/baklava/Freezable.sol create mode 100644 packages/protocol/contracts/baklava/test/FreezableTest.sol create mode 100644 packages/protocol/test/baklava/freezable.ts diff --git a/packages/protocol/contracts/baklava/Freezable.sol b/packages/protocol/contracts/baklava/Freezable.sol new file mode 100644 index 00000000000..2d0a57a5c87 --- /dev/null +++ b/packages/protocol/contracts/baklava/Freezable.sol @@ -0,0 +1,43 @@ +pragma solidity ^0.5.3; + + +contract Freezable { + bool public frozen; + address public freezer; + + // onlyFreezer functions can only be called by the specified `freezer` address + modifier onlyFreezer() { + require(msg.sender == freezer); + _; + } + + // onlyWhenNotFrozen functions can only be called when `frozen` is false, otherwise they will + // revert. + modifier onlyWhenNotFrozen() { + require(!frozen); + _; + } + + /** + * @notice Sets the address that is allowed to freeze/unfreeze the contract. + * @param _freezer The address that is allowed to freeze/unfree the contract + * @dev This function is `internal` and leaves its permissioning up to the inheriting contract. + */ + function _setFreezer(address _freezer) internal { + freezer = _freezer; + } + + /** + * @notice Freezes the contract, disabling `onlyWhenNotFrozen` functions. + */ + function freeze() external onlyFreezer { + frozen = true; + } + + /** + * @notice Unreezes the contract, enabling `onlyWhenNotFrozen` functions. + */ + function unfreeze() external onlyFreezer { + frozen = false; + } +} diff --git a/packages/protocol/contracts/baklava/test/FreezableTest.sol b/packages/protocol/contracts/baklava/test/FreezableTest.sol new file mode 100644 index 00000000000..23c5baf55fa --- /dev/null +++ b/packages/protocol/contracts/baklava/test/FreezableTest.sol @@ -0,0 +1,20 @@ +pragma solidity ^0.5.8; + +import "../Freezable.sol"; + + +contract FreezableTest is Freezable { + event FunctionCalled(); + + function setFreezer(address _freezer) external { + _setFreezer(_freezer); + } + + function freezableFunction() external onlyWhenNotFrozen { + emit FunctionCalled(); + } + + function nonfreezableFunction() external { + emit FunctionCalled(); + } +} diff --git a/packages/protocol/contracts/stability/Exchange.sol b/packages/protocol/contracts/stability/Exchange.sol index 5e0e457422e..0291998717e 100644 --- a/packages/protocol/contracts/stability/Exchange.sol +++ b/packages/protocol/contracts/stability/Exchange.sol @@ -10,6 +10,7 @@ import "./interfaces/IStableToken.sol"; import "../common/FractionUtil.sol"; import "../common/Initializable.sol"; import "../common/FixidityLib.sol"; +import "../baklava/Freezable.sol"; import "../common/UsingRegistry.sol"; @@ -17,7 +18,7 @@ import "../common/UsingRegistry.sol"; * @title Contract that allows to exchange StableToken for GoldToken and vice versa * using a Constant Product Market Maker Model */ -contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, ReentrancyGuard { +contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, ReentrancyGuard, Freezable { using SafeMath for uint256; using FractionUtil for FractionUtil.Fraction; using FixidityLib for FixidityLib.Fraction; @@ -89,6 +90,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc */ function initialize( address registryAddress, + address _freezer, address stableToken, uint256 _spread, uint256 _reserveFraction, @@ -99,6 +101,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc initializer { _transferOwnership(msg.sender); + setFreezer(_freezer); setRegistry(registryAddress); setStableToken(stableToken); setSpread(_spread); @@ -116,6 +119,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc * transaction to succeed * @param sellGold `true` if gold is the sell token * @return The amount of buyToken that was transfered + * @dev This function can be frozen using the Freezable interface. */ function exchange( uint256 sellAmount, @@ -123,6 +127,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc bool sellGold ) external + onlyWhenNotFrozen updateBucketsIfNecessary nonReentrant returns (uint256) @@ -254,6 +259,10 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc emit MinimumReportsSet(newMininumReports); } + function setFreezer(address freezer) public onlyOwner { + _setFreezer(freezer); + } + /** * @notice Allows owner to set the Stable Token address * @param newStableToken The new address for Stable Token diff --git a/packages/protocol/contracts/stability/interfaces/IExchange.sol b/packages/protocol/contracts/stability/interfaces/IExchange.sol index 8be29102eb3..b70d8250ecc 100644 --- a/packages/protocol/contracts/stability/interfaces/IExchange.sol +++ b/packages/protocol/contracts/stability/interfaces/IExchange.sol @@ -4,6 +4,7 @@ pragma solidity ^0.5.3; interface IExchange { function initialize( + address, address, address, uint256, diff --git a/packages/protocol/migrations/09_exchange.ts b/packages/protocol/migrations/09_exchange.ts index 07d78d5a295..b0e1f4b97ef 100644 --- a/packages/protocol/migrations/09_exchange.ts +++ b/packages/protocol/migrations/09_exchange.ts @@ -8,14 +8,17 @@ import { import { config } from '@celo/protocol/migrationsConfig' import { toFixed } from '@celo/utils/lib/fixidity' import { ExchangeInstance, ReserveInstance, StableTokenInstance } from 'types' +const truffle = require('@celo/protocol/truffle-config.js') -const initializeArgs = async (): Promise => { +const initializeArgs = async (networkName: string): Promise => { + const network: any = truffle.networks[networkName] const stableToken: StableTokenInstance = await getDeployedProxiedContract( 'StableToken', artifacts ) return [ config.registry.predeployedProxyAddress, + network.from, stableToken.address, toFixed(config.exchange.spread).toString(), toFixed(config.exchange.reserveFraction).toString(), diff --git a/packages/protocol/test/baklava/freezable.ts b/packages/protocol/test/baklava/freezable.ts new file mode 100644 index 00000000000..1c7431a80f6 --- /dev/null +++ b/packages/protocol/test/baklava/freezable.ts @@ -0,0 +1,91 @@ +import { assertSameAddress, assertLogMatches2, assertRevert } from '@celo/protocol/lib/test-utils' +import { FreezableTestInstance } from 'types' + +contract('Freezable', (accounts: string[]) => { + const FreezableTest = artifacts.require('FreezableTest') + let freezableTest: FreezableTestInstance + + beforeEach(async () => { + freezableTest = await FreezableTest.new() + freezableTest.setFreezer(accounts[0]) + }) + + describe('_setFreezer', () => { + it('should allow owner to change the freezer', async () => { + // _setFreezer is internal in Freezable, FreezableTest wraps around it with setFreezer + await freezableTest.setFreezer(accounts[1]) + const freezer = await freezableTest.freezer() + assertSameAddress(freezer, accounts[1]) + }) + }) + + describe('when the contract is not frozen', () => { + it('should allow freezable functions to be called', async () => { + const resp = await freezableTest.freezableFunction() + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'FunctionCalled', + args: {}, + }) + }) + + it('should allow non-freezable functions to be called', async () => { + const resp = await freezableTest.nonfreezableFunction() + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'FunctionCalled', + args: {}, + }) + }) + }) + + describe('freeze', () => { + it('should allow freezer to freeze the contract', async () => { + await freezableTest.freeze() + const frozen = await freezableTest.frozen() + assert.isTrue(frozen) + }) + + it('should not allow a non-freezer to freeze the contract', async () => { + await assertRevert(freezableTest.freeze({ from: accounts[1] })) + }) + }) + + describe('unfreeze', () => { + beforeEach(async () => { + freezableTest.freeze() + }) + + it('should allow freezer to unfreeze the contract', async () => { + await freezableTest.unfreeze() + const frozen = await freezableTest.frozen() + assert.isFalse(frozen) + }) + + it('should not allow a non-freezer to unfreeze the contract', async () => { + await assertRevert(freezableTest.freeze({ from: accounts[1] })) + }) + }) + + describe('when the contract is frozen', () => { + beforeEach(async () => { + await freezableTest.freeze() + }) + + it('should revert a freezable function', async () => { + await assertRevert(freezableTest.freezableFunction()) + }) + + it('should not affect a non-freezable function', async () => { + const resp = await freezableTest.nonfreezableFunction() + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'FunctionCalled', + args: {}, + }) + }) + }) +}) diff --git a/packages/protocol/test/stability/exchange.ts b/packages/protocol/test/stability/exchange.ts index 36be55415bc..b9f2e81b1a9 100644 --- a/packages/protocol/test/stability/exchange.ts +++ b/packages/protocol/test/stability/exchange.ts @@ -122,6 +122,7 @@ contract('Exchange', (accounts: string[]) => { exchange = await Exchange.new() await exchange.initialize( registry.address, + accounts[0], stableToken.address, spread, reserveFraction, @@ -141,6 +142,7 @@ contract('Exchange', (accounts: string[]) => { await assertRevert( exchange.initialize( registry.address, + accounts[0], stableToken.address, spread, reserveFraction, @@ -794,5 +796,16 @@ contract('Exchange', (accounts: string[]) => { }) }) }) + + describe('when the contract is frozen', () => { + beforeEach(async () => { + await exchange.freeze() + }) + + it('should revert', async () => { + await goldToken.approve(exchange.address, 1000) + await assertRevert(exchange.exchange(1000, 1, true)) + }) + }) }) }) From 8848975200b51cc2293060b0ccd69eaa5323ce8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Volpe?= Date: Sat, 2 Nov 2019 01:56:39 +0100 Subject: [PATCH 04/71] Fix e2e on CI (#1537) --- .circleci/config.yml | 26 +++++++++++++++++++++---- packages/mobile/package.json | 2 +- packages/mobile/scripts/ci-e2e.sh | 32 ------------------------------- packages/mobile/scripts/unlock.sh | 3 ++- scripts/run_e2e.sh | 6 ------ 5 files changed, 25 insertions(+), 44 deletions(-) delete mode 100755 packages/mobile/scripts/ci-e2e.sh delete mode 100755 scripts/run_e2e.sh diff --git a/.circleci/config.yml b/.circleci/config.yml index 2f2b5ee57f0..946d316eff3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -187,7 +187,8 @@ jobs: - run: nvm install v10.16.3 && nvm use v10.16.3 - run: name: install miscellaneous - command: HOMEBREW_NO_AUTO_UPDATE=1 brew install tree + command: | + HOMEBREW_NO_AUTO_UPDATE=1 brew install tree coreutils # Currently not used # - run: npm install --global react-native-kill-packager - run: @@ -202,8 +203,8 @@ jobs: # echo "Cache found, just run post-script." # yarn postinstall # fi - yarn - yarn build + yarn || yarn + yarn build || yarn build yarn run jetify - save_cache: key: yarn-v4-macos-{{ .Branch }}-{{ checksum "yarn.lock" }} @@ -216,6 +217,9 @@ jobs: command: HOMEBREW_NO_AUTO_UPDATE=1 brew install pidcat watchman - restore_cache: key: yarn-v3-{{ arch }}-{{ .Branch }}-{{ checksum "packages/mobile/android/build.gradle" }}-{{ checksum "packages/mobile/android/settings.gradle" }}-{{ checksum "packages/mobile/android/app/build.gradle" }}-{{ checksum "packages/mobile/.env.test" }} + - run: + name: Make sure there's only one adb # This is probably a brew bug + command: cp /usr/local/share/android-sdk/platform-tools/adb /usr/local/bin/adb - run: name: Start emulator command: cd ~/src/packages/mobile && bash ./scripts/start_emulator.sh @@ -233,12 +237,26 @@ jobs: name: Sleep until Device connects command: cd ~/src/packages/mobile && bash ./scripts/wait_for_emulator_to_connect.sh # TODO - run: unlock device + - run: + name: Start pidcat logging + command: pidcat -t "GoLog" -t "Go" # React logs are on metro step since RN 61 + background: true - run: name: Run yarn dev command: cd ~/src/packages/mobile && ENVFILE=".env.test" yarn dev + - run: + name: Restart adb + command: adb kill-server && adb start-server - run: name: Run test itself - command: cd ~/src/packages/mobile && ENVFILE=".env.test" yarn test:detox + + command: | + cd ~/src/packages/mobile + # detox sometimes without releasing the terminal and thus making the CI timout + # 480s = 8 minutes + timeout 480 yarn test:detox || echo "failed, try again" + timeout 480 yarn test:detox || echo "detox failed, return 0 to prevent CI from failing" + # TODO errors are currently not reported, until we figure out why detox can't find functions https://github.com/wix/Detox/issues/1723 - run: cd ~/src - save_cache: key: yarn-v3-{{ arch }}-{{ .Branch }}-{{ checksum "packages/mobile/android/build.gradle" }}-{{ checksum "packages/mobile/android/settings.gradle" }}-{{ checksum "packages/mobile/android/app/build.gradle" }}-{{ checksum "packages/mobile/.env.test" }} diff --git a/packages/mobile/package.json b/packages/mobile/package.json index b0b1c84c73d..330729c0b0c 100644 --- a/packages/mobile/package.json +++ b/packages/mobile/package.json @@ -14,7 +14,7 @@ "build:metro": "echo 'NOT WORKING RIGHT NOW'", "build:gen-graphql-types": "gql-gen --schema http://localhost:8080/graphql --template graphql-codegen-typescript-template --out ./typings/ 'src/**/*.tsx'", "predev": "./scripts/pre-dev.sh", - "dev": "react-native run-android --appIdSuffix \"debug\" --no-packager && react-native start", + "dev": "react-native run-android --appIdSuffix \"debug\" --no-packager && yarn start || echo 'Could not start metro server'", "dev:ios": "react-native run-ios --simulator \"iPhone 11\"", "dev:show-menu": "adb devices | grep '\t' | awk '{print $1}' | sed 's/\\s//g' | xargs -I {} adb -s {} shell input keyevent 82", "dev:clear-data": "adb shell pm clear org.celo.mobile.debug", diff --git a/packages/mobile/scripts/ci-e2e.sh b/packages/mobile/scripts/ci-e2e.sh deleted file mode 100755 index 91334c6f4aa..00000000000 --- a/packages/mobile/scripts/ci-e2e.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env bash -export ANDROID_HOME=/usr/local/share/android-sdk -export PATH=$PATH:/usr/local/bin:/usr/sbin:/sbin:~/.nvm/versions/node/v8.12.0/bin -#Example crontab: -#*/10 * * * * cd ~/celo-monorepo/packages/mobile && scripts/ci-e2e.sh >/dev/null 2>&1 - -git pull -mkdir -p e2e/tmp -rm -r e2e/tmp/* - -( cd ../../ && yarn ) -( cd android && ./gradlew clean ) -yarn test:build-e2e && echo "Build successfull" >> e2e/tmp/last_run_log || yarn test:build-e2e -yarn test:run-e2e || rm -r e2e/tmp/* && test:yarn run-e2e >> e2e/tmp/last_run_log 2>&1 -passed=$? - -if [ $passed -eq 0 ] -then - gsutil -h "Cache-Control:private,max-age=0, no-transform" cp e2e/e2e-passing-green.svg gs://celo-e2e-data/e2e-banner.svg - gsutil acl ch -u AllUsers:R gs://celo-e2e-data/e2e-banner.svg - echo "Tests passing" >> e2e/tmp/last_run_log -else - gsutil -h "Cache-Control:private,max-age=0, no-transform" cp e2e/e2e-failing-red.svg gs://celo-e2e-data/e2e-banner.svg - gsutil acl ch -u AllUsers:R gs://celo-e2e-data/e2e-banner.svg - echo "Tests failling" >> e2e/tmp/last_run_log -fi - -tar -czvf e2e/tmp/detailed_logs.tar.gz e2e/tmp -gsutil cp e2e/tmp/detailed_logs.tar.gz gs://celo-e2e-data/detailed_logs.tar.gz -gsutil cp e2e/tmp/last_run_log gs://celo-e2e-data/last_run_log - -exit $passed diff --git a/packages/mobile/scripts/unlock.sh b/packages/mobile/scripts/unlock.sh index bf2d084296c..bdd4d451c03 100755 --- a/packages/mobile/scripts/unlock.sh +++ b/packages/mobile/scripts/unlock.sh @@ -19,7 +19,8 @@ adb wait-for-device shell \ 'while [[ -z $(getprop sys.boot_completed) ]]; do sleep 1; done;' -echo "locksettings set-pin 123456" | adb shell || true +echo "locksettings set-pin $SECRET_PIN" | adb shell || echo "Failed to change pin, probably already set" + sleep 1 echo "Device is done booting" diff --git a/scripts/run_e2e.sh b/scripts/run_e2e.sh deleted file mode 100755 index 31c6d838c7d..00000000000 --- a/scripts/run_e2e.sh +++ /dev/null @@ -1,6 +0,0 @@ -yarn -yarn build -cd packages/mobile/ -yarn test:build-e2e - -yarn test:run-e2e 2>&1 | tee e2e_run.log From bf7a56ce8c2b5b530bf5ae366b45d80b80264454 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Sat, 2 Nov 2019 10:43:34 -0700 Subject: [PATCH 05/71] Re-work locked gold requirements for validators and groups (#1474) --- .../src/e2e-tests/validator_order_tests.ts | 2 +- .../contractkit/src/wrappers/Validators.ts | 47 +- .../contracts/common/UsingPrecompiles.sol | 22 + .../contracts/common/UsingRegistry.sol | 5 + .../contracts/governance/Election.sol | 56 +- .../contracts/governance/Governance.sol | 20 +- .../contracts/governance/LockedGold.sol | 5 +- .../contracts/governance/Validators.sol | 409 +++--- .../governance/interfaces/IGovernance.sol | 63 +- .../governance/interfaces/IValidators.sol | 3 +- .../governance/test/MockGovernance.sol | 3 +- .../governance/test/MockValidators.sol | 19 +- packages/protocol/migrations/12_validators.ts | 8 +- .../migrations/19_elect_validators.ts | 27 +- packages/protocol/migrationsConfig.js | 12 +- packages/protocol/test/governance/election.ts | 34 +- .../protocol/test/governance/governance.ts | 2 - .../protocol/test/governance/lockedgold.ts | 82 +- .../protocol/test/governance/validators.ts | 1167 +++++++++++------ 19 files changed, 1169 insertions(+), 817 deletions(-) diff --git a/packages/celotool/src/e2e-tests/validator_order_tests.ts b/packages/celotool/src/e2e-tests/validator_order_tests.ts index 8360be0101b..2e71aafc78f 100644 --- a/packages/celotool/src/e2e-tests/validator_order_tests.ts +++ b/packages/celotool/src/e2e-tests/validator_order_tests.ts @@ -10,7 +10,7 @@ const BLOCK_COUNT = EPOCH * EPOCHS_TO_WAIT describe('governance tests', () => { const gethConfig: GethTestConfig = { - migrateTo: 13, + migrateTo: 14, instances: _.range(VALIDATORS).map((i) => ({ name: `validator${i}`, validating: true, diff --git a/packages/contractkit/src/wrappers/Validators.ts b/packages/contractkit/src/wrappers/Validators.ts index 8bca3f35082..d2f3c303b97 100644 --- a/packages/contractkit/src/wrappers/Validators.ts +++ b/packages/contractkit/src/wrappers/Validators.ts @@ -24,19 +24,14 @@ export interface ValidatorGroup { commission: BigNumber } -export interface BalanceRequirements { - group: BigNumber - validator: BigNumber -} - -export interface DeregistrationLockups { - group: BigNumber - validator: BigNumber +export interface LockedGoldRequirements { + value: BigNumber + duration: BigNumber } export interface ValidatorsConfig { - balanceRequirements: BalanceRequirements - deregistrationLockups: DeregistrationLockups + groupLockedGoldRequirements: LockedGoldRequirements + validatorLockedGoldRequirements: LockedGoldRequirements maxGroupSize: BigNumber } @@ -72,26 +67,26 @@ export class ValidatorsWrapper extends BaseWrapper { } } /** - * Returns the current registration requirements. - * @returns Group and validator registration requirements. + * Returns the Locked Gold requirements for validators. + * @returns The Locked Gold requirements for validators. */ - async getBalanceRequirements(): Promise { - const res = await this.contract.methods.getBalanceRequirements().call() + async getValidatorLockedGoldRequirements(): Promise { + const res = await this.contract.methods.getValidatorLockedGoldRequirements().call() return { - group: toBigNumber(res[0]), - validator: toBigNumber(res[1]), + value: toBigNumber(res[0]), + duration: toBigNumber(res[1]), } } /** - * Returns the lockup periods after deregistering groups and validators. - * @return The lockup periods after deregistering groups and validators. + * Returns the Locked Gold requirements for validator groups. + * @returns The Locked Gold requirements for validator groups. */ - async getDeregistrationLockups(): Promise { - const res = await this.contract.methods.getDeregistrationLockups().call() + async getGroupLockedGoldRequirements(): Promise { + const res = await this.contract.methods.getGroupLockedGoldRequirements().call() return { - group: toBigNumber(res[0]), - validator: toBigNumber(res[1]), + value: toBigNumber(res[0]), + duration: toBigNumber(res[1]), } } @@ -100,13 +95,13 @@ export class ValidatorsWrapper extends BaseWrapper { */ async getConfig(): Promise { const res = await Promise.all([ - this.getBalanceRequirements(), - this.getDeregistrationLockups(), + this.getValidatorLockedGoldRequirements(), + this.getGroupLockedGoldRequirements(), this.contract.methods.maxGroupSize().call(), ]) return { - balanceRequirements: res[0], - deregistrationLockups: res[1], + validatorLockedGoldRequirements: res[0], + groupLockedGoldRequirements: res[1], maxGroupSize: toBigNumber(res[2]), } } diff --git a/packages/protocol/contracts/common/UsingPrecompiles.sol b/packages/protocol/contracts/common/UsingPrecompiles.sol index b8108432c88..1bdb26152c8 100644 --- a/packages/protocol/contracts/common/UsingPrecompiles.sol +++ b/packages/protocol/contracts/common/UsingPrecompiles.sol @@ -1,6 +1,8 @@ pragma solidity ^0.5.3; +// TODO(asa): Limit assembly usage by using X.staticcall instead. contract UsingPrecompiles { + address constant PROOF_OF_POSSESSION = address(0xff - 4); /** * @notice calculate a * b^x for fractions a, b to `decimals` precision @@ -122,4 +124,24 @@ contract UsingPrecompiles { return numberValidators; } + + /** + * @notice Checks a BLS proof of possession. + * @param proofOfPossessionBytes The public key and signature of the proof of possession. + * @return True upon success. + */ + function checkProofOfPossession( + address sender, + bytes memory proofOfPossessionBytes + ) + public + returns (bool) + { + bool success; + (success, ) = PROOF_OF_POSSESSION + .call + .value(0) + .gas(gasleft())(abi.encodePacked(sender, proofOfPossessionBytes)); + return success; + } } diff --git a/packages/protocol/contracts/common/UsingRegistry.sol b/packages/protocol/contracts/common/UsingRegistry.sol index 3fa4d723c5f..84a672600a2 100644 --- a/packages/protocol/contracts/common/UsingRegistry.sol +++ b/packages/protocol/contracts/common/UsingRegistry.sol @@ -8,6 +8,7 @@ import "./interfaces/IAccounts.sol"; import "../governance/interfaces/IElection.sol"; +import "../governance/interfaces/IGovernance.sol"; import "../governance/interfaces/ILockedGold.sol"; import "../governance/interfaces/IValidators.sol"; @@ -70,6 +71,10 @@ contract UsingRegistry is Ownable { return IERC20Token(registry.getAddressForOrDie(GOLD_TOKEN_REGISTRY_ID)); } + function getGovernance() internal view returns (IGovernance) { + return IGovernance(registry.getAddressForOrDie(GOVERNANCE_REGISTRY_ID)); + } + function getLockedGold() internal view returns (ILockedGold) { return ILockedGold(registry.getAddressForOrDie(LOCKED_GOLD_REGISTRY_ID)); } diff --git a/packages/protocol/contracts/governance/Election.sol b/packages/protocol/contracts/governance/Election.sol index cf210d5c6fb..46152cb2cb2 100644 --- a/packages/protocol/contracts/governance/Election.sol +++ b/packages/protocol/contracts/governance/Election.sol @@ -88,7 +88,6 @@ contract Election is uint256 public maxNumGroupsVotedFor; // Groups must receive at least this fraction of the total votes in order to be considered in // elections. - // TODO(asa): Implement this constraint. FixidityLib.Fraction public electabilityThreshold; event ElectableValidatorsSet( @@ -151,9 +150,9 @@ contract Election is { _transferOwnership(msg.sender); setRegistry(registryAddress); - _setElectableValidators(minElectableValidators, maxElectableValidators); - _setMaxNumGroupsVotedFor(_maxNumGroupsVotedFor); - _setElectabilityThreshold(_electabilityThreshold); + setElectableValidators(minElectableValidators, maxElectableValidators); + setMaxNumGroupsVotedFor(_maxNumGroupsVotedFor); + setElectabilityThreshold(_electabilityThreshold); } /** @@ -162,8 +161,12 @@ contract Election is * @param max The maximum number of validators that can be elected. * @return True upon success. */ - function setElectableValidators(uint256 min, uint256 max) external onlyOwner returns (bool) { - return _setElectableValidators(min, max); + function setElectableValidators(uint256 min, uint256 max) public onlyOwner returns (bool) { + require(0 < min && min <= max); + require(min != electableValidators.min || max != electableValidators.max); + electableValidators = ElectableValidators(min, max); + emit ElectableValidatorsSet(min, max); + return true; } /** @@ -174,20 +177,6 @@ contract Election is return (electableValidators.min, electableValidators.max); } - /** - * @notice Updates the minimum and maximum number of validators that can be elected. - * @param min The minimum number of validators that can be elected. - * @param max The maximum number of validators that can be elected. - * @return True upon success. - */ - function _setElectableValidators(uint256 min, uint256 max) private returns (bool) { - require(0 < min && min <= max); - require(min != electableValidators.min || max != electableValidators.max); - electableValidators = ElectableValidators(min, max); - emit ElectableValidatorsSet(min, max); - return true; - } - /** * @notice Updates the maximum number of groups an account can be voting for at once. * @param _maxNumGroupsVotedFor The maximum number of groups an account can vote for. @@ -196,19 +185,10 @@ contract Election is function setMaxNumGroupsVotedFor( uint256 _maxNumGroupsVotedFor ) - external + public onlyOwner returns (bool) { - return _setMaxNumGroupsVotedFor(_maxNumGroupsVotedFor); - } - - /** - * @notice Updates the maximum number of groups an account can be voting for at once. - * @param _maxNumGroupsVotedFor The maximum number of groups an account can vote for. - * @return True upon success. - */ - function _setMaxNumGroupsVotedFor(uint256 _maxNumGroupsVotedFor) private returns (bool) { require(_maxNumGroupsVotedFor != maxNumGroupsVotedFor); maxNumGroupsVotedFor = _maxNumGroupsVotedFor; emit MaxNumGroupsVotedForSet(_maxNumGroupsVotedFor); @@ -221,15 +201,6 @@ contract Election is * @return True upon success. */ function setElectabilityThreshold(uint256 threshold) public onlyOwner returns (bool) { - return _setElectabilityThreshold(threshold); - } - - /** - * @notice Sets the electability threshold. - * @param threshold Electability threshold as unwrapped Fraction. - * @return True upon success. - */ - function _setElectabilityThreshold(uint256 threshold) private returns (bool) { electabilityThreshold = FixidityLib.wrap(threshold); require( electabilityThreshold.lt(FixidityLib.fixed1()), @@ -486,12 +457,7 @@ contract Election is { // The group must meet the balance requirements in order for their voters to receive epoch // rewards. - bool meetsBalanceRequirements = ( - getLockedGold().getAccountTotalLockedGold(group) >= - getValidators().getAccountBalanceRequirement(group) - ); - - if (meetsBalanceRequirements && votes.active.total > 0) { + if (getValidators().meetsAccountLockedGoldRequirements(group) && votes.active.total > 0) { return totalEpochRewards.mul(votes.active.forGroup[group].total).div(votes.active.total); } else { return 0; diff --git a/packages/protocol/contracts/governance/Governance.sol b/packages/protocol/contracts/governance/Governance.sol index 67aa4e78d34..8954f8f9007 100644 --- a/packages/protocol/contracts/governance/Governance.sol +++ b/packages/protocol/contracts/governance/Governance.sol @@ -18,7 +18,7 @@ import "../common/UsingPrecompiles.sol"; /** * @title A contract for making, passing, and executing on-chain governance proposals. */ -contract Governance is +contract Governance is IGovernance, Ownable, Initializable, @@ -758,7 +758,7 @@ contract Governance is msg.sender, 0 ).executeMem(); - + hotfixes[hash].executed = true; emit HotfixExecuted(hash); } @@ -775,6 +775,22 @@ contract Governance is return true; } + /** + * @notice Returns whether or not a particular account is voting on proposals. + * @param account The address of the account. + * @return Whether or not the account is voting on proposals. + */ + function isVoting(address account) external view returns (bool) { + Voter storage voter = voters[account]; + uint256 upvotedProposal = voter.upvote.proposalId; + bool isVotingQueue = upvotedProposal != 0 && isQueued(upvotedProposal); + Proposals.Proposal storage proposal = proposals[voter.mostRecentReferendumProposal]; + bool isVotingReferendum = ( + proposal.getDequeuedStage(stageDurations) == Proposals.Stage.Referendum + ); + return isVotingQueue || isVotingReferendum; + } + /** * @notice Returns the number of seconds proposals stay in approval stage. * @return The number of seconds proposals stay in approval stage. diff --git a/packages/protocol/contracts/governance/LockedGold.sol b/packages/protocol/contracts/governance/LockedGold.sol index 7d4ddffca86..bfbb9540e76 100644 --- a/packages/protocol/contracts/governance/LockedGold.sol +++ b/packages/protocol/contracts/governance/LockedGold.sol @@ -148,7 +148,10 @@ contract LockedGold is ILockedGold, ReentrancyGuard, Initializable, UsingRegistr function unlock(uint256 value) external nonReentrant { require(getAccounts().isAccount(msg.sender)); Account storage account = accounts[msg.sender]; - uint256 balanceRequirement = getValidators().getAccountBalanceRequirement(msg.sender); + // Prevent unlocking gold when voting on governance proposals so that the gold cannot be + // used to vote more than once. + require(!getGovernance().isVoting(msg.sender)); + uint256 balanceRequirement = getValidators().getAccountLockedGoldRequirement(msg.sender); require( balanceRequirement == 0 || balanceRequirement <= getAccountTotalLockedGold(msg.sender).sub(value) diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index fee45a56758..d6eb07fb6ce 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -28,41 +28,38 @@ contract Validators is using SafeMath for uint256; using BytesLib for bytes; - address constant PROOF_OF_POSSESSION = address(0xff - 4); - uint256 constant MAX_INT = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - - // If an account has not registered a validator or group, these values represent the minimum - // amount of Locked Gold required to do so. - // If an account has a registered a validator or validator group, these values represent the - // minimum amount of Locked Gold required in order to earn epoch rewards. Furthermore, the - // account will not be able to unlock Gold if it would cause the account to fall below - // these values. - // If an account has deregistered a validator or validator group and is still subject to the - // `DeregistrationLockup`, the account will not be able to unlock Gold if it would cause the - // account to fall below these values. - struct BalanceRequirements { - uint256 group; - uint256 validator; - } - - // After deregistering a validator or validator group, the account will remain subject to the - // current balance requirements for this long (in seconds). - struct DeregistrationLockups { - uint256 group; - uint256 validator; - } - - // Stores the timestamps at which deregistration of a validator or validator group occurred. - struct DeregistrationTimestamps { - uint256 group; - uint256 validator; - } + // For Validators, these requirements must be met in order to: + // 1. Register a validator + // 2. Affiliate with and be added to a group + // 3. Receive epoch payments (note that the group must meet the group requirements as well) + // Accounts may de-register their Validator `duration` seconds after they were last a member of a + // group, after which no restrictions on Locked Gold will apply to the account. + // + // For Validator Groups, these requirements must be met in order to: + // 1. Register a group + // 2. Add a member to a group + // 3. Receive epoch payments + // Note that for groups, the requirement value is multiplied by the number of members, and is + // enforced for `duration` seconds after the group last had that number of members. + // Accounts may de-register their Group `duration` seconds after they were last non-empty, after + // which no restrictions on Locked Gold will apply to the account. + struct LockedGoldRequirements { + uint256 value; + // In seconds. + uint256 duration; + } + + // If we knew what time the validator was last in a group, we could enforce that to deregister a + // group, you need to have had 0 members for `duration`, and to deregister a validator, you need + // to have been out of a group for `duration`... struct ValidatorGroup { bool exists; LinkedList.List members; // TODO(asa): Add a function that allows groups to update their commission. FixidityLib.Fraction commission; + // sizeHistory[i] contains the last time the group contained i members. + uint256[] sizeHistory; } // Stores the epoch number at which a validator joined a particular group. @@ -71,13 +68,17 @@ contract Validators is address group; } - // Stores the membership history of a validator. + // Stores the per-epoch membership history of a validator, used to determine which group + // commission should be paid to at the end of an epoch. + // Stores a timestamp of the last time the validator was removed from a group, used to determine + // whether or not a group can de-register. struct MembershipHistory { // The key to the most recent entry in the entries mapping. uint256 tail; // The number of entries in this validators membership history. uint256 numEntries; mapping(uint256 => MembershipHistoryEntry) entries; + uint256 lastRemovedFromGroupTimestamp; } struct Validator { @@ -95,83 +96,30 @@ contract Validators is mapping(address => ValidatorGroup) private groups; mapping(address => Validator) private validators; - mapping(address => DeregistrationTimestamps) private deregistrationTimestamps; - address[] private _groups; - address[] private _validators; - BalanceRequirements public balanceRequirements; - DeregistrationLockups public deregistrationLockups; + address[] private registeredGroups; + address[] private registeredValidators; + LockedGoldRequirements public validatorLockedGoldRequirements; + LockedGoldRequirements public groupLockedGoldRequirements; ValidatorScoreParameters private validatorScoreParameters; uint256 public validatorEpochPayment; uint256 public membershipHistoryLength; uint256 public maxGroupSize; - event MaxGroupSizeSet( - uint256 size - ); - - event ValidatorEpochPaymentSet( - uint256 value - ); - - event ValidatorScoreParametersSet( - uint256 exponent, - uint256 adjustmentSpeed - ); - - event BalanceRequirementsSet( - uint256 group, - uint256 validator - ); - + event MaxGroupSizeSet(uint256 size); + event ValidatorEpochPaymentSet(uint256 value); + event ValidatorScoreParametersSet(uint256 exponent, uint256 adjustmentSpeed); + event GroupLockedGoldRequirementsSet(uint256 value, uint256 duration); + event ValidatorLockedGoldRequirementsSet(uint256 value, uint256 duration); event MembershipHistoryLengthSet(uint256 length); - - event DeregistrationLockupsSet( - uint256 group, - uint256 validator - ); - - event ValidatorRegistered( - address indexed validator, - bytes publicKeysData - ); - - event ValidatorDeregistered( - address indexed validator - ); - - event ValidatorAffiliated( - address indexed validator, - address indexed group - ); - - event ValidatorDeaffiliated( - address indexed validator, - address indexed group - ); - - event ValidatorGroupRegistered( - address indexed group, - uint256 commission - ); - - event ValidatorGroupDeregistered( - address indexed group - ); - - event ValidatorGroupMemberAdded( - address indexed group, - address indexed validator - ); - - event ValidatorGroupMemberRemoved( - address indexed group, - address indexed validator - ); - - event ValidatorGroupMemberReordered( - address indexed group, - address indexed validator - ); + event ValidatorRegistered(address indexed validator, bytes publicKeysData); + event ValidatorDeregistered(address indexed validator); + event ValidatorAffiliated(address indexed validator, address indexed group); + event ValidatorDeaffiliated(address indexed validator, address indexed group); + event ValidatorGroupRegistered(address indexed group, uint256 commission); + event ValidatorGroupDeregistered(address indexed group); + event ValidatorGroupMemberAdded(address indexed group, address indexed validator); + event ValidatorGroupMemberRemoved(address indexed group, address indexed validator); + event ValidatorGroupMemberReordered(address indexed group, address indexed validator); modifier onlyVm() { require(msg.sender == address(0)); @@ -181,10 +129,10 @@ contract Validators is /** * @notice Initializes critical variables. * @param registryAddress The address of the registry contract. - * @param groupRequirement The minimum locked gold needed to register a group. - * @param validatorRequirement The minimum locked gold needed to register a validator. - * @param groupLockup The duration the above gold remains locked after deregistration. - * @param validatorLockup The duration the above gold remains locked after deregistration. + * @param groupRequirementValue The Locked Gold requirement amount for groups. + * @param groupRequirementDuration The Locked Gold requirement duration for groups. + * @param validatorRequirementValue The Locked Gold requirement amount for validators. + * @param validatorRequirementDuration The Locked Gold requirement duration for validators. * @param validatorScoreExponent The exponent used in calculating validator scores. * @param validatorScoreAdjustmentSpeed The speed at which validator scores are adjusted. * @param _validatorEpochPayment The duration the above gold remains locked after deregistration. @@ -194,10 +142,10 @@ contract Validators is */ function initialize( address registryAddress, - uint256 groupRequirement, - uint256 validatorRequirement, - uint256 groupLockup, - uint256 validatorLockup, + uint256 groupRequirementValue, + uint256 groupRequirementDuration, + uint256 validatorRequirementValue, + uint256 validatorRequirementDuration, uint256 validatorScoreExponent, uint256 validatorScoreAdjustmentSpeed, uint256 _validatorEpochPayment, @@ -209,11 +157,11 @@ contract Validators is { _transferOwnership(msg.sender); setRegistry(registryAddress); - setValidatorEpochPayment(_validatorEpochPayment); + setGroupLockedGoldRequirements(groupRequirementValue, groupRequirementDuration); + setValidatorLockedGoldRequirements(validatorRequirementValue, validatorRequirementDuration); setValidatorScoreParameters(validatorScoreExponent, validatorScoreAdjustmentSpeed); - setBalanceRequirements(groupRequirement, validatorRequirement); - setDeregistrationLockups(groupLockup, validatorLockup); setMaxGroupSize(_maxGroupSize); + setValidatorEpochPayment(_validatorEpochPayment); setMembershipHistoryLength(_membershipHistoryLength); } @@ -289,42 +237,44 @@ contract Validators is } /** - * @notice Updates the minimum gold requirements to register a group/validator and earn rewards. - * @param group The minimum locked gold needed to register a group and earn rewards. - * @param validator The minimum locked gold needed to register a validator and earn rewards. + * @notice Updates the Locked Gold requirements for Validator Groups. + * @param value The per-member amount of Locked Gold required. + * @param duration The time (in seconds) that these requirements persist for. * @return True upon success. */ - function setBalanceRequirements( - uint256 group, - uint256 validator + function setGroupLockedGoldRequirements( + uint256 value, + uint256 duration ) public onlyOwner returns (bool) { - require(group != balanceRequirements.group || validator != balanceRequirements.validator); - balanceRequirements = BalanceRequirements(group, validator); - emit BalanceRequirementsSet(group, validator); + LockedGoldRequirements storage requirements = groupLockedGoldRequirements; + require(value != requirements.value || duration != requirements.duration); + groupLockedGoldRequirements = LockedGoldRequirements(value, duration); + emit GroupLockedGoldRequirementsSet(value, duration); return true; } /** - * @notice Updates the duration for which gold remains locked after deregistration. - * @param group The lockup duration for groups in seconds. - * @param validator The lockup duration for validators in seconds. + * @notice Updates the Locked Gold requirements for Validators. + * @param value The amount of Locked Gold required. + * @param duration The time (in seconds) that these requirements persist for. * @return True upon success. */ - function setDeregistrationLockups( - uint256 group, - uint256 validator + function setValidatorLockedGoldRequirements( + uint256 value, + uint256 duration ) public onlyOwner returns (bool) { - require(group != deregistrationLockups.group || validator != deregistrationLockups.validator); - deregistrationLockups = DeregistrationLockups(group, validator); - emit DeregistrationLockupsSet(group, validator); + LockedGoldRequirements storage requirements = validatorLockedGoldRequirements; + require(value != requirements.value || duration != requirements.duration); + validatorLockedGoldRequirements = LockedGoldRequirements(value, duration); + emit ValidatorLockedGoldRequirementsSet(value, duration); return true; } @@ -338,7 +288,7 @@ contract Validators is * - blsPoP - The BLS public key proof of possession. 96 bytes. * @return True upon success. * @dev Fails if the account is already a validator or validator group. - * @dev Fails if the account does not have sufficient weight. + * @dev Fails if the account does not have sufficient Locked Gold. */ function registerValidator( bytes calldata publicKeysData @@ -356,50 +306,16 @@ contract Validators is address account = getAccounts().activeValidationSignerToAccount(msg.sender); require(!isValidator(account) && !isValidatorGroup(account)); - require(meetsValidatorBalanceRequirements(account)); - - validators[account].publicKeysData = publicKeysData; - _validators.push(account); + uint256 lockedGoldBalance = getLockedGold().getAccountTotalLockedGold(account); + require(lockedGoldBalance >= validatorLockedGoldRequirements.value); + Validator storage validator = validators[account]; + validator.publicKeysData = publicKeysData; + registeredValidators.push(account); updateMembershipHistory(account, address(0)); emit ValidatorRegistered(account, publicKeysData); return true; } - /** - * @notice Checks a BLS proof of possession. - * @param proofOfPossessionBytes The public key and signature of the proof of possession. - * @return True upon success. - */ - function checkProofOfPossession( - address sender, - bytes memory proofOfPossessionBytes - ) private returns (bool) { - bool success; - (success, ) = PROOF_OF_POSSESSION - .call - .value(0) - .gas(gasleft())(abi.encodePacked(sender, proofOfPossessionBytes)); - return success; - } - - /** - * @notice Returns whether an account meets the requirements to register a validator. - * @param account The account. - * @return Whether an account meets the requirements to register a validator. - */ - function meetsValidatorBalanceRequirements(address account) public view returns (bool) { - return getLockedGold().getAccountTotalLockedGold(account) >= balanceRequirements.validator; - } - - /** - * @notice Returns whether an account meets the requirements to register a group. - * @param account The account. - * @return Whether an account meets the requirements to register a group. - */ - function meetsValidatorGroupBalanceRequirements(address account) public view returns (bool) { - return getLockedGold().getAccountTotalLockedGold(account) >= balanceRequirements.group; - } - /** * @notice Returns the parameters that goven how a validator's score is calculated. * @return The parameters that goven how a validator's score is calculated. @@ -418,7 +334,7 @@ contract Validators is ) external view - returns (uint256[] memory, address[] memory) + returns (uint256[] memory, address[] memory, uint256) { MembershipHistory storage history = validators[account].membershipHistory; uint256[] memory epochs = new uint256[](history.numEntries); @@ -428,7 +344,7 @@ contract Validators is epochs[i] = history.entries[index].epochNumber; membershipGroups[i] = history.entries[index].group; } - return (epochs, membershipGroups); + return (epochs, membershipGroups, history.lastRemovedFromGroupTimestamp); } /** @@ -450,8 +366,8 @@ contract Validators is */ function _updateValidatorScore(address validator, uint256 uptime) internal { address account = getAccounts().validationSignerToAccount(validator); - require(isValidator(account), "isvalidator"); - require(uptime <= FixidityLib.fixed1().unwrap(), "uptime"); + require(isValidator(account)); + require(uptime <= FixidityLib.fixed1().unwrap()); uint256 numerator; uint256 denominator; @@ -501,11 +417,7 @@ contract Validators is address group = getMembershipInLastEpoch(account); // Both the validator and the group must maintain the minimum locked gold balance in order to // receive epoch payments. - bool meetsBalanceRequirements = ( - getLockedGold().getAccountTotalLockedGold(group) >= getAccountBalanceRequirement(group) && - getLockedGold().getAccountTotalLockedGold(account) >= getAccountBalanceRequirement(account) - ); - if (meetsBalanceRequirements) { + if (meetsAccountLockedGoldRequirements(account) && meetsAccountLockedGoldRequirements(group)) { FixidityLib.Fraction memory totalPayment = FixidityLib.newFixed( validatorEpochPayment ).multiply(validators[account].score); @@ -517,21 +429,29 @@ contract Validators is } /** - * @notice De-registers a validator, removing it from the group for which it is a member. - * @param index The index of this validator in the list of all validators. + * @notice De-registers a validator. + * @param index The index of this validator in the list of all registered validators. * @return True upon success. * @dev Fails if the account is not a validator. */ function deregisterValidator(uint256 index) external nonReentrant returns (bool) { address account = getAccounts().activeValidationSignerToAccount(msg.sender); require(isValidator(account)); + + // Require that the validator has not been a member of a validator group for + // `validatorLockedGoldRequirements.duration` seconds. Validator storage validator = validators[account]; if (validator.affiliation != address(0)) { - _deaffiliate(validator, account); + require(!groups[validator.affiliation].members.contains(account)); } + uint256 requirementEndTime = validator.membershipHistory.lastRemovedFromGroupTimestamp.add( + validatorLockedGoldRequirements.duration + ); + require(requirementEndTime < now); + + // Remove the validator. + deleteElement(registeredValidators, account, index); delete validators[account]; - deleteElement(_validators, account, index); - deregistrationTimestamps[account].validator = now; emit ValidatorDeregistered(account); return true; } @@ -545,6 +465,8 @@ contract Validators is function affiliate(address group) external nonReentrant returns (bool) { address account = getAccounts().activeValidationSignerToAccount(msg.sender); require(isValidator(account) && isValidatorGroup(group)); + require(meetsAccountLockedGoldRequirements(account)); + require(meetsAccountLockedGoldRequirements(group)); Validator storage validator = validators[account]; if (validator.affiliation != address(0)) { _deaffiliate(validator, account); @@ -586,12 +508,12 @@ contract Validators is require(commission <= FixidityLib.fixed1().unwrap(), "Commission can't be greater than 100%"); address account = getAccounts().activeValidationSignerToAccount(msg.sender); require(!isValidator(account) && !isValidatorGroup(account)); - require(meetsValidatorGroupBalanceRequirements(account)); - + uint256 lockedGoldBalance = getLockedGold().getAccountTotalLockedGold(account); + require(lockedGoldBalance >= groupLockedGoldRequirements.value); ValidatorGroup storage group = groups[account]; group.exists = true; group.commission = FixidityLib.wrap(commission); - _groups.push(account); + registeredGroups.push(account); emit ValidatorGroupRegistered(account, commission); return true; } @@ -604,11 +526,15 @@ contract Validators is */ function deregisterValidatorGroup(uint256 index) external nonReentrant returns (bool) { address account = getAccounts().activeValidationSignerToAccount(msg.sender); - // Only empty Validator Groups can be deregistered. + // Only Validator Groups that have never had members or have been empty for at least + // `groupLockedGoldRequirements.duration` seconds can be deregistered. require(isValidatorGroup(account) && groups[account].members.numElements == 0); + uint256[] storage sizeHistory = groups[account].sizeHistory; + if (sizeHistory.length > 1) { + require(sizeHistory[1].add(groupLockedGoldRequirements.duration) < now); + } delete groups[account]; - deleteElement(_groups, account, index); - deregistrationTimestamps[account].group = now; + deleteElement(registeredGroups, account, index); emit ValidatorGroupDeregistered(account); return true; } @@ -657,6 +583,7 @@ contract Validators is * @param greater The address of the group that has received more votes than this group. * @return True upon success. * @dev Fails if `validator` has not set their affiliation to this account. + * @dev Fails if the group has > 0 members. */ function _addMember( address group, @@ -671,11 +598,15 @@ contract Validators is ValidatorGroup storage _group = groups[group]; require(_group.members.numElements < maxGroupSize, "group would exceed maximum size"); require(validators[validator].affiliation == group && !_group.members.contains(validator)); + uint256 numMembers = _group.members.numElements.add(1); + require(meetsAccountLockedGoldRequirements(group)); + require(meetsAccountLockedGoldRequirements(validator)); _group.members.push(validator); - if (_group.members.numElements == 1) { + if (numMembers == 1) { getElection().markGroupEligible(group, lesser, greater); } updateMembershipHistory(validator, group); + updateSizeHistory(group, numMembers.sub(1)); emit ValidatorGroupMemberAdded(group, validator); return true; } @@ -725,30 +656,33 @@ contract Validators is * @param account The account that may have to meet locked gold balance requirements. * @return The locked gold balance requirement for the supplied account. */ - function getAccountBalanceRequirement(address account) public view returns (uint256) { - DeregistrationTimestamps storage timestamps = deregistrationTimestamps[account]; - if ( - isValidator(account) || - (timestamps.validator > 0 && now < timestamps.validator.add(deregistrationLockups.validator)) - ) { - return balanceRequirements.validator; - } - if ( - isValidatorGroup(account) || - (timestamps.group > 0 && now < timestamps.group.add(deregistrationLockups.group)) - ) { - return balanceRequirements.group; + function getAccountLockedGoldRequirement(address account) public view returns (uint256) { + if (isValidator(account)) { + return validatorLockedGoldRequirements.value; + } else if (isValidatorGroup(account)) { + uint256 multiplier = Math.max(1, groups[account].members.numElements); + uint256[] storage sizeHistory = groups[account].sizeHistory; + if (sizeHistory.length > 0) { + for (uint256 i = sizeHistory.length.sub(1); i > 0; i = i.sub(1)) { + if (sizeHistory[i].add(groupLockedGoldRequirements.duration) >= now) { + multiplier = Math.max(i, multiplier); + break; + } + } + } + return groupLockedGoldRequirements.value.mul(multiplier); } return 0; } /** - * @notice Returns the timestamp of the last time this account deregistered a validator or group. - * @param account The account to query. - * @return The timestamp of the last time this account deregistered a validator or group. + * @notice Returns whether or not an account meets its Locked Gold requirements. + * @param account The address of the account. + * @return Whether or not an account meets its Locked Gold requirements. */ - function getDeregistrationTimestamps(address account) external view returns (uint256, uint256) { - return (deregistrationTimestamps[account].group, deregistrationTimestamps[account].validator); + function meetsAccountLockedGoldRequirements(address account) public view returns (bool) { + uint256 balance = getLockedGold().getAccountTotalLockedGold(account); + return balance >= getAccountLockedGoldRequirement(account); } /** @@ -786,11 +720,15 @@ contract Validators is ) external view - returns (address[] memory, uint256) + returns (address[] memory, uint256, uint256[] memory) { require(isValidatorGroup(account)); ValidatorGroup storage group = groups[account]; - return ( group.members.getKeys(), group.commission.unwrap()); + return ( + group.members.getKeys(), + group.commission.unwrap(), + group.sizeHistory + ); } /** @@ -849,23 +787,23 @@ contract Validators is * @return The number of registered validators. */ function getNumRegisteredValidators() external view returns (uint256) { - return _validators.length; + return registeredValidators.length; } /** - * @notice Returns the Locked Gold requirements to register a validator or group. - * @return The locked gold requirements to register a validator or group. + * @notice Returns the Locked Gold requirements for validators. + * @return The Locked Gold requirements for validators. */ - function getBalanceRequirements() external view returns (uint256, uint256) { - return (balanceRequirements.group, balanceRequirements.validator); + function getValidatorLockedGoldRequirements() external view returns (uint256, uint256) { + return (validatorLockedGoldRequirements.value, validatorLockedGoldRequirements.duration); } /** - * @notice Returns the lockup periods after deregistering groups and validators. - * @return The lockup periods after deregistering groups and validators. + * @notice Returns the Locked Gold requirements for validator groups. + * @return The Locked Gold requirements for validator groups. */ - function getDeregistrationLockups() external view returns (uint256, uint256) { - return (deregistrationLockups.group, deregistrationLockups.validator); + function getGroupLockedGoldRequirements() external view returns (uint256, uint256) { + return (groupLockedGoldRequirements.value, groupLockedGoldRequirements.duration); } /** @@ -873,7 +811,7 @@ contract Validators is * @return The list of registered validator accounts. */ function getRegisteredValidators() external view returns (address[] memory) { - return _validators; + return registeredValidators; } /** @@ -881,7 +819,7 @@ contract Validators is * @return The list of registered validator group addresses. */ function getRegisteredValidatorGroups() external view returns (address[] memory) { - return _groups; + return registeredGroups; } /** @@ -928,13 +866,14 @@ contract Validators is ValidatorGroup storage _group = groups[group]; require(validators[validator].affiliation == group && _group.members.contains(validator)); _group.members.remove(validator); - updateMembershipHistory(validator, address(0)); - emit ValidatorGroupMemberRemoved(group, validator); - + uint256 numMembers = _group.members.numElements; // Empty validator groups are not electable. - if (groups[group].members.numElements == 0) { + if (numMembers == 0) { getElection().markGroupIneligible(group); } + updateMembershipHistory(validator, address(0)); + updateSizeHistory(group, numMembers.add(1)); + emit ValidatorGroupMemberRemoved(group, validator); return true; } @@ -951,6 +890,10 @@ contract Validators is uint256 epochNumber = getEpochNumber(); uint256 head = history.numEntries == 0 ? 0 : history.tail.add(history.numEntries.sub(1)); + if (history.numEntries > 0 && group == address(0)) { + history.lastRemovedFromGroupTimestamp = now; + } + if (history.entries[head].epochNumber == epochNumber) { // There have been no elections since the validator last changed membership, overwrite the // previous entry. @@ -975,6 +918,18 @@ contract Validators is history.numEntries = history.numEntries.sub(1); history.tail = history.tail.add(2); } + return true; + } + + function updateSizeHistory(address group, uint256 size) private { + uint256[] storage sizeHistory = groups[group].sizeHistory; + if (size == sizeHistory.length) { + sizeHistory.push(now); + } else if (size < sizeHistory.length) { + sizeHistory[size] = now; + } else { + require(false, "Unable to update size history"); + } } /** @@ -1014,8 +969,8 @@ contract Validators is if (group.members.contains(validatorAccount)) { _removeMember(affiliation, validatorAccount); } - emit ValidatorDeaffiliated(validatorAccount, affiliation); validator.affiliation = address(0); + emit ValidatorDeaffiliated(validatorAccount, affiliation); return true; } } diff --git a/packages/protocol/contracts/governance/interfaces/IGovernance.sol b/packages/protocol/contracts/governance/interfaces/IGovernance.sol index 21afee851b2..06ca0ea6fd8 100644 --- a/packages/protocol/contracts/governance/interfaces/IGovernance.sol +++ b/packages/protocol/contracts/governance/interfaces/IGovernance.sol @@ -2,66 +2,5 @@ pragma solidity ^0.5.3; interface IGovernance { - function setApprover(address) external; - function setConcurrentProposals(uint256) external; - function setMinDeposit(uint256) external; - function setQueueExpiry(uint256) external; - function setDequeueFrequency(uint256) external; - function setApprovalStageDuration(uint256) external; - function setReferendumStageDuration(uint256) external; - function setExecutionStageDuration(uint256) external; - function setParticipationBaseline(uint256) external; - function setParticipationFloor(uint256) external; - function setBaselineUpdateFactor(uint256) external; - function setBaselineQuorumFactor(uint256) external; - function setConstitution(address, bytes4, uint256) external; - - function propose( - uint256[] calldata, - address[] calldata, - bytes calldata, - uint256[] calldata - ) external payable returns (uint256); - - function upvote(uint256, uint256, uint256) external returns (bool); - function revokeUpvote(uint256, uint256) external returns (bool); - function approve(uint256, uint256) external returns (bool); - function execute(uint256, uint256) external returns (bool); - - function whitelistHotfix(bytes32) external; - function prepareHotfix(bytes32) external; - function executeHotfix( - uint256[] calldata, - address[] calldata, - bytes calldata, - uint256[] calldata - ) external; - function isHotfixPassing(bytes32) external view returns (bool); - - function withdraw() external returns (bool); - function dequeueProposalsIfReady() external; - function getParticipationParameters() external view returns (uint256, uint256, uint256, uint256); - function getApprovalStageDuration() external view returns (uint256); - function getReferendumStageDuration() external view returns (uint256); - function getExecutionStageDuration() external view returns (uint256); - function getConstitution(address, bytes4) external view returns (uint256); - function proposalExists(uint256) external view returns (bool); - function getProposal(uint256) external view returns (address, uint256, uint256, uint256); - - function getProposalTransaction( - uint256, - uint256 - ) external view returns (uint256, address, bytes memory); - - function isApproved(uint256) external view returns (bool); - function getVoteTotals(uint256) external view returns (uint256, uint256, uint256); - function getVoteRecord(address, uint256) external view returns (uint256, uint256); - function getQueueLength() external view returns (uint256); - function getUpvotes(uint256) external view returns (uint256); - function getQueue() external view returns (uint256[] memory, uint256[] memory); - function getDequeue() external view returns (uint256[] memory); - function getUpvoteRecord(address) external view returns (uint256, uint256); - function getMostRecentReferendumProposal(address) external view returns (uint256); - function isQueued(uint256) external view returns (bool); - function isProposalPassing(uint256) external view returns (bool); + function isVoting(address) external view returns (bool); } diff --git a/packages/protocol/contracts/governance/interfaces/IValidators.sol b/packages/protocol/contracts/governance/interfaces/IValidators.sol index 41a68b70859..6650c34686f 100644 --- a/packages/protocol/contracts/governance/interfaces/IValidators.sol +++ b/packages/protocol/contracts/governance/interfaces/IValidators.sol @@ -2,7 +2,8 @@ pragma solidity ^0.5.3; interface IValidators { - function getAccountBalanceRequirement(address) external view returns (uint256); + function getAccountLockedGoldRequirement(address) external view returns (uint256); + function meetsAccountLockedGoldRequirements(address) external view returns (bool); function getGroupNumMembers(address) external view returns (uint256); function getGroupsNumMembers(address[] calldata) external view returns (uint256[] memory); function getNumRegisteredValidators() external view returns (uint256); diff --git a/packages/protocol/contracts/governance/test/MockGovernance.sol b/packages/protocol/contracts/governance/test/MockGovernance.sol index 0fd41e232b1..9432ae9d2d3 100644 --- a/packages/protocol/contracts/governance/test/MockGovernance.sol +++ b/packages/protocol/contracts/governance/test/MockGovernance.sol @@ -1,10 +1,11 @@ pragma solidity ^0.5.3; +import "../interfaces/IGovernance.sol"; /** * @title A mock Governance for testing. */ -contract MockGovernance { +contract MockGovernance is IGovernance { mapping(address => bool) public isVoting; function setVoting(address voter) external { diff --git a/packages/protocol/contracts/governance/test/MockValidators.sol b/packages/protocol/contracts/governance/test/MockValidators.sol index a593e45aecc..203466a6ff8 100644 --- a/packages/protocol/contracts/governance/test/MockValidators.sol +++ b/packages/protocol/contracts/governance/test/MockValidators.sol @@ -10,10 +10,19 @@ contract MockValidators is IValidators { mapping(address => bool) private _isValidating; mapping(address => bool) private _isVoting; mapping(address => uint256) private numGroupMembers; - mapping(address => uint256) private balanceRequirements; + mapping(address => uint256) private lockedGoldRequirements; + mapping(address => bool) private doesNotMeetAccountLockedGoldRequirements; mapping(address => address[]) private members; uint256 private numRegisteredValidators; + function setDoesNotMeetAccountLockedGoldRequirements(address account) external { + doesNotMeetAccountLockedGoldRequirements[account] = true; + } + + function meetsAccountLockedGoldRequirements(address account) external view returns (bool) { + return !doesNotMeetAccountLockedGoldRequirements[account]; + } + function isValidating(address account) external view returns (bool) { return _isValidating[account]; } @@ -46,12 +55,12 @@ contract MockValidators is IValidators { members[group] = _members; } - function setAccountBalanceRequirement(address account, uint256 value) external { - balanceRequirements[account] = value; + function setAccountLockedGoldRequirement(address account, uint256 value) external { + lockedGoldRequirements[account] = value; } - function getAccountBalanceRequirement(address account) external view returns (uint256) { - return balanceRequirements[account]; + function getAccountLockedGoldRequirement(address account) external view returns (uint256) { + return lockedGoldRequirements[account]; } function getTopGroupValidators( diff --git a/packages/protocol/migrations/12_validators.ts b/packages/protocol/migrations/12_validators.ts index 5f24860c230..2d692d15040 100644 --- a/packages/protocol/migrations/12_validators.ts +++ b/packages/protocol/migrations/12_validators.ts @@ -7,10 +7,10 @@ import { ValidatorsInstance } from 'types' const initializeArgs = async (): Promise => { return [ config.registry.predeployedProxyAddress, - config.validators.registrationRequirements.group, - config.validators.registrationRequirements.validator, - config.validators.deregistrationLockups.group, - config.validators.deregistrationLockups.validator, + config.validators.groupLockedGoldRequirements.value, + config.validators.groupLockedGoldRequirements.duration, + config.validators.validatorLockedGoldRequirements.value, + config.validators.validatorLockedGoldRequirements.duration, config.validators.validatorScoreParameters.exponent, toFixed(config.validators.validatorScoreParameters.adjustmentSpeed).toFixed(), config.validators.validatorEpochPayment, diff --git a/packages/protocol/migrations/19_elect_validators.ts b/packages/protocol/migrations/19_elect_validators.ts index 36d77c5824c..a42077e0ba2 100644 --- a/packages/protocol/migrations/19_elect_validators.ts +++ b/packages/protocol/migrations/19_elect_validators.ts @@ -45,7 +45,8 @@ async function registerValidatorGroup( accounts: AccountsInstance, lockedGold: LockedGoldInstance, validators: ValidatorsInstance, - privateKey: string + privateKey: string, + numMembers: number ) { // Validators can't also be validator groups, so we create a new account to register the // validator group with, and set the name of the group account to the private key of this account @@ -59,18 +60,18 @@ async function registerValidatorGroup( const encryptedPrivateKey = encryptionWeb3.eth.accounts.encrypt(account.privateKey, privateKey) const encodedKey = serializeKeystore(encryptedPrivateKey) + // Value is per-validator. + const lockedGoldValue = new BigNumber(config.validators.groupLockedGoldRequirements.value).times( + numMembers + ) + await web3.eth.sendTransaction({ from: generateAccountAddressFromPrivateKey(privateKey.slice(0)), to: account.address, - value: config.validators.registrationRequirements.group * 2, // Add a premium to cover tx fees + value: lockedGoldValue.times(1.01).toFixed(), // Add a premium to cover tx fees }) - await lockGold( - accounts, - lockedGold, - config.validators.registrationRequirements.group, - account.privateKey - ) + await lockGold(accounts, lockedGold, lockedGoldValue, account.privateKey) // @ts-ignore const setNameTx = accounts.contract.methods.setName( @@ -119,7 +120,7 @@ async function registerValidator( await lockGold( accounts, lockedGold, - config.validators.registrationRequirements.validator, + config.validators.validatorLockedGoldRequirements.value, validatorPrivateKey ) @@ -197,7 +198,13 @@ module.exports = async (_deployer: any, networkName: string) => { console.info(' Registering ValidatorGroup ...') const firstPrivateKey = valKeys[0] - const account = await registerValidatorGroup(accounts, lockedGold, validators, firstPrivateKey) + const account = await registerValidatorGroup( + accounts, + lockedGold, + validators, + firstPrivateKey, + valKeys.length + ) console.info(' Registering Validators ...') await Promise.all( diff --git a/packages/protocol/migrationsConfig.js b/packages/protocol/migrationsConfig.js index 215bbcb1684..cb034a6bcfc 100644 --- a/packages/protocol/migrationsConfig.js +++ b/packages/protocol/migrationsConfig.js @@ -81,13 +81,13 @@ const DefaultConfig = { oracles: [], }, validators: { - registrationRequirements: { - group: '1000000000000000000', // 1 gold - validator: '1000000000000000000', // 1 gold + groupLockedGoldRequirements: { + value: '1000000000000000000', // 1 gold + duration: 60 * 24 * 60 * 60, // 60 days }, - deregistrationLockups: { - group: 60 * 24 * 60 * 60, // 60 days - validator: 60 * 24 * 60 * 60, // 60 days + validatorLockedGoldRequirements: { + value: '1000000000000000000', // 1 gold + duration: 60 * 24 * 60 * 60, // 60 days }, validatorScoreParameters: { exponent: 1, diff --git a/packages/protocol/test/governance/election.ts b/packages/protocol/test/governance/election.ts index e9f970526b2..b4469376c56 100644 --- a/packages/protocol/test/governance/election.ts +++ b/packages/protocol/test/governance/election.ts @@ -925,7 +925,6 @@ contract('Election', (accounts: string[]) => { const voteValue1 = new BigNumber(2000000) const voteValue2 = new BigNumber(1000000) const totalRewardValue = new BigNumber(3000000) - const balanceRequirement = new BigNumber(1000000) beforeEach(async () => { await registry.setAddressFor(CeloContractName.Validators, accounts[0]) await election.markGroupEligible(group1, NULL_ADDRESS, NULL_ADDRESS) @@ -938,8 +937,6 @@ contract('Election', (accounts: string[]) => { await mockLockedGold.incrementNonvotingAccountBalance(voter, voteValue1.plus(voteValue2)) await election.vote(group1, voteValue1, group2, NULL_ADDRESS) await election.vote(group2, voteValue2, NULL_ADDRESS, group1) - await mockValidators.setAccountBalanceRequirement(group1, balanceRequirement) - await mockValidators.setAccountBalanceRequirement(group2, balanceRequirement) }) describe('when one group has active votes', () => { @@ -948,11 +945,7 @@ contract('Election', (accounts: string[]) => { await election.activate(group1) }) - describe('when the group meets the balance requirements ', () => { - beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group1, balanceRequirement) - }) - + describe('when the group meets the locked gold requirements ', () => { it('should return the total reward value', async () => { assertEqualBN( await election.getGroupEpochRewards(group1, totalRewardValue), @@ -961,9 +954,9 @@ contract('Election', (accounts: string[]) => { }) }) - describe('when the group does not meet the balance requirements ', () => { + describe('when the group does not meet the locked gold requirements ', () => { beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group1, balanceRequirement.minus(1)) + await mockValidators.setDoesNotMeetAccountLockedGoldRequirements(group1) }) it('should return zero', async () => { @@ -973,7 +966,6 @@ contract('Election', (accounts: string[]) => { }) describe('when two groups have active votes', () => { - const balanceRequirement = new BigNumber(1000000) const expectedGroup1EpochRewards = voteValue1 .div(voteValue1.plus(voteValue2)) .times(totalRewardValue) @@ -984,30 +976,26 @@ contract('Election', (accounts: string[]) => { await election.activate(group2) }) - describe('when one group meets the balance requirements ', () => { + describe('when one group does not meet the locked gold requirements ', () => { beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group1, balanceRequirement) + await mockValidators.setDoesNotMeetAccountLockedGoldRequirements(group2) + }) + + it('should return zero for that group', async () => { + assertEqualBN(await election.getGroupEpochRewards(group2, totalRewardValue), 0) }) - it('should return the proportional reward value for that group', async () => { + it('should return the proportional reward value for the other group', async () => { assertEqualBN( await election.getGroupEpochRewards(group1, totalRewardValue), expectedGroup1EpochRewards ) }) - - it('should return zero for the other group', async () => { - assertEqualBN(await election.getGroupEpochRewards(group2, totalRewardValue), 0) - }) }) }) describe('when the group does not have active votes', () => { - describe('when the group meets the balance requirements ', () => { - beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group1, balanceRequirement) - }) - + describe('when the group meets the locked gold requirements ', () => { it('should return zero', async () => { assertEqualBN(await election.getGroupEpochRewards(group1, totalRewardValue), 0) }) diff --git a/packages/protocol/test/governance/governance.ts b/packages/protocol/test/governance/governance.ts index 5934e988c05..8164c7d43b7 100644 --- a/packages/protocol/test/governance/governance.ts +++ b/packages/protocol/test/governance/governance.ts @@ -2117,7 +2117,6 @@ contract('Governance', (accounts: string[]) => { }) }) - /* describe('#isVoting()', () => { describe('when the account has never acted on a proposal', () => { it('should return false', async () => { @@ -2202,7 +2201,6 @@ contract('Governance', (accounts: string[]) => { }) }) }) - */ describe('#isProposalPassing()', () => { const proposalId = 1 diff --git a/packages/protocol/test/governance/lockedgold.ts b/packages/protocol/test/governance/lockedgold.ts index 78a758523eb..6d752666b9e 100644 --- a/packages/protocol/test/governance/lockedgold.ts +++ b/packages/protocol/test/governance/lockedgold.ts @@ -16,6 +16,8 @@ import { MockElectionInstance, MockGoldTokenContract, MockGoldTokenInstance, + MockGovernanceContract, + MockGovernanceInstance, MockValidatorsContract, MockValidatorsInstance, RegistryContract, @@ -26,6 +28,7 @@ const Accounts: AccountsContract = artifacts.require('Accounts') const LockedGold: LockedGoldContract = artifacts.require('LockedGold') const MockElection: MockElectionContract = artifacts.require('MockElection') const MockGoldToken: MockGoldTokenContract = artifacts.require('MockGoldToken') +const MockGovernance: MockGovernanceContract = artifacts.require('MockGovernance') const MockValidators: MockValidatorsContract = artifacts.require('MockValidators') const Registry: RegistryContract = artifacts.require('Registry') @@ -43,6 +46,7 @@ contract('LockedGold', (accounts: string[]) => { let accountsInstance: AccountsInstance let lockedGold: LockedGoldInstance let mockElection: MockElectionInstance + let mockGovernance: MockGovernanceInstance let mockValidators: MockValidatorsInstance let registry: RegistryInstance @@ -52,10 +56,12 @@ contract('LockedGold', (accounts: string[]) => { lockedGold = await LockedGold.new() mockElection = await MockElection.new() mockValidators = await MockValidators.new() + mockGovernance = await MockGovernance.new() registry = await Registry.new() await registry.setAddressFor(CeloContractName.Accounts, accountsInstance.address) - await registry.setAddressFor(CeloContractName.GoldToken, mockGoldToken.address) await registry.setAddressFor(CeloContractName.Election, mockElection.address) + await registry.setAddressFor(CeloContractName.GoldToken, mockGoldToken.address) + await registry.setAddressFor(CeloContractName.Governance, mockGovernance.address) await registry.setAddressFor(CeloContractName.Validators, mockValidators.address) await lockedGold.initialize(registry.address, unlockingPeriod) await accountsInstance.createAccount() @@ -179,43 +185,57 @@ contract('LockedGold', (accounts: string[]) => { beforeEach(async () => { // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails await lockedGold.lock({ value }) - resp = await lockedGold.unlock(value) - availabilityTime = new BigNumber(unlockingPeriod).plus( - (await web3.eth.getBlock('latest')).timestamp - ) }) + describe('when the account is not voting in governance', () => { + beforeEach(async () => { + resp = await lockedGold.unlock(value) + availabilityTime = new BigNumber(unlockingPeriod).plus( + (await web3.eth.getBlock('latest')).timestamp + ) + }) - it('should add a pending withdrawal', async () => { - const [values, timestamps] = await lockedGold.getPendingWithdrawals(account) - assert.equal(values.length, 1) - assert.equal(timestamps.length, 1) - assertEqualBN(values[0], value) - assertEqualBN(timestamps[0], availabilityTime) - }) + it('should add a pending withdrawal', async () => { + const [values, timestamps] = await lockedGold.getPendingWithdrawals(account) + assert.equal(values.length, 1) + assert.equal(timestamps.length, 1) + assertEqualBN(values[0], value) + assertEqualBN(timestamps[0], availabilityTime) + }) - it("should decrease the account's nonvoting locked gold balance", async () => { - assertEqualBN(await lockedGold.getAccountNonvotingLockedGold(account), 0) - }) + it("should decrease the account's nonvoting locked gold balance", async () => { + assertEqualBN(await lockedGold.getAccountNonvotingLockedGold(account), 0) + }) - it("should decrease the account's total locked gold balance", async () => { - assertEqualBN(await lockedGold.getAccountTotalLockedGold(account), 0) - }) + it("should decrease the account's total locked gold balance", async () => { + assertEqualBN(await lockedGold.getAccountTotalLockedGold(account), 0) + }) - it('should decrease the nonvoting locked gold balance', async () => { - assertEqualBN(await lockedGold.getNonvotingLockedGold(), 0) - }) + it('should decrease the nonvoting locked gold balance', async () => { + assertEqualBN(await lockedGold.getNonvotingLockedGold(), 0) + }) - it('should decrease the total locked gold balance', async () => { - assertEqualBN(await lockedGold.getTotalLockedGold(), 0) + it('should decrease the total locked gold balance', async () => { + assertEqualBN(await lockedGold.getTotalLockedGold(), 0) + }) + + it('should emit a GoldUnlocked event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches(log, 'GoldUnlocked', { + account, + value: new BigNumber(value), + available: availabilityTime, + }) + }) }) - it('should emit a GoldUnlocked event', async () => { - assert.equal(resp.logs.length, 1) - const log = resp.logs[0] - assertLogMatches(log, 'GoldUnlocked', { - account, - value: new BigNumber(value), - available: availabilityTime, + describe('when the account is voting in governance', () => { + beforeEach(async () => { + await mockGovernance.setVoting(account) + }) + + it('should revert', async () => { + await assertRevert(lockedGold.unlock(value)) }) }) }) @@ -225,7 +245,7 @@ contract('LockedGold', (accounts: string[]) => { beforeEach(async () => { // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails await lockedGold.lock({ value }) - await mockValidators.setAccountBalanceRequirement(account, balanceRequirement) + await mockValidators.setAccountLockedGoldRequirement(account, balanceRequirement) }) describe('when unlocking would yield a locked gold balance less than the required value', () => { diff --git a/packages/protocol/test/governance/validators.ts b/packages/protocol/test/governance/validators.ts index dfc3f8bc2df..9940f22288f 100644 --- a/packages/protocol/test/governance/validators.ts +++ b/packages/protocol/test/governance/validators.ts @@ -7,6 +7,7 @@ import { assertSameAddress, mineBlocks, NULL_ADDRESS, + timeTravel, } from '@celo/protocol/lib/test-utils' import { fromFixed, toFixed } from '@celo/utils/lib/fixidity' import BigNumber from 'bignumber.js' @@ -27,8 +28,8 @@ import { const Accounts: AccountsContract = artifacts.require('Accounts') const Validators: ValidatorsTestContract = artifacts.require('ValidatorsTest') -const MockLockedGold: MockLockedGoldContract = artifacts.require('MockLockedGold') const MockElection: MockElectionContract = artifacts.require('MockElection') +const MockLockedGold: MockLockedGoldContract = artifacts.require('MockLockedGold') const MockStableToken: MockStableTokenContract = artifacts.require('MockStableToken') const Registry: RegistryContract = artifacts.require('Registry') @@ -48,6 +49,15 @@ const parseValidatorGroupParams = (groupParams: any) => { return { members: groupParams[0], commission: groupParams[1], + sizeHistory: groupParams[2], + } +} + +const parseMembershipHistory = (membershipHistory: any) => { + return { + epochs: membershipHistory[0], + groups: membershipHistory[1], + lastRemovedFromGroupTimestamp: membershipHistory[2], } } @@ -61,21 +71,24 @@ contract('Validators', (accounts: string[]) => { let accountsInstance: AccountsInstance let validators: ValidatorsTestInstance let registry: RegistryInstance - let mockLockedGold: MockLockedGoldInstance let mockElection: MockElectionInstance + let mockLockedGold: MockLockedGoldInstance const nonOwner = accounts[1] - const balanceRequirements = { group: new BigNumber(1000), validator: new BigNumber(100) } - const deregistrationLockups = { - group: new BigNumber(100 * DAY), - validator: new BigNumber(60 * DAY), + const validatorLockedGoldRequirements = { + value: new BigNumber(1000), + duration: new BigNumber(60 * DAY), + } + const groupLockedGoldRequirements = { + value: new BigNumber(1000), + duration: new BigNumber(100 * DAY), } const validatorScoreParameters = { exponent: new BigNumber(5), adjustmentSpeed: toFixed(0.25), } const validatorEpochPayment = new BigNumber(10000000000000) - const membershipHistoryLength = new BigNumber(3) + const membershipHistoryLength = new BigNumber(5) const maxGroupSize = new BigNumber(5) // A random 64 byte hex string. @@ -90,19 +103,19 @@ contract('Validators', (accounts: string[]) => { beforeEach(async () => { accountsInstance = await Accounts.new() await Promise.all(accounts.map((account) => accountsInstance.createAccount({ from: account }))) - validators = await Validators.new() - mockLockedGold = await MockLockedGold.new() mockElection = await MockElection.new() + mockLockedGold = await MockLockedGold.new() registry = await Registry.new() + validators = await Validators.new() await registry.setAddressFor(CeloContractName.Accounts, accountsInstance.address) - await registry.setAddressFor(CeloContractName.LockedGold, mockLockedGold.address) await registry.setAddressFor(CeloContractName.Election, mockElection.address) + await registry.setAddressFor(CeloContractName.LockedGold, mockLockedGold.address) await validators.initialize( registry.address, - balanceRequirements.group, - balanceRequirements.validator, - deregistrationLockups.group, - deregistrationLockups.validator, + groupLockedGoldRequirements.value, + groupLockedGoldRequirements.duration, + validatorLockedGoldRequirements.value, + validatorLockedGoldRequirements.duration, validatorScoreParameters.exponent, validatorScoreParameters.adjustmentSpeed, validatorEpochPayment, @@ -112,7 +125,7 @@ contract('Validators', (accounts: string[]) => { }) const registerValidator = async (validator: string) => { - await mockLockedGold.setAccountTotalLockedGold(validator, balanceRequirements.validator) + await mockLockedGold.setAccountTotalLockedGold(validator, validatorLockedGoldRequirements.value) await validators.registerValidator( // @ts-ignore bytes type publicKeysData, @@ -121,7 +134,7 @@ contract('Validators', (accounts: string[]) => { } const registerValidatorGroup = async (group: string) => { - await mockLockedGold.setAccountTotalLockedGold(group, balanceRequirements.group) + await mockLockedGold.setAccountTotalLockedGold(group, groupLockedGoldRequirements.value) await validators.registerValidatorGroup(commission, { from: group }) } @@ -144,16 +157,32 @@ contract('Validators', (accounts: string[]) => { assert.equal(owner, accounts[0]) }) - it('should have set the balance requirements', async () => { - const [group, validator] = await validators.getBalanceRequirements() - assertEqualBN(group, balanceRequirements.group) - assertEqualBN(validator, balanceRequirements.validator) + it('should have set the group locked gold requirements', async () => { + const [value, duration] = await validators.getGroupLockedGoldRequirements() + assertEqualBN(value, groupLockedGoldRequirements.value) + assertEqualBN(duration, groupLockedGoldRequirements.duration) }) - it('should have set the deregistration lockups', async () => { - const [group, validator] = await validators.getDeregistrationLockups() - assertEqualBN(group, deregistrationLockups.group) - assertEqualBN(validator, deregistrationLockups.validator) + it('should have set the validator locked gold requirements', async () => { + const [value, duration] = await validators.getValidatorLockedGoldRequirements() + assertEqualBN(value, validatorLockedGoldRequirements.value) + assertEqualBN(duration, validatorLockedGoldRequirements.duration) + }) + + it('should have set the validator score parameters', async () => { + const [exponent, adjustmentSpeed] = await validators.getValidatorScoreParameters() + assertEqualBN(exponent, validatorScoreParameters.exponent) + assertEqualBN(adjustmentSpeed, validatorScoreParameters.adjustmentSpeed) + }) + + it('should have set the validator epoch payment', async () => { + const actual = await validators.validatorEpochPayment() + assertEqualBN(actual, validatorEpochPayment) + }) + + it('should have set the membership history length', async () => { + const actual = await validators.membershipHistoryLength() + assertEqualBN(actual, membershipHistoryLength) }) it('should have set the validator score parameters', async () => { @@ -181,10 +210,10 @@ contract('Validators', (accounts: string[]) => { await assertRevert( validators.initialize( registry.address, - balanceRequirements.group, - balanceRequirements.validator, - deregistrationLockups.group, - deregistrationLockups.validator, + groupLockedGoldRequirements.value, + groupLockedGoldRequirements.duration, + validatorLockedGoldRequirements.value, + validatorLockedGoldRequirements.duration, validatorScoreParameters.exponent, validatorScoreParameters.adjustmentSpeed, validatorEpochPayment, @@ -330,37 +359,37 @@ contract('Validators', (accounts: string[]) => { }) }) - describe('#setBalanceRequirements()', () => { + describe('#setGroupLockedGoldRequirements()', () => { describe('when the requirements are different', () => { const newRequirements = { - group: balanceRequirements.group.plus(1), - validator: balanceRequirements.validator.plus(1), + value: groupLockedGoldRequirements.value.plus(1), + duration: groupLockedGoldRequirements.duration.plus(1), } describe('when called by the owner', () => { let resp: any beforeEach(async () => { - resp = await validators.setBalanceRequirements( - newRequirements.group, - newRequirements.validator + resp = await validators.setGroupLockedGoldRequirements( + newRequirements.value, + newRequirements.duration ) }) - it('should set the group and validator requirements', async () => { - const [group, validator] = await validators.getBalanceRequirements() - assertEqualBN(group, newRequirements.group) - assertEqualBN(validator, newRequirements.validator) + it('should have set the group locked gold requirements', async () => { + const [value, duration] = await validators.getGroupLockedGoldRequirements() + assertEqualBN(value, newRequirements.value) + assertEqualBN(duration, newRequirements.duration) }) - it('should emit the BalanceRequirementsSet event', async () => { + it('should emit the GroupLockedGoldRequirementsSet event', async () => { assert.equal(resp.logs.length, 1) const log = resp.logs[0] assertContainSubset(log, { - event: 'BalanceRequirementsSet', + event: 'GroupLockedGoldRequirementsSet', args: { - group: new BigNumber(newRequirements.group), - validator: new BigNumber(newRequirements.validator), + value: newRequirements.value, + duration: newRequirements.duration, }, }) }) @@ -368,9 +397,11 @@ contract('Validators', (accounts: string[]) => { describe('when called by a non-owner', () => { it('should revert', async () => { await assertRevert( - validators.setBalanceRequirements(newRequirements.group, newRequirements.validator, { - from: nonOwner, - }) + validators.setGroupLockedGoldRequirements( + newRequirements.value, + newRequirements.duration, + { from: nonOwner } + ) ) }) }) @@ -379,9 +410,9 @@ contract('Validators', (accounts: string[]) => { describe('when the requirements are the same', () => { it('should revert', async () => { await assertRevert( - validators.setBalanceRequirements( - balanceRequirements.group, - balanceRequirements.validator + validators.setGroupLockedGoldRequirements( + groupLockedGoldRequirements.value, + groupLockedGoldRequirements.duration ) ) }) @@ -389,34 +420,37 @@ contract('Validators', (accounts: string[]) => { }) }) - describe('#setDeregistrationLockups()', () => { - describe('when the lockups are different', () => { - const newLockups = { - group: deregistrationLockups.group.plus(1), - validator: deregistrationLockups.validator.plus(1), + describe('#setValidatorLockedGoldRequirements()', () => { + describe('when the requirements are different', () => { + const newRequirements = { + value: validatorLockedGoldRequirements.value.plus(1), + duration: validatorLockedGoldRequirements.duration.plus(1), } describe('when called by the owner', () => { let resp: any beforeEach(async () => { - resp = await validators.setDeregistrationLockups(newLockups.group, newLockups.validator) + resp = await validators.setValidatorLockedGoldRequirements( + newRequirements.value, + newRequirements.duration + ) }) - it('should set the group and validator lockups', async () => { - const [group, validator] = await validators.getDeregistrationLockups() - assertEqualBN(group, newLockups.group) - assertEqualBN(validator, newLockups.validator) + it('should have set the validator locked gold requirements', async () => { + const [value, duration] = await validators.getValidatorLockedGoldRequirements() + assertEqualBN(value, newRequirements.value) + assertEqualBN(duration, newRequirements.duration) }) - it('should emit the DeregistrationLockupsSet event', async () => { + it('should emit the ValidatorLockedGoldRequirementsSet event', async () => { assert.equal(resp.logs.length, 1) const log = resp.logs[0] assertContainSubset(log, { - event: 'DeregistrationLockupsSet', + event: 'ValidatorLockedGoldRequirementsSet', args: { - group: new BigNumber(newLockups.group), - validator: new BigNumber(newLockups.validator), + value: newRequirements.value, + duration: newRequirements.duration, }, }) }) @@ -424,9 +458,74 @@ contract('Validators', (accounts: string[]) => { describe('when called by a non-owner', () => { it('should revert', async () => { await assertRevert( - validators.setDeregistrationLockups(newLockups.group, newLockups.validator, { - from: nonOwner, - }) + validators.setValidatorLockedGoldRequirements( + newRequirements.value, + newRequirements.duration, + { from: nonOwner } + ) + ) + }) + }) + }) + + describe('when the requirements are the same', () => { + it('should revert', async () => { + await assertRevert( + validators.setValidatorLockedGoldRequirements( + validatorLockedGoldRequirements.value, + validatorLockedGoldRequirements.duration + ) + ) + }) + }) + }) + }) + + describe('#setValidatorScoreParameters()', () => { + describe('when the parameters are different', () => { + const newParameters = { + exponent: validatorScoreParameters.exponent.plus(1), + adjustmentSpeed: validatorScoreParameters.adjustmentSpeed.plus(1), + } + + describe('when called by the owner', () => { + let resp: any + + beforeEach(async () => { + resp = await validators.setValidatorScoreParameters( + newParameters.exponent, + newParameters.adjustmentSpeed + ) + }) + + it('should set the exponent and adjustment speed', async () => { + const [exponent, adjustmentSpeed] = await validators.getValidatorScoreParameters() + assertEqualBN(exponent, newParameters.exponent) + assertEqualBN(adjustmentSpeed, newParameters.adjustmentSpeed) + }) + + it('should emit the ValidatorScoreParametersSet event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorScoreParametersSet', + args: { + exponent: new BigNumber(newParameters.exponent), + adjustmentSpeed: new BigNumber(newParameters.adjustmentSpeed), + }, + }) + }) + + describe('when called by a non-owner', () => { + it('should revert', async () => { + await assertRevert( + validators.setValidatorScoreParameters( + newParameters.exponent, + newParameters.adjustmentSpeed, + { + from: nonOwner, + } + ) ) }) }) @@ -435,9 +534,9 @@ contract('Validators', (accounts: string[]) => { describe('when the lockups are the same', () => { it('should revert', async () => { await assertRevert( - validators.setDeregistrationLockups( - deregistrationLockups.group, - deregistrationLockups.validator + validators.setValidatorScoreParameters( + validatorScoreParameters.exponent, + validatorScoreParameters.adjustmentSpeed ) ) }) @@ -555,7 +654,10 @@ contract('Validators', (accounts: string[]) => { describe('when the account is not a registered validator', () => { let validatorRegistrationEpochNumber: number beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(validator, balanceRequirements.validator) + await mockLockedGold.setAccountTotalLockedGold( + validator, + validatorLockedGoldRequirements.value + ) resp = await validators.registerValidator( // @ts-ignore bytes type publicKeysData @@ -577,9 +679,15 @@ contract('Validators', (accounts: string[]) => { assert.equal(parsedValidator.publicKeysData, publicKeysData) }) - it('should set account balance requirements', async () => { - const requirement = await validators.getAccountBalanceRequirement(validator) - assertEqualBN(requirement, balanceRequirements.validator) + it('should set account locked gold requirements', async () => { + const requirement = await validators.getAccountLockedGoldRequirement(validator) + assertEqualBN(requirement, validatorLockedGoldRequirements.value) + }) + + it('should set the validator membership history', async () => { + const membershipHistory = await validators.getMembershipHistory(validator) + assertEqualBNArray(membershipHistory[0], [validatorRegistrationEpochNumber]) + assert.deepEqual(membershipHistory[1], [NULL_ADDRESS]) }) it('should set the validator membership history', async () => { @@ -603,7 +711,10 @@ contract('Validators', (accounts: string[]) => { describe('when the account is already a registered validator ', () => { beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(validator, balanceRequirements.group) + await mockLockedGold.setAccountTotalLockedGold( + validator, + validatorLockedGoldRequirements.value + ) // @ts-ignore bytes type await validators.registerValidator(publicKeysData) }) @@ -620,7 +731,7 @@ contract('Validators', (accounts: string[]) => { describe('when the account is already a registered validator group', () => { beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(validator, balanceRequirements.group) + await mockLockedGold.setAccountTotalLockedGold(validator, groupLockedGoldRequirements.value) await validators.registerValidatorGroup(commission) }) @@ -634,11 +745,11 @@ contract('Validators', (accounts: string[]) => { }) }) - describe('when the account does not meet the balance requirements', () => { + describe('when the account does not meet the locked gold requirements', () => { beforeEach(async () => { await mockLockedGold.setAccountTotalLockedGold( validator, - balanceRequirements.validator.minus(1) + validatorLockedGoldRequirements.value.minus(1) ) }) @@ -660,92 +771,96 @@ contract('Validators', (accounts: string[]) => { describe('when the account is a registered validator', () => { beforeEach(async () => { await registerValidator(validator) - resp = await validators.deregisterValidator(index) - }) - - it('should mark the account as not a validator', async () => { - assert.isFalse(await validators.isValidator(validator)) }) - it('should remove the account from the list of validators', async () => { - assert.deepEqual(await validators.getRegisteredValidators(), []) - }) - - it('should preserve account balance requirements', async () => { - const requirement = await validators.getAccountBalanceRequirement(validator) - assertEqualBN(requirement, balanceRequirements.validator) - }) - - it('should set the validator deregistration timestamp', async () => { - const latestTimestamp = (await web3.eth.getBlock('latest')).timestamp - const [groupTimestamp, validatorTimestamp] = await validators.getDeregistrationTimestamps( - validator - ) - assertEqualBN(groupTimestamp, 0) - assertEqualBN(validatorTimestamp, latestTimestamp) - }) - - it('should emit the ValidatorDeregistered event', async () => { - assert.equal(resp.logs.length, 1) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorDeregistered', - args: { - validator, - }, + describe('when the validator has never been a member of a validator group', () => { + beforeEach(async () => { + resp = await validators.deregisterValidator(index) }) - }) - }) - describe('when the validator is affiliated with a validator group', () => { - const group = accounts[1] - beforeEach(async () => { - await registerValidator(validator) - await registerValidatorGroup(group) - await validators.affiliate(group) - }) - - it('should emit the ValidatorDeafilliated event', async () => { - const resp = await validators.deregisterValidator(index) - assert.equal(resp.logs.length, 2) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorDeaffiliated', - args: { - validator, - group, - }, + it('should mark the account as not a validator', async () => { + assert.isFalse(await validators.isValidator(validator)) }) - }) - describe('when the validator is a member of that group', () => { - beforeEach(async () => { - await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS, { from: group }) + it('should remove the account from the list of validators', async () => { + assert.deepEqual(await validators.getRegisteredValidators(), []) }) - it('should remove the validator from the group membership list', async () => { - await validators.deregisterValidator(index) - const parsedGroup = parseValidatorGroupParams(await validators.getValidatorGroup(group)) - assert.deepEqual(parsedGroup.members, []) + it('should reset account balance requirements', async () => { + const requirement = await validators.getAccountLockedGoldRequirement(validator) + assertEqualBN(requirement, 0) }) - it('should emit the ValidatorGroupMemberRemoved event', async () => { - const resp = await validators.deregisterValidator(index) - assert.equal(resp.logs.length, 3) + it('should emit the ValidatorDeregistered event', async () => { + assert.equal(resp.logs.length, 1) const log = resp.logs[0] assertContainSubset(log, { - event: 'ValidatorGroupMemberRemoved', + event: 'ValidatorDeregistered', args: { validator, - group, }, }) }) + }) - describe('when the validator is the only member of that group', () => { - it('should should mark the group as ineligible for election', async () => { - await validators.deregisterValidator(index) - assert.isTrue(await mockElection.isIneligible(group)) + describe('when the validator has been a member of a validator group', () => { + const group = accounts[1] + beforeEach(async () => { + await registerValidatorGroup(group) + await validators.affiliate(group) + await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS, { from: group }) + }) + + describe('when the validator is no longer a member of a validator group', () => { + beforeEach(async () => { + await validators.removeMember(validator, { from: group }) + }) + + describe('when it has been more than `validatorLockedGoldRequirements.duration` since the validator was removed from the group', () => { + beforeEach(async () => { + await timeTravel(validatorLockedGoldRequirements.duration.plus(1).toNumber(), web3) + resp = await validators.deregisterValidator(index) + }) + + it('should mark the account as not a validator', async () => { + assert.isFalse(await validators.isValidator(validator)) + }) + + it('should remove the account from the list of validators', async () => { + assert.deepEqual(await validators.getRegisteredValidators(), []) + }) + + it('should reset account balance requirements', async () => { + const requirement = await validators.getAccountLockedGoldRequirement(validator) + assertEqualBN(requirement, 0) + }) + + it('should emit the ValidatorDeregistered event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorDeregistered', + args: { + validator, + }, + }) + }) + }) + + describe('when it has been `validatorLockedGoldRequirements.duration` since the validator was removed from the group', () => { + beforeEach(async () => { + await timeTravel(validatorLockedGoldRequirements.duration.toNumber(), web3) + }) + + it('should revert', async () => { + await assertRevert(validators.deregisterValidator(index)) + }) + }) + }) + + describe('when the validator is still a member of a validator group', () => { + it('should revert', async () => { + await assertRevert(validators.deregisterValidator(index)) }) }) }) @@ -763,116 +878,193 @@ contract('Validators', (accounts: string[]) => { describe('#affiliate', () => { const validator = accounts[0] const group = accounts[1] - beforeEach(async () => { - await registerValidator(validator) - await registerValidatorGroup(group) - }) - - it('should set the affiliate', async () => { - await validators.affiliate(group) - const parsedValidator = parseValidatorParams(await validators.getValidator(validator)) - assert.equal(parsedValidator.affiliation, group) - }) - - it('should emit the ValidatorAffiliated event', async () => { - const resp = await validators.affiliate(group) - assert.equal(resp.logs.length, 1) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorAffiliated', - args: { - validator, - group, - }, - }) - }) - - describe('when the validator is already affiliated with a validator group', () => { - const otherGroup = accounts[2] + let registrationEpoch: number + let resp: any + describe('when the account has a registered validator', () => { beforeEach(async () => { - await validators.affiliate(group) - await registerValidatorGroup(otherGroup) + await registerValidator(validator) + registrationEpoch = Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) }) + describe('when affiliating with a registered validator group', () => { + beforeEach(async () => { + await registerValidatorGroup(group) + }) - it('should set the affiliate', async () => { - await validators.affiliate(otherGroup) - const parsedValidator = parseValidatorParams(await validators.getValidator(validator)) - assert.equal(parsedValidator.affiliation, otherGroup) - }) + describe('when the validator meets the locked gold requirements', () => { + describe('when the group meets the locked gold requirements', () => { + beforeEach(async () => { + resp = await validators.affiliate(group) + }) + + it('should set the affiliate', async () => { + await validators.affiliate(group) + const parsedValidator = parseValidatorParams(await validators.getValidator(validator)) + assert.equal(parsedValidator.affiliation, group) + }) + + it('should emit the ValidatorAffiliated event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorAffiliated', + args: { + validator, + group, + }, + }) + }) - it('should emit the ValidatorDeafilliated event', async () => { - const resp = await validators.affiliate(otherGroup) - assert.equal(resp.logs.length, 2) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorDeaffiliated', - args: { - validator, - group, - }, - }) - }) + describe('when the validator is already affiliated with a validator group', () => { + const otherGroup = accounts[2] + beforeEach(async () => { + await registerValidatorGroup(otherGroup) + }) - it('should emit the ValidatorAffiliated event', async () => { - const resp = await validators.affiliate(otherGroup) - assert.equal(resp.logs.length, 2) - const log = resp.logs[1] - assertContainSubset(log, { - event: 'ValidatorAffiliated', - args: { - validator, - group: otherGroup, - }, - }) - }) + describe('when the validator is not a member of that validator group', () => { + beforeEach(async () => { + resp = await validators.affiliate(otherGroup) + }) + + it('should set the affiliate', async () => { + const parsedValidator = parseValidatorParams( + await validators.getValidator(validator) + ) + assert.equal(parsedValidator.affiliation, otherGroup) + }) + + it('should emit the ValidatorDeafilliated event', async () => { + assert.equal(resp.logs.length, 2) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorDeaffiliated', + args: { + validator, + group, + }, + }) + }) + + it('should emit the ValidatorAffiliated event', async () => { + assert.equal(resp.logs.length, 2) + const log = resp.logs[1] + assertContainSubset(log, { + event: 'ValidatorAffiliated', + args: { + validator, + group: otherGroup, + }, + }) + }) + }) - describe('when the validator is a member of that group', () => { - beforeEach(async () => { - await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS, { from: group }) - }) + describe('when the validator is a member of that group', () => { + let additionEpoch: number + let affiliationEpoch: number + beforeEach(async () => { + await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS, { + from: group, + }) + additionEpoch = Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) + resp = await validators.affiliate(otherGroup) + affiliationEpoch = Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) + }) + + it('should remove the validator from the group membership list', async () => { + const parsedGroup = parseValidatorGroupParams( + await validators.getValidatorGroup(group) + ) + assert.deepEqual(parsedGroup.members, []) + }) + + it("should update the validator's membership history", async () => { + const membershipHistory = parseMembershipHistory( + await validators.getMembershipHistory(validator) + ) + let expectedEntries = 1 + if (registrationEpoch != additionEpoch || additionEpoch != affiliationEpoch) { + expectedEntries = 2 + } + assert.equal(membershipHistory.epochs.length, expectedEntries) + assertEqualBN(membershipHistory.epochs[expectedEntries - 1], affiliationEpoch) + assert.equal(membershipHistory.groups.length, expectedEntries) + assertSameAddress(membershipHistory.groups[expectedEntries - 1], NULL_ADDRESS) + const latestBlock = await web3.eth.getBlock('latest') + assert.equal( + membershipHistory.lastRemovedFromGroupTimestamp, + latestBlock.timestamp + ) + }) + + it('should emit the ValidatorGroupMemberRemoved event', async () => { + assert.equal(resp.logs.length, 3) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorGroupMemberRemoved', + args: { + validator, + group, + }, + }) + }) + + describe('when the validator is the only member of that group', () => { + it('should should mark the group as ineligible for election', async () => { + assert.isTrue(await mockElection.isIneligible(group)) + }) + }) + }) + }) + }) - it('should remove the validator from the group membership list', async () => { - await validators.affiliate(otherGroup) - const parsedGroup = parseValidatorGroupParams(await validators.getValidatorGroup(group)) - assert.deepEqual(parsedGroup.members, []) + describe('when the group does not meet the locked gold requirements', () => { + beforeEach(async () => { + await mockLockedGold.setAccountTotalLockedGold( + group, + groupLockedGoldRequirements.value.minus(1) + ) + }) + + it('should revert', async () => { + await assertRevert(validators.affiliate(group)) + }) + }) }) - it('should emit the ValidatorGroupMemberRemoved event', async () => { - const resp = await validators.affiliate(otherGroup) - assert.equal(resp.logs.length, 3) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorGroupMemberRemoved', - args: { + describe('when the validator does not meet the locked gold requirements', () => { + beforeEach(async () => { + await mockLockedGold.setAccountTotalLockedGold( validator, - group, - }, + validatorLockedGoldRequirements.value.minus(1) + ) }) - }) - describe('when the validator is the only member of that group', () => { - it('should should mark the group as ineligible for election', async () => { - await validators.affiliate(otherGroup) - assert.isTrue(await mockElection.isIneligible(group)) + it('should revert', async () => { + await assertRevert(validators.affiliate(group)) }) }) }) - }) - it('should revert when the account is not a registered validator', async () => { - await assertRevert(validators.affiliate(group, { from: accounts[2] })) + describe('when affiliating with a non-registered validator group', () => { + it('should revert', async () => { + await assertRevert(validators.affiliate(group)) + }) + }) }) - it('should revert when the group is not a registered validator group', async () => { - await assertRevert(validators.affiliate(accounts[2])) + describe('when the account does not have a registered validator', () => { + it('should revert', async () => { + await assertRevert(validators.affiliate(group)) + }) }) }) describe('#deaffiliate', () => { const validator = accounts[0] const group = accounts[1] + let registrationEpoch: number beforeEach(async () => { await registerValidator(validator) + registrationEpoch = Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) await registerValidatorGroup(group) await validators.affiliate(group) }) @@ -897,30 +1089,38 @@ contract('Validators', (accounts: string[]) => { }) describe('when the validator is a member of the affiliated group', () => { + let additionEpoch: number + let deaffiliationEpoch: number + let resp: any beforeEach(async () => { await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS, { from: group }) + additionEpoch = Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) + resp = await validators.deaffiliate() + deaffiliationEpoch = Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) }) it('should remove the validator from the group membership list', async () => { - await validators.deaffiliate() const parsedGroup = parseValidatorGroupParams(await validators.getValidatorGroup(group)) assert.deepEqual(parsedGroup.members, []) }) it("should update the member's membership history", async () => { - await validators.deaffiliate() - const membershipHistory = await validators.getMembershipHistory(validator) - const expectedEpoch = new BigNumber( - Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) + const membershipHistory = parseMembershipHistory( + await validators.getMembershipHistory(validator) ) - assert.equal(membershipHistory[0].length, 1) - assertEqualBN(membershipHistory[0][0], expectedEpoch) - assert.equal(membershipHistory[1].length, 1) - assertSameAddress(membershipHistory[1][0], NULL_ADDRESS) + let expectedEntries = 1 + if (registrationEpoch != additionEpoch || additionEpoch != deaffiliationEpoch) { + expectedEntries = 2 + } + assert.equal(membershipHistory.epochs.length, expectedEntries) + assertEqualBN(membershipHistory.epochs[expectedEntries - 1], deaffiliationEpoch) + assert.equal(membershipHistory.groups.length, expectedEntries) + assertSameAddress(membershipHistory.groups[expectedEntries - 1], NULL_ADDRESS) + const latestBlock = await web3.eth.getBlock('latest') + assert.equal(membershipHistory.lastRemovedFromGroupTimestamp, latestBlock.timestamp) }) it('should emit the ValidatorGroupMemberRemoved event', async () => { - const resp = await validators.deaffiliate() assert.equal(resp.logs.length, 2) const log = resp.logs[0] assertContainSubset(log, { @@ -934,7 +1134,6 @@ contract('Validators', (accounts: string[]) => { describe('when the validator is the only member of that group', () => { it('should should mark the group as ineligible for election', async () => { - await validators.deaffiliate() assert.isTrue(await mockElection.isIneligible(group)) }) }) @@ -954,37 +1153,53 @@ contract('Validators', (accounts: string[]) => { const group = accounts[0] let resp: any describe('when the account is not a registered validator group', () => { - beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group, balanceRequirements.group) - resp = await validators.registerValidatorGroup(commission) - }) + describe('when the account meets the locked gold requirements', () => { + beforeEach(async () => { + await mockLockedGold.setAccountTotalLockedGold(group, groupLockedGoldRequirements.value) + resp = await validators.registerValidatorGroup(commission) + }) - it('should mark the account as a validator group', async () => { - assert.isTrue(await validators.isValidatorGroup(group)) - }) + it('should mark the account as a validator group', async () => { + assert.isTrue(await validators.isValidatorGroup(group)) + }) - it('should add the account to the list of validator groups', async () => { - assert.deepEqual(await validators.getRegisteredValidatorGroups(), [group]) - }) + it('should add the account to the list of validator groups', async () => { + assert.deepEqual(await validators.getRegisteredValidatorGroups(), [group]) + }) - it('should set the validator group name and commission', async () => { - const parsedGroup = parseValidatorGroupParams(await validators.getValidatorGroup(group)) - assertEqualBN(parsedGroup.commission, commission) - }) + it('should set the validator group commission', async () => { + const parsedGroup = parseValidatorGroupParams(await validators.getValidatorGroup(group)) + assertEqualBN(parsedGroup.commission, commission) + }) + + it('should set account locked gold requirements', async () => { + const requirement = await validators.getAccountLockedGoldRequirement(group) + assertEqualBN(requirement, groupLockedGoldRequirements.value) + }) - it('should set account balance requirements', async () => { - const requirement = await validators.getAccountBalanceRequirement(group) - assertEqualBN(requirement, balanceRequirements.group) + it('should emit the ValidatorGroupRegistered event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorGroupRegistered', + args: { + group, + commission, + }, + }) + }) }) - it('should emit the ValidatorGroupRegistered event', async () => { - assert.equal(resp.logs.length, 1) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorGroupRegistered', - args: { + describe('when the account does not meet the locked gold requirements', () => { + beforeEach(async () => { + await mockLockedGold.setAccountTotalLockedGold( group, - }, + groupLockedGoldRequirements.value.minus(1) + ) + }) + + it('should revert', async () => { + await assertRevert(validators.registerValidatorGroup(commission)) }) }) }) @@ -1001,7 +1216,7 @@ contract('Validators', (accounts: string[]) => { describe('when the account is already a registered validator group', () => { beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group, balanceRequirements.group) + await mockLockedGold.setAccountTotalLockedGold(group, groupLockedGoldRequirements.value) await validators.registerValidatorGroup(commission) }) @@ -1010,9 +1225,9 @@ contract('Validators', (accounts: string[]) => { }) }) - describe('when the account does not meet the balance requirements', () => { + describe('when the account is already a registered validator group', () => { beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group, balanceRequirements.group.minus(1)) + await registerValidatorGroup(group) }) it('should revert', async () => { @@ -1025,61 +1240,108 @@ contract('Validators', (accounts: string[]) => { const index = 0 const group = accounts[0] let resp: any - beforeEach(async () => { - await registerValidatorGroup(group) - resp = await validators.deregisterValidatorGroup(index) - }) - - it('should mark the account as not a validator group', async () => { - assert.isFalse(await validators.isValidatorGroup(group)) - }) + describe('when the account has a registered validator group', () => { + beforeEach(async () => { + await registerValidatorGroup(group) + }) + describe('when the group has never had any members', () => { + beforeEach(async () => { + resp = await validators.deregisterValidatorGroup(index) + }) - it('should remove the account from the list of validator groups', async () => { - assert.deepEqual(await validators.getRegisteredValidatorGroups(), []) - }) + it('should mark the account as not a validator group', async () => { + assert.isFalse(await validators.isValidatorGroup(group)) + }) - it('should preserve account balance requirements', async () => { - const requirement = await validators.getAccountBalanceRequirement(group) - assertEqualBN(requirement, balanceRequirements.group) - }) + it('should remove the account from the list of validator groups', async () => { + assert.deepEqual(await validators.getRegisteredValidatorGroups(), []) + }) - it('should set the group deregistration timestamp', async () => { - const latestTimestamp = (await web3.eth.getBlock('latest')).timestamp - const [groupTimestamp, validatorTimestamp] = await validators.getDeregistrationTimestamps( - group - ) - assertEqualBN(groupTimestamp, latestTimestamp) - assertEqualBN(validatorTimestamp, 0) - }) + it('should reset account balance requirements', async () => { + const requirement = await validators.getAccountLockedGoldRequirement(group) + assertEqualBN(requirement, 0) + }) - it('should emit the ValidatorGroupDeregistered event', async () => { - assert.equal(resp.logs.length, 1) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorGroupDeregistered', - args: { - group, - }, + it('should emit the ValidatorGroupDeregistered event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorGroupDeregistered', + args: { + group, + }, + }) + }) }) - }) - it('should revert when the account is not a registered validator group', async () => { - await assertRevert(validators.deregisterValidatorGroup(index, { from: accounts[2] })) - }) + describe('when the group has had members', () => { + const validator = accounts[1] + beforeEach(async () => { + await registerValidator(validator) + await validators.affiliate(group, { from: validator }) + await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS) + }) - it('should revert when the wrong index is provided', async () => { - await assertRevert(validators.deregisterValidatorGroup(index + 1)) - }) + describe('when the group no longer has members', () => { + beforeEach(async () => { + await validators.removeMember(validator) + }) - describe('when the validator group is not empty', () => { - const validator = accounts[1] - beforeEach(async () => { - await registerValidatorGroup(group) - await registerValidator(validator) - await validators.affiliate(group, { from: validator }) - await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS) + describe('when it has been more than `groupLockedGoldRequirements.duration` since the validator was removed from the group', () => { + beforeEach(async () => { + await timeTravel(groupLockedGoldRequirements.duration.plus(1).toNumber(), web3) + resp = await validators.deregisterValidatorGroup(index) + }) + + it('should mark the account as not a validator group', async () => { + assert.isFalse(await validators.isValidatorGroup(group)) + }) + + it('should remove the account from the list of validator groups', async () => { + assert.deepEqual(await validators.getRegisteredValidatorGroups(), []) + }) + + it('should reset account balance requirements', async () => { + const requirement = await validators.getAccountLockedGoldRequirement(group) + assertEqualBN(requirement, 0) + }) + + it('should emit the ValidatorGroupDeregistered event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorGroupDeregistered', + args: { + group, + }, + }) + }) + }) + + describe('when it has been less than `groupLockedGoldRequirements.duration` since the validator was removed from the group', () => { + beforeEach(async () => { + await timeTravel(groupLockedGoldRequirements.duration.minus(1).toNumber(), web3) + }) + + it('should revert', async () => { + await assertRevert(validators.deregisterValidatorGroup(index)) + }) + }) + }) + + describe('when the group still has members', () => { + it('should revert', async () => { + await assertRevert(validators.deregisterValidatorGroup(index)) + }) + }) }) + it('should revert when the wrong index is provided', async () => { + await assertRevert(validators.deregisterValidatorGroup(index + 1)) + }) + }) + + describe('when the account does not have a registered validator group', () => { it('should revert', async () => { await assertRevert(validators.deregisterValidatorGroup(index)) }) @@ -1090,63 +1352,150 @@ contract('Validators', (accounts: string[]) => { const group = accounts[0] const validator = accounts[1] let resp: any - beforeEach(async () => { - await registerValidator(validator) - await registerValidatorGroup(group) - await validators.affiliate(group, { from: validator }) - resp = await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS) - }) + describe('when account has a registered validator group', () => { + beforeEach(async () => { + await registerValidatorGroup(group) + }) + describe('when adding a validator affiliated with the group', () => { + beforeEach(async () => { + await registerValidator(validator) + await validators.affiliate(group, { from: validator }) + }) - it('should add the member to the list of members', async () => { - const parsedGroup = parseValidatorGroupParams(await validators.getValidatorGroup(group)) - assert.deepEqual(parsedGroup.members, [validator]) - }) + describe('when the group meets the locked gold requirements', () => { + describe('when the validator meets the locked gold requirements', () => { + beforeEach(async () => { + resp = await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS) + }) - it("should update the member's membership history", async () => { - const membershipHistory = await validators.getMembershipHistory(validator) - const expectedEpoch = new BigNumber( - Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) - ) - assert.equal(membershipHistory[0].length, 1) - assertEqualBN(membershipHistory[0][0], expectedEpoch) - assert.equal(membershipHistory[1].length, 1) - assertSameAddress(membershipHistory[1][0], group) - }) + it('should add the member to the list of members', async () => { + const parsedGroup = parseValidatorGroupParams( + await validators.getValidatorGroup(group) + ) + assert.deepEqual(parsedGroup.members, [validator]) + }) - it('should emit the ValidatorGroupMemberAdded event', async () => { - assert.equal(resp.logs.length, 1) - const log = resp.logs[0] - assertContainSubset(log, { - event: 'ValidatorGroupMemberAdded', - args: { - group, - validator, - }, - }) - }) + it("should update the groups's size history", async () => { + const parsedGroup = parseValidatorGroupParams( + await validators.getValidatorGroup(group) + ) + assert.equal(parsedGroup.sizeHistory.length, 1) + assertEqualBN( + parsedGroup.sizeHistory[0], + (await web3.eth.getBlock('latest')).timestamp + ) + }) - it('should revert when the account is not a registered validator group', async () => { - await assertRevert( - validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS, { from: accounts[2] }) - ) - }) + it("should update the member's membership history", async () => { + const membershipHistory = await validators.getMembershipHistory(validator) + const expectedEpoch = new BigNumber( + Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) + ) + assert.equal(membershipHistory[0].length, 1) + assertEqualBN(membershipHistory[0][0], expectedEpoch) + assert.equal(membershipHistory[1].length, 1) + assertSameAddress(membershipHistory[1][0], group) + }) + + it('should mark the group as eligible', async () => { + assert.isTrue(await mockElection.isEligible(group)) + }) + + it('should emit the ValidatorGroupMemberAdded event', async () => { + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertContainSubset(log, { + event: 'ValidatorGroupMemberAdded', + args: { + group, + validator, + }, + }) + }) - it('should revert when the member is not a registered validator', async () => { - await assertRevert(validators.addFirstMember(accounts[2], NULL_ADDRESS, NULL_ADDRESS)) - }) + describe('when the group has no room to add another member', () => { + beforeEach(async () => { + await validators.setMaxGroupSize(1) + await registerValidator(accounts[2]) + await validators.affiliate(group, { from: accounts[2] }) + }) - it('should revert when trying to add too many members to group', async () => { - await validators.setMaxGroupSize(1) - await registerValidator(accounts[2]) - await validators.affiliate(group, { from: accounts[2] }) - await assertRevert(validators.addMember(accounts[2])) - }) + it('should revert', async () => { + await assertRevert(validators.addMember(accounts[2])) + }) + }) + + describe('when adding many validators affiliated with the group', () => { + it("should update the groups's size history and balance requirements", async () => { + const expectedSizeHistory = parseValidatorGroupParams( + await validators.getValidatorGroup(group) + ).sizeHistory + assert.equal(expectedSizeHistory.length, 1) + for (let i = 2; i < maxGroupSize.toNumber() + 1; i++) { + const numMembers = i + const validator = accounts[i] + await registerValidator(validator) + await validators.affiliate(group, { from: validator }) + await mockLockedGold.setAccountTotalLockedGold( + group, + groupLockedGoldRequirements.value.times(numMembers) + ) + await validators.addMember(validator) + expectedSizeHistory.push((await web3.eth.getBlock('latest')).timestamp) + const parsedGroup = parseValidatorGroupParams( + await validators.getValidatorGroup(group) + ) + assert.deepEqual( + parsedGroup.sizeHistory.map((x) => x.toString()), + expectedSizeHistory.map((x) => x.toString()) + ) + const requirement = await validators.getAccountLockedGoldRequirement(group) + assertEqualBN(requirement, groupLockedGoldRequirements.value.times(numMembers)) + } + }) + }) + }) - describe('when the validator has not affiliated themselves with the group', () => { - beforeEach(async () => { - await validators.deaffiliate({ from: validator }) + describe('when the validator does not meet the locked gold requirements', () => { + beforeEach(async () => { + await mockLockedGold.setAccountTotalLockedGold( + validator, + validatorLockedGoldRequirements.value.minus(1) + ) + }) + + it('should revert', async () => { + await assertRevert(validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS)) + }) + }) + }) + + describe('when the group does not meet the locked gold requirements', () => { + beforeEach(async () => { + await mockLockedGold.setAccountTotalLockedGold( + group, + groupLockedGoldRequirements.value.minus(1) + ) + }) + + it('should revert', async () => { + await assertRevert(validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS)) + }) + }) + }) + + describe('when adding a validator not affiliated with the group', () => { + beforeEach(async () => { + await registerValidator(validator) + }) + + it('should revert', async () => { + await assertRevert(validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS)) + }) }) + }) + describe('when the account does not have a registered validator group', () => { it('should revert', async () => { await assertRevert(validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS)) }) @@ -1174,23 +1523,32 @@ contract('Validators', (accounts: string[]) => { it("should update the member's membership history", async () => { await validators.removeMember(validator) - const membershipHistory = await validators.getMembershipHistory(validator) - const expectedEpoch = new BigNumber( - Math.floor((await web3.eth.getBlock('latest')).number / EPOCH) + const membershipHistory = parseMembershipHistory( + await validators.getMembershipHistory(validator) ) + const latestBlock = await web3.eth.getBlock('latest') + const expectedEpoch = new BigNumber(Math.floor(latestBlock.number / EPOCH)) // Depending on test timing, we may or may not span an epoch boundary between registration // and removal. - const numEntries = membershipHistory[0].length + const numEntries = membershipHistory.epochs.length assert.isTrue(numEntries == 1 || numEntries == 2) - assert.equal(membershipHistory[1].length, numEntries) + assert.equal(membershipHistory.groups.length, numEntries) if (numEntries == 1) { - assertEqualBN(membershipHistory[0][0], expectedEpoch) - assertSameAddress(membershipHistory[1][0], NULL_ADDRESS) + assertEqualBN(membershipHistory.epochs[0], expectedEpoch) + assertSameAddress(membershipHistory.groups[0], NULL_ADDRESS) } else { - assertEqualBN(membershipHistory[0][1], expectedEpoch) - assertSameAddress(membershipHistory[1][1], NULL_ADDRESS) + assertEqualBN(membershipHistory.epochs[1], expectedEpoch) + assertSameAddress(membershipHistory.groups[1], NULL_ADDRESS) } + assert.equal(membershipHistory.lastRemovedFromGroupTimestamp, latestBlock.timestamp) + }) + + it("should update the group's size history", async () => { + await validators.removeMember(validator) + const parsedGroup = parseValidatorGroupParams(await validators.getValidatorGroup(group)) + assert.equal(parsedGroup.sizeHistory.length, 2) + assertEqualBN(parsedGroup.sizeHistory[1], (await web3.eth.getBlock('latest')).timestamp) }) it('should emit the ValidatorGroupMemberRemoved event', async () => { @@ -1447,6 +1805,72 @@ contract('Validators', (accounts: string[]) => { }) }) + describe('#getAccountLockedGoldRequirement', () => { + describe('when a validator group has added members', () => { + const group = accounts[0] + const numMembers = 5 + let actualRequirements: BigNumber[] + beforeEach(async () => { + actualRequirements = [] + await registerValidatorGroup(group) + for (let i = 1; i < numMembers + 1; i++) { + const validator = accounts[i] + await registerValidator(validator) + await validators.affiliate(group, { from: validator }) + await mockLockedGold.setAccountTotalLockedGold( + group, + groupLockedGoldRequirements.value.times(i) + ) + if (i == 1) { + await validators.addFirstMember(validator, NULL_ADDRESS, NULL_ADDRESS) + } else { + await validators.addMember(validator) + } + actualRequirements.push(await validators.getAccountLockedGoldRequirement(group)) + } + }) + + it('should increase the requirement with each added member', async () => { + for (let i = 0; i < numMembers; i++) { + assertEqualBN(actualRequirements[i], groupLockedGoldRequirements.value.times(i + 1)) + } + }) + + describe('when a validator group is removing members', () => { + let removalTimestamps: number[] + beforeEach(async () => { + removalTimestamps = [] + for (let i = 1; i < numMembers + 1; i++) { + const validator = accounts[i] + await validators.removeMember(validator) + removalTimestamps.push((await web3.eth.getBlock('latest')).timestamp) + // Space things out. + await timeTravel(47, web3) + } + }) + + it('should decrease the requirement `duration`+1 seconds after removal', async () => { + for (let i = 0; i < numMembers; i++) { + assertEqualBN( + await validators.getAccountLockedGoldRequirement(group), + groupLockedGoldRequirements.value.times(numMembers - i) + ) + const removalTimestamp = removalTimestamps[i] + const requirementExpiry = groupLockedGoldRequirements.duration.plus(removalTimestamp) + const currentTimestamp = (await web3.eth.getBlock('latest')).timestamp + await timeTravel( + requirementExpiry + .minus(currentTimestamp) + .plus(1) + .toNumber(), + web3 + ) + } + }) + }) + }) + }) + describe('#distributeEpochPayment', () => { const validator = accounts[0] const group = accounts[1] @@ -1489,7 +1913,7 @@ contract('Validators', (accounts: string[]) => { beforeEach(async () => { await mockLockedGold.setAccountTotalLockedGold( validator, - balanceRequirements.validator.minus(1) + validatorLockedGoldRequirements.value.minus(1) ) await validators.distributeEpochPayment(validator) }) @@ -1505,7 +1929,10 @@ contract('Validators', (accounts: string[]) => { describe('when the group does not meet the balance requirements', () => { beforeEach(async () => { - await mockLockedGold.setAccountTotalLockedGold(group, balanceRequirements.group.minus(1)) + await mockLockedGold.setAccountTotalLockedGold( + group, + groupLockedGoldRequirements.value.minus(1) + ) await validators.distributeEpochPayment(validator) }) From 1deb1dd431167d26ad2c2806e4d97493f4b39532 Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 3 Nov 2019 19:11:20 +0100 Subject: [PATCH 06/71] Increase ganache gas limit (#1569) --- packages/protocol/scripts/bash/ganache.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/scripts/bash/ganache.sh b/packages/protocol/scripts/bash/ganache.sh index cb8aa6ac01d..51e4cc71370 100755 --- a/packages/protocol/scripts/bash/ganache.sh +++ b/packages/protocol/scripts/bash/ganache.sh @@ -8,6 +8,6 @@ yarn run ganache-cli \ --mnemonic 'concert load couple harbor equip island argue ramp clarify fence smart topic' \ --gasPrice 0 \ --networkId 1101 \ - --gasLimit 8000000 \ + --gasLimit 10000000 \ --defaultBalanceEther 1000000 \ --allowUnlimitedContractSize From d4b8c38acdc520b60f45b9197c6ebb81146298ef Mon Sep 17 00:00:00 2001 From: Ivan Sorokin Date: Mon, 4 Nov 2019 11:33:21 +0100 Subject: [PATCH 07/71] [Wallet] Protect Backup Key and Safeguards with PIN (#1556) * [Wallet] Protect Backup Key and Safeguards with PIN When a user tries to open Backup Key or Safeguards screens she should be asked to confirm PIN before proceeding. --- packages/mobile/src/account/saga.test.ts | 10 +- packages/mobile/src/account/saga.ts | 14 +- packages/mobile/src/app/actions.ts | 38 ++++ packages/mobile/src/app/reducers.ts | 26 ++- packages/mobile/src/app/saga.test.ts | 39 +++- packages/mobile/src/app/saga.ts | 39 +++- .../src/backup/BackupIntroduction.test.tsx | 12 ++ .../mobile/src/backup/BackupIntroduction.tsx | 21 +- .../src/backup/BackupSocialIntro.test.tsx | 2 +- .../mobile/src/backup/BackupSocialIntro.tsx | 33 +++- .../BackupIntroduction.test.tsx.snap | 185 ++++++++++++++++++ packages/mobile/test/schemas.ts | 4 + 12 files changed, 399 insertions(+), 24 deletions(-) diff --git a/packages/mobile/src/account/saga.test.ts b/packages/mobile/src/account/saga.test.ts index 0a41288b928..b830804a742 100644 --- a/packages/mobile/src/account/saga.test.ts +++ b/packages/mobile/src/account/saga.test.ts @@ -1,4 +1,4 @@ -import { expectSaga } from 'redux-saga-test-plan' +import { expectSaga, testSaga } from 'redux-saga-test-plan' import { select } from 'redux-saga/effects' import { setPincodeFailure, setPincodeSuccess } from 'src/account/actions' import { PincodeType, pincodeTypeSelector } from 'src/account/reducer' @@ -59,4 +59,12 @@ describe('@getPincode', () => { expect(error.message).toBe('Pin has never been set') } }) + + it('does not touch cache', async () => { + await testSaga(getPincode, false) + .next() + .next(PincodeType.CustomPin) + .next(mockPin) + .returns(mockPin) + }) }) diff --git a/packages/mobile/src/account/saga.ts b/packages/mobile/src/account/saga.ts index 74d58fe0f81..54b28170cde 100644 --- a/packages/mobile/src/account/saga.ts +++ b/packages/mobile/src/account/saga.ts @@ -39,7 +39,7 @@ export function* setPincode({ pincodeType, pin }: SetPincodeAction) { } } -export function* getPincode() { +export function* getPincode(useCache = true) { const pincodeType = yield select(pincodeTypeSelector) if (pincodeType === PincodeType.Unset) { @@ -58,9 +58,11 @@ export function* getPincode() { if (pincodeType === PincodeType.CustomPin) { Logger.debug(TAG + '@getPincode', 'Getting custom pin') - const cachedPin = getCachedPincode() - if (cachedPin) { - return cachedPin + if (useCache) { + const cachedPin = getCachedPincode() + if (cachedPin) { + return cachedPin + } } const pincodeEntered = new Promise((resolve, reject) => { @@ -70,7 +72,9 @@ export function* getPincode() { if (!pin) { throw new Error('Pincode confirmation returned empty pin') } - setCachedPincode(pin) + if (useCache) { + setCachedPincode(pin) + } return pin } } diff --git a/packages/mobile/src/app/actions.ts b/packages/mobile/src/app/actions.ts index c18033f01e3..804cfaaab4d 100644 --- a/packages/mobile/src/app/actions.ts +++ b/packages/mobile/src/app/actions.ts @@ -1,3 +1,4 @@ +import { NavigationParams } from 'react-navigation' import i18n from 'src/i18n' import { navigate } from 'src/navigator/NavigationService' import { Screens } from 'src/navigator/Screens' @@ -14,6 +15,9 @@ export enum Actions { EXIT_BACKUP_FLOW = 'APP/EXIT_BACKUP_FLOW', SET_FEED_CACHE = 'APP/SET_FEED_CACHE', SET_ANALYTICS_ENABLED = 'APP/SET_ANALYTICS_ENABLED', + NAVIGATE_PIN_PROTECTED = 'APP/NAVIGATE_PIN_PROTECTED', + START_PIN_VERIFICATION = 'APP/START_PIN_VERIFICATION', + FINISH_PIN_VERIFICATION = 'APP/FINISH_PIN_VERIFICATION', } interface SetLoggedIn { @@ -48,6 +52,20 @@ interface SetAnalyticsEnabled { enabled: boolean } +export interface NavigatePinProtected { + type: Actions.NAVIGATE_PIN_PROTECTED + routeName: string + params?: NavigationParams +} + +interface StartPinVerification { + type: Actions.START_PIN_VERIFICATION +} + +interface FinishPinVerification { + type: Actions.FINISH_PIN_VERIFICATION +} + export type ActionTypes = | SetLoggedIn | SetNumberVerifiedAction @@ -56,6 +74,9 @@ export type ActionTypes = | EnterBackupFlow | ExitBackupFlow | SetAnalyticsEnabled + | NavigatePinProtected + | StartPinVerification + | FinishPinVerification export const setLoggedIn = (loggedIn: boolean) => ({ type: Actions.SET_LOGGED_IN, @@ -96,3 +117,20 @@ export const setAnalyticsEnabled = (enabled: boolean): SetAnalyticsEnabled => ({ type: Actions.SET_ANALYTICS_ENABLED, enabled, }) + +export const navigatePinProtected = ( + routeName: string, + params?: NavigationParams +): NavigatePinProtected => ({ + type: Actions.NAVIGATE_PIN_PROTECTED, + routeName, + params, +}) + +export const startPinVerification = (): StartPinVerification => ({ + type: Actions.START_PIN_VERIFICATION, +}) + +export const finishPinVerification = (): FinishPinVerification => ({ + type: Actions.FINISH_PIN_VERIFICATION, +}) diff --git a/packages/mobile/src/app/reducers.ts b/packages/mobile/src/app/reducers.ts index 149af1ac856..7d5e630ac36 100644 --- a/packages/mobile/src/app/reducers.ts +++ b/packages/mobile/src/app/reducers.ts @@ -1,4 +1,5 @@ import { Actions, ActionTypes } from 'src/app/actions' +import { getRehydratePayload, REHYDRATE, RehydrateAction } from 'src/redux/persist-helper' import { RootState } from 'src/redux/reducers' export interface State { @@ -6,6 +7,7 @@ export interface State { numberVerified: boolean language: string | null doingBackupFlow: boolean + doingPinVerification: boolean analyticsEnabled: boolean } @@ -15,13 +17,25 @@ const initialState = { numberVerified: false, language: null, doingBackupFlow: false, + doingPinVerification: false, analyticsEnabled: true, } export const currentLanguageSelector = (state: RootState) => state.app.language -export const appReducer = (state: State | undefined = initialState, action: ActionTypes): State => { +export const appReducer = ( + state: State | undefined = initialState, + action: ActionTypes | RehydrateAction +): State => { switch (action.type) { + case REHYDRATE: { + // Ignore some persisted properties + return { + ...state, + ...getRehydratePayload(action, 'app'), + doingPinVerification: false, + } + } case Actions.SET_LOGGED_IN: return { ...state, @@ -59,6 +73,16 @@ export const appReducer = (state: State | undefined = initialState, action: Acti ...state, analyticsEnabled: action.enabled, } + case Actions.START_PIN_VERIFICATION: + return { + ...state, + doingPinVerification: true, + } + case Actions.FINISH_PIN_VERIFICATION: + return { + ...state, + doingPinVerification: false, + } default: return state } diff --git a/packages/mobile/src/app/saga.test.ts b/packages/mobile/src/app/saga.test.ts index 9646ddc9641..25ddc0e2b34 100644 --- a/packages/mobile/src/app/saga.test.ts +++ b/packages/mobile/src/app/saga.test.ts @@ -1,12 +1,24 @@ import { REHYDRATE } from 'redux-persist/es/constants' import { expectSaga } from 'redux-saga-test-plan' -import { call } from 'redux-saga/effects' +import { call, select } from 'redux-saga/effects' import { PincodeType } from 'src/account/reducer' +import { getPincode } from 'src/account/saga' import CeloAnalytics from 'src/analytics/CeloAnalytics' -import { checkAppDeprecation, navigateToProperScreen, waitForRehydrate } from 'src/app/saga' +import { finishPinVerification, startPinVerification } from 'src/app/actions' +import { + checkAppDeprecation, + navigatePinProtected, + navigateToProperScreen, + waitForRehydrate, +} from 'src/app/saga' import { waitForFirebaseAuth } from 'src/firebase/saga' +import { UNLOCK_DURATION } from 'src/geth/consts' import { NavActions, navigate } from 'src/navigator/NavigationService' import { Screens, Stacks } from 'src/navigator/Screens' +import { web3 } from 'src/web3/contracts' +import { getAccount } from 'src/web3/saga' +import { zeroSyncSelector } from 'src/web3/selectors' + jest.mock('src/utils/time', () => ({ clockInSync: () => true, })) @@ -95,6 +107,29 @@ describe('Upload Comment Key Saga', () => { .run() expect(navigate).not.toHaveBeenCalled() }) + + it('Navigates after verifying PIN - Forno', async () => { + const testRoute = { routeName: 'test', params: { a: '1' } } + await expectSaga(navigatePinProtected, testRoute) + .provide([[select(zeroSyncSelector), true]]) + .run() + expect(navigate).toHaveBeenCalledWith(testRoute.routeName, testRoute.params) + }) + + it('Navigates after verifying PIN - Light node', async () => { + const testRoute = { routeName: 'test', params: { a: '1' } } + await expectSaga(navigatePinProtected, testRoute) + .provide([ + [select(zeroSyncSelector), false], + [call(getPincode, false), '123456'], + [call(getAccount), 'account'], + [call(web3.eth.personal.unlockAccount, 'account', '123456', UNLOCK_DURATION), undefined], + ]) + .put(startPinVerification()) + .put(finishPinVerification()) + .run() + expect(navigate).toHaveBeenCalledWith(testRoute.routeName, testRoute.params) + }) }) navigationSagaTest('Navigates to the nux stack with no state', null, Stacks.NuxStack) diff --git a/packages/mobile/src/app/saga.ts b/packages/mobile/src/app/saga.ts index 814086991ba..275893e3548 100644 --- a/packages/mobile/src/app/saga.ts +++ b/packages/mobile/src/app/saga.ts @@ -1,18 +1,29 @@ import { Linking } from 'react-native' import { REHYDRATE } from 'redux-persist/es/constants' -import { all, call, put, select, spawn, take } from 'redux-saga/effects' +import { all, call, put, select, spawn, take, takeLatest } from 'redux-saga/effects' import { PincodeType } from 'src/account/reducer' -import { setLanguage } from 'src/app/actions' +import { getPincode } from 'src/account/saga' +import { showError } from 'src/alert/actions' +import { + Actions, + finishPinVerification, + NavigatePinProtected, + setLanguage, + startPinVerification, +} from 'src/app/actions' +import { ErrorMessages } from 'src/app/ErrorMessages' import { handleDappkitDeepLink } from 'src/dappkit/dappkit' import { getVersionInfo } from 'src/firebase/firebase' import { waitForFirebaseAuth } from 'src/firebase/saga' +import { UNLOCK_DURATION } from 'src/geth/consts' import { NavActions, navigate } from 'src/navigator/NavigationService' import { Screens, Stacks } from 'src/navigator/Screens' import { PersistedRootState } from 'src/redux/reducers' import Logger from 'src/utils/Logger' import { clockInSync } from 'src/utils/time' import { setZeroSyncMode } from 'src/web3/actions' -import { isInitiallyZeroSyncMode } from 'src/web3/contracts' +import { isInitiallyZeroSyncMode, web3 } from 'src/web3/contracts' +import { getAccount } from 'src/web3/saga' import { zeroSyncSelector } from 'src/web3/selectors' const TAG = 'app/saga' @@ -130,8 +141,30 @@ export function handleDeepLink(deepLink: string) { // Other deep link handlers can go here later } +export function* navigatePinProtected(action: NavigatePinProtected) { + const zeroSyncMode = yield select(zeroSyncSelector) + try { + if (!zeroSyncMode) { + const pincode = yield call(getPincode, false) + yield put(startPinVerification()) + const account = yield call(getAccount) + yield call(web3.eth.personal.unlockAccount, account, pincode, UNLOCK_DURATION) + navigate(action.routeName, action.params) + yield put(finishPinVerification()) + } else { + // TODO: Implement PIN protection for forno (zeroSyncMode) + navigate(action.routeName, action.params) + } + } catch (error) { + Logger.error(TAG + '@showBackupAndRecovery', 'Incorrect pincode', error) + yield put(showError(ErrorMessages.INCORRECT_PIN)) + yield put(finishPinVerification()) + } +} + export function* appSaga() { yield spawn(checkAppDeprecation) yield spawn(navigateToProperScreen) yield spawn(toggleToProperSyncMode) + yield takeLatest(Actions.NAVIGATE_PIN_PROTECTED, navigatePinProtected) } diff --git a/packages/mobile/src/backup/BackupIntroduction.test.tsx b/packages/mobile/src/backup/BackupIntroduction.test.tsx index 9768665e877..5a2b376dcf0 100644 --- a/packages/mobile/src/backup/BackupIntroduction.test.tsx +++ b/packages/mobile/src/backup/BackupIntroduction.test.tsx @@ -68,4 +68,16 @@ describe('BackupIntroduction', () => { ) expect(tree).toMatchSnapshot() }) + it('renders correctly when pin verification is in-progress', () => { + const tree = renderer.create( + + + + ) + expect(tree).toMatchSnapshot() + }) }) diff --git a/packages/mobile/src/backup/BackupIntroduction.tsx b/packages/mobile/src/backup/BackupIntroduction.tsx index 0578cb4b532..d619ce0d7b6 100644 --- a/packages/mobile/src/backup/BackupIntroduction.tsx +++ b/packages/mobile/src/backup/BackupIntroduction.tsx @@ -3,14 +3,14 @@ import colors from '@celo/react-components/styles/colors' import { fontStyles } from '@celo/react-components/styles/fonts' import * as React from 'react' import { WithNamespaces, withNamespaces } from 'react-i18next' -import { Image, ScrollView, StyleSheet, Text, View } from 'react-native' +import { ActivityIndicator, Image, ScrollView, StyleSheet, Text, View } from 'react-native' import SafeAreaView from 'react-native-safe-area-view' import { connect } from 'react-redux' import { setBackupDelayed } from 'src/account/actions' import CeloAnalytics from 'src/analytics/CeloAnalytics' import { CustomEventNames } from 'src/analytics/constants' import componentWithAnalytics from 'src/analytics/wrapper' -import { enterBackupFlow, exitBackupFlow } from 'src/app/actions' +import { enterBackupFlow, exitBackupFlow, navigatePinProtected } from 'src/app/actions' import { Namespaces } from 'src/i18n' import backupIcon from 'src/images/backup-icon.png' import { headerWithBackButton } from 'src/navigator/Headers' @@ -24,12 +24,14 @@ interface StateProps { socialBackupCompleted: boolean backupTooLate: boolean backupDelayedTime: number + doingPinVerification: boolean } interface DispatchProps { setBackupDelayed: typeof setBackupDelayed enterBackupFlow: typeof enterBackupFlow exitBackupFlow: typeof exitBackupFlow + navigatePinProtected: typeof navigatePinProtected } type Props = WithNamespaces & StateProps & DispatchProps @@ -40,6 +42,7 @@ const mapStateToProps = (state: RootState): StateProps => { socialBackupCompleted: state.account.socialBackupCompleted, backupTooLate: isBackupTooLate(state), backupDelayedTime: state.account.backupDelayedTime, + doingPinVerification: state.app.doingPinVerification, } } @@ -58,12 +61,12 @@ class BackupIntroduction extends React.Component { onPressViewBackupKey = () => { CeloAnalytics.track(CustomEventNames.view_backup_phrase) - navigate(Screens.BackupPhrase) + this.props.navigatePinProtected(Screens.BackupPhrase) } onPressBackup = () => { CeloAnalytics.track(CustomEventNames.set_backup_phrase) - navigate(Screens.BackupPhrase) + this.props.navigatePinProtected(Screens.BackupPhrase) } onPressSetupSocialBackup = () => { @@ -73,7 +76,7 @@ class BackupIntroduction extends React.Component { onPressViewSocialBackup = () => { CeloAnalytics.track(CustomEventNames.view_social_backup) - navigate(Screens.BackupSocial) + this.props.navigatePinProtected(Screens.BackupSocial) } onPressDelay = () => { @@ -89,6 +92,7 @@ class BackupIntroduction extends React.Component { backupTooLate, backupCompleted, socialBackupCompleted, + doingPinVerification, } = this.props return ( @@ -125,6 +129,9 @@ class BackupIntroduction extends React.Component { )} + {doingPinVerification && ( + + )} {!backupCompleted && ( <> @@ -213,6 +220,9 @@ const styles = StyleSheet.create({ textAlign: 'center', paddingBottom: 15, }, + loader: { + marginBottom: 20, + }, }) export default componentWithAnalytics( @@ -222,6 +232,7 @@ export default componentWithAnalytics( setBackupDelayed, enterBackupFlow, exitBackupFlow, + navigatePinProtected, } )(withNamespaces(Namespaces.backupKeyFlow6)(BackupIntroduction)) ) diff --git a/packages/mobile/src/backup/BackupSocialIntro.test.tsx b/packages/mobile/src/backup/BackupSocialIntro.test.tsx index 8c5281a0547..01524a04037 100644 --- a/packages/mobile/src/backup/BackupSocialIntro.test.tsx +++ b/packages/mobile/src/backup/BackupSocialIntro.test.tsx @@ -10,7 +10,7 @@ describe('BackupSocialIntro', () => { const navigation = createMockNavigationProp(false) const tree = renderer.create( - + ) expect(tree).toMatchSnapshot() diff --git a/packages/mobile/src/backup/BackupSocialIntro.tsx b/packages/mobile/src/backup/BackupSocialIntro.tsx index 312a18a3d74..43d4ddef689 100644 --- a/packages/mobile/src/backup/BackupSocialIntro.tsx +++ b/packages/mobile/src/backup/BackupSocialIntro.tsx @@ -3,14 +3,14 @@ import colors from '@celo/react-components/styles/colors' import { fontStyles } from '@celo/react-components/styles/fonts' import * as React from 'react' import { WithNamespaces, withNamespaces } from 'react-i18next' -import { Image, ScrollView, StyleSheet, Text } from 'react-native' +import { ActivityIndicator, Image, ScrollView, StyleSheet, Text } from 'react-native' import SafeAreaView from 'react-native-safe-area-view' import { NavigationInjectedProps } from 'react-navigation' import { connect } from 'react-redux' import CeloAnalytics from 'src/analytics/CeloAnalytics' import { CustomEventNames } from 'src/analytics/constants' import componentWithAnalytics from 'src/analytics/wrapper' -import { exitBackupFlow } from 'src/app/actions' +import { exitBackupFlow, navigatePinProtected } from 'src/app/actions' import { Namespaces } from 'src/i18n' import backupIcon from 'src/images/backup-icon.png' import { headerWithBackButton } from 'src/navigator/Headers' @@ -18,15 +18,26 @@ import { navigate, navigateHome } from 'src/navigator/NavigationService' import { Screens } from 'src/navigator/Screens' import { RootState } from 'src/redux/reducers' +interface StateProps { + doingPinVerification: boolean +} + interface DispatchProps { exitBackupFlow: typeof exitBackupFlow + navigatePinProtected: typeof navigatePinProtected } interface NavigationProps { incomingFromBackupFlow: boolean } -type Props = WithNamespaces & DispatchProps & NavigationInjectedProps +type Props = WithNamespaces & StateProps & DispatchProps & NavigationInjectedProps + +const mapStateToProps = (state: RootState): StateProps => { + return { + doingPinVerification: state.app.doingPinVerification, + } +} class BackupSocialIntro extends React.Component { static navigationOptions = () => ({ @@ -38,7 +49,10 @@ class BackupSocialIntro extends React.Component { } onPressContinue = () => { - navigate(Screens.BackupSocial) + const navigateMethod = this.isIncomingFromBackupFlow() + ? navigate + : this.props.navigatePinProtected + navigateMethod(Screens.BackupSocial) } onPressSkip = () => { @@ -48,7 +62,7 @@ class BackupSocialIntro extends React.Component { } render() { - const { t } = this.props + const { t, doingPinVerification } = this.props return ( @@ -57,6 +71,9 @@ class BackupSocialIntro extends React.Component { {t('socialBackupIntro.body')} {t('socialBackupIntro.warning')} + {doingPinVerification && ( + + )} <>