Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/allow approval in execution #296

Merged
merged 15 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/two-squids-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@celo/celocli': patch
---

Allows approving proposals when in Approval or Referendum stages
nicolasbrugneaux marked this conversation as resolved.
Show resolved Hide resolved
127 changes: 124 additions & 3 deletions packages/cli/src/commands/governance/approve.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { StrongAddress } from '@celo/base'
import { Provider, getRandomId } from '@celo/connect'
import { newKitFromWeb3 } from '@celo/contractkit'
import { GovernanceWrapper } from '@celo/contractkit/lib/wrappers/Governance'
import { GovernanceWrapper, ProposalStage } from '@celo/contractkit/lib/wrappers/Governance'
import { testWithAnvil } from '@celo/dev-utils/lib/anvil-test'
import { timeTravel } from '@celo/dev-utils/lib/ganache-test'
import { ux } from '@oclif/core'
Expand All @@ -12,6 +13,28 @@ import Approve from './approve'
process.env.NO_SYNCCHECK = 'true'

testWithAnvil('governance:approve cmd', (web3: Web3) => {
function sendRawTx(method: string, params: any[]) {
return new Promise<string>((resolve, reject) => {
;(kit.web3.currentProvider as Provider).send(
{
id: getRandomId(),
jsonrpc: '2.0',
method,
params,
},
(error, resp) => {
if (error) {
reject(error)
} else if (resp) {
resolve(resp.result as string)
} else {
reject(new Error('empty-response'))
}
}
)
})
}

const kit = newKitFromWeb3(web3)
const proposalID = '1'
let minDeposit: string
Expand Down Expand Up @@ -72,7 +95,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 ",
Expand Down Expand Up @@ -110,7 +133,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 ",
Expand All @@ -128,4 +151,102 @@ 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 sendRawTx('anvil_impersonateAccount', [approver])
nicolasbrugneaux marked this conversation as resolved.
Show resolved Hide resolved
await sendRawTx('anvil_setBalance', [approver, '0x10000000000000000000'])
nicolasbrugneaux marked this conversation as resolved.
Show resolved Hide resolved

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:",
],
[
" ✔ 0x1B46C60e8B3B427d91d35df22FCfc3FF36407f35 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:",
],
[
" ✔ 0x1B46C60e8B3B427d91d35df22FCfc3FF36407f35 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",
],
]
`)
})
})
})
6 changes: 3 additions & 3 deletions packages/cli/src/commands/governance/approve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ export default class Approve extends BaseCommand {

await checkBuilder
.proposalExists(id)
.proposalInStage(
.proposalInStages(
id,
governanceVersion.storage === '1' && Number(governanceVersion.major) < 3
? 'Approval'
: 'Referendum'
? ['Approval']
nicolasbrugneaux marked this conversation as resolved.
Show resolved Hide resolved
: ['Referendum', 'Execution']
)
.addCheck(`${id} not already approved`, async () => !(await governance.isApproved(id)))
.runChecks()
Expand Down
11 changes: 9 additions & 2 deletions packages/cli/src/utils/checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,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)
Expand Down
Loading