From de1e5a02bb0361aa42b486e9227222b20dfb8416 Mon Sep 17 00:00:00 2001 From: Nicolas Brugneaux Date: Wed, 24 Jul 2024 11:09:33 +0200 Subject: [PATCH] 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 Co-authored-by: boqdan <304771+bowd@users.noreply.github.com> --- .changeset/two-squids-clean.md | 5 + .../src/commands/governance/approve.test.ts | 105 +++++++++++++++++- .../cli/src/commands/governance/approve.ts | 9 +- packages/cli/src/utils/checks.ts | 11 +- packages/dev-utils/src/anvil-test.ts | 14 ++- packages/dev-utils/src/chain-setup.ts | 15 ++- 6 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 .changeset/two-squids-clean.md diff --git a/.changeset/two-squids-clean.md b/.changeset/two-squids-clean.md new file mode 100644 index 000000000..101c4f221 --- /dev/null +++ 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 index 1fca384ce..b4b062a37 100644 --- 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 index 988f4eb01..1d05ad3c3 100644 --- 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 index 82b858192..b62779d3c 100644 --- 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 index 0f7798f30..c961f6119 100644 --- 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 index 716607f27..435a049d2 100644 --- 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) }) }