From 3588479106503e8316c7bc9e767cc5e494a8832c Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 4 Aug 2020 12:28:04 -0400 Subject: [PATCH 01/31] WIP make-release --- .../protocol/scripts/truffle/make-release.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 packages/protocol/scripts/truffle/make-release.ts diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts new file mode 100644 index 00000000000..39d4e641117 --- /dev/null +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -0,0 +1,60 @@ +/* tslint:disable:no-console */ +import assert = require('assert') +import { readJsonSync } from 'fs-extra' + +import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' + +/* + * A script that reads a backwards compatibility report, deploys changed contracts, and creates + * a corresponding JSON file to be proposed with `celocli governance:propose` + * + * Expects the following flags: + * report: The filepath of the backwards compatibility report + * network: The network for which artifacts should be + * + * Run using truffle exec, e.g.: + * truffle exec scripts/truffle/make-release --report TODO + * + */ + +/* + * For each change in the compatibility report: + * deploy new implementation + * if storage change + * deploy new proxy + * transfer ownership to governance contract (needed?) + * add setAndInitializeImplementation template call to governance proposal + * add registry repointing call to governance proposal + * else + * find existing proxy address (how to do when not registered?) + * add setImplementation call to governance proposal + */ +module.exports = async (callback: (error?: any) => number) => { + try { + const argv = require('minimist')(process.argv.slice(2), { + string: ['report', 'network'], + }) + const report = readJsonSync(argv.report) + const proposal = [] + for (let contract of Object.keys(report.contracts)) { + console.log(`Deploying ${contract}`) + const Contract = artifacts.require(contract) + deployer.deploy(Contract) + const deployedContract = await Contract.deployed() + if (report.contracts[contract].changes.storage.length === 0) { + deployer.then(async () => { + proposal.append({ + contract: `${contract}Proxy`, + function: '_setImplementation', + args: [deployedContract.address], + value: '0', + }) + }) + } else { + } + } + callback() + } catch (error) { + callback(error) + } +} From 6c4ba95a4ed7b0da9bfd3435eecf2c2265ca0ffc Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 4 Aug 2020 16:38:26 -0400 Subject: [PATCH 02/31] Somewhat functional --- packages/contractkit/src/base.ts | 24 +------- .../protocol/scripts/truffle/make-release.ts | 57 ++++++++++++------- packages/protocol/truffle-config.js | 2 +- 3 files changed, 40 insertions(+), 43 deletions(-) diff --git a/packages/contractkit/src/base.ts b/packages/contractkit/src/base.ts index 4eec1b91ab5..ccae542622d 100644 --- a/packages/contractkit/src/base.ts +++ b/packages/contractkit/src/base.ts @@ -26,29 +26,7 @@ export enum CeloContract { Validators = 'Validators', } -export const ProxyContracts = [ - 'AccountsProxy', - 'AttestationsProxy', - 'BlockchainParametersProxy', - 'DoubleSigningSlasherProxy', - 'DowntimeSlasherProxy', - 'ElectionProxy', - 'EpochRewardsProxy', - 'EscrowProxy', - 'ExchangeProxy', - 'FeeCurrencyWhitelistProxy', - 'FreezerProxy', - 'GasPriceMinimumProxy', - 'GoldTokenProxy', - 'GovernanceApproverMultiSigProxy', - 'GovernanceProxy', - 'LockedGoldProxy', - 'ReserveProxy', - 'ReserveSpenderMultiSigProxy', - 'StableTokenProxy', - 'SortedOraclesProxy', - 'RegistryProxy', -] +export const ProxyContracts = Object.keys(CeloContract).map((c) => `${c}Proxy`) export type CeloToken = CeloContract.GoldToken | CeloContract.StableToken diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 39d4e641117..f4f0ae2359e 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -1,8 +1,8 @@ /* tslint:disable:no-console */ -import assert = require('assert') -import { readJsonSync } from 'fs-extra' +import { readJsonSync, writeJsonSync } from 'fs-extra' +import { ASTDetailedVersionedReport } from '@celo/protocol/lib/compatibility/report' -import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' +// import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' /* * A script that reads a backwards compatibility report, deploys changed contracts, and creates @@ -18,6 +18,7 @@ import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' */ /* + * Ensure libraries are linked * For each change in the compatibility report: * deploy new implementation * if storage change @@ -26,33 +27,51 @@ import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' * add setAndInitializeImplementation template call to governance proposal * add registry repointing call to governance proposal * else - * find existing proxy address (how to do when not registered?) + * find existing proxy address (how to do when not registered?) - not needed for proposal. * add setImplementation call to governance proposal */ module.exports = async (callback: (error?: any) => number) => { try { const argv = require('minimist')(process.argv.slice(2), { - string: ['report', 'network'], + string: ['report', 'network', 'proposal'], }) - const report = readJsonSync(argv.report) + const report: ASTDetailedVersionedReport = readJsonSync(argv.report).report + const registry = await artifacts + .require('Registry') + .at('0x000000000000000000000000000000000000ce10') + const governanceAddress = await registry.getAddressForString('Governance') const proposal = [] - for (let contract of Object.keys(report.contracts)) { - console.log(`Deploying ${contract}`) - const Contract = artifacts.require(contract) - deployer.deploy(Contract) - const deployedContract = await Contract.deployed() - if (report.contracts[contract].changes.storage.length === 0) { - deployer.then(async () => { - proposal.append({ - contract: `${contract}Proxy`, - function: '_setImplementation', - args: [deployedContract.address], - value: '0', - }) + for (let contractName of Object.keys(report.contracts)) { + console.log(`Deploying ${contractName}`) + const Contract = artifacts.require(contractName) + const contract = await Contract.new() + if (report.contracts[contractName].changes.storage.length === 0) { + proposal.push({ + contract: `${contractName}Proxy`, + function: '_setImplementation', + args: [contract.address], + value: '0', }) } else { + const Proxy = artifacts.require(`${contractName}Proxy`) + const proxy = await Proxy.new() + await proxy._transferOwnership(governanceAddress) + proposal.push({ + contract: 'Registry', + function: 'setAddressFor', + args: [web3.utils.soliditySha3({ type: 'string', value: contractName }), proxy.address], + value: '0', + }) + proposal.push({ + contract: `${contractName}Proxy`, + function: '_setAndInitializeImplementation', + args: [contract.address, 'data: POPULATE ME'], + value: '0', + }) + // process.exit(1) } } + writeJsonSync(argv.proposal, proposal, { spaces: 2 }) callback() } catch (error) { callback(error) diff --git a/packages/protocol/truffle-config.js b/packages/protocol/truffle-config.js index df31f79cd3d..683629f0a0a 100644 --- a/packages/protocol/truffle-config.js +++ b/packages/protocol/truffle-config.js @@ -37,7 +37,7 @@ const BAKLAVA_FROM = '0x0Cc59Ed03B3e763c02d54D695FFE353055f1502D' const BAKLAVASTAGING_FROM = '0x4588ABb84e1BBEFc2BcF4b2296F785fB7AD9F285' // Gas limit is doubled for initial contract deployment. -const gasLimit = argv.reset ? 20000000 : 10000000 +const gasLimit = argv.reset ? 20000000 : 12500000 const defaultConfig = { host: '127.0.0.1', From 58755737106af0f3fc919f9901b6efe880f4911d Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 11 Aug 2020 12:48:51 -0400 Subject: [PATCH 03/31] Smarter releases WIP --- .../protocol/scripts/truffle/make-release.ts | 165 +++++++++++++----- 1 file changed, 126 insertions(+), 39 deletions(-) diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index f4f0ae2359e..0e4510d9a98 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -1,6 +1,7 @@ /* tslint:disable:no-console */ import { readJsonSync, writeJsonSync } from 'fs-extra' import { ASTDetailedVersionedReport } from '@celo/protocol/lib/compatibility/report' +import { linkedLibraries } from '@celo/protocol/migrationsConfig' // import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' @@ -18,59 +19,145 @@ import { ASTDetailedVersionedReport } from '@celo/protocol/lib/compatibility/rep */ /* - * Ensure libraries are linked - * For each change in the compatibility report: - * deploy new implementation - * if storage change - * deploy new proxy - * transfer ownership to governance contract (needed?) - * add setAndInitializeImplementation template call to governance proposal - * add registry repointing call to governance proposal - * else - * find existing proxy address (how to do when not registered?) - not needed for proposal. - * add setImplementation call to governance proposal +const linkLibraries = async () => { + // How to get libAddress??? + const libAddress = '0x123456789009876543211234567890098765432112345678900987654321' + Object.keys(linkedLibraries).forEach(async (lib: string) => { + const Contracts = linkedLibraries[lib].map((contract: string) => artifacts.require(contract)) + await Promise.all(Contracts.map((C) => C.link(lib, libAddress)) + }) +} +*/ + +class ContractDependencies { + dependencies: Map + constructor(linkedLibraries: any) { + this.dependencies = new Map() + Object.keys(linkedLibraries).forEach((lib: string) => { + linkedLibraries[lib].forEach((contract: string) => { + if (this.dependencies.has(contract)) { + this.dependencies.get(contract).push(lib) + } else { + this.dependencies.set(contract, [lib]) + } + }) + }) + } + + public get = (contract: string): string[] => { + return this.dependencies.has(contract) ? this.dependencies.get(contract) : [] + } +} + +class ContractAddresses { + dependencies: Map + constructor(contracts: string[], registry: Registry) { + this.addresses = new Map() + contracts.forEach((contract: string) => { + const registeredAddress = await registry.getAddressForString(contract) + if (!eqAddress(registeredAddress, nullAddress)) { + this.addresses.set(contract, registeredAddress) + } + }) + } + + public get = (contract: string): string[] => { + if (this.addresses.has(contract)) { + return this.addresses.get(contract) + } else { + throw new Error(`Unable to find address for ${contract}`) + } + } + + public set = (contract: string, address: Address) => { + this.addresses.set(contract, address) + } +} + +/* + * TOOD: + * Ensure libraries are linked + * First, create a dependency graph: + * For each contract in the build directory, see if it links a library + * Second, create a mapping of contract -> address: + * Fetch from registry, when possible + * Otherwise, read from a file + * Then, do the following recursion: + * if the contract has been marked as upgraded, nothing + * else if the contract has not been marked as upgraded: + * upgrade each of its dependencies + * link libraries in the contract + * upgrade the contract + * set the contract -> address mapping + * Figure out setAndInitialize data */ module.exports = async (callback: (error?: any) => number) => { try { const argv = require('minimist')(process.argv.slice(2), { - string: ['report', 'network', 'proposal'], + string: ['report', 'network', 'proposal', 'libraries', 'build_directory'], }) const report: ASTDetailedVersionedReport = readJsonSync(argv.report).report + const dependencies = new ContractDependencies(linkedLibraries) + const contracts = fs.readdirSync(argv.build_directory).map((x) => path.basename(x)) const registry = await artifacts .require('Registry') .at('0x000000000000000000000000000000000000ce10') - const governanceAddress = await registry.getAddressForString('Governance') + const addresses = new ContractAddresses(contracts, registry) + const released: Set = new Set([]) const proposal = [] - for (let contractName of Object.keys(report.contracts)) { - console.log(`Deploying ${contractName}`) - const Contract = artifacts.require(contractName) - const contract = await Contract.new() - if (report.contracts[contractName].changes.storage.length === 0) { - proposal.push({ - contract: `${contractName}Proxy`, - function: '_setImplementation', - args: [contract.address], - value: '0', - }) + const release = async (contractName: string) => { + console.log(`Releasing ${contractName}`) + if (released.has(contractName)) { + console.log(`Already released ${contractName}`) + return } else { - const Proxy = artifacts.require(`${contractName}Proxy`) - const proxy = await Proxy.new() - await proxy._transferOwnership(governanceAddress) - proposal.push({ - contract: 'Registry', - function: 'setAddressFor', - args: [web3.utils.soliditySha3({ type: 'string', value: contractName }), proxy.address], - value: '0', + // 1. Release all dependencies. + const contractDependencies = dependencies.get(contractName) + contractDependencies.forEach(async (dependency: string) => { + await release(dependency) }) - proposal.push({ - contract: `${contractName}Proxy`, - function: '_setAndInitializeImplementation', - args: [contract.address, 'data: POPULATE ME'], - value: '0', - }) - // process.exit(1) + // 2. Link dependencies. + const Contract = artifacts.require(contractName) + await Promise.all(contractDependencies.map((d) => Contract.link(d, addresses.get(d)))) + // 3. Deploy new versions of the contract and proxy, if needed. + if (Object.keys(report.contracts).includes(contractName)) { + console.log(`Deploying implementation of ${contractName}`) + const contract = await Contract.new() + proposal.push({ + contract: `${contractName}Proxy`, + function: '_setImplementation', + args: [contract.address], + value: '0', + }) + if (report.contracts[contractName].changes.storage.length) { + console.log(`Deploying proxy for ${contractName}`) + const Proxy = artifacts.require(`${contractName}Proxy`) + const proxy = await Proxy.new() + proposal.push({ + contract: 'Registry', + function: 'setAddressFor', + args: [ + web3.utils.soliditySha3({ type: 'string', value: contractName }), + proxy.address, + ], + value: '0', + }) + // TODO(asa): Need to actually pass something here... + await proxy._setAndInitializeImplementation('0x') + // TODO(asa): This makes essentially every contract dependent on Governance. + // How to handle? + await proxy._transferOwnership(addresses.get('Governance')) + // 4. Update the contract's address, if needed. + addresses.set(contractName, proxy.address) + } + } + // 5. Mark the contract as released + released.add(contractName) } } + contracts.forEach(async (contractName: string) => { + await release(contractName) + }) writeJsonSync(argv.proposal, proposal, { spaces: 2 }) callback() } catch (error) { From 8b6ea77dab5c825ad9db67501a4647f41fcd23e3 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 11 Aug 2020 15:38:14 -0400 Subject: [PATCH 04/31] Add library proxies --- .../contracts/common/proxies/AddressLinkedListProxy.sol | 6 ++++++ .../common/proxies/AddressSortedLinkedListProxy.sol | 6 ++++++ .../proxies/AddressSortedLinkedListWithMedianProxy.sol | 6 ++++++ .../protocol/contracts/common/proxies/FixidityLibProxy.sol | 6 ++++++ .../common/proxies/IntegerSortedLinkedListProxy.sol | 6 ++++++ .../protocol/contracts/common/proxies/LinkedListProxy.sol | 6 ++++++ .../protocol/contracts/common/proxies/SignaturesProxy.sol | 6 ++++++ .../contracts/common/proxies/SortedLinkedListProxy.sol | 6 ++++++ .../common/proxies/SortedLinkedListWithMedianProxy.sol | 6 ++++++ 9 files changed, 54 insertions(+) create mode 100644 packages/protocol/contracts/common/proxies/AddressLinkedListProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/AddressSortedLinkedListProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/AddressSortedLinkedListWithMedianProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/FixidityLibProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/IntegerSortedLinkedListProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/LinkedListProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/SignaturesProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol create mode 100644 packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol diff --git a/packages/protocol/contracts/common/proxies/AddressLinkedListProxy.sol b/packages/protocol/contracts/common/proxies/AddressLinkedListProxy.sol new file mode 100644 index 00000000000..f6eeda126cd --- /dev/null +++ b/packages/protocol/contracts/common/proxies/AddressLinkedListProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract AddressLinkedListProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/AddressSortedLinkedListProxy.sol b/packages/protocol/contracts/common/proxies/AddressSortedLinkedListProxy.sol new file mode 100644 index 00000000000..289c5a99bf0 --- /dev/null +++ b/packages/protocol/contracts/common/proxies/AddressSortedLinkedListProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract AddressSortedLinkedListProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/AddressSortedLinkedListWithMedianProxy.sol b/packages/protocol/contracts/common/proxies/AddressSortedLinkedListWithMedianProxy.sol new file mode 100644 index 00000000000..d7d101fab12 --- /dev/null +++ b/packages/protocol/contracts/common/proxies/AddressSortedLinkedListWithMedianProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract AddressSortedLinkedListWithMedianProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/FixidityLibProxy.sol b/packages/protocol/contracts/common/proxies/FixidityLibProxy.sol new file mode 100644 index 00000000000..41424ca8954 --- /dev/null +++ b/packages/protocol/contracts/common/proxies/FixidityLibProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract FixidityLibProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/IntegerSortedLinkedListProxy.sol b/packages/protocol/contracts/common/proxies/IntegerSortedLinkedListProxy.sol new file mode 100644 index 00000000000..6d4b93bf9c2 --- /dev/null +++ b/packages/protocol/contracts/common/proxies/IntegerSortedLinkedListProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract IntegerSortedLinkedListProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/LinkedListProxy.sol b/packages/protocol/contracts/common/proxies/LinkedListProxy.sol new file mode 100644 index 00000000000..5804cb74827 --- /dev/null +++ b/packages/protocol/contracts/common/proxies/LinkedListProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract LinkedListProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/SignaturesProxy.sol b/packages/protocol/contracts/common/proxies/SignaturesProxy.sol new file mode 100644 index 00000000000..a618258cfbb --- /dev/null +++ b/packages/protocol/contracts/common/proxies/SignaturesProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract SignaturesProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol b/packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol new file mode 100644 index 00000000000..d6c00b7a139 --- /dev/null +++ b/packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract SortedLinkedListProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol b/packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol new file mode 100644 index 00000000000..2b6dbfa9a43 --- /dev/null +++ b/packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract SortedLinkedListWithMedianProxy is Proxy {} From 29a2d561ed0c9508624d456acc4e6af507ae57c4 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 11 Aug 2020 15:39:05 -0400 Subject: [PATCH 05/31] Missing proxy --- .../contracts/governance/proxies/ProposalsProxy.sol | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 packages/protocol/contracts/governance/proxies/ProposalsProxy.sol diff --git a/packages/protocol/contracts/governance/proxies/ProposalsProxy.sol b/packages/protocol/contracts/governance/proxies/ProposalsProxy.sol new file mode 100644 index 00000000000..90b2aba044d --- /dev/null +++ b/packages/protocol/contracts/governance/proxies/ProposalsProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.3; + +import "../../common/Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract ProposalsProxy is Proxy {} From 220c76e733de0720d076eda945694a237f0c17c5 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 11 Aug 2020 16:16:11 -0400 Subject: [PATCH 06/31] Make it work with upgradable libraries --- .../scripts/truffle/deployedGrants.json | 2 - .../protocol/scripts/truffle/make-release.ts | 123 +++++++++++------- .../truffle/releaseGoldExampleConfigs.json | 34 ----- 3 files changed, 76 insertions(+), 83 deletions(-) delete mode 100644 packages/protocol/scripts/truffle/deployedGrants.json delete mode 100644 packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json diff --git a/packages/protocol/scripts/truffle/deployedGrants.json b/packages/protocol/scripts/truffle/deployedGrants.json deleted file mode 100644 index 0d4f101c7a3..00000000000 --- a/packages/protocol/scripts/truffle/deployedGrants.json +++ /dev/null @@ -1,2 +0,0 @@ -[ -] diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 0e4510d9a98..5aa8d45f270 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -1,7 +1,10 @@ /* tslint:disable:no-console */ -import { readJsonSync, writeJsonSync } from 'fs-extra' +import { setAndInitializeImplementation } from '@celo/protocol/lib/proxy-utils' +import { readdirSync, readJsonSync, writeJsonSync } from 'fs-extra' import { ASTDetailedVersionedReport } from '@celo/protocol/lib/compatibility/report' +import { Address, eqAddress, NULL_ADDRESS } from '@celo/utils/lib/address' import { linkedLibraries } from '@celo/protocol/migrationsConfig' +import { basename, join } from 'path' // import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' @@ -50,18 +53,19 @@ class ContractDependencies { } class ContractAddresses { - dependencies: Map - constructor(contracts: string[], registry: Registry) { - this.addresses = new Map() - contracts.forEach((contract: string) => { + constructor(public addresses: Map) {} + static async create(contracts: string[], registry: any) { + const addresses = new Map() + contracts.forEach(async (contract: string) => { const registeredAddress = await registry.getAddressForString(contract) - if (!eqAddress(registeredAddress, nullAddress)) { - this.addresses.set(contract, registeredAddress) + if (!eqAddress(registeredAddress, NULL_ADDRESS)) { + addresses.set(contract, registeredAddress) } }) + return new ContractAddresses(addresses) } - public get = (contract: string): string[] => { + public get = (contract: string): Address => { if (this.addresses.has(contract)) { return this.addresses.get(contract) } else { @@ -75,20 +79,10 @@ class ContractAddresses { } /* - * TOOD: - * Ensure libraries are linked - * First, create a dependency graph: - * For each contract in the build directory, see if it links a library - * Second, create a mapping of contract -> address: - * Fetch from registry, when possible - * Otherwise, read from a file - * Then, do the following recursion: - * if the contract has been marked as upgraded, nothing - * else if the contract has not been marked as upgraded: - * upgrade each of its dependencies - * link libraries in the contract - * upgrade the contract - * set the contract -> address mapping + * TODO: + * Deal with libraries: + * For the first iteration of this tool, all libraries should be re-deployed. - DONE + * Should check that all contracts linking libraries have changes.- NOT DONE * Figure out setAndInitialize data */ module.exports = async (callback: (error?: any) => number) => { @@ -98,41 +92,75 @@ module.exports = async (callback: (error?: any) => number) => { }) const report: ASTDetailedVersionedReport = readJsonSync(argv.report).report const dependencies = new ContractDependencies(linkedLibraries) - const contracts = fs.readdirSync(argv.build_directory).map((x) => path.basename(x)) + const contracts = readdirSync(join(argv.build_directory, 'contracts')).map((x) => + basename(x, '.json') + ) const registry = await artifacts .require('Registry') .at('0x000000000000000000000000000000000000ce10') - const addresses = new ContractAddresses(contracts, registry) + const addresses = await ContractAddresses.create(contracts, registry) const released: Set = new Set([]) const proposal = [] const release = async (contractName: string) => { - console.log(`Releasing ${contractName}`) if (released.has(contractName)) { - console.log(`Already released ${contractName}`) return } else { // 1. Release all dependencies. const contractDependencies = dependencies.get(contractName) - contractDependencies.forEach(async (dependency: string) => { + for (const dependency of contractDependencies) { await release(dependency) - }) + } // 2. Link dependencies. - const Contract = artifacts.require(contractName) + const Contract = await artifacts.require(contractName) await Promise.all(contractDependencies.map((d) => Contract.link(d, addresses.get(d)))) + + // TODO(asa): Redeploying all libraries is only needed for release 1. + // Remove `isLibrary` for future releases. + const isLibrary = Object.keys(linkedLibraries).includes(contractName) + const deployImplementation = Object.keys(report.contracts).includes(contractName) + const deployProxy = + deployImplementation && report.contracts[contractName].changes.storage.length + // 3. Deploy new versions of the contract and proxy, if needed. - if (Object.keys(report.contracts).includes(contractName)) { + if (deployImplementation || isLibrary) { console.log(`Deploying implementation of ${contractName}`) const contract = await Contract.new() - proposal.push({ - contract: `${contractName}Proxy`, - function: '_setImplementation', - args: [contract.address], - value: '0', - }) - if (report.contracts[contractName].changes.storage.length) { + if (deployProxy || isLibrary) { console.log(`Deploying proxy for ${contractName}`) - const Proxy = artifacts.require(`${contractName}Proxy`) + const Proxy = await artifacts.require(`${contractName}Proxy`) const proxy = await Proxy.new() + if (isLibrary) { + await proxy._setImplementation(contract.address) + } else { + // TODO(asa): This is a hack! + const initializerAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'initialize' + ) + const args = ['0x000000000000000000000000000000000000ce10', 10, 5, 20] + await setAndInitializeImplementation( + web3, + proxy, + contract.address, + initializerAbi, + {}, + ...args + ) + } + // TODO(asa): This makes essentially every contract dependent on Governance. + // How to handle? + await proxy._transferOwnership(addresses.get('Governance')) + const proxiedContract = await artifacts.require(contractName).at(proxy.address) + // TODO(asa): How to deal with non-ownable + const transferOwnershipAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'transferOwnership' + ) + if (transferOwnershipAbi) { + await proxiedContract.transferOwnership(addresses.get('Governance')) + } else { + console.log(`Unable to transfer ownership for ${contractName}`) + } + // 4. Update the contract's address, if needed. + addresses.set(contractName, proxy.address) proposal.push({ contract: 'Registry', function: 'setAddressFor', @@ -141,23 +169,24 @@ module.exports = async (callback: (error?: any) => number) => { proxy.address, ], value: '0', + description: `Registry: ${contractName} -> ${proxy.address}`, + }) + } else { + proposal.push({ + contract: `${contractName}Proxy`, + function: '_setImplementation', + args: [contract.address], + value: '0', }) - // TODO(asa): Need to actually pass something here... - await proxy._setAndInitializeImplementation('0x') - // TODO(asa): This makes essentially every contract dependent on Governance. - // How to handle? - await proxy._transferOwnership(addresses.get('Governance')) - // 4. Update the contract's address, if needed. - addresses.set(contractName, proxy.address) } } // 5. Mark the contract as released released.add(contractName) } } - contracts.forEach(async (contractName: string) => { + for (const contractName of contracts) { await release(contractName) - }) + } writeJsonSync(argv.proposal, proposal, { spaces: 2 }) callback() } catch (error) { diff --git a/packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json b/packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json deleted file mode 100644 index 9abcff822be..00000000000 --- a/packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json +++ /dev/null @@ -1,34 +0,0 @@ -[ - { - "identifier": "0x111111111", - "releaseStartTime": "MAINNET", - "releaseCliffTime": 0, - "numReleasePeriods": 1, - "releasePeriod": 100000000, - "amountReleasedPerPeriod": 10002, - "revocable": false, - "beneficiary": "0x5409ED021D9299bf6814279A6A1411A7e866A631", - "releaseOwner": "0x6Ecbe1DB9EF729CBe972C83Fb886247691Fb6beb", - "refundAddress": "0x0000000000000000000000000000000000000000", - "subjectToLiquidityProvision": true, - "initialDistributionRatio": 1000, - "canVote": true, - "canValidate": true - }, - { - "identifier": "0x222222222", - "releaseStartTime": "MAINNET", - "releaseCliffTime": 0, - "numReleasePeriods": 1, - "releasePeriod": 100000000, - "amountReleasedPerPeriod": 2, - "revocable": true, - "beneficiary": "0x6Ecbe1DB9EF729CBe972C83Fb886247691Fb6beb", - "releaseOwner": "0x5409ED021D9299bf6814279A6A1411A7e866A631", - "refundAddress": "0x5409ED021D9299bf6814279A6A1411A7e866A631", - "subjectToLiquidityProvision": false, - "initialDistributionRatio": 1000, - "canVote": true, - "canValidate": false - } -] From 3326ccff2b25f480594b88d6f4e06951aeedd3ba Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 13 Aug 2020 13:12:40 -0400 Subject: [PATCH 07/31] Read initialize data json --- .../protocol/scripts/truffle/make-release.ts | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 5aa8d45f270..86a3d9379a2 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -78,19 +78,13 @@ class ContractAddresses { } } -/* - * TODO: - * Deal with libraries: - * For the first iteration of this tool, all libraries should be re-deployed. - DONE - * Should check that all contracts linking libraries have changes.- NOT DONE - * Figure out setAndInitialize data - */ module.exports = async (callback: (error?: any) => number) => { try { const argv = require('minimist')(process.argv.slice(2), { - string: ['report', 'network', 'proposal', 'libraries', 'build_directory'], + string: ['report', 'network', 'proposal', 'libraries', 'initialize_data', 'build_directory'], }) const report: ASTDetailedVersionedReport = readJsonSync(argv.report).report + const initializationData = readJsonSync(argv.initialize_data) const dependencies = new ContractDependencies(linkedLibraries) const contracts = readdirSync(join(argv.build_directory, 'contracts')).map((x) => basename(x, '.json') @@ -120,44 +114,45 @@ module.exports = async (callback: (error?: any) => number) => { const deployImplementation = Object.keys(report.contracts).includes(contractName) const deployProxy = deployImplementation && report.contracts[contractName].changes.storage.length - // 3. Deploy new versions of the contract and proxy, if needed. if (deployImplementation || isLibrary) { - console.log(`Deploying implementation of ${contractName}`) + console.log(`Deploying ${contractName}`) const contract = await Contract.new() if (deployProxy || isLibrary) { - console.log(`Deploying proxy for ${contractName}`) + console.log(`Deploying ${contractName}Proxy`) const Proxy = await artifacts.require(`${contractName}Proxy`) const proxy = await Proxy.new() - if (isLibrary) { - await proxy._setImplementation(contract.address) - } else { - // TODO(asa): This is a hack! - const initializerAbi = (contract as any).abi.find( - (abi: any) => abi.type === 'function' && abi.name === 'initialize' - ) - const args = ['0x000000000000000000000000000000000000ce10', 10, 5, 20] + const initializeAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'initialize' + ) + console.log(`Setting ${contractName}Proxy implementation to ${contract.address}`) + if (initializeAbi) { + const args = initializationData[contractName] + console.log(`Initializing ${contractName} with: ${args}`) await setAndInitializeImplementation( web3, proxy, contract.address, - initializerAbi, + initializeAbi, {}, ...args ) + } else { + await proxy._setImplementation(contract.address) } // TODO(asa): This makes essentially every contract dependent on Governance. // How to handle? + console.log(`Transferring ownership of ${contractName}Proxy to Governance`) await proxy._transferOwnership(addresses.get('Governance')) const proxiedContract = await artifacts.require(contractName).at(proxy.address) - // TODO(asa): How to deal with non-ownable const transferOwnershipAbi = (contract as any).abi.find( (abi: any) => abi.type === 'function' && abi.name === 'transferOwnership' ) if (transferOwnershipAbi) { + console.log(`Transferring ownership of ${contractName} to Governance`) await proxiedContract.transferOwnership(addresses.get('Governance')) } else { - console.log(`Unable to transfer ownership for ${contractName}`) + console.log(`${contractName} is not ownable`) } // 4. Update the contract's address, if needed. addresses.set(contractName, proxy.address) From 18170e541b6fb3f1128733746c10a9cab2ae1db3 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 13 Aug 2020 13:49:18 -0400 Subject: [PATCH 08/31] Sanity checks for release 1 --- ...tAttestations.sol => AttestationsTest.sol} | 2 +- .../test/{TestRandom.sol => RandomTest.sol} | 2 +- .../protocol/scripts/bash/check-versions.sh | 4 +- .../protocol/scripts/truffle/make-release.ts | 37 ++++++++++++++++++- .../protocol/test/identity/attestations.ts | 8 ++-- packages/protocol/test/identity/random.ts | 6 +-- 6 files changed, 47 insertions(+), 12 deletions(-) rename packages/protocol/contracts/identity/test/{TestAttestations.sol => AttestationsTest.sol} (93%) rename packages/protocol/contracts/identity/test/{TestRandom.sol => RandomTest.sol} (94%) diff --git a/packages/protocol/contracts/identity/test/TestAttestations.sol b/packages/protocol/contracts/identity/test/AttestationsTest.sol similarity index 93% rename from packages/protocol/contracts/identity/test/TestAttestations.sol rename to packages/protocol/contracts/identity/test/AttestationsTest.sol index dd3d744b672..15546b27b77 100644 --- a/packages/protocol/contracts/identity/test/TestAttestations.sol +++ b/packages/protocol/contracts/identity/test/AttestationsTest.sol @@ -7,7 +7,7 @@ import "../Attestations.sol"; * but mocks the implementations of the validator set getters. Otherwise we * couldn't test `request` with the current ganache local testnet. */ -contract TestAttestations is Attestations { +contract AttestationsTest is Attestations { address[] private __testValidators; function __setValidators(address[] memory validators) public { diff --git a/packages/protocol/contracts/identity/test/TestRandom.sol b/packages/protocol/contracts/identity/test/RandomTest.sol similarity index 94% rename from packages/protocol/contracts/identity/test/TestRandom.sol rename to packages/protocol/contracts/identity/test/RandomTest.sol index 7b82b55ec5b..3095aca73b4 100644 --- a/packages/protocol/contracts/identity/test/TestRandom.sol +++ b/packages/protocol/contracts/identity/test/RandomTest.sol @@ -2,7 +2,7 @@ pragma solidity ^0.5.3; import "../Random.sol"; -contract TestRandom is Random { +contract RandomTest is Random { function addTestRandomness(uint256 blockNumber, bytes32 randomness) external { addRandomness(blockNumber, randomness); } diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index 5924fb445ef..d578ff3fdce 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -38,5 +38,7 @@ yarn build:sol rm -rf $BUILD_DIR_2 && mkdir -p $BUILD_DIR_2 mv build/contracts $BUILD_DIR_2 -CONTRACT_EXCLUSION_REGEX=".*Test.*|.*LinkedList.*|.*MultiSig.*|.*Mock.*|I[A-Z].*|ReleaseGold" +# Exclude test contracts, mock contracts, contract interfaces, MultiSig contracts, and the +# ReleaseGold contract. +CONTRACT_EXCLUSION_REGEX=".*Test|Mock.*|I[A-Z].*|MultiSig.*|ReleaseGold" yarn ts-node scripts/check-backward.ts sem_check -o $BUILD_DIR_1/contracts -n $BUILD_DIR_2/contracts -e $CONTRACT_EXCLUSION_REGEX diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 86a3d9379a2..27a5b57a646 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -95,6 +95,29 @@ module.exports = async (callback: (error?: any) => number) => { const addresses = await ContractAddresses.create(contracts, registry) const released: Set = new Set([]) const proposal = [] + // Release 1 will deploy all libraries with proxies so that they're more easily + // upgradable. All contracts that link libraries should be upgraded to instead link the proxied + // library. + // To ensure this actually happens, we check that all contracts that link libraries are marked + // as needing to be redeployed. + // TODO(asa): Remove this check after release 1. + const linksLibrariesWithoutChanges = contracts.map( + (c) => + dependencies.get(c).length && + !Object.keys(report.contracts).includes(c) && + !c.endsWith('Test') + ) + if (linksLibrariesWithoutChanges.some((b) => b)) { + contracts.forEach((c, i) => { + if (linksLibrariesWithoutChanges[i]) { + console.log( + `${c} links ${dependencies.get(c)} and needs to be upgraded to link proxied libraries.` + ) + } + }) + throw new Error('All contracts linking libraries should be upgraded in release 1') + } + const release = async (contractName: string) => { if (released.has(contractName)) { return @@ -108,8 +131,9 @@ module.exports = async (callback: (error?: any) => number) => { const Contract = await artifacts.require(contractName) await Promise.all(contractDependencies.map((d) => Contract.link(d, addresses.get(d)))) - // TODO(asa): Redeploying all libraries is only needed for release 1. - // Remove `isLibrary` for future releases. + // This is a hack that will re-deploy all libraries with proxies, whether or not they have + // changes to them. + // TODO(asa): Remove `isLibrary` for future releases. const isLibrary = Object.keys(linkedLibraries).includes(contractName) const deployImplementation = Object.keys(report.contracts).includes(contractName) const deployProxy = @@ -118,6 +142,15 @@ module.exports = async (callback: (error?: any) => number) => { if (deployImplementation || isLibrary) { console.log(`Deploying ${contractName}`) const contract = await Contract.new() + // Sanity check that any contracts that are being changed set a version number. + const getVersionNumberAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'getVersionNumber' + ) + if (!getVersionNumberAbi) { + throw new Error( + `Contract ${contractName} has changes but does not specify a version number` + ) + } if (deployProxy || isLibrary) { console.log(`Deploying ${contractName}Proxy`) const Proxy = await artifacts.require(`${contractName}Proxy`) diff --git a/packages/protocol/test/identity/attestations.ts b/packages/protocol/test/identity/attestations.ts index 7a4abd0c01b..d9d8ea46d25 100644 --- a/packages/protocol/test/identity/attestations.ts +++ b/packages/protocol/test/identity/attestations.ts @@ -27,8 +27,8 @@ import { MockValidatorsContract, RegistryContract, RegistryInstance, - TestAttestationsContract, - TestAttestationsInstance, + AttestationsTestContract, + AttestationsTestInstance, } from 'types' import Web3 from 'web3' import { getParsedSignatureOfAddress } from '../../lib/signing-utils' @@ -43,7 +43,7 @@ const Accounts: AccountsContract = artifacts.require('Accounts') * contracts, which are not available in our current ganache fork, which we use * for Truffle unit tests. */ -const Attestations: TestAttestationsContract = artifacts.require('TestAttestations') +const Attestations: AttestationsTestContract = artifacts.require('AttestationsTest') const MockStableToken: MockStableTokenContract = artifacts.require('MockStableToken') const MockElection: MockElectionContract = artifacts.require('MockElection') const MockLockedGold: MockLockedGoldContract = artifacts.require('MockLockedGold') @@ -53,7 +53,7 @@ const Registry: RegistryContract = artifacts.require('Registry') contract('Attestations', (accounts: string[]) => { let accountsInstance: AccountsInstance - let attestations: TestAttestationsInstance + let attestations: AttestationsTestInstance let mockStableToken: MockStableTokenInstance let otherMockStableToken: MockStableTokenInstance let random: MockRandomInstance diff --git a/packages/protocol/test/identity/random.ts b/packages/protocol/test/identity/random.ts index 58457d27ca6..c0f941742f1 100644 --- a/packages/protocol/test/identity/random.ts +++ b/packages/protocol/test/identity/random.ts @@ -7,16 +7,16 @@ import { timeTravel, } from '@celo/protocol/lib/test-utils' import { BigNumber } from 'bignumber.js' -import { TestRandomContract, TestRandomInstance } from 'types' +import { RandomTestContract, RandomTestInstance } from 'types' -const Random: TestRandomContract = artifacts.require('TestRandom') +const Random: RandomTestContract = artifacts.require('RandomTest') // @ts-ignore // TODO(mcortesi): Use BN Random.numberFormat = 'BigNumber' contract('Random', (accounts: string[]) => { - let random: TestRandomInstance + let random: RandomTestInstance beforeEach(async () => { random = await Random.new() From 3d0f7835bdab0001d68d7cc9ef6580378d6ef894 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 13 Aug 2020 14:17:24 -0400 Subject: [PATCH 09/31] Add version number to contracts that link libraries --- .../protocol/contracts/common/Accounts.sol | 18 +++++++++++++++++- .../contracts/common/GasPriceMinimum.sol | 17 ++++++++++++++++- .../protocol/contracts/common/Signatures.sol | 8 ++++++++ .../contracts/governance/Proposals.sol | 8 ++++++++ .../protocol/contracts/identity/Escrow.sol | 18 +++++++++++++++++- .../contracts/stability/StableToken.sol | 12 +++++++++++- 6 files changed, 77 insertions(+), 4 deletions(-) diff --git a/packages/protocol/contracts/common/Accounts.sol b/packages/protocol/contracts/common/Accounts.sol index bf49e884f9f..250776a161b 100644 --- a/packages/protocol/contracts/common/Accounts.sol +++ b/packages/protocol/contracts/common/Accounts.sol @@ -6,11 +6,19 @@ import "openzeppelin-solidity/contracts/ownership/Ownable.sol"; import "./interfaces/IAccounts.sol"; import "../common/Initializable.sol"; +import "../common/interfaces/ICeloVersionedContract.sol"; import "../common/Signatures.sol"; import "../common/UsingRegistry.sol"; import "../common/libraries/ReentrancyGuard.sol"; -contract Accounts is IAccounts, Ownable, ReentrancyGuard, Initializable, UsingRegistry { +contract Accounts is + IAccounts, + ICeloVersionedContract, + Ownable, + ReentrancyGuard, + Initializable, + UsingRegistry +{ using SafeMath for uint256; struct Signers { @@ -60,6 +68,14 @@ contract Accounts is IAccounts, Ownable, ReentrancyGuard, Initializable, UsingRe event AccountWalletAddressSet(address indexed account, address walletAddress); event AccountCreated(address indexed account); + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 0, 1, 0); + } + /** * @notice Used in place of the constructor to allow the contract to be upgradable via proxy. * @param registryAddress The address of the registry core smart contract. diff --git a/packages/protocol/contracts/common/GasPriceMinimum.sol b/packages/protocol/contracts/common/GasPriceMinimum.sol index 67390885dae..eaea23bc5c8 100644 --- a/packages/protocol/contracts/common/GasPriceMinimum.sol +++ b/packages/protocol/contracts/common/GasPriceMinimum.sol @@ -5,6 +5,7 @@ import "openzeppelin-solidity/contracts/ownership/Ownable.sol"; import "./CalledByVm.sol"; import "./Initializable.sol"; +import "./interfaces/ICeloVersionedContract.sol"; import "./FixidityLib.sol"; import "./UsingRegistry.sol"; import "../stability/interfaces/ISortedOracles.sol"; @@ -12,7 +13,13 @@ import "../stability/interfaces/ISortedOracles.sol"; /** * @title Stores and provides gas price minimum for various currencies. */ -contract GasPriceMinimum is Ownable, Initializable, UsingRegistry, CalledByVm { +contract GasPriceMinimum is + ICeloVersionedContract, + Ownable, + Initializable, + UsingRegistry, + CalledByVm +{ using FixidityLib for FixidityLib.Fraction; using SafeMath for uint256; @@ -30,6 +37,14 @@ contract GasPriceMinimum is Ownable, Initializable, UsingRegistry, CalledByVm { // Speed of gas price minimum adjustment due to congestion. FixidityLib.Fraction public adjustmentSpeed; + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 0, 1, 0); + } + /** * @notice Used in place of the constructor to allow the contract to be upgradable via proxy. * @param _registryAddress The address of the registry core smart contract. diff --git a/packages/protocol/contracts/common/Signatures.sol b/packages/protocol/contracts/common/Signatures.sol index e52176a64ed..54c78c7fbdd 100644 --- a/packages/protocol/contracts/common/Signatures.sol +++ b/packages/protocol/contracts/common/Signatures.sol @@ -3,6 +3,14 @@ pragma solidity ^0.5.3; import "openzeppelin-solidity/contracts/cryptography/ECDSA.sol"; library Signatures { + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 0, 1, 0); + } + /** * @notice Given a signed address, returns the signer of the address. * @param message The address that was signed. diff --git a/packages/protocol/contracts/governance/Proposals.sol b/packages/protocol/contracts/governance/Proposals.sol index 92975514244..b386449a4c6 100644 --- a/packages/protocol/contracts/governance/Proposals.sol +++ b/packages/protocol/contracts/governance/Proposals.sol @@ -48,6 +48,14 @@ library Proposals { string descriptionUrl; } + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 0, 1, 0); + } + /** * @notice Constructs a proposal. * @param proposal The proposal struct to be constructed. diff --git a/packages/protocol/contracts/identity/Escrow.sol b/packages/protocol/contracts/identity/Escrow.sol index 1d03f659d8a..5ae059ab741 100644 --- a/packages/protocol/contracts/identity/Escrow.sol +++ b/packages/protocol/contracts/identity/Escrow.sol @@ -7,11 +7,19 @@ import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol"; import "./interfaces/IAttestations.sol"; import "./interfaces/IEscrow.sol"; import "../common/Initializable.sol"; +import "../common/interfaces/ICeloVersionedContract.sol"; import "../common/UsingRegistry.sol"; import "../common/Signatures.sol"; import "../common/libraries/ReentrancyGuard.sol"; -contract Escrow is IEscrow, ReentrancyGuard, Ownable, Initializable, UsingRegistry { +contract Escrow is + IEscrow, + ICeloVersionedContract, + ReentrancyGuard, + Ownable, + Initializable, + UsingRegistry +{ using SafeMath for uint256; event Transfer( @@ -61,6 +69,14 @@ contract Escrow is IEscrow, ReentrancyGuard, Ownable, Initializable, UsingRegist // Maps senders' addresses to a list of sent escrowed payment IDs. mapping(address => address[]) public sentPaymentIds; + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 0, 1, 0); + } + /** * @notice Used in place of the constructor to allow the contract to be upgradable via proxy. * @param registryAddress The address of the registry core smart contract. diff --git a/packages/protocol/contracts/stability/StableToken.sol b/packages/protocol/contracts/stability/StableToken.sol index abc794c280a..155c5dfe0f0 100644 --- a/packages/protocol/contracts/stability/StableToken.sol +++ b/packages/protocol/contracts/stability/StableToken.sol @@ -6,6 +6,7 @@ import "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol"; import "./interfaces/IStableToken.sol"; import "../common/interfaces/ICeloToken.sol"; +import "../common/interfaces/ICeloVersionedContract.sol"; import "../common/CalledByVm.sol"; import "../common/Initializable.sol"; import "../common/FixidityLib.sol"; @@ -26,7 +27,8 @@ contract StableToken is CalledByVm, IStableToken, IERC20, - ICeloToken + ICeloToken, + ICeloVersionedContract { using FixidityLib for FixidityLib.Fraction; using SafeMath for uint256; @@ -83,6 +85,14 @@ contract StableToken is _; } + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 0, 1, 0); + } + /** * @param _name The name of the stable token (English) * @param _symbol A short symbol identifying the token (e.g. "cUSD") From 4ea8b4ea10f1f19cc25d73bad7f1f4bef0090b29 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 13 Aug 2020 16:06:30 -0400 Subject: [PATCH 10/31] update version numbers --- packages/protocol/contracts/common/Accounts.sol | 2 +- packages/protocol/contracts/common/GasPriceMinimum.sol | 2 +- packages/protocol/contracts/common/Signatures.sol | 2 +- .../contracts/common/linkedlists/AddressLinkedList.sol | 8 ++++++++ .../common/linkedlists/AddressSortedLinkedList.sol | 8 ++++++++ .../linkedlists/AddressSortedLinkedListWithMedian.sol | 8 ++++++++ .../common/linkedlists/IntegerSortedLinkedList.sol | 8 ++++++++ .../contracts/common/linkedlists/SortedLinkedList.sol | 8 ++++++++ .../common/linkedlists/SortedLinkedListWithMedian.sol | 8 ++++++++ packages/protocol/contracts/governance/Proposals.sol | 2 +- packages/protocol/contracts/identity/Escrow.sol | 2 +- packages/protocol/contracts/stability/StableToken.sol | 2 +- 12 files changed, 54 insertions(+), 6 deletions(-) diff --git a/packages/protocol/contracts/common/Accounts.sol b/packages/protocol/contracts/common/Accounts.sol index 250776a161b..eebca833566 100644 --- a/packages/protocol/contracts/common/Accounts.sol +++ b/packages/protocol/contracts/common/Accounts.sol @@ -73,7 +73,7 @@ contract Accounts is * @return The storage, major, minor, and patch version of the contract. */ function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 0, 1, 0); + return (1, 1, 1, 0); } /** diff --git a/packages/protocol/contracts/common/GasPriceMinimum.sol b/packages/protocol/contracts/common/GasPriceMinimum.sol index eaea23bc5c8..5e21fcc534e 100644 --- a/packages/protocol/contracts/common/GasPriceMinimum.sol +++ b/packages/protocol/contracts/common/GasPriceMinimum.sol @@ -42,7 +42,7 @@ contract GasPriceMinimum is * @return The storage, major, minor, and patch version of the contract. */ function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 0, 1, 0); + return (1, 1, 1, 0); } /** diff --git a/packages/protocol/contracts/common/Signatures.sol b/packages/protocol/contracts/common/Signatures.sol index 54c78c7fbdd..420f575567f 100644 --- a/packages/protocol/contracts/common/Signatures.sol +++ b/packages/protocol/contracts/common/Signatures.sol @@ -8,7 +8,7 @@ library Signatures { * @return The storage, major, minor, and patch version of the contract. */ function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 0, 1, 0); + return (1, 1, 1, 0); } /** diff --git a/packages/protocol/contracts/common/linkedlists/AddressLinkedList.sol b/packages/protocol/contracts/common/linkedlists/AddressLinkedList.sol index 63400142a1c..7980ed8d9ad 100644 --- a/packages/protocol/contracts/common/linkedlists/AddressLinkedList.sol +++ b/packages/protocol/contracts/common/linkedlists/AddressLinkedList.sol @@ -12,6 +12,14 @@ library AddressLinkedList { using LinkedList for LinkedList.List; using SafeMath for uint256; + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + function toBytes(address a) public pure returns (bytes32) { return bytes32(uint256(a) << 96); } diff --git a/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedList.sol b/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedList.sol index 7e1bf1e5fad..b337f6c9594 100644 --- a/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedList.sol +++ b/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedList.sol @@ -12,6 +12,14 @@ library AddressSortedLinkedList { using SafeMath for uint256; using SortedLinkedList for SortedLinkedList.List; + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + function toBytes(address a) public pure returns (bytes32) { return bytes32(uint256(a) << 96); } diff --git a/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedListWithMedian.sol b/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedListWithMedian.sol index e199dadb560..60b365997f9 100644 --- a/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedListWithMedian.sol +++ b/packages/protocol/contracts/common/linkedlists/AddressSortedLinkedListWithMedian.sol @@ -11,6 +11,14 @@ library AddressSortedLinkedListWithMedian { using SafeMath for uint256; using SortedLinkedListWithMedian for SortedLinkedListWithMedian.List; + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + function toBytes(address a) public pure returns (bytes32) { return bytes32(uint256(a) << 96); } diff --git a/packages/protocol/contracts/common/linkedlists/IntegerSortedLinkedList.sol b/packages/protocol/contracts/common/linkedlists/IntegerSortedLinkedList.sol index f7237ecf496..41d858be887 100644 --- a/packages/protocol/contracts/common/linkedlists/IntegerSortedLinkedList.sol +++ b/packages/protocol/contracts/common/linkedlists/IntegerSortedLinkedList.sol @@ -11,6 +11,14 @@ library IntegerSortedLinkedList { using SafeMath for uint256; using SortedLinkedList for SortedLinkedList.List; + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + /** * @notice Inserts an element into a doubly linked list. * @param list A storage pointer to the underlying list. diff --git a/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol b/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol index 590533c5894..81f8efc66c1 100644 --- a/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol +++ b/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol @@ -15,6 +15,14 @@ library SortedLinkedList { mapping(bytes32 => uint256) values; } + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + /** * @notice Inserts an element into a doubly linked list. * @param list A storage pointer to the underlying list. diff --git a/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol b/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol index 34c91ad9c79..e06056fac3a 100644 --- a/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol +++ b/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol @@ -21,6 +21,14 @@ library SortedLinkedListWithMedian { mapping(bytes32 => MedianRelation) relation; } + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + /** * @notice Inserts an element into a doubly linked list. * @param list A storage pointer to the underlying list. diff --git a/packages/protocol/contracts/governance/Proposals.sol b/packages/protocol/contracts/governance/Proposals.sol index b386449a4c6..5a498567d42 100644 --- a/packages/protocol/contracts/governance/Proposals.sol +++ b/packages/protocol/contracts/governance/Proposals.sol @@ -53,7 +53,7 @@ library Proposals { * @return The storage, major, minor, and patch version of the contract. */ function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 0, 1, 0); + return (1, 1, 1, 0); } /** diff --git a/packages/protocol/contracts/identity/Escrow.sol b/packages/protocol/contracts/identity/Escrow.sol index 5ae059ab741..d4753456de5 100644 --- a/packages/protocol/contracts/identity/Escrow.sol +++ b/packages/protocol/contracts/identity/Escrow.sol @@ -74,7 +74,7 @@ contract Escrow is * @return The storage, major, minor, and patch version of the contract. */ function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 0, 1, 0); + return (1, 1, 1, 0); } /** diff --git a/packages/protocol/contracts/stability/StableToken.sol b/packages/protocol/contracts/stability/StableToken.sol index 155c5dfe0f0..01c1f19a5af 100644 --- a/packages/protocol/contracts/stability/StableToken.sol +++ b/packages/protocol/contracts/stability/StableToken.sol @@ -90,7 +90,7 @@ contract StableToken is * @return The storage, major, minor, and patch version of the contract. */ function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 0, 1, 0); + return (1, 1, 1, 0); } /** From 3b123dfc021a111088a5aa36eb1799da1f2ddedc Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 13 Aug 2020 16:30:00 -0400 Subject: [PATCH 11/31] Add version to linkedlist --- .../protocol/contracts/common/linkedlists/LinkedList.sol | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/protocol/contracts/common/linkedlists/LinkedList.sol b/packages/protocol/contracts/common/linkedlists/LinkedList.sol index 1d142b89b5a..19df7521d34 100644 --- a/packages/protocol/contracts/common/linkedlists/LinkedList.sol +++ b/packages/protocol/contracts/common/linkedlists/LinkedList.sol @@ -22,6 +22,14 @@ library LinkedList { mapping(bytes32 => Element) elements; } + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + /** * @notice Inserts an element into a doubly linked list. * @param list A storage pointer to the underlying list. From 67ca83700d831baa19dd0c5c78ce135b4a66ac5b Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 09:40:32 -0400 Subject: [PATCH 12/31] Ignore proxies when generating compatibility report --- packages/protocol/scripts/bash/check-versions.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index d578ff3fdce..87bc8301f9e 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -38,7 +38,7 @@ yarn build:sol rm -rf $BUILD_DIR_2 && mkdir -p $BUILD_DIR_2 mv build/contracts $BUILD_DIR_2 -# Exclude test contracts, mock contracts, contract interfaces, MultiSig contracts, and the -# ReleaseGold contract. -CONTRACT_EXCLUSION_REGEX=".*Test|Mock.*|I[A-Z].*|MultiSig.*|ReleaseGold" +# Exclude test contracts, mock contracts, contract interfaces, Proxy contracts, MultiSig contracts, +# and the ReleaseGold contract. +CONTRACT_EXCLUSION_REGEX=".*Test|Mock.*|I[A-Z].*|.*Proxy|MultiSig.*|ReleaseGold" yarn ts-node scripts/check-backward.ts sem_check -o $BUILD_DIR_1/contracts -n $BUILD_DIR_2/contracts -e $CONTRACT_EXCLUSION_REGEX From 86d1abdc76776ea2c0f4c1a19e704b01ee188a5e Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 10:03:19 -0400 Subject: [PATCH 13/31] Version fixidity lib --- packages/protocol/contracts/common/FixidityLib.sol | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/protocol/contracts/common/FixidityLib.sol b/packages/protocol/contracts/common/FixidityLib.sol index 222155dcc87..97be3bb4a99 100644 --- a/packages/protocol/contracts/common/FixidityLib.sol +++ b/packages/protocol/contracts/common/FixidityLib.sol @@ -17,6 +17,14 @@ library FixidityLib { uint256 value; } + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 1, 0); + } + /** * @notice Number of positions that the comma is shifted to the right. */ From de9e1fdda7ad65d43cf2b8cb5dc0791826c0af7d Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 10:28:22 -0400 Subject: [PATCH 14/31] cleanup --- .../protocol/scripts/truffle/make-release.ts | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 27a5b57a646..88bfa1d7f87 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -17,21 +17,12 @@ import { basename, join } from 'path' * network: The network for which artifacts should be * * Run using truffle exec, e.g.: - * truffle exec scripts/truffle/make-release --report TODO + * truffle exec scripts/truffle/make-release \ + * --network alfajores --build_directory build/alfajores/ --report report.json \ + * --initialize_data initialize_data.json --proposal proposal.json * */ -/* -const linkLibraries = async () => { - // How to get libAddress??? - const libAddress = '0x123456789009876543211234567890098765432112345678900987654321' - Object.keys(linkedLibraries).forEach(async (lib: string) => { - const Contracts = linkedLibraries[lib].map((contract: string) => artifacts.require(contract)) - await Promise.all(Contracts.map((C) => C.link(lib, libAddress)) - }) -} -*/ - class ContractDependencies { dependencies: Map constructor(linkedLibraries: any) { @@ -152,6 +143,16 @@ module.exports = async (callback: (error?: any) => number) => { ) } if (deployProxy || isLibrary) { + // Explicitly forbid upgrading to a new Governance proxy contract. + // Upgrading to a new Governance proxy contract would require ownership of all + // contracts to be moved to the new governance contract, possibly including contracts + // deployed in this script. + // Because this depends on ordering (i.e. was the new GovernanceProxy deployed + // before or after other contracts in this script?), and that ordering is not being + // checked, fail if there are storage incompatible changes to Governance. + if (contractName == 'Governance') { + throw new Error(`Storage incompatible changes to Governance are not yet supported`) + } console.log(`Deploying ${contractName}Proxy`) const Proxy = await artifacts.require(`${contractName}Proxy`) const proxy = await Proxy.new() @@ -173,8 +174,7 @@ module.exports = async (callback: (error?: any) => number) => { } else { await proxy._setImplementation(contract.address) } - // TODO(asa): This makes essentially every contract dependent on Governance. - // How to handle? + // This makes essentially every contract dependent on Governance. console.log(`Transferring ownership of ${contractName}Proxy to Governance`) await proxy._transferOwnership(addresses.get('Governance')) const proxiedContract = await artifacts.require(contractName).at(proxy.address) @@ -184,8 +184,6 @@ module.exports = async (callback: (error?: any) => number) => { if (transferOwnershipAbi) { console.log(`Transferring ownership of ${contractName} to Governance`) await proxiedContract.transferOwnership(addresses.get('Governance')) - } else { - console.log(`${contractName} is not ownable`) } // 4. Update the contract's address, if needed. addresses.set(contractName, proxy.address) From 6f308ccf5c23fa463fadd581155495307a0f1829 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 11:10:51 -0400 Subject: [PATCH 15/31] Add script for making releases --- .circleci/config.yml | 2 +- packages/protocol/package.json | 3 +- .../protocol/scripts/bash/check-versions.sh | 23 ++++++++---- .../protocol/scripts/bash/make-release.sh | 37 +++++++++++++++++++ 4 files changed, 56 insertions(+), 9 deletions(-) create mode 100755 packages/protocol/scripts/bash/make-release.sh diff --git a/.circleci/config.yml b/.circleci/config.yml index 0c3e674a1f2..8292eeb52d0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -476,7 +476,7 @@ jobs: ssh-keyscan github.com >> ~/.ssh/known_hosts git fetch --all RELEASE_BRANCH=$(git branch -a --list *release/contracts/* | tail -1 | sed -e 's/remotes\/origin\///g') - yarn --cwd packages/protocol check-versions -o $RELEASE_BRANCH -n $CIRCLE_BRANCH + yarn --cwd packages/protocol check-versions -a $RELEASE_BRANCH -b $CIRCLE_BRANCH LIBRARIES_CHANGED=$(git diff $RELEASE_BRANCH --name-only packages/protocol/contracts/common/linkedlists) if [[ $LIBRARIES_CHANGED ]]; then diff --git a/packages/protocol/package.json b/packages/protocol/package.json index a44b45581fc..3b9a2ffe870 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -30,6 +30,7 @@ "govern": "./scripts/bash/govern.sh", "console": "./scripts/bash/console.sh", "check-versions": "./scripts/bash/check-versions.sh", + "make-release": "./scripts/bash/make-release.sh", "invite": "./scripts/bash/invite.sh", "ganache-dev": "./scripts/bash/ganache.sh", "pretruffle:migrate": "yarn build:ts", @@ -103,4 +104,4 @@ "typechain-target-truffle": "1.0.2", "yargs": "^14.0.0" } -} \ No newline at end of file +} diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index 87bc8301f9e..afbf30196ef 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -5,22 +5,26 @@ set -euo pipefail # a released branch. # # Flags: -# -o: Old branch containing smart contracts, which has likely been released. -# -n: New branch containing smart contracts, on which version numbers may be updated. +# -a: Old branch containing smart contracts, which has likely been released. +# -b: New branch containing smart contracts, on which version numbers may be updated. BRANCH_1="" BRANCH_2="" +NETWORK="" +REPORT="" -while getopts 'o:n:' flag; do +while getopts 'a:b:n:r:' flag; do case "${flag}" in - o) BRANCH_1="${OPTARG}" ;; - n) BRANCH_2="${OPTARG}" ;; + a) BRANCH_1="${OPTARG}" ;; + b) BRANCH_2="${OPTARG}" ;; + n) NETWORK="$OPTARG" ;; + r) REPORT="${OPTARG}" ;; *) error "Unexpected option ${flag}" ;; esac done -[ -z "$BRANCH_1" ] && echo "Need to set the first branch via the -n flag" && exit 1; -[ -z "$BRANCH_2" ] && echo "Need to set the second branch via the -o flag" && exit 1; +[ -z "$BRANCH_1" ] && echo "Need to set the first branch via the -a flag" && exit 1; +[ -z "$BRANCH_2" ] && echo "Need to set the second branch via the -b flag" && exit 1; BUILD_DIR_1=$(echo build/$(echo $BRANCH_1 | sed -e 's/\//_/g')) git checkout $BRANCH_1 @@ -38,6 +42,11 @@ yarn build:sol rm -rf $BUILD_DIR_2 && mkdir -p $BUILD_DIR_2 mv build/contracts $BUILD_DIR_2 +REPORT_FLAG="" +if [ -z "$REPORT" ]; then + REPORT_FLAG="-f "$REPORT +fi + # Exclude test contracts, mock contracts, contract interfaces, Proxy contracts, MultiSig contracts, # and the ReleaseGold contract. CONTRACT_EXCLUSION_REGEX=".*Test|Mock.*|I[A-Z].*|.*Proxy|MultiSig.*|ReleaseGold" diff --git a/packages/protocol/scripts/bash/make-release.sh b/packages/protocol/scripts/bash/make-release.sh new file mode 100755 index 00000000000..34dd6c243d8 --- /dev/null +++ b/packages/protocol/scripts/bash/make-release.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Checks that the contract version numbers in a provided branch are as expected given +# a released branch. +# +# Flags: +# -a: Old branch containing smart contracts, which has likely been released. +# -b: New branch containing smart contracts, on which version numbers may be updated. + +BRANCH_1="" +BRANCH_2="" +NETWORK="" +REPORT="" + +while getopts 'a:b:n:r:' flag; do + case "${flag}" in + a) BRANCH_1="${OPTARG}" ;; + b) BRANCH_2="${OPTARG}" ;; + n) NETWORK="$OPTARG" ;; + p) PROPOSAL="$OPTARG" ;; + i) INITIALIZE_DATA="$OPTARG" ;; + r) REPORT="${OPTARG}" ;; + *) error "Unexpected option ${flag}" ;; + esac +done + +[ -z "$BRANCH_1" ] && echo "Need to set the first branch via the -a flag" && exit 1; +[ -z "$BRANCH_2" ] && echo "Need to set the second branch via the -b flag" && exit 1; +[ -z "$NETWORK" ] && echo "Need to set the NETWORK via the -n flag" && exit 1; +[ -z "$INITIALIZE_DATA" ] && echo "Need to set the initialization data via the -i flag" && exit 1; +[ -z "$PROPOSAL" ] && echo "Need to set the proposal outfile via the -p flag" && exit 1; +[ -z "$REPORT" ] && echo "Need to set the compatibility report outfile via the -r flag" && exit 1; + +yarn check-versions -o $BRANCH_1 -n $BRANCH_2 -r $REPORT +yarn build +yarn run truffle exec ./scripts/truffle/make-release.js --network $NETWORK --build_directory build/ --report $REPORT --proposal $PROPOSAL --initialize_data $INITITIALIZE_DATA From 295e5e029656408e215667e88b1c9c0eec612be6 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 11:11:56 -0400 Subject: [PATCH 16/31] oops --- packages/protocol/scripts/bash/make-release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/scripts/bash/make-release.sh b/packages/protocol/scripts/bash/make-release.sh index 34dd6c243d8..f578d02684f 100755 --- a/packages/protocol/scripts/bash/make-release.sh +++ b/packages/protocol/scripts/bash/make-release.sh @@ -13,7 +13,7 @@ BRANCH_2="" NETWORK="" REPORT="" -while getopts 'a:b:n:r:' flag; do +while getopts 'a:b:n:p:i:r:' flag; do case "${flag}" in a) BRANCH_1="${OPTARG}" ;; b) BRANCH_2="${OPTARG}" ;; From 995dfc830168382de98e3cdb5a1220a9e1c3a735 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 11:12:22 -0400 Subject: [PATCH 17/31] oops --- packages/protocol/scripts/bash/make-release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/scripts/bash/make-release.sh b/packages/protocol/scripts/bash/make-release.sh index f578d02684f..e454f34c776 100755 --- a/packages/protocol/scripts/bash/make-release.sh +++ b/packages/protocol/scripts/bash/make-release.sh @@ -32,6 +32,6 @@ done [ -z "$PROPOSAL" ] && echo "Need to set the proposal outfile via the -p flag" && exit 1; [ -z "$REPORT" ] && echo "Need to set the compatibility report outfile via the -r flag" && exit 1; -yarn check-versions -o $BRANCH_1 -n $BRANCH_2 -r $REPORT +yarn check-versions -a $BRANCH_1 -b $BRANCH_2 -r $REPORT yarn build yarn run truffle exec ./scripts/truffle/make-release.js --network $NETWORK --build_directory build/ --report $REPORT --proposal $PROPOSAL --initialize_data $INITITIALIZE_DATA From 150aaf7398d489ac29e1fecb8d85e108711eac87 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 11:16:51 -0400 Subject: [PATCH 18/31] cleanup --- packages/protocol/scripts/bash/check-versions.sh | 2 +- packages/protocol/scripts/truffle/make-release.ts | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index afbf30196ef..38293d8c3ab 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -50,4 +50,4 @@ fi # Exclude test contracts, mock contracts, contract interfaces, Proxy contracts, MultiSig contracts, # and the ReleaseGold contract. CONTRACT_EXCLUSION_REGEX=".*Test|Mock.*|I[A-Z].*|.*Proxy|MultiSig.*|ReleaseGold" -yarn ts-node scripts/check-backward.ts sem_check -o $BUILD_DIR_1/contracts -n $BUILD_DIR_2/contracts -e $CONTRACT_EXCLUSION_REGEX +yarn ts-node scripts/check-backward.ts sem_check -o $BUILD_DIR_1/contracts -n $BUILD_DIR_2/contracts -e $CONTRACT_EXCLUSION_REGEX $REPORT_FLAG diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 88bfa1d7f87..0e33c48a264 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -6,8 +6,6 @@ import { Address, eqAddress, NULL_ADDRESS } from '@celo/utils/lib/address' import { linkedLibraries } from '@celo/protocol/migrationsConfig' import { basename, join } from 'path' -// import { getDeployedProxiedContract } from '@celo/protocol/lib/web3-utils' - /* * A script that reads a backwards compatibility report, deploys changed contracts, and creates * a corresponding JSON file to be proposed with `celocli governance:propose` From 1832bf9abf9ff2a9edc0037edef643cc6aa10c7a Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 11:35:33 -0400 Subject: [PATCH 19/31] typo --- packages/protocol/scripts/bash/make-release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/scripts/bash/make-release.sh b/packages/protocol/scripts/bash/make-release.sh index e454f34c776..07abe69def2 100755 --- a/packages/protocol/scripts/bash/make-release.sh +++ b/packages/protocol/scripts/bash/make-release.sh @@ -34,4 +34,4 @@ done yarn check-versions -a $BRANCH_1 -b $BRANCH_2 -r $REPORT yarn build -yarn run truffle exec ./scripts/truffle/make-release.js --network $NETWORK --build_directory build/ --report $REPORT --proposal $PROPOSAL --initialize_data $INITITIALIZE_DATA +yarn run truffle exec ./scripts/truffle/make-release.js --network $NETWORK --build_directory build/ --report $REPORT --proposal $PROPOSAL --initialize_data $INITIALIZE_DATA From 1ca0365675ffbb28b5a84c1438665e1ae45d2d1d Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 11:42:17 -0400 Subject: [PATCH 20/31] Fix --- packages/protocol/scripts/bash/check-versions.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index 38293d8c3ab..b1c7eb5f53e 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -43,7 +43,7 @@ rm -rf $BUILD_DIR_2 && mkdir -p $BUILD_DIR_2 mv build/contracts $BUILD_DIR_2 REPORT_FLAG="" -if [ -z "$REPORT" ]; then +if [ ! -z "$REPORT" ]; then REPORT_FLAG="-f "$REPORT fi From 88e8fc09ae95594c36209fd49fed40d6c41ebd8d Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Fri, 14 Aug 2020 11:59:26 -0400 Subject: [PATCH 21/31] Cleanup --- .../protocol/scripts/bash/check-versions.sh | 2 ++ .../protocol/scripts/bash/make-release.sh | 4 +++ .../scripts/truffle/deployedGrants.json | 2 ++ .../truffle/releaseGoldExampleConfigs.json | 34 +++++++++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 packages/protocol/scripts/truffle/deployedGrants.json create mode 100644 packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index b1c7eb5f53e..296e451a0fc 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -7,6 +7,8 @@ set -euo pipefail # Flags: # -a: Old branch containing smart contracts, which has likely been released. # -b: New branch containing smart contracts, on which version numbers may be updated. +# -n: The network to deploy to. +# -r: Path that the contract compatibility report should be written to. BRANCH_1="" BRANCH_2="" diff --git a/packages/protocol/scripts/bash/make-release.sh b/packages/protocol/scripts/bash/make-release.sh index 07abe69def2..7b1bfb9216f 100755 --- a/packages/protocol/scripts/bash/make-release.sh +++ b/packages/protocol/scripts/bash/make-release.sh @@ -7,6 +7,10 @@ set -euo pipefail # Flags: # -a: Old branch containing smart contracts, which has likely been released. # -b: New branch containing smart contracts, on which version numbers may be updated. +# -n: The network to deploy to. +# -p: Path that the governance proposal should be written to. +# -i: Path to the data needed to initialize contracts. +# -r: Path that the contract compatibility report should be written to. BRANCH_1="" BRANCH_2="" diff --git a/packages/protocol/scripts/truffle/deployedGrants.json b/packages/protocol/scripts/truffle/deployedGrants.json new file mode 100644 index 00000000000..0d4f101c7a3 --- /dev/null +++ b/packages/protocol/scripts/truffle/deployedGrants.json @@ -0,0 +1,2 @@ +[ +] diff --git a/packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json b/packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json new file mode 100644 index 00000000000..9abcff822be --- /dev/null +++ b/packages/protocol/scripts/truffle/releaseGoldExampleConfigs.json @@ -0,0 +1,34 @@ +[ + { + "identifier": "0x111111111", + "releaseStartTime": "MAINNET", + "releaseCliffTime": 0, + "numReleasePeriods": 1, + "releasePeriod": 100000000, + "amountReleasedPerPeriod": 10002, + "revocable": false, + "beneficiary": "0x5409ED021D9299bf6814279A6A1411A7e866A631", + "releaseOwner": "0x6Ecbe1DB9EF729CBe972C83Fb886247691Fb6beb", + "refundAddress": "0x0000000000000000000000000000000000000000", + "subjectToLiquidityProvision": true, + "initialDistributionRatio": 1000, + "canVote": true, + "canValidate": true + }, + { + "identifier": "0x222222222", + "releaseStartTime": "MAINNET", + "releaseCliffTime": 0, + "numReleasePeriods": 1, + "releasePeriod": 100000000, + "amountReleasedPerPeriod": 2, + "revocable": true, + "beneficiary": "0x6Ecbe1DB9EF729CBe972C83Fb886247691Fb6beb", + "releaseOwner": "0x5409ED021D9299bf6814279A6A1411A7e866A631", + "refundAddress": "0x5409ED021D9299bf6814279A6A1411A7e866A631", + "subjectToLiquidityProvision": false, + "initialDistributionRatio": 1000, + "canVote": true, + "canValidate": false + } +] From bdd7de8f794e7168864c02fa0b9060e28f0a99ca Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 18 Aug 2020 10:34:34 -0400 Subject: [PATCH 22/31] Start work on dry-run --- .../protocol/scripts/truffle/make-release.ts | 139 ++++++++++-------- 1 file changed, 76 insertions(+), 63 deletions(-) diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 0e33c48a264..70f6f6cc287 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -45,12 +45,14 @@ class ContractAddresses { constructor(public addresses: Map) {} static async create(contracts: string[], registry: any) { const addresses = new Map() - contracts.forEach(async (contract: string) => { - const registeredAddress = await registry.getAddressForString(contract) - if (!eqAddress(registeredAddress, NULL_ADDRESS)) { - addresses.set(contract, registeredAddress) - } - }) + await Promise.all( + contracts.map(async (contract: string) => { + const registeredAddress = await registry.getAddressForString(contract) + if (!eqAddress(registeredAddress, NULL_ADDRESS)) { + addresses.set(contract, registeredAddress) + } + }) + ) return new ContractAddresses(addresses) } @@ -107,6 +109,68 @@ module.exports = async (callback: (error?: any) => number) => { throw new Error('All contracts linking libraries should be upgraded in release 1') } + const deployImplementation = async (contractName: string, Contract: any) => { + console.log(`Deploying ${contractName}`) + // const contract = await Contract.new() + const contract = await Contract.at(web3.utils.randomHex(20)) + // Sanity check that any contracts that are being changed set a version number. + const getVersionNumberAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'getVersionNumber' + ) + if (!getVersionNumberAbi) { + throw new Error( + `Contract ${contractName} has changes but does not specify a version number` + ) + } + return contract + } + + const deployProxy = async (contractName: string, contract: any) => { + // Explicitly forbid upgrading to a new Governance proxy contract. + // Upgrading to a new Governance proxy contract would require ownership of all + // contracts to be moved to the new governance contract, possibly including contracts + // deployed in this script. + // Because this depends on ordering (i.e. was the new GovernanceProxy deployed + // before or after other contracts in this script?), and that ordering is not being + // checked, fail if there are storage incompatible changes to Governance. + if (contractName == 'Governance') { + throw new Error(`Storage incompatible changes to Governance are not yet supported`) + } + console.log(`Deploying ${contractName}Proxy`) + const Proxy = await artifacts.require(`${contractName}Proxy`) + const proxy = await Proxy.new() + const initializeAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'initialize' + ) + console.log(`Setting ${contractName}Proxy implementation to ${contract.address}`) + if (initializeAbi) { + const args = initializationData[contractName] + console.log(`Initializing ${contractName} with: ${args}`) + await setAndInitializeImplementation( + web3, + proxy, + contract.address, + initializeAbi, + {}, + ...args + ) + } else { + await proxy._setImplementation(contract.address) + } + // This makes essentially every contract dependent on Governance. + console.log(`Transferring ownership of ${contractName}Proxy to Governance`) + await proxy._transferOwnership(addresses.get('Governance')) + const proxiedContract = await artifacts.require(contractName).at(proxy.address) + const transferOwnershipAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'transferOwnership' + ) + if (transferOwnershipAbi) { + console.log(`Transferring ownership of ${contractName} to Governance`) + await proxiedContract.transferOwnership(addresses.get('Governance')) + } + return proxy + } + const release = async (contractName: string) => { if (released.has(contractName)) { return @@ -124,65 +188,14 @@ module.exports = async (callback: (error?: any) => number) => { // changes to them. // TODO(asa): Remove `isLibrary` for future releases. const isLibrary = Object.keys(linkedLibraries).includes(contractName) - const deployImplementation = Object.keys(report.contracts).includes(contractName) - const deployProxy = + const shouldDeployImplementation = Object.keys(report.contracts).includes(contractName) + const shouldDeployProxy = deployImplementation && report.contracts[contractName].changes.storage.length // 3. Deploy new versions of the contract and proxy, if needed. - if (deployImplementation || isLibrary) { - console.log(`Deploying ${contractName}`) - const contract = await Contract.new() - // Sanity check that any contracts that are being changed set a version number. - const getVersionNumberAbi = (contract as any).abi.find( - (abi: any) => abi.type === 'function' && abi.name === 'getVersionNumber' - ) - if (!getVersionNumberAbi) { - throw new Error( - `Contract ${contractName} has changes but does not specify a version number` - ) - } - if (deployProxy || isLibrary) { - // Explicitly forbid upgrading to a new Governance proxy contract. - // Upgrading to a new Governance proxy contract would require ownership of all - // contracts to be moved to the new governance contract, possibly including contracts - // deployed in this script. - // Because this depends on ordering (i.e. was the new GovernanceProxy deployed - // before or after other contracts in this script?), and that ordering is not being - // checked, fail if there are storage incompatible changes to Governance. - if (contractName == 'Governance') { - throw new Error(`Storage incompatible changes to Governance are not yet supported`) - } - console.log(`Deploying ${contractName}Proxy`) - const Proxy = await artifacts.require(`${contractName}Proxy`) - const proxy = await Proxy.new() - const initializeAbi = (contract as any).abi.find( - (abi: any) => abi.type === 'function' && abi.name === 'initialize' - ) - console.log(`Setting ${contractName}Proxy implementation to ${contract.address}`) - if (initializeAbi) { - const args = initializationData[contractName] - console.log(`Initializing ${contractName} with: ${args}`) - await setAndInitializeImplementation( - web3, - proxy, - contract.address, - initializeAbi, - {}, - ...args - ) - } else { - await proxy._setImplementation(contract.address) - } - // This makes essentially every contract dependent on Governance. - console.log(`Transferring ownership of ${contractName}Proxy to Governance`) - await proxy._transferOwnership(addresses.get('Governance')) - const proxiedContract = await artifacts.require(contractName).at(proxy.address) - const transferOwnershipAbi = (contract as any).abi.find( - (abi: any) => abi.type === 'function' && abi.name === 'transferOwnership' - ) - if (transferOwnershipAbi) { - console.log(`Transferring ownership of ${contractName} to Governance`) - await proxiedContract.transferOwnership(addresses.get('Governance')) - } + if (shouldDeployImplementation || isLibrary) { + const contract = await deployImplementation(contractName, Contract) + if (shouldDeployProxy || isLibrary) { + const proxy = await deployProxy(contractName, contract) // 4. Update the contract's address, if needed. addresses.set(contractName, proxy.address) proposal.push({ From 784b476ce3fdafdfa233cc7da5e0b0a74f041d04 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 18 Aug 2020 16:11:13 -0400 Subject: [PATCH 23/31] Address comments, fix lint --- .../protocol/scripts/truffle/make-release.ts | 66 +++++++++++-------- .../protocol/test/identity/attestations.ts | 4 +- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 70f6f6cc287..f54219150c4 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -1,9 +1,10 @@ -/* tslint:disable:no-console */ -import { setAndInitializeImplementation } from '@celo/protocol/lib/proxy-utils' -import { readdirSync, readJsonSync, writeJsonSync } from 'fs-extra' +// tslint:disable: max-classes-per-file +// tslint:disable: no-console import { ASTDetailedVersionedReport } from '@celo/protocol/lib/compatibility/report' -import { Address, eqAddress, NULL_ADDRESS } from '@celo/utils/lib/address' +import { setAndInitializeImplementation } from '@celo/protocol/lib/proxy-utils' import { linkedLibraries } from '@celo/protocol/migrationsConfig' +import { Address, eqAddress, NULL_ADDRESS } from '@celo/utils/lib/address' +import { readdirSync, readJsonSync, writeJsonSync } from 'fs-extra' import { basename, join } from 'path' /* @@ -23,10 +24,10 @@ import { basename, join } from 'path' class ContractDependencies { dependencies: Map - constructor(linkedLibraries: any) { + constructor(libraries: any) { this.dependencies = new Map() - Object.keys(linkedLibraries).forEach((lib: string) => { - linkedLibraries[lib].forEach((contract: string) => { + Object.keys(libraries).forEach((lib: string) => { + libraries[lib].forEach((contract: string) => { if (this.dependencies.has(contract)) { this.dependencies.get(contract).push(lib) } else { @@ -42,7 +43,6 @@ class ContractDependencies { } class ContractAddresses { - constructor(public addresses: Map) {} static async create(contracts: string[], registry: any) { const addresses = new Map() await Promise.all( @@ -56,6 +56,8 @@ class ContractAddresses { return new ContractAddresses(addresses) } + constructor(public addresses: Map) {} + public get = (contract: string): Address => { if (this.addresses.has(contract)) { return this.addresses.get(contract) @@ -69,10 +71,13 @@ class ContractAddresses { } } +const REGISTRY_ADDRESS = '0x000000000000000000000000000000000000ce10' + module.exports = async (callback: (error?: any) => number) => { try { const argv = require('minimist')(process.argv.slice(2), { string: ['report', 'network', 'proposal', 'libraries', 'initialize_data', 'build_directory'], + boolean: ['dry_run'], }) const report: ASTDetailedVersionedReport = readJsonSync(argv.report).report const initializationData = readJsonSync(argv.initialize_data) @@ -80,9 +85,7 @@ module.exports = async (callback: (error?: any) => number) => { const contracts = readdirSync(join(argv.build_directory, 'contracts')).map((x) => basename(x, '.json') ) - const registry = await artifacts - .require('Registry') - .at('0x000000000000000000000000000000000000ce10') + const registry = await artifacts.require('Registry').at(REGISTRY_ADDRESS) const addresses = await ContractAddresses.create(contracts, registry) const released: Set = new Set([]) const proposal = [] @@ -111,8 +114,8 @@ module.exports = async (callback: (error?: any) => number) => { const deployImplementation = async (contractName: string, Contract: any) => { console.log(`Deploying ${contractName}`) - // const contract = await Contract.new() - const contract = await Contract.at(web3.utils.randomHex(20)) + // Hack to trick truffle, which checks that the provided address has code + const contract = await (argv.dry_run ? Contract.at(REGISTRY_ADDRESS) : Contract.new()) // Sanity check that any contracts that are being changed set a version number. const getVersionNumberAbi = (contract as any).abi.find( (abi: any) => abi.type === 'function' && abi.name === 'getVersionNumber' @@ -133,12 +136,13 @@ module.exports = async (callback: (error?: any) => number) => { // Because this depends on ordering (i.e. was the new GovernanceProxy deployed // before or after other contracts in this script?), and that ordering is not being // checked, fail if there are storage incompatible changes to Governance. - if (contractName == 'Governance') { + if (contractName === 'Governance') { throw new Error(`Storage incompatible changes to Governance are not yet supported`) } console.log(`Deploying ${contractName}Proxy`) const Proxy = await artifacts.require(`${contractName}Proxy`) - const proxy = await Proxy.new() + // Hack to trick truffle, which checks that the provided address has code + const proxy = await (argv.dry_run ? Proxy.at(REGISTRY_ADDRESS) : Proxy.new()) const initializeAbi = (contract as any).abi.find( (abi: any) => abi.type === 'function' && abi.name === 'initialize' ) @@ -146,27 +150,35 @@ module.exports = async (callback: (error?: any) => number) => { if (initializeAbi) { const args = initializationData[contractName] console.log(`Initializing ${contractName} with: ${args}`) - await setAndInitializeImplementation( - web3, - proxy, - contract.address, - initializeAbi, - {}, - ...args - ) + if (!argv.dry_run) { + await setAndInitializeImplementation( + web3, + proxy, + contract.address, + initializeAbi, + {}, + ...args + ) + } } else { - await proxy._setImplementation(contract.address) + if (!argv.dry_run) { + await proxy._setImplementation(contract.address) + } } // This makes essentially every contract dependent on Governance. console.log(`Transferring ownership of ${contractName}Proxy to Governance`) - await proxy._transferOwnership(addresses.get('Governance')) + if (!argv.dry_run) { + await proxy._transferOwnership(addresses.get('Governance')) + } const proxiedContract = await artifacts.require(contractName).at(proxy.address) const transferOwnershipAbi = (contract as any).abi.find( (abi: any) => abi.type === 'function' && abi.name === 'transferOwnership' ) if (transferOwnershipAbi) { console.log(`Transferring ownership of ${contractName} to Governance`) - await proxiedContract.transferOwnership(addresses.get('Governance')) + if (!argv.dry_run) { + await proxiedContract.transferOwnership(addresses.get('Governance')) + } } return proxy } @@ -190,7 +202,7 @@ module.exports = async (callback: (error?: any) => number) => { const isLibrary = Object.keys(linkedLibraries).includes(contractName) const shouldDeployImplementation = Object.keys(report.contracts).includes(contractName) const shouldDeployProxy = - deployImplementation && report.contracts[contractName].changes.storage.length + shouldDeployImplementation && report.contracts[contractName].changes.storage.length > 0 // 3. Deploy new versions of the contract and proxy, if needed. if (shouldDeployImplementation || isLibrary) { const contract = await deployImplementation(contractName, Contract) diff --git a/packages/protocol/test/identity/attestations.ts b/packages/protocol/test/identity/attestations.ts index a357ab71bcf..4d04992e6ac 100644 --- a/packages/protocol/test/identity/attestations.ts +++ b/packages/protocol/test/identity/attestations.ts @@ -16,6 +16,8 @@ import { range, uniq } from 'lodash' import { AccountsContract, AccountsInstance, + AttestationsTestContract, + AttestationsTestInstance, MockElectionContract, MockElectionInstance, MockLockedGoldContract, @@ -27,8 +29,6 @@ import { MockValidatorsContract, RegistryContract, RegistryInstance, - AttestationsTestContract, - AttestationsTestInstance, } from 'types' import Web3 from 'web3' import { getParsedSignatureOfAddress } from '../../lib/signing-utils' From 052b2f8e4ee3b8d74a6b93c3b959ea76c4b03f94 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 18 Aug 2020 16:12:12 -0400 Subject: [PATCH 24/31] Fix migrationsconfig --- packages/protocol/migrationsConfig.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/migrationsConfig.js b/packages/protocol/migrationsConfig.js index d16c9b98774..a26323518b1 100644 --- a/packages/protocol/migrationsConfig.js +++ b/packages/protocol/migrationsConfig.js @@ -533,8 +533,8 @@ const linkedLibraries = { AddressSortedLinkedListWithMedian: ['SortedOracles', 'AddressSortedLinkedListWithMedianTest'], Signatures: [ 'Accounts', - 'TestAttestations', 'Attestations', + 'AttestationsTest', 'LockedGold', 'Escrow', 'MetaTransactionWallet', From a3b58e5c23e3da15d5484cf11f8e93bf395f3a72 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 20 Aug 2020 09:10:30 -0400 Subject: [PATCH 25/31] Address comments --- .../protocol/scripts/truffle/make-release.ts | 199 ++++++++++-------- 1 file changed, 109 insertions(+), 90 deletions(-) diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index f54219150c4..f8d34c1f6f9 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -73,6 +73,106 @@ class ContractAddresses { const REGISTRY_ADDRESS = '0x000000000000000000000000000000000000ce10' +const ensureAllContractsThatLinkLibrariesHaveChanges = ( + dependencies: ContractDependencies, + report: ASTDetailedVersionedReport, + contracts: string[] +) => { + let anyContractViolates = false + contracts.map((contract) => { + const hasDependency = dependencies.get(contract).length > 0 + const hasChanges = report.contracts[contract] + const isTest = contract.endsWith('Test') + + if (hasDependency && !hasChanges && !isTest) { + console.log( + `${contract} links ${dependencies.get( + contract + )} and needs to be upgraded to link proxied libraries.` + ) + anyContractViolates = true + } + }) + + if (anyContractViolates) { + throw new Error('All contracts linking libraries should be upgraded in release 1') + } +} + +const deployImplementation = async (contractName: string, Contract: any, dryRun: boolean) => { + console.log(`Deploying ${contractName}`) + // Hack to trick truffle, which checks that the provided address has code + const contract = await (dryRun ? Contract.at(REGISTRY_ADDRESS) : Contract.new()) + // Sanity check that any contracts that are being changed set a version number. + const getVersionNumberAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'getVersionNumber' + ) + if (!getVersionNumberAbi) { + throw new Error(`Contract ${contractName} has changes but does not specify a version number`) + } + return contract +} + +const deployProxy = async ( + contractName: string, + contract: any, + initializationData: any, + dryRun: boolean +) => { + // Explicitly forbid upgrading to a new Governance proxy contract. + // Upgrading to a new Governance proxy contract would require ownership of all + // contracts to be moved to the new governance contract, possibly including contracts + // deployed in this script. + // Because this depends on ordering (i.e. was the new GovernanceProxy deployed + // before or after other contracts in this script?), and that ordering is not being + // checked, fail if there are storage incompatible changes to Governance. + if (contractName === 'Governance') { + throw new Error(`Storage incompatible changes to Governance are not yet supported`) + } + console.log(`Deploying ${contractName}Proxy`) + const Proxy = await artifacts.require(`${contractName}Proxy`) + // Hack to trick truffle, which checks that the provided address has code + const proxy = await (dryRun ? Proxy.at(REGISTRY_ADDRESS) : Proxy.new()) + const initializeAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'initialize' + ) + console.log(`Setting ${contractName}Proxy implementation to ${contract.address}`) + if (initializeAbi) { + const args = initializationData[contractName] + console.log(`Initializing ${contractName} with: ${args}`) + if (!argv.dry_run) { + await setAndInitializeImplementation( + web3, + proxy, + contract.address, + initializeAbi, + {}, + ...args + ) + } + } else { + if (!argv.dry_run) { + await proxy._setImplementation(contract.address) + } + } + // This makes essentially every contract dependent on Governance. + console.log(`Transferring ownership of ${contractName}Proxy to Governance`) + if (!dryRun) { + await proxy._transferOwnership(addresses.get('Governance')) + } + const proxiedContract = await artifacts.require(contractName).at(proxy.address) + const transferOwnershipAbi = (contract as any).abi.find( + (abi: any) => abi.type === 'function' && abi.name === 'transferOwnership' + ) + if (transferOwnershipAbi) { + console.log(`Transferring ownership of ${contractName} to Governance`) + if (!dryRun) { + await proxiedContract.transferOwnership(addresses.get('Governance')) + } + } + return proxy +} + module.exports = async (callback: (error?: any) => number) => { try { const argv = require('minimist')(process.argv.slice(2), { @@ -95,93 +195,7 @@ module.exports = async (callback: (error?: any) => number) => { // To ensure this actually happens, we check that all contracts that link libraries are marked // as needing to be redeployed. // TODO(asa): Remove this check after release 1. - const linksLibrariesWithoutChanges = contracts.map( - (c) => - dependencies.get(c).length && - !Object.keys(report.contracts).includes(c) && - !c.endsWith('Test') - ) - if (linksLibrariesWithoutChanges.some((b) => b)) { - contracts.forEach((c, i) => { - if (linksLibrariesWithoutChanges[i]) { - console.log( - `${c} links ${dependencies.get(c)} and needs to be upgraded to link proxied libraries.` - ) - } - }) - throw new Error('All contracts linking libraries should be upgraded in release 1') - } - - const deployImplementation = async (contractName: string, Contract: any) => { - console.log(`Deploying ${contractName}`) - // Hack to trick truffle, which checks that the provided address has code - const contract = await (argv.dry_run ? Contract.at(REGISTRY_ADDRESS) : Contract.new()) - // Sanity check that any contracts that are being changed set a version number. - const getVersionNumberAbi = (contract as any).abi.find( - (abi: any) => abi.type === 'function' && abi.name === 'getVersionNumber' - ) - if (!getVersionNumberAbi) { - throw new Error( - `Contract ${contractName} has changes but does not specify a version number` - ) - } - return contract - } - - const deployProxy = async (contractName: string, contract: any) => { - // Explicitly forbid upgrading to a new Governance proxy contract. - // Upgrading to a new Governance proxy contract would require ownership of all - // contracts to be moved to the new governance contract, possibly including contracts - // deployed in this script. - // Because this depends on ordering (i.e. was the new GovernanceProxy deployed - // before or after other contracts in this script?), and that ordering is not being - // checked, fail if there are storage incompatible changes to Governance. - if (contractName === 'Governance') { - throw new Error(`Storage incompatible changes to Governance are not yet supported`) - } - console.log(`Deploying ${contractName}Proxy`) - const Proxy = await artifacts.require(`${contractName}Proxy`) - // Hack to trick truffle, which checks that the provided address has code - const proxy = await (argv.dry_run ? Proxy.at(REGISTRY_ADDRESS) : Proxy.new()) - const initializeAbi = (contract as any).abi.find( - (abi: any) => abi.type === 'function' && abi.name === 'initialize' - ) - console.log(`Setting ${contractName}Proxy implementation to ${contract.address}`) - if (initializeAbi) { - const args = initializationData[contractName] - console.log(`Initializing ${contractName} with: ${args}`) - if (!argv.dry_run) { - await setAndInitializeImplementation( - web3, - proxy, - contract.address, - initializeAbi, - {}, - ...args - ) - } - } else { - if (!argv.dry_run) { - await proxy._setImplementation(contract.address) - } - } - // This makes essentially every contract dependent on Governance. - console.log(`Transferring ownership of ${contractName}Proxy to Governance`) - if (!argv.dry_run) { - await proxy._transferOwnership(addresses.get('Governance')) - } - const proxiedContract = await artifacts.require(contractName).at(proxy.address) - const transferOwnershipAbi = (contract as any).abi.find( - (abi: any) => abi.type === 'function' && abi.name === 'transferOwnership' - ) - if (transferOwnershipAbi) { - console.log(`Transferring ownership of ${contractName} to Governance`) - if (!argv.dry_run) { - await proxiedContract.transferOwnership(addresses.get('Governance')) - } - } - return proxy - } + ensureAllContractsThatLinkLibrariesHaveChanges(dependencies, report, contracts) const release = async (contractName: string) => { if (released.has(contractName)) { @@ -199,15 +213,20 @@ module.exports = async (callback: (error?: any) => number) => { // This is a hack that will re-deploy all libraries with proxies, whether or not they have // changes to them. // TODO(asa): Remove `isLibrary` for future releases. - const isLibrary = Object.keys(linkedLibraries).includes(contractName) + const isLibrary = linkedLibraries[contractName] const shouldDeployImplementation = Object.keys(report.contracts).includes(contractName) const shouldDeployProxy = shouldDeployImplementation && report.contracts[contractName].changes.storage.length > 0 // 3. Deploy new versions of the contract and proxy, if needed. if (shouldDeployImplementation || isLibrary) { - const contract = await deployImplementation(contractName, Contract) + const contract = await deployImplementation(contractName, Contract, argv.dry_run) if (shouldDeployProxy || isLibrary) { - const proxy = await deployProxy(contractName, contract) + const proxy = await deployProxy( + contractName, + contract, + initializationData, + argv.dry_run + ) // 4. Update the contract's address, if needed. addresses.set(contractName, proxy.address) proposal.push({ From 10a3278cc905e40a104144f90817e00f7f2b566b Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 20 Aug 2020 09:11:07 -0400 Subject: [PATCH 26/31] Update docs --- .../contractkit/reference/modules/_base_.md | 30 +++---------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/packages/docs/developer-resources/contractkit/reference/modules/_base_.md b/packages/docs/developer-resources/contractkit/reference/modules/_base_.md index 607efa6b4c2..ed5c0f93c17 100644 --- a/packages/docs/developer-resources/contractkit/reference/modules/_base_.md +++ b/packages/docs/developer-resources/contractkit/reference/modules/_base_.md @@ -31,7 +31,7 @@ ___ Ƭ **CeloToken**: *[GoldToken](../enums/_base_.celocontract.md#goldtoken) | [StableToken](../enums/_base_.celocontract.md#stabletoken)* -*Defined in [contractkit/src/base.ts:53](https://github.com/celo-org/celo-monorepo/blob/master/packages/contractkit/src/base.ts#L53)* +*Defined in [contractkit/src/base.ts:31](https://github.com/celo-org/celo-monorepo/blob/master/packages/contractkit/src/base.ts#L31)* ## Variables @@ -41,7 +41,7 @@ ___ (k) => (CeloContract as any)[k as any] ) as CeloContract[] -*Defined in [contractkit/src/base.ts:55](https://github.com/celo-org/celo-monorepo/blob/master/packages/contractkit/src/base.ts#L55)* +*Defined in [contractkit/src/base.ts:33](https://github.com/celo-org/celo-monorepo/blob/master/packages/contractkit/src/base.ts#L33)* ___ @@ -49,34 +49,12 @@ ___ • **NULL_ADDRESS**: *string* = '0x0000000000000000000000000000000000000000' as Address -*Defined in [contractkit/src/base.ts:59](https://github.com/celo-org/celo-monorepo/blob/master/packages/contractkit/src/base.ts#L59)* +*Defined in [contractkit/src/base.ts:37](https://github.com/celo-org/celo-monorepo/blob/master/packages/contractkit/src/base.ts#L37)* ___ ### `Const` ProxyContracts -• **ProxyContracts**: *string[]* = [ - 'AccountsProxy', - 'AttestationsProxy', - 'BlockchainParametersProxy', - 'DoubleSigningSlasherProxy', - 'DowntimeSlasherProxy', - 'ElectionProxy', - 'EpochRewardsProxy', - 'EscrowProxy', - 'ExchangeProxy', - 'FeeCurrencyWhitelistProxy', - 'FreezerProxy', - 'GasPriceMinimumProxy', - 'GoldTokenProxy', - 'GovernanceApproverMultiSigProxy', - 'GovernanceProxy', - 'LockedGoldProxy', - 'ReserveProxy', - 'ReserveSpenderMultiSigProxy', - 'StableTokenProxy', - 'SortedOraclesProxy', - 'RegistryProxy', -] +• **ProxyContracts**: *string[]* = Object.keys(CeloContract).map((c) => `${c}Proxy`) *Defined in [contractkit/src/base.ts:29](https://github.com/celo-org/celo-monorepo/blob/master/packages/contractkit/src/base.ts#L29)* From 4429df1a4d749f49d5301c1e3fb23566748b3721 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Thu, 20 Aug 2020 09:13:49 -0400 Subject: [PATCH 27/31] Make work --- packages/protocol/scripts/truffle/make-release.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index f8d34c1f6f9..5847b044e76 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -116,6 +116,7 @@ const deployImplementation = async (contractName: string, Contract: any, dryRun: const deployProxy = async ( contractName: string, contract: any, + addresses: ContractAddresses, initializationData: any, dryRun: boolean ) => { @@ -140,7 +141,7 @@ const deployProxy = async ( if (initializeAbi) { const args = initializationData[contractName] console.log(`Initializing ${contractName} with: ${args}`) - if (!argv.dry_run) { + if (!dryRun) { await setAndInitializeImplementation( web3, proxy, @@ -151,7 +152,7 @@ const deployProxy = async ( ) } } else { - if (!argv.dry_run) { + if (!dryRun) { await proxy._setImplementation(contract.address) } } @@ -224,6 +225,7 @@ module.exports = async (callback: (error?: any) => number) => { const proxy = await deployProxy( contractName, contract, + addresses, initializationData, argv.dry_run ) From 14bd5b3e824381a0a9aee352c961984e733b5a2b Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Mon, 24 Aug 2020 18:13:50 -0400 Subject: [PATCH 28/31] Inline libraries where possible --- .../common/linkedlists/LinkedList.sol | 22 ++++--------- .../common/linkedlists/SortedLinkedList.sol | 32 ++++++++----------- .../SortedLinkedListWithMedian.sol | 28 ++++++---------- .../common/proxies/LinkedListProxy.sol | 6 ---- .../common/proxies/SortedLinkedListProxy.sol | 6 ---- .../SortedLinkedListWithMedianProxy.sol | 6 ---- packages/protocol/migrationsConfig.js | 7 ---- 7 files changed, 31 insertions(+), 76 deletions(-) delete mode 100644 packages/protocol/contracts/common/proxies/LinkedListProxy.sol delete mode 100644 packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol delete mode 100644 packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol diff --git a/packages/protocol/contracts/common/linkedlists/LinkedList.sol b/packages/protocol/contracts/common/linkedlists/LinkedList.sol index 19df7521d34..6fd90836861 100644 --- a/packages/protocol/contracts/common/linkedlists/LinkedList.sol +++ b/packages/protocol/contracts/common/linkedlists/LinkedList.sol @@ -22,14 +22,6 @@ library LinkedList { mapping(bytes32 => Element) elements; } - /** - * @notice Returns the storage, major, minor, and patch version of the contract. - * @return The storage, major, minor, and patch version of the contract. - */ - function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 1, 1, 0); - } - /** * @notice Inserts an element into a doubly linked list. * @param list A storage pointer to the underlying list. @@ -37,7 +29,7 @@ library LinkedList { * @param previousKey The key of the element that comes before the element to insert. * @param nextKey The key of the element that comes after the element to insert. */ - function insert(List storage list, bytes32 key, bytes32 previousKey, bytes32 nextKey) public { + function insert(List storage list, bytes32 key, bytes32 previousKey, bytes32 nextKey) internal { require(key != bytes32(0), "Key must be defined"); require(!contains(list, key), "Can't insert an existing element"); require( @@ -90,7 +82,7 @@ library LinkedList { * @param list A storage pointer to the underlying list. * @param key The key of the element to insert. */ - function push(List storage list, bytes32 key) public { + function push(List storage list, bytes32 key) internal { insert(list, key, bytes32(0), list.tail); } @@ -99,7 +91,7 @@ library LinkedList { * @param list A storage pointer to the underlying list. * @param key The key of the element to remove. */ - function remove(List storage list, bytes32 key) public { + function remove(List storage list, bytes32 key) internal { Element storage element = list.elements[key]; require(key != bytes32(0) && contains(list, key), "key not in list"); if (element.previousKey != bytes32(0)) { @@ -127,7 +119,7 @@ library LinkedList { * @param previousKey The key of the element that comes before the updated element. * @param nextKey The key of the element that comes after the updated element. */ - function update(List storage list, bytes32 key, bytes32 previousKey, bytes32 nextKey) public { + function update(List storage list, bytes32 key, bytes32 previousKey, bytes32 nextKey) internal { require( key != bytes32(0) && key != previousKey && key != nextKey && contains(list, key), "key on in list" @@ -142,7 +134,7 @@ library LinkedList { * @param key The element key. * @return Whether or not the key is in the sorted list. */ - function contains(List storage list, bytes32 key) public view returns (bool) { + function contains(List storage list, bytes32 key) internal view returns (bool) { return list.elements[key].exists; } @@ -153,7 +145,7 @@ library LinkedList { * @return The keys of the N elements at the head of the list. * @dev Reverts if n is greater than the number of elements in the list. */ - function headN(List storage list, uint256 n) public view returns (bytes32[] memory) { + function headN(List storage list, uint256 n) internal view returns (bytes32[] memory) { require(n <= list.numElements, "not enough elements"); bytes32[] memory keys = new bytes32[](n); bytes32 key = list.head; @@ -169,7 +161,7 @@ library LinkedList { * @param list A storage pointer to the underlying list. * @return All element keys from head to tail. */ - function getKeys(List storage list) public view returns (bytes32[] memory) { + function getKeys(List storage list) internal view returns (bytes32[] memory) { return headN(list, list.numElements); } } diff --git a/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol b/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol index 81f8efc66c1..57f63b0fe77 100644 --- a/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol +++ b/packages/protocol/contracts/common/linkedlists/SortedLinkedList.sol @@ -15,14 +15,6 @@ library SortedLinkedList { mapping(bytes32 => uint256) values; } - /** - * @notice Returns the storage, major, minor, and patch version of the contract. - * @return The storage, major, minor, and patch version of the contract. - */ - function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 1, 1, 0); - } - /** * @notice Inserts an element into a doubly linked list. * @param list A storage pointer to the underlying list. @@ -37,7 +29,7 @@ library SortedLinkedList { uint256 value, bytes32 lesserKey, bytes32 greaterKey - ) public { + ) internal { require( key != bytes32(0) && key != lesserKey && key != greaterKey && !contains(list, key), "invalid key" @@ -58,7 +50,7 @@ library SortedLinkedList { * @param list A storage pointer to the underlying list. * @param key The key of the element to remove. */ - function remove(List storage list, bytes32 key) public { + function remove(List storage list, bytes32 key) internal { list.list.remove(key); list.values[key] = 0; } @@ -78,7 +70,7 @@ library SortedLinkedList { uint256 value, bytes32 lesserKey, bytes32 greaterKey - ) public { + ) internal { // TODO(asa): Optimize by not making any changes other than value if lesserKey and greaterKey // don't change. // TODO(asa): Optimize by not updating lesserKey/greaterKey for key @@ -91,7 +83,7 @@ library SortedLinkedList { * @param list A storage pointer to the underlying list. * @param key The key of the element to insert. */ - function push(List storage list, bytes32 key) public { + function push(List storage list, bytes32 key) internal { insert(list, key, 0, bytes32(0), list.list.tail); } @@ -101,7 +93,7 @@ library SortedLinkedList { * @param n The number of elements to pop. * @return The keys of the popped elements. */ - function popN(List storage list, uint256 n) public returns (bytes32[] memory) { + function popN(List storage list, uint256 n) internal returns (bytes32[] memory) { require(n <= list.list.numElements, "not enough elements"); bytes32[] memory keys = new bytes32[](n); for (uint256 i = 0; i < n; i = i.add(1)) { @@ -118,7 +110,7 @@ library SortedLinkedList { * @param key The element key. * @return Whether or not the key is in the sorted list. */ - function contains(List storage list, bytes32 key) public view returns (bool) { + function contains(List storage list, bytes32 key) internal view returns (bool) { return list.list.contains(key); } @@ -128,7 +120,7 @@ library SortedLinkedList { * @param key The element key. * @return The element value. */ - function getValue(List storage list, bytes32 key) public view returns (uint256) { + function getValue(List storage list, bytes32 key) internal view returns (uint256) { return list.values[key]; } @@ -137,7 +129,11 @@ library SortedLinkedList { * @param list A storage pointer to the underlying list. * @return An unpacked list of elements from largest to smallest. */ - function getElements(List storage list) public view returns (bytes32[] memory, uint256[] memory) { + function getElements(List storage list) + internal + view + returns (bytes32[] memory, uint256[] memory) + { bytes32[] memory keys = getKeys(list); uint256[] memory values = new uint256[](keys.length); for (uint256 i = 0; i < keys.length; i = i.add(1)) { @@ -151,7 +147,7 @@ library SortedLinkedList { * @param list A storage pointer to the underlying list. * @return All element keys from head to tail. */ - function getKeys(List storage list) public view returns (bytes32[] memory) { + function getKeys(List storage list) internal view returns (bytes32[] memory) { return list.list.getKeys(); } @@ -162,7 +158,7 @@ library SortedLinkedList { * @return The keys of the first n elements. * @dev Reverts if n is greater than the number of elements in the list. */ - function headN(List storage list, uint256 n) public view returns (bytes32[] memory) { + function headN(List storage list, uint256 n) internal view returns (bytes32[] memory) { return list.list.headN(n); } diff --git a/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol b/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol index e06056fac3a..a58f042a881 100644 --- a/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol +++ b/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol @@ -21,14 +21,6 @@ library SortedLinkedListWithMedian { mapping(bytes32 => MedianRelation) relation; } - /** - * @notice Returns the storage, major, minor, and patch version of the contract. - * @return The storage, major, minor, and patch version of the contract. - */ - function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 1, 1, 0); - } - /** * @notice Inserts an element into a doubly linked list. * @param list A storage pointer to the underlying list. @@ -43,7 +35,7 @@ library SortedLinkedListWithMedian { uint256 value, bytes32 lesserKey, bytes32 greaterKey - ) public { + ) internal { list.list.insert(key, value, lesserKey, greaterKey); LinkedList.Element storage element = list.list.list.elements[key]; @@ -85,7 +77,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @param key The key of the element to remove. */ - function remove(List storage list, bytes32 key) public { + function remove(List storage list, bytes32 key) internal { MedianAction action = MedianAction.None; if (list.list.list.numElements == 0) { list.median = bytes32(0); @@ -128,7 +120,7 @@ library SortedLinkedListWithMedian { uint256 value, bytes32 lesserKey, bytes32 greaterKey - ) public { + ) internal { // TODO(asa): Optimize by not making any changes other than value if lesserKey and greaterKey // don't change. // TODO(asa): Optimize by not updating lesserKey/greaterKey for key @@ -141,7 +133,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @param key The key of the element to insert. */ - function push(List storage list, bytes32 key) public { + function push(List storage list, bytes32 key) internal { insert(list, key, 0, bytes32(0), list.list.list.tail); } @@ -151,7 +143,7 @@ library SortedLinkedListWithMedian { * @param n The number of elements to pop. * @return The keys of the popped elements. */ - function popN(List storage list, uint256 n) public returns (bytes32[] memory) { + function popN(List storage list, uint256 n) internal returns (bytes32[] memory) { require(n <= list.list.list.numElements, "not enough elements"); bytes32[] memory keys = new bytes32[](n); for (uint256 i = 0; i < n; i = i.add(1)) { @@ -168,7 +160,7 @@ library SortedLinkedListWithMedian { * @param key The element key. * @return Whether or not the key is in the sorted list. */ - function contains(List storage list, bytes32 key) public view returns (bool) { + function contains(List storage list, bytes32 key) internal view returns (bool) { return list.list.contains(key); } @@ -178,7 +170,7 @@ library SortedLinkedListWithMedian { * @param key The element key. * @return The element value. */ - function getValue(List storage list, bytes32 key) public view returns (uint256) { + function getValue(List storage list, bytes32 key) internal view returns (uint256) { return list.list.values[key]; } @@ -187,7 +179,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @return The median value. */ - function getMedianValue(List storage list) public view returns (uint256) { + function getMedianValue(List storage list) internal view returns (uint256) { return getValue(list, list.median); } @@ -233,7 +225,7 @@ library SortedLinkedListWithMedian { * @return An unpacked list of elements from largest to smallest. */ function getElements(List storage list) - public + internal view returns (bytes32[] memory, uint256[] memory, MedianRelation[] memory) { @@ -252,7 +244,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @return All element keys from head to tail. */ - function getKeys(List storage list) public view returns (bytes32[] memory) { + function getKeys(List storage list) internal view returns (bytes32[] memory) { return list.list.getKeys(); } diff --git a/packages/protocol/contracts/common/proxies/LinkedListProxy.sol b/packages/protocol/contracts/common/proxies/LinkedListProxy.sol deleted file mode 100644 index 5804cb74827..00000000000 --- a/packages/protocol/contracts/common/proxies/LinkedListProxy.sol +++ /dev/null @@ -1,6 +0,0 @@ -pragma solidity ^0.5.3; - -import "../Proxy.sol"; - -/* solhint-disable no-empty-blocks */ -contract LinkedListProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol b/packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol deleted file mode 100644 index d6c00b7a139..00000000000 --- a/packages/protocol/contracts/common/proxies/SortedLinkedListProxy.sol +++ /dev/null @@ -1,6 +0,0 @@ -pragma solidity ^0.5.3; - -import "../Proxy.sol"; - -/* solhint-disable no-empty-blocks */ -contract SortedLinkedListProxy is Proxy {} diff --git a/packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol b/packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol deleted file mode 100644 index 2b6dbfa9a43..00000000000 --- a/packages/protocol/contracts/common/proxies/SortedLinkedListWithMedianProxy.sol +++ /dev/null @@ -1,6 +0,0 @@ -pragma solidity ^0.5.3; - -import "../Proxy.sol"; - -/* solhint-disable no-empty-blocks */ -contract SortedLinkedListWithMedianProxy is Proxy {} diff --git a/packages/protocol/migrationsConfig.js b/packages/protocol/migrationsConfig.js index a26323518b1..9f2a3bbc3ac 100644 --- a/packages/protocol/migrationsConfig.js +++ b/packages/protocol/migrationsConfig.js @@ -520,13 +520,6 @@ const linkedLibraries = { 'Validators', ], Proposals: ['Governance', 'GovernanceTest', 'ProposalsTest'], - LinkedList: ['AddressLinkedList', 'SortedLinkedList', 'LinkedListTest'], - SortedLinkedList: [ - 'AddressSortedLinkedList', - 'IntegerSortedLinkedList', - 'SortedLinkedListWithMedian', - ], - SortedLinkedListWithMedian: ['AddressSortedLinkedListWithMedian'], AddressLinkedList: ['Validators', 'ValidatorsTest'], AddressSortedLinkedList: ['Election', 'ElectionTest'], IntegerSortedLinkedList: ['Governance', 'GovernanceTest', 'IntegerSortedLinkedListTest'], From 783e5b7b0eb0b87ef3dbfe526601ca9d29b37820 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Mon, 24 Aug 2020 19:14:09 -0400 Subject: [PATCH 29/31] Address feedback --- .../protocol/scripts/bash/check-versions.sh | 2 +- .../protocol/scripts/bash/make-release.sh | 6 ++-- .../protocol/scripts/truffle/make-release.ts | 28 +++++++++++++------ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index 296e451a0fc..779a1fd046d 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -19,7 +19,7 @@ while getopts 'a:b:n:r:' flag; do case "${flag}" in a) BRANCH_1="${OPTARG}" ;; b) BRANCH_2="${OPTARG}" ;; - n) NETWORK="$OPTARG" ;; + n) NETWORK="${OPTARG}" ;; r) REPORT="${OPTARG}" ;; *) error "Unexpected option ${flag}" ;; esac diff --git a/packages/protocol/scripts/bash/make-release.sh b/packages/protocol/scripts/bash/make-release.sh index 7b1bfb9216f..a1c72443446 100755 --- a/packages/protocol/scripts/bash/make-release.sh +++ b/packages/protocol/scripts/bash/make-release.sh @@ -21,9 +21,9 @@ while getopts 'a:b:n:p:i:r:' flag; do case "${flag}" in a) BRANCH_1="${OPTARG}" ;; b) BRANCH_2="${OPTARG}" ;; - n) NETWORK="$OPTARG" ;; - p) PROPOSAL="$OPTARG" ;; - i) INITIALIZE_DATA="$OPTARG" ;; + n) NETWORK="${OPTARG}" ;; + p) PROPOSAL="${OPTARG}" ;; + i) INITIALIZE_DATA="${OPTARG}" ;; r) REPORT="${OPTARG}" ;; *) error "Unexpected option ${flag}" ;; esac diff --git a/packages/protocol/scripts/truffle/make-release.ts b/packages/protocol/scripts/truffle/make-release.ts index 5847b044e76..2193edaeb07 100644 --- a/packages/protocol/scripts/truffle/make-release.ts +++ b/packages/protocol/scripts/truffle/make-release.ts @@ -6,6 +6,7 @@ import { linkedLibraries } from '@celo/protocol/migrationsConfig' import { Address, eqAddress, NULL_ADDRESS } from '@celo/utils/lib/address' import { readdirSync, readJsonSync, writeJsonSync } from 'fs-extra' import { basename, join } from 'path' +import { RegistryInstance } from 'types' /* * A script that reads a backwards compatibility report, deploys changed contracts, and creates @@ -19,12 +20,11 @@ import { basename, join } from 'path' * truffle exec scripts/truffle/make-release \ * --network alfajores --build_directory build/alfajores/ --report report.json \ * --initialize_data initialize_data.json --proposal proposal.json - * */ class ContractDependencies { dependencies: Map - constructor(libraries: any) { + constructor(libraries: { [library: string]: string[] }) { this.dependencies = new Map() Object.keys(libraries).forEach((lib: string) => { libraries[lib].forEach((contract: string) => { @@ -43,7 +43,7 @@ class ContractDependencies { } class ContractAddresses { - static async create(contracts: string[], registry: any) { + static async create(contracts: string[], registry: RegistryInstance) { const addresses = new Map() await Promise.all( contracts.map(async (contract: string) => { @@ -99,12 +99,16 @@ const ensureAllContractsThatLinkLibrariesHaveChanges = ( } } -const deployImplementation = async (contractName: string, Contract: any, dryRun: boolean) => { +const deployImplementation = async ( + contractName: string, + Contract: Truffle.Contract, + dryRun: boolean +) => { console.log(`Deploying ${contractName}`) // Hack to trick truffle, which checks that the provided address has code const contract = await (dryRun ? Contract.at(REGISTRY_ADDRESS) : Contract.new()) // Sanity check that any contracts that are being changed set a version number. - const getVersionNumberAbi = (contract as any).abi.find( + const getVersionNumberAbi = contract.abi.find( (abi: any) => abi.type === 'function' && abi.name === 'getVersionNumber' ) if (!getVersionNumberAbi) { @@ -115,9 +119,9 @@ const deployImplementation = async (contractName: string, Contract: any, dryRun: const deployProxy = async ( contractName: string, - contract: any, + contract: Truffle.ContractInstance, addresses: ContractAddresses, - initializationData: any, + initializationData: { [contract: string]: any[] }, dryRun: boolean ) => { // Explicitly forbid upgrading to a new Governance proxy contract. @@ -174,6 +178,14 @@ const deployProxy = async ( return proxy } +interface ProposalTx { + contract: string + function: string + args: string[] + value: string + description?: string +} + module.exports = async (callback: (error?: any) => number) => { try { const argv = require('minimist')(process.argv.slice(2), { @@ -189,7 +201,7 @@ module.exports = async (callback: (error?: any) => number) => { const registry = await artifacts.require('Registry').at(REGISTRY_ADDRESS) const addresses = await ContractAddresses.create(contracts, registry) const released: Set = new Set([]) - const proposal = [] + const proposal: ProposalTx[] = [] // Release 1 will deploy all libraries with proxies so that they're more easily // upgradable. All contracts that link libraries should be upgraded to instead link the proxied // library. From 36e977b80296f0aeb40019eb5380bbea97fe52b9 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 25 Aug 2020 09:32:00 -0400 Subject: [PATCH 30/31] Inlined library should be inlined --- .../common/linkedlists/SortedLinkedListWithMedian.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol b/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol index a58f042a881..2255483daa9 100644 --- a/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol +++ b/packages/protocol/contracts/common/linkedlists/SortedLinkedListWithMedian.sol @@ -188,7 +188,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @return The key of the first element in the list. */ - function getHead(List storage list) external view returns (bytes32) { + function getHead(List storage list) internal view returns (bytes32) { return list.list.list.head; } @@ -197,7 +197,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @return The key of the median element in the list. */ - function getMedian(List storage list) external view returns (bytes32) { + function getMedian(List storage list) internal view returns (bytes32) { return list.median; } @@ -206,7 +206,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @return The key of the last element in the list. */ - function getTail(List storage list) external view returns (bytes32) { + function getTail(List storage list) internal view returns (bytes32) { return list.list.list.tail; } @@ -215,7 +215,7 @@ library SortedLinkedListWithMedian { * @param list A storage pointer to the underlying list. * @return The number of elements in the list. */ - function getNumElements(List storage list) external view returns (uint256) { + function getNumElements(List storage list) internal view returns (uint256) { return list.list.list.numElements; } From 2c22704e560227c524d8e5a68e3146d14ad65009 Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 25 Aug 2020 09:33:52 -0400 Subject: [PATCH 31/31] Ignore inlined libraries --- packages/protocol/scripts/bash/check-versions.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/protocol/scripts/bash/check-versions.sh b/packages/protocol/scripts/bash/check-versions.sh index 779a1fd046d..b15f13f98a4 100755 --- a/packages/protocol/scripts/bash/check-versions.sh +++ b/packages/protocol/scripts/bash/check-versions.sh @@ -49,7 +49,7 @@ if [ ! -z "$REPORT" ]; then REPORT_FLAG="-f "$REPORT fi -# Exclude test contracts, mock contracts, contract interfaces, Proxy contracts, MultiSig contracts, -# and the ReleaseGold contract. -CONTRACT_EXCLUSION_REGEX=".*Test|Mock.*|I[A-Z].*|.*Proxy|MultiSig.*|ReleaseGold" +# Exclude test contracts, mock contracts, contract interfaces, Proxy contracts, inlined libraries, +# MultiSig contracts, and the ReleaseGold contract. +CONTRACT_EXCLUSION_REGEX=".*Test|Mock.*|I[A-Z].*|.*Proxy|LinkedList|SortedLinkedList|SortedLinkedListWithMedian|MultiSig.*|ReleaseGold" yarn ts-node scripts/check-backward.ts sem_check -o $BUILD_DIR_1/contracts -n $BUILD_DIR_2/contracts -e $CONTRACT_EXCLUSION_REGEX $REPORT_FLAG