Make the same hotfix proposal executable with unique salts (#2357)

packages/celotool/src/cmds/deploy/upgrade/hotfix.ts

@@ -2,12 +2,13 @@
 
 import { newKit } from '@celo/contractkit'
 import {
+  hotfixToHash,
   ProposalBuilder,
-  proposalToHash,
   proposalToJSON,
 } from '@celo/contractkit/lib/governance/proposals'
 import { privateKeyToAddress } from '@celo/utils/lib/address'
 import { concurrentMap } from '@celo/utils/lib/async'
+import { randomBytes } from 'crypto'
 import { getFornoUrl } from 'src/lib/endpoints'
 import { envVar, fetchEnv } from 'src/lib/env-utils'
 import { AccountType, getPrivateKeysFor } from 'src/lib/generate_utils'
@@ -78,7 +79,9 @@ export const handler = async (argv: EthstatsArgv) => {
       console.error('\nPlease see examples in hotfix.ts and add transactions')
       process.exit(1)
     }
-    const proposalHash = proposalToHash(kit, proposal)
+
+    const salt = randomBytes(32)
+    const proposalHash = hotfixToHash(kit, proposal, salt)
 
     // If your proposal is just made of Celo Registry contract methods, you can print it out
     console.info('Proposal: ', await proposalToJSON(kit, proposal))
@@ -119,7 +122,7 @@ export const handler = async (argv: EthstatsArgv) => {
       throw new Error()
     }
     console.info('\nExecute the hotfix')
-    await governance.executeHotfix(proposal).sendAndWaitForReceipt({ from: addresses[0] })
+    await governance.executeHotfix(proposal, salt).sendAndWaitForReceipt({ from: addresses[0] })
 
     hotfixRecord = await governance.getHotfixRecord(proposalHash)
     console.info('\nHotfix Record: ', hotfixRecord)

packages/cli/src/commands/governance/executehotfix.ts

@@ -11,6 +11,7 @@ export default class ExecuteHotfix extends BaseCommand {
     ...BaseCommand.flags,
     from: Flags.address({ required: true, description: "Executors's address" }),
     jsonTransactions: flags.string({ required: true, description: 'Path to json transactions' }),
+    salt: flags.string({ required: true, description: 'Secret salt associated with hotfix' }),
   }
 
   static examples = []
@@ -20,7 +21,9 @@ export default class ExecuteHotfix extends BaseCommand {
 
     const governance = await this.kit.contracts.getGovernance()
     const hotfix = await buildProposalFromJsonFile(this.kit, res.flags.jsonTransactions)
-    const tx = governance.executeHotfix(hotfix)
+
+    const saltBuff = Buffer.from(res.flags.salt, 'hex')
+    const tx = governance.executeHotfix(hotfix, saltBuff)
     await displaySendTx('executeHotfixTx', tx, { from: res.flags.from })
   }
 }

packages/contractkit/src/governance/proposals.ts

@@ -7,19 +7,18 @@ import { obtainKitContractDetails } from '../explorer/base'
 import { BlockExplorer } from '../explorer/block-explorer'
 import { ABI as GovernanceABI } from '../generated/Governance'
 import { ContractKit } from '../kit'
+import { getAbiTypes } from '../utils/web3-utils'
 import { CeloTransactionObject, valueToString } from '../wrappers/BaseWrapper'
-import { GovernanceWrapper, Proposal, ProposalTransaction } from '../wrappers/Governance'
+import { hotfixToParams, Proposal, ProposalTransaction } from '../wrappers/Governance'
 import { setImplementationOnProxy } from './proxy'
 
-export const PROPOSE_PARAM_ABI_TYPES = (GovernanceABI.find(
-  (abiEntry) => abiEntry.name! === 'propose'
-)!.inputs! as Array<{ type: string }>).map((abiInput) => abiInput.type)
+export const HOTFIX_PARAM_ABI_TYPES = getAbiTypes(GovernanceABI as any, 'executeHotfix')
 
-export const proposalToEncodedParams = (kit: ContractKit, proposal: Proposal) =>
-  kit.web3.eth.abi.encodeParameters(PROPOSE_PARAM_ABI_TYPES, GovernanceWrapper.toParams(proposal))
+export const hotfixToEncodedParams = (kit: ContractKit, proposal: Proposal, salt: Buffer) =>
+  kit.web3.eth.abi.encodeParameters(HOTFIX_PARAM_ABI_TYPES, hotfixToParams(proposal, salt))
 
-export const proposalToHash = (kit: ContractKit, proposal: Proposal) =>
-  keccak256(proposalToEncodedParams(kit, proposal)) as Buffer
+export const hotfixToHash = (kit: ContractKit, proposal: Proposal, salt: Buffer) =>
+  keccak256(hotfixToEncodedParams(kit, proposal, salt)) as Buffer
 
 export interface ProposalTransactionJSON {
   contract: CeloContract

packages/contractkit/src/utils/web3-utils.ts

@@ -1,5 +1,6 @@
 import debugFactory from 'debug'
 import Web3 from 'web3'
+import { ABIDefinition } from 'web3/eth/abi'
 import { CeloProvider } from '../providers/celo-provider'
 
 const debug = debugFactory('kit:web3:utils')
@@ -18,3 +19,6 @@ export function addLocalAccount(web3: Web3, privateKey: string): Web3 {
   debug('Providers configured')
   return web3
 }
+
+export const getAbiTypes = (abi: ABIDefinition[], methodName: string) =>
+  abi.find((entry) => entry.name! === methodName)!.inputs!.map((input) => input.type)

packages/contractkit/src/wrappers/Governance.ts

@@ -54,6 +54,16 @@ export type ProposalParams = Parameters<Governance['methods']['propose']>
 export type ProposalTransaction = Pick<Transaction, 'to' | 'input' | 'value'>
 export type Proposal = ProposalTransaction[]
 
+export const proposalToParams = (proposal: Proposal): ProposalParams => {
+  const data = proposal.map((tx) => stringToBuffer(tx.input))
+  return [
+    proposal.map((tx) => tx.value),
+    proposal.map((tx) => tx.to),
+    bufferToBytes(Buffer.concat(data)),
+    data.map((inp) => inp.length),
+  ]
+}
+
 export interface ProposalRecord {
   stage: ProposalStage
   metadata: ProposalMetadata
@@ -80,8 +90,13 @@ export interface Votes {
   [VoteValue.Abstain]: BigNumber
 }
 
+export type HotfixParams = Parameters<Governance['methods']['executeHotfix']>
+export const hotfixToParams = (proposal: Proposal, salt: Buffer): HotfixParams => {
+  const p = proposalToParams(proposal)
+  return [p[0], p[1], p[2], p[3], bufferToString(salt)]
+}
+
 export interface HotfixRecord {
-  hash: Buffer
   approved: boolean
   executed: boolean
   preparedEpoch: BigNumber
@@ -183,16 +198,6 @@ export class GovernanceWrapper extends BaseWrapper<Governance> {
     })
   )
 
-  static toParams = (proposal: Proposal): ProposalParams => {
-    const data = proposal.map((tx) => stringToBuffer(tx.input))
-    return [
-      proposal.map((tx) => tx.value),
-      proposal.map((tx) => tx.to),
-      bufferToBytes(Buffer.concat(data)),
-      data.map((inp) => inp.length),
-    ]
-  }
-
   /**
    * Returns whether a given proposal is approved.
    * @param proposalID Governance proposal UUID
@@ -259,7 +264,7 @@ export class GovernanceWrapper extends BaseWrapper<Governance> {
    * Submits a new governance proposal.
    * @param proposal Governance proposal
    */
-  propose = proxySend(this.kit, this.contract.methods.propose, GovernanceWrapper.toParams)
+  propose = proxySend(this.kit, this.contract.methods.propose, proposalToParams)
 
   /**
    * Returns whether a governance proposal exists with the given ID.
@@ -509,7 +514,6 @@ export class GovernanceWrapper extends BaseWrapper<Governance> {
   async getHotfixRecord(hash: Buffer): Promise<HotfixRecord> {
     const res = await this.contract.methods.getHotfixRecord(bufferToString(hash)).call()
     return {
-      hash,
       approved: res[0],
       executed: res[1],
       preparedEpoch: valueToBigNumber(res[2]),
@@ -575,11 +579,8 @@ export class GovernanceWrapper extends BaseWrapper<Governance> {
   /**
    * Executes a given sequence of transactions if the corresponding hash is prepared and approved.
    * @param hotfix Governance hotfix proposal
+   * @param salt Secret which guarantees uniqueness of hash
    * @notice keccak256 hash of abi encoded transactions computed on-chain
    */
-  executeHotfix = proxySend(
-    this.kit,
-    this.contract.methods.executeHotfix,
-    GovernanceWrapper.toParams
-  )
+  executeHotfix = proxySend(this.kit, this.contract.methods.executeHotfix, hotfixToParams)
 }

packages/docs/command-line-interface/governance.md

@@ -60,6 +60,7 @@ USAGE
 OPTIONS
   --from=0xc1912fEE45d61C87Cc5EA59DaE31190FFFFf232d  (required) Executors's address
   --jsonTransactions=jsonTransactions                (required) Path to json transactions
+  --salt=salt                                        (required) Secret salt associated with hotfix
 ```
 
 _See code: [packages/cli/src/commands/governance/executehotfix.ts](https://github.com/celo-org/celo-monorepo/tree/master/packages/cli/src/commands/governance/executehotfix.ts)_

packages/protocol/contracts/governance/Governance.sol

@@ -655,6 +655,7 @@ contract Governance is
    * @param hash The abi encoded keccak256 hash of the hotfix transaction(s) to be whitelisted.
    */
   function approveHotfix(bytes32 hash) external {
+    require(!hotfixes[hash].executed, "hotfix already executed");
     require(msg.sender == approver, "Not approver");
     hotfixes[hash].approved = true;
     emit HotfixApproved(hash);
@@ -674,6 +675,7 @@ contract Governance is
    * @param hash The abi encoded keccak256 hash of the hotfix transaction(s) to be whitelisted.
    */
   function whitelistHotfix(bytes32 hash) external {
+    require(!hotfixes[hash].executed, "hotfix already executed");
     hotfixes[hash].whitelisted[msg.sender] = true;
     emit HotfixWhitelisted(hash, msg.sender);
   }
@@ -683,6 +685,7 @@ contract Governance is
    * @param hash The hash of the hotfix to be prepared.
    */
   function prepareHotfix(bytes32 hash) external {
+    require(!hotfixes[hash].executed, "hotfix already executed");
     require(isHotfixPassing(hash), "hotfix not whitelisted by 2f+1 validators");
     uint256 epoch = getEpochNumber();
     require(hotfixes[hash].preparedEpoch < epoch, "hotfix already prepared for this epoch");
@@ -696,15 +699,17 @@ contract Governance is
    * @param destinations The destination addresses of the proposed transactions.
    * @param data The concatenated data to be included in the proposed transactions.
    * @param dataLengths The lengths of each transaction's data.
+   * @param salt Secret associated with hotfix which guarantees uniqueness of hash.
    * @dev Reverts if hotfix is already executed, not approved, or not prepared for current epoch.
    */
   function executeHotfix(
     uint256[] calldata values,
     address[] calldata destinations,
     bytes calldata data,
-    uint256[] calldata dataLengths
+    uint256[] calldata dataLengths,
+    bytes32 salt
   ) external {
-    bytes32 hash = keccak256(abi.encode(values, destinations, data, dataLengths));
+    bytes32 hash = keccak256(abi.encode(values, destinations, data, dataLengths, salt));
 
     (bool approved, bool executed, uint256 preparedEpoch) = getHotfixRecord(hash);
     require(!executed, "hotfix already executed");