Fix/allow approval in execution (#296)

* feat: add proposal checker with multiple allowed stages * fix: mimic code style better * test: add approval tests * test: snapshots oops * chore: add changeset * chore: snapshots * fix: changeset typo * fix: use impersonateAccount and add a balance helper * fix: hex -> number/bigint/bignumber --------- Co-authored-by: bowd <[email protected]> Co-authored-by: boqdan <[email protected]>
celo-org · Jul 24, 2024 · de1e5a0 · de1e5a0
1 parent 6d6c3fb
commit de1e5a0
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 21 deletions.
diff --git a/.changeset/two-squids-clean.md b/.changeset/two-squids-clean.md
@@ -0,0 +1,5 @@
+---
+'@celo/celocli': patch
+---
+
+Allows approving proposals when in Execution or Referendum stages
diff --git a/packages/cli/src/commands/governance/approve.test.ts b/packages/cli/src/commands/governance/approve.test.ts
@@ -1,7 +1,7 @@
 import { StrongAddress } from '@celo/base'
 import { newKitFromWeb3 } from '@celo/contractkit'
-import { GovernanceWrapper } from '@celo/contractkit/lib/wrappers/Governance'
-import { testWithAnvil } from '@celo/dev-utils/lib/anvil-test'
+import { GovernanceWrapper, ProposalStage } from '@celo/contractkit/lib/wrappers/Governance'
+import { impersonateAccount, testWithAnvil } from '@celo/dev-utils/lib/anvil-test'
 import { timeTravel } from '@celo/dev-utils/lib/ganache-test'
 import { ux } from '@oclif/core'
 import Web3 from 'web3'
@@ -72,7 +72,7 @@ testWithAnvil('governance:approve cmd', (web3: Web3) => {
           "   ✔  1 is an existing proposal ",
         ],
         [
-          "   ✔  1 is in stage Referendum ",
+          "   ✔  1 is in stage Referendum or Execution ",
         ],
         [
           "   ✔  1 not already approved ",
@@ -110,7 +110,7 @@ testWithAnvil('governance:approve cmd', (web3: Web3) => {
           "   ✔  1 is an existing proposal ",
         ],
         [
-          "   ✔  1 is in stage Referendum ",
+          "   ✔  1 is in stage Referendum or Execution ",
         ],
         [
           "   ✔  1 not already approved ",
@@ -128,4 +128,101 @@ testWithAnvil('governance:approve cmd', (web3: Web3) => {
     `)
     expect(writeMock.mock.calls).toMatchInlineSnapshot(`[]`)
   })
+
+  describe('approve succeeds if stage is "Referendum or Execution" or "Approval"', () => {
+    test('can be approved if version >= 3 (default)', async () => {
+      const logMock = jest.spyOn(console, 'log')
+
+      await governance
+        .propose([], 'https://example.com')
+        .sendAndWaitForReceipt({ value: minDeposit })
+
+      const approver = await governance.getApprover()
+      await impersonateAccount(web3, approver, 1000000000000000000n)
+
+      let proposalId = (await governance.getQueue())[0].proposalID
+      await expect(governance.getProposalStage(proposalId)).resolves.toBe(ProposalStage.Queued)
+      await expect(
+        testLocallyWithWeb3Node(
+          Approve,
+          ['--from', approver, '--proposalID', proposalId.toString()],
+          web3
+        )
+      ).rejects.not.toBeUndefined()
+      const schedule = await governance.proposalSchedule(proposalId)
+      expect(logMock.mock.calls.map((args) => args.map(stripAnsiCodes))).toMatchInlineSnapshot(`
+        [
+          [
+            "Running Checks:",
+          ],
+          [
+            "   ✔  0x078B932B0d1e56554974A431B8B33973D94E002b is approver address ",
+          ],
+          [
+            "   ✔  2 is an existing proposal ",
+          ],
+          [
+            "Expiration: ${schedule.Expiration?.toString()} (~${schedule.Expiration?.toExponential(
+        3
+      )})
+        Queued: ${schedule.Queued?.toString()} (~${schedule.Queued?.toExponential(3)})",
+          ],
+          [
+            "   ✘  2 is in stage Referendum or Execution ",
+          ],
+          [
+            "   ✔  2 not already approved ",
+          ],
+        ]
+      `)
+      logMock.mockClear()
+
+      const dequeueFrequency = (await governance.dequeueFrequency()).toNumber()
+      await timeTravel(dequeueFrequency, web3)
+      await governance.dequeueProposalsIfReady().sendAndWaitForReceipt()
+
+      await governance.vote(proposalId, 'Yes')
+      await expect(governance.getProposalStage(proposalId)).resolves.toBe(ProposalStage.Referendum)
+      await testLocallyWithWeb3Node(
+        Approve,
+        ['--from', approver, '--proposalID', proposalId.toString()],
+        web3
+      )
+      const txHash = stripAnsiCodes(logMock.mock.calls.at(-3)![0].split(':')[1].trim())
+      expect(logMock.mock.calls.map((args) => args.map(stripAnsiCodes))).toMatchInlineSnapshot(`
+        [
+          [
+            "Running Checks:",
+          ],
+          [
+            "   ✔  0x078B932B0d1e56554974A431B8B33973D94E002b is approver address ",
+          ],
+          [
+            "   ✔  2 is an existing proposal ",
+          ],
+          [
+            "   ✔  2 is in stage Referendum or Execution ",
+          ],
+          [
+            "   ✔  2 not already approved ",
+          ],
+          [
+            "All checks passed",
+          ],
+          [
+            "SendTransaction: approveTx",
+          ],
+          [
+            "txHash: ${txHash}",
+          ],
+          [
+            "ProposalApproved:",
+          ],
+          [
+            "proposalId: 2",
+          ],
+        ]
+      `)
+    })
+  })
 })
diff --git a/packages/cli/src/commands/governance/approve.ts b/packages/cli/src/commands/governance/approve.ts
@@ -92,16 +92,9 @@ export default class Approve extends BaseCommand {
         await governance.dequeueProposalsIfReady().sendAndWaitForReceipt()
       }
 
-      const governanceVersion = await governance.version()
-
       await checkBuilder
         .proposalExists(id)
-        .proposalInStage(
-          id,
-          governanceVersion.storage === '1' && Number(governanceVersion.major) < 3
-            ? 'Approval'
-            : 'Referendum'
-        )
+        .proposalInStages(id, ['Referendum', 'Execution'])
         .addCheck(`${id} not already approved`, async () => !(await governance.isApproved(id)))
         .runChecks()
       governanceTx = await governance.approve(id)

diff --git a/packages/cli/src/utils/checks.ts b/packages/cli/src/utils/checks.ts
@@ -163,10 +163,17 @@ class CheckBuilder {
     )
 
   proposalInStage = (proposalID: string, stage: keyof typeof ProposalStage) =>
+    this.proposalInStages(proposalID, [stage])
+
+  proposalInStages = (proposalID: string, stages: (keyof typeof ProposalStage)[]) =>
     this.addCheck(
-      `${proposalID} is in stage ${stage}`,
+      `${proposalID} is in stage ${stages.join(' or ')}`,
       this.withGovernance(async (governance) => {
-        const match = (await governance.getProposalStage(proposalID)) === stage
+        const stage = await governance.getProposalStage(proposalID)
+        let match = false
+        for (const allowedStage of stages) {
+          match = stage === allowedStage || match
+        }
         if (!match) {
           const schedule = await governance.proposalSchedule(proposalID)
           printValueMapRecursive(schedule)

diff --git a/packages/dev-utils/src/anvil-test.ts b/packages/dev-utils/src/anvil-test.ts
@@ -1,6 +1,7 @@
 import { StrongAddress } from '@celo/base'
 import { PROXY_ADMIN_ADDRESS } from '@celo/connect'
 import { Anvil, CreateAnvilOptions, createAnvil } from '@viem/anvil'
+import BigNumber from 'bignumber.js'
 import Web3 from 'web3'
 import {
   TEST_BALANCE,
@@ -52,8 +53,17 @@ export function testWithAnvil(name: string, fn: (web3: Web3) => void) {
   })
 }
 
-export function impersonateAccount(web3: Web3, address: string) {
-  return jsonRpcCall(web3, 'anvil_impersonateAccount', [address])
+export function impersonateAccount(
+  web3: Web3,
+  address: string,
+  withBalance?: number | bigint | BigNumber
+) {
+  return Promise.all([
+    jsonRpcCall(web3, 'anvil_impersonateAccount', [address]),
+    withBalance
+      ? jsonRpcCall(web3, 'anvil_setBalance', [address, `0x${withBalance.toString(16)}`])
+      : undefined,
+  ])
 }
 
 export function stopImpersonatingAccount(web3: Web3, address: string) {

diff --git a/packages/dev-utils/src/chain-setup.ts b/packages/dev-utils/src/chain-setup.ts
@@ -12,9 +12,12 @@ export async function setCommissionUpdateDelay(
   withImpersonatedAccount(web3, DEFAULT_OWNER_ADDRESS, async () => {
     const validators = newValidators(web3, validatorsContractAddress)
 
-    await validators.methods.setCommissionUpdateDelay(delayInBlocks).send({
-      from: DEFAULT_OWNER_ADDRESS,
-    })
+    const { transactionHash } = await validators.methods
+      .setCommissionUpdateDelay(delayInBlocks)
+      .send({
+        from: DEFAULT_OWNER_ADDRESS,
+      })
+    await web3.eth.getTransactionReceipt(transactionHash)
   })
 }
 
@@ -26,9 +29,10 @@ export async function setDequeueFrequency(
   withImpersonatedAccount(web3, DEFAULT_OWNER_ADDRESS, async () => {
     const governance = newGovernance(web3, governanceContractAddress)
 
-    await governance.methods.setDequeueFrequency(frequency).send({
+    const { transactionHash } = await governance.methods.setDequeueFrequency(frequency).send({
       from: DEFAULT_OWNER_ADDRESS,
     })
+    await web3.eth.getTransactionReceipt(transactionHash)
   })
 }
 
@@ -40,8 +44,9 @@ export async function setReferendumStageDuration(
   withImpersonatedAccount(web3, DEFAULT_OWNER_ADDRESS, async () => {
     const governance = newGovernance(web3, governanceContractAddress)
 
-    await governance.methods.setReferendumStageDuration(duration).send({
+    const { transactionHash } = await governance.methods.setReferendumStageDuration(duration).send({
       from: DEFAULT_OWNER_ADDRESS,
     })
+    await web3.eth.getTransactionReceipt(transactionHash)
   })
 }