Skip to content

Commit 9d19100

Browse files
pahor167Pavel Hornakclaude
authored
fix(governance): proxy-repoint log + show proposal id in governance:propose (#782)
## Summary Two `governance:propose` output bugs reported on celocli 9.0.0: 1. **`undefined is a proxy, repointing to ...`** — `ProposalBuilder.fromJsonTx` logged `tx.address`, which is undefined for core-contract proxy repoints that identify the target by `contract` (the map key already fell back to `tx.contract`). Now logs the real proxy id (`address || contract`). 2. **Proposal id missing from output** — after submitting, `governance:propose` printed only the tx hash. It now decodes the `ProposalQueued` event via `displayViemTx`, so the new proposal id is shown. ## Test plan - [x] `@celo/governance` build + tests (42/42) - [x] `@celo/celocli` build + `governance:propose` test (14/14) - [ ] CI green 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- start pr-codex --> --- ## PR-Codex overview This PR focuses on enhancing governance commands and utilities, improving proposal handling, and refining event logging. It introduces new checks, updates to proposal execution, and better handling of slashing penalties in rewards. ### Detailed summary - Added `proposalIsApproved` check in `governance/execute.ts`. - Introduced `--slashing` flag in `rewards/show.ts` for slashing penalties. - Improved logging in tests to confirm successful checks and actions. - Updated error messages for deprecated flags in `authorize.test.ts`. - Enhanced proposal handling in `governance/propose.ts` with new simulation options. - Refined event decoding and logging in several governance commands. - Fixed proposal filtering logic to ensure only groups with pending votes are considered. - Improved handling of pending withdrawals in `lockedcelo/withdraw.ts`. - Added system anvil binary resolution for test harness compatibility. > The following files were skipped due to too many changes: `packages/cli/src/utils/cli.ts` > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex --> --------- Co-authored-by: Pavel Hornak <pavel.hornak@clabs.co> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e2641eb commit 9d19100

41 files changed

Lines changed: 1079 additions & 195 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
'@celo/governance': patch
3+
'@celo/actions': patch
4+
'@celo/dev-utils': patch
5+
'@celo/explorer': patch
6+
'@celo/celocli': patch
7+
---
8+
9+
Fix several `governance`/`celocli` command output & safety issues:
10+
- `governance:propose` logged `undefined is a proxy, repointing to ...` for
11+
core-contract proxy repoints (logged `tx.address` which is undefined when the
12+
tx is keyed by `contract`); now logs the real proxy id.
13+
- `governance:propose` now surfaces the new proposal id (`ProposalQueued`), and
14+
the `--useMultiSig` path surfaces the multisig transaction id (`Submission` on
15+
submit, `Confirmation` on a later signer) plus the proposal id when the submit
16+
reaches threshold and executes in the same receipt.
17+
- `@celo/actions` `getGroupsWithPendingVotes` now filters on pending votes `> 0`
18+
(was `>= 0`, which returned every group); fixes `election:activate` selecting
19+
groups with no pending votes.
20+
- `governance:execute` now checks the proposal is approved before sending, so it
21+
fails the precondition cleanly instead of reverting with "Proposal not approved".
22+
- `governance:upvote`/`revokeupvote`/`votePartially` and `multisig:approve` now
23+
decode and print their on-chain events (proposal id / transaction id).
24+
- `governance:propose` can now build a core-contract call whose method is added
25+
by an earlier upgrade tx in the same proposal: when the method is absent from
26+
the bundled ABI, it is resolved from the implementation a prior tx repoints the
27+
proxy to (verified metadata), with a raw `function: "name(uint256)"` signature
28+
fallback.
29+
- `governance:propose` now simulates the proposal by default against a
30+
self-contained local fork (bundled `@foundry-rs/anvil`) of the connected node,
31+
applying the transactions in order so a transaction that depends on an earlier
32+
one (e.g. a method added by a prior upgrade tx) simulates correctly. Use
33+
`--simulate <rpcUrl>` to target an external fork, or `--no-simulate` to fall
34+
back to the previous independent per-transaction `eth_call` checks.
35+
- `lockedcelo:withdraw` (and `releasecelo:locked-gold` withdraw) no longer spin
36+
in an infinite loop when no pending withdrawal is available, and re-fetch
37+
between withdrawals to avoid stale indices.
38+
- `@celo/dev-utils` anvil test harness now resolves the foundry-installed
39+
`anvil` (snapshot-compatible) instead of a package-manager `anvil` bin shim,
40+
so packages that bundle a newer anvil don't break the devchain state load.
41+
- `@celo/explorer` `fetchMetadata` now uses the Sourcify v2 API (the v1 repo API
42+
has been sunset / returns 503), so contract ABI resolution (used by
43+
`governance:propose` to build calls to verified contracts, including
44+
implementations added by an in-proposal upgrade) works again.
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import type { Address } from 'viem'
2+
import { describe, expect, it, vi } from 'vitest'
3+
import { getGroupsWithPendingVotes } from './election.js'
4+
5+
// resolveAddress hits the on-chain registry; stub it so the unit test only
6+
// exercises the pending-votes filtering logic.
7+
vi.mock('./registry.js', () => ({
8+
resolveAddress: vi.fn(async () => '0x0000000000000000000000000000000000000001'),
9+
}))
10+
11+
const ACCOUNT = '0x00000000000000000000000000000000000000aa' as Address
12+
const GROUP_A = '0x000000000000000000000000000000000000000a' as Address
13+
const GROUP_B = '0x000000000000000000000000000000000000000b' as Address
14+
const GROUP_C = '0x000000000000000000000000000000000000000c' as Address
15+
16+
function clients(groups: Address[], pendingVotes: bigint[]) {
17+
return {
18+
public: {
19+
readContract: vi.fn(async () => groups),
20+
multicall: vi.fn(async () => pendingVotes),
21+
},
22+
} as any
23+
}
24+
25+
describe('getGroupsWithPendingVotes', () => {
26+
it('returns only groups whose pending votes are strictly greater than zero', async () => {
27+
// Regression: the filter used `>= 0`, which kept every group (including
28+
// those with 0 pending votes). It must be `> 0`.
29+
const result = await getGroupsWithPendingVotes(
30+
clients([GROUP_A, GROUP_B, GROUP_C], [BigInt(0), BigInt(5), BigInt(0)]),
31+
ACCOUNT
32+
)
33+
expect(result).toEqual([GROUP_B])
34+
})
35+
36+
it('returns an empty array when every group has zero pending votes', async () => {
37+
const result = await getGroupsWithPendingVotes(
38+
clients([GROUP_A, GROUP_B], [BigInt(0), BigInt(0)]),
39+
ACCOUNT
40+
)
41+
expect(result).toEqual([])
42+
})
43+
})

packages/actions/src/contracts/election.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { electionABI } from '@celo/abis'
2-
import { Address, getContract, GetContractReturnType, Hex, isAddressEqual } from 'viem'
2+
import { Address, GetContractReturnType, getContract, Hex, isAddressEqual } from 'viem'
33
import { Clients } from '../client.js'
44
import { resolveAddress } from './registry.js'
55

@@ -47,7 +47,7 @@ export async function getGroupsWithPendingVotes(
4747
}) as const
4848
),
4949
})
50-
const groupsWithPendingVotes = groups.filter((_, i) => pendingVotes[i] >= 0)
50+
const groupsWithPendingVotes = groups.filter((_, i) => pendingVotes[i] > BigInt(0))
5151
return groupsWithPendingVotes
5252
}
5353

packages/cli/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
"@celo/wallet-hsm-azure": "^8.0.4",
5555
"@celo/wallet-ledger": "^8.0.4",
5656
"@celo/wallet-local": "^8.0.4",
57+
"@foundry-rs/anvil": "1.7.1",
5758
"@ledgerhq/hw-transport-node-hid": "^6.28.5",
5859
"@oclif/core": "^3.27.0",
5960
"@oclif/plugin-autocomplete": "^3.2.0",
@@ -65,6 +66,7 @@
6566
"@safe-global/protocol-kit": "^5.0.4",
6667
"@safe-global/types-kit": "^1.0.0",
6768
"@types/command-exists": "^1.2.3",
69+
"@viem/anvil": "^0.0.9",
6870
"bignumber.js": "9.0.0",
6971
"chalk": "^2.4.2",
7072
"command-exists": "^1.2.9",

packages/cli/src/commands/account/authorize.test.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,7 @@ testWithAnvilL2('account:authorize cmd', (provider) => {
256256

257257
provider
258258
)
259-
).rejects.toMatchInlineSnapshot(`
260-
[Error: Nonexistent flags: --blsKey, --blsPop
261-
See more help with --help]
262-
`)
259+
).rejects.toMatchInlineSnapshot(`[Error: BLS keys are not supported anymore]`)
263260

264261
expect(stripAnsiCodesFromNestedArray(logMock.mock.calls)).toMatchInlineSnapshot(`[]`)
265262
})

packages/cli/src/commands/account/authorize.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,18 @@ export default class Authorize extends BaseCommand {
2929
default: false,
3030
hidden: true,
3131
}),
32+
// Declared (hidden, deprecated) only so passing them yields the clear
33+
// "BLS keys are not supported anymore" error below instead of oclif's
34+
// generic unknown-flag rejection.
35+
blsKey: Flags.string({ hidden: true, deprecated: true }),
36+
blsPop: Flags.string({ hidden: true, deprecated: true }),
3237
}
3338

3439
static args = {}
3540

3641
static examples = [
3742
'authorize --from 0x5409ED021D9299bf6814279A6A1411A7e866A631 --role vote --signer 0x6ecbe1db9ef729cbe972c83fb886247691fb6beb --signature 0x1b9fca4bbb5bfb1dbe69ef1cddbd9b4202dcb6b134c5170611e1e36ecfa468d7b46c85328d504934fce6c2a1571603a50ae224d2b32685e84d4d1a1eebad8452eb',
38-
'authorize --from 0x5409ED021D9299bf6814279A6A1411A7e866A631 --role validator --signer 0x6ecbe1db9ef729cbe972c83fb886247691fb6beb --signature 0x1b9fca4bbb5bfb1dbe69ef1cddbd9b4202dcb6b134c5170611e1e36ecfa468d7b46c85328d504934fce6c2a1571603a50ae224d2b32685e84d4d1a1eebad8452eb --blsKey 0x4fa3f67fc913878b068d1fa1cdddc54913d3bf988dbe5a36a20fa888f20d4894c408a6773f3d7bde11154f2a3076b700d345a42fd25a0e5e83f4db5586ac7979ac2053cd95d8f2efd3e959571ceccaa743e02cf4be3f5d7aaddb0b06fc9aff00 --blsPop 0xcdb77255037eb68897cd487fdd85388cbda448f617f874449d4b11588b0b7ad8ddc20d9bb450b513bb35664ea3923900',
43+
'authorize --from 0x5409ED021D9299bf6814279A6A1411A7e866A631 --role validator --signer 0x6ecbe1db9ef729cbe972c83fb886247691fb6beb --signature 0x1b9fca4bbb5bfb1dbe69ef1cddbd9b4202dcb6b134c5170611e1e36ecfa468d7b46c85328d504934fce6c2a1571603a50ae224d2b32685e84d4d1a1eebad8452eb',
3944
]
4045

4146
async run() {

packages/cli/src/commands/account/deauthorize.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ testWithAnvilL2('account:deauthorize cmd', (provider) => {
4747

4848
expect(stripAnsiCodesFromNestedArray(logMock.mock.calls)).toMatchInlineSnapshot(`
4949
[
50+
[
51+
"Running Checks:",
52+
],
53+
[
54+
" ✔ 0x5409ED021D9299bf6814279A6A1411A7e866A631 is a registered Account ",
55+
],
56+
[
57+
"All checks passed",
58+
],
5059
[
5160
"SendTransaction: deauthorizeTx",
5261
],

packages/cli/src/commands/account/deauthorize.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Flags } from '@oclif/core'
22
import { BaseCommand } from '../../base'
3+
import { newCheckBuilder } from '../../utils/checks'
34
import { displayViemTx } from '../../utils/cli'
45
import { CustomFlags } from '../../utils/command'
56

@@ -36,6 +37,8 @@ export default class Deauthorize extends BaseCommand {
3637
return
3738
}
3839

40+
await newCheckBuilder(this).isAccount(res.flags.from).runChecks()
41+
3942
const attestationSigner = await accounts.getAttestationSigner(res.flags.from)
4043

4144
if (res.flags.signer !== attestationSigner) {

packages/cli/src/commands/account/delete-payment-delegation.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { BaseCommand } from '../../base'
2+
import { newCheckBuilder } from '../../utils/checks'
23
import { displayViemTx } from '../../utils/cli'
34
import { CustomFlags } from '../../utils/command'
45

@@ -24,6 +25,8 @@ export default class DeletePaymentDelegation extends BaseCommand {
2425
kit.defaultAccount = res.flags.account
2526
const accounts = await kit.contracts.getAccounts()
2627

28+
await newCheckBuilder(this).isAccount(res.flags.account).runChecks()
29+
2730
await displayViemTx('deletePaymentDelegation', accounts.deletePaymentDelegation(), publicClient)
2831

2932
console.log('Deleted payment delegation.')

packages/cli/src/commands/account/set-payment-delegation.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { valueToFixidityString } from '@celo/contractkit/lib/wrappers/BaseWrapper'
22
import { Flags } from '@oclif/core'
33
import { BaseCommand } from '../../base'
4+
import { newCheckBuilder } from '../../utils/checks'
45
import { displayViemTx } from '../../utils/cli'
56
import { CustomFlags } from '../../utils/command'
67

@@ -28,6 +29,8 @@ export default class SetPaymentDelegation extends BaseCommand {
2829
kit.defaultAccount = res.flags.account
2930
const accounts = await kit.contracts.getAccounts()
3031

32+
await newCheckBuilder(this).isAccount(res.flags.account).runChecks()
33+
3134
await displayViemTx(
3235
'setPaymentDelegation',
3336
accounts.setPaymentDelegation(

0 commit comments

Comments
 (0)