diff --git a/.changeset/fix-propose-proxy-log-and-proposalid.md b/.changeset/fix-propose-proxy-log-and-proposalid.md new file mode 100644 index 0000000000..8bd79d0c58 --- /dev/null +++ b/.changeset/fix-propose-proxy-log-and-proposalid.md @@ -0,0 +1,44 @@ +--- +'@celo/governance': patch +'@celo/actions': patch +'@celo/dev-utils': patch +'@celo/explorer': patch +'@celo/celocli': patch +--- + +Fix several `governance`/`celocli` command output & safety issues: +- `governance:propose` logged `undefined is a proxy, repointing to ...` for + core-contract proxy repoints (logged `tx.address` which is undefined when the + tx is keyed by `contract`); now logs the real proxy id. +- `governance:propose` now surfaces the new proposal id (`ProposalQueued`), and + the `--useMultiSig` path surfaces the multisig transaction id (`Submission` on + submit, `Confirmation` on a later signer) plus the proposal id when the submit + reaches threshold and executes in the same receipt. +- `@celo/actions` `getGroupsWithPendingVotes` now filters on pending votes `> 0` + (was `>= 0`, which returned every group); fixes `election:activate` selecting + groups with no pending votes. +- `governance:execute` now checks the proposal is approved before sending, so it + fails the precondition cleanly instead of reverting with "Proposal not approved". +- `governance:upvote`/`revokeupvote`/`votePartially` and `multisig:approve` now + decode and print their on-chain events (proposal id / transaction id). +- `governance:propose` can now build a core-contract call whose method is added + by an earlier upgrade tx in the same proposal: when the method is absent from + the bundled ABI, it is resolved from the implementation a prior tx repoints the + proxy to (verified metadata), with a raw `function: "name(uint256)"` signature + fallback. +- `governance:propose` now simulates the proposal by default against a + self-contained local fork (bundled `@foundry-rs/anvil`) of the connected node, + applying the transactions in order so a transaction that depends on an earlier + one (e.g. a method added by a prior upgrade tx) simulates correctly. Use + `--simulate ` to target an external fork, or `--no-simulate` to fall + back to the previous independent per-transaction `eth_call` checks. +- `lockedcelo:withdraw` (and `releasecelo:locked-gold` withdraw) no longer spin + in an infinite loop when no pending withdrawal is available, and re-fetch + between withdrawals to avoid stale indices. +- `@celo/dev-utils` anvil test harness now resolves the foundry-installed + `anvil` (snapshot-compatible) instead of a package-manager `anvil` bin shim, + so packages that bundle a newer anvil don't break the devchain state load. +- `@celo/explorer` `fetchMetadata` now uses the Sourcify v2 API (the v1 repo API + has been sunset / returns 503), so contract ABI resolution (used by + `governance:propose` to build calls to verified contracts, including + implementations added by an in-proposal upgrade) works again. diff --git a/packages/actions/src/contracts/election.test.ts b/packages/actions/src/contracts/election.test.ts new file mode 100644 index 0000000000..4ce2f83d46 --- /dev/null +++ b/packages/actions/src/contracts/election.test.ts @@ -0,0 +1,43 @@ +import type { Address } from 'viem' +import { describe, expect, it, vi } from 'vitest' +import { getGroupsWithPendingVotes } from './election.js' + +// resolveAddress hits the on-chain registry; stub it so the unit test only +// exercises the pending-votes filtering logic. +vi.mock('./registry.js', () => ({ + resolveAddress: vi.fn(async () => '0x0000000000000000000000000000000000000001'), +})) + +const ACCOUNT = '0x00000000000000000000000000000000000000aa' as Address +const GROUP_A = '0x000000000000000000000000000000000000000a' as Address +const GROUP_B = '0x000000000000000000000000000000000000000b' as Address +const GROUP_C = '0x000000000000000000000000000000000000000c' as Address + +function clients(groups: Address[], pendingVotes: bigint[]) { + return { + public: { + readContract: vi.fn(async () => groups), + multicall: vi.fn(async () => pendingVotes), + }, + } as any +} + +describe('getGroupsWithPendingVotes', () => { + it('returns only groups whose pending votes are strictly greater than zero', async () => { + // Regression: the filter used `>= 0`, which kept every group (including + // those with 0 pending votes). It must be `> 0`. + const result = await getGroupsWithPendingVotes( + clients([GROUP_A, GROUP_B, GROUP_C], [BigInt(0), BigInt(5), BigInt(0)]), + ACCOUNT + ) + expect(result).toEqual([GROUP_B]) + }) + + it('returns an empty array when every group has zero pending votes', async () => { + const result = await getGroupsWithPendingVotes( + clients([GROUP_A, GROUP_B], [BigInt(0), BigInt(0)]), + ACCOUNT + ) + expect(result).toEqual([]) + }) +}) diff --git a/packages/actions/src/contracts/election.ts b/packages/actions/src/contracts/election.ts index acd41759b6..8861c3216a 100644 --- a/packages/actions/src/contracts/election.ts +++ b/packages/actions/src/contracts/election.ts @@ -1,5 +1,5 @@ import { electionABI } from '@celo/abis' -import { Address, getContract, GetContractReturnType, Hex, isAddressEqual } from 'viem' +import { Address, GetContractReturnType, getContract, Hex, isAddressEqual } from 'viem' import { Clients } from '../client.js' import { resolveAddress } from './registry.js' @@ -47,7 +47,7 @@ export async function getGroupsWithPendingVotes( }) as const ), }) - const groupsWithPendingVotes = groups.filter((_, i) => pendingVotes[i] >= 0) + const groupsWithPendingVotes = groups.filter((_, i) => pendingVotes[i] > BigInt(0)) return groupsWithPendingVotes } diff --git a/packages/cli/package.json b/packages/cli/package.json index 2b16dd5758..7ef9bb00bf 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -54,6 +54,7 @@ "@celo/wallet-hsm-azure": "^8.0.4", "@celo/wallet-ledger": "^8.0.4", "@celo/wallet-local": "^8.0.4", + "@foundry-rs/anvil": "1.7.1", "@ledgerhq/hw-transport-node-hid": "^6.28.5", "@oclif/core": "^3.27.0", "@oclif/plugin-autocomplete": "^3.2.0", @@ -65,6 +66,7 @@ "@safe-global/protocol-kit": "^5.0.4", "@safe-global/types-kit": "^1.0.0", "@types/command-exists": "^1.2.3", + "@viem/anvil": "^0.0.9", "bignumber.js": "9.0.0", "chalk": "^2.4.2", "command-exists": "^1.2.9", diff --git a/packages/cli/src/commands/account/authorize.test.ts b/packages/cli/src/commands/account/authorize.test.ts index 7c6139d7b2..02f2ab7efe 100644 --- a/packages/cli/src/commands/account/authorize.test.ts +++ b/packages/cli/src/commands/account/authorize.test.ts @@ -256,10 +256,7 @@ testWithAnvilL2('account:authorize cmd', (provider) => { provider ) - ).rejects.toMatchInlineSnapshot(` - [Error: Nonexistent flags: --blsKey, --blsPop - See more help with --help] - `) + ).rejects.toMatchInlineSnapshot(`[Error: BLS keys are not supported anymore]`) expect(stripAnsiCodesFromNestedArray(logMock.mock.calls)).toMatchInlineSnapshot(`[]`) }) diff --git a/packages/cli/src/commands/account/authorize.ts b/packages/cli/src/commands/account/authorize.ts index 5777c33a9c..471b1efab1 100644 --- a/packages/cli/src/commands/account/authorize.ts +++ b/packages/cli/src/commands/account/authorize.ts @@ -29,13 +29,18 @@ export default class Authorize extends BaseCommand { default: false, hidden: true, }), + // Declared (hidden, deprecated) only so passing them yields the clear + // "BLS keys are not supported anymore" error below instead of oclif's + // generic unknown-flag rejection. + blsKey: Flags.string({ hidden: true, deprecated: true }), + blsPop: Flags.string({ hidden: true, deprecated: true }), } static args = {} static examples = [ 'authorize --from 0x5409ED021D9299bf6814279A6A1411A7e866A631 --role vote --signer 0x6ecbe1db9ef729cbe972c83fb886247691fb6beb --signature 0x1b9fca4bbb5bfb1dbe69ef1cddbd9b4202dcb6b134c5170611e1e36ecfa468d7b46c85328d504934fce6c2a1571603a50ae224d2b32685e84d4d1a1eebad8452eb', - 'authorize --from 0x5409ED021D9299bf6814279A6A1411A7e866A631 --role validator --signer 0x6ecbe1db9ef729cbe972c83fb886247691fb6beb --signature 0x1b9fca4bbb5bfb1dbe69ef1cddbd9b4202dcb6b134c5170611e1e36ecfa468d7b46c85328d504934fce6c2a1571603a50ae224d2b32685e84d4d1a1eebad8452eb --blsKey 0x4fa3f67fc913878b068d1fa1cdddc54913d3bf988dbe5a36a20fa888f20d4894c408a6773f3d7bde11154f2a3076b700d345a42fd25a0e5e83f4db5586ac7979ac2053cd95d8f2efd3e959571ceccaa743e02cf4be3f5d7aaddb0b06fc9aff00 --blsPop 0xcdb77255037eb68897cd487fdd85388cbda448f617f874449d4b11588b0b7ad8ddc20d9bb450b513bb35664ea3923900', + 'authorize --from 0x5409ED021D9299bf6814279A6A1411A7e866A631 --role validator --signer 0x6ecbe1db9ef729cbe972c83fb886247691fb6beb --signature 0x1b9fca4bbb5bfb1dbe69ef1cddbd9b4202dcb6b134c5170611e1e36ecfa468d7b46c85328d504934fce6c2a1571603a50ae224d2b32685e84d4d1a1eebad8452eb', ] async run() { diff --git a/packages/cli/src/commands/account/deauthorize.test.ts b/packages/cli/src/commands/account/deauthorize.test.ts index a00566cb26..5055a38dd7 100644 --- a/packages/cli/src/commands/account/deauthorize.test.ts +++ b/packages/cli/src/commands/account/deauthorize.test.ts @@ -47,6 +47,15 @@ testWithAnvilL2('account:deauthorize cmd', (provider) => { expect(stripAnsiCodesFromNestedArray(logMock.mock.calls)).toMatchInlineSnapshot(` [ + [ + "Running Checks:", + ], + [ + " ✔ 0x5409ED021D9299bf6814279A6A1411A7e866A631 is a registered Account ", + ], + [ + "All checks passed", + ], [ "SendTransaction: deauthorizeTx", ], diff --git a/packages/cli/src/commands/account/deauthorize.ts b/packages/cli/src/commands/account/deauthorize.ts index ba84e62b7e..8a69b30de9 100644 --- a/packages/cli/src/commands/account/deauthorize.ts +++ b/packages/cli/src/commands/account/deauthorize.ts @@ -1,5 +1,6 @@ import { Flags } from '@oclif/core' import { BaseCommand } from '../../base' +import { newCheckBuilder } from '../../utils/checks' import { displayViemTx } from '../../utils/cli' import { CustomFlags } from '../../utils/command' @@ -36,6 +37,8 @@ export default class Deauthorize extends BaseCommand { return } + await newCheckBuilder(this).isAccount(res.flags.from).runChecks() + const attestationSigner = await accounts.getAttestationSigner(res.flags.from) if (res.flags.signer !== attestationSigner) { diff --git a/packages/cli/src/commands/account/delete-payment-delegation.ts b/packages/cli/src/commands/account/delete-payment-delegation.ts index b87a62f385..6c48236118 100644 --- a/packages/cli/src/commands/account/delete-payment-delegation.ts +++ b/packages/cli/src/commands/account/delete-payment-delegation.ts @@ -1,4 +1,5 @@ import { BaseCommand } from '../../base' +import { newCheckBuilder } from '../../utils/checks' import { displayViemTx } from '../../utils/cli' import { CustomFlags } from '../../utils/command' @@ -24,6 +25,8 @@ export default class DeletePaymentDelegation extends BaseCommand { kit.defaultAccount = res.flags.account const accounts = await kit.contracts.getAccounts() + await newCheckBuilder(this).isAccount(res.flags.account).runChecks() + await displayViemTx('deletePaymentDelegation', accounts.deletePaymentDelegation(), publicClient) console.log('Deleted payment delegation.') diff --git a/packages/cli/src/commands/account/set-payment-delegation.ts b/packages/cli/src/commands/account/set-payment-delegation.ts index 09c2f5b2b0..9241ef968a 100644 --- a/packages/cli/src/commands/account/set-payment-delegation.ts +++ b/packages/cli/src/commands/account/set-payment-delegation.ts @@ -1,6 +1,7 @@ import { valueToFixidityString } from '@celo/contractkit/lib/wrappers/BaseWrapper' import { Flags } from '@oclif/core' import { BaseCommand } from '../../base' +import { newCheckBuilder } from '../../utils/checks' import { displayViemTx } from '../../utils/cli' import { CustomFlags } from '../../utils/command' @@ -28,6 +29,8 @@ export default class SetPaymentDelegation extends BaseCommand { kit.defaultAccount = res.flags.account const accounts = await kit.contracts.getAccounts() + await newCheckBuilder(this).isAccount(res.flags.account).runChecks() + await displayViemTx( 'setPaymentDelegation', accounts.setPaymentDelegation( diff --git a/packages/cli/src/commands/account/set-wallet.ts b/packages/cli/src/commands/account/set-wallet.ts index 51f8dab3f5..2fa51721f1 100644 --- a/packages/cli/src/commands/account/set-wallet.ts +++ b/packages/cli/src/commands/account/set-wallet.ts @@ -39,17 +39,19 @@ export default class SetWallet extends BaseCommand { await newCheckBuilder(this).isAccount(res.flags.account).runChecks() if (res.flags.signature !== undefined) { + let signature: ReturnType try { - accounts.parseSignatureOfAddress(res.flags.account, res.flags.signer, res.flags.signature) - } catch (error) { - console.error('Error: Failed to parse signature') + signature = accounts.parseSignatureOfAddress( + res.flags.account, + res.flags.signer, + res.flags.signature + ) + } catch (_error) { + return this.error('Failed to parse signature') } await displayViemTx( 'setWalletAddress', - accounts.setWalletAddress( - res.flags.wallet, - accounts.parseSignatureOfAddress(res.flags.account, res.flags.signer, res.flags.signature) - ), + accounts.setWalletAddress(res.flags.wallet, signature), publicClient ) } else { diff --git a/packages/cli/src/commands/governance/execute.test.ts b/packages/cli/src/commands/governance/execute.test.ts index 287406bf73..0c317759f8 100644 --- a/packages/cli/src/commands/governance/execute.test.ts +++ b/packages/cli/src/commands/governance/execute.test.ts @@ -1,3 +1,4 @@ +import path from 'node:path' import { AbiItem, PROXY_ADMIN_ADDRESS } from '@celo/connect' import { newKitFromProvider } from '@celo/contractkit' import { Proposal } from '@celo/contractkit/lib/wrappers/Governance' @@ -9,10 +10,9 @@ import { } from '@celo/dev-utils/anvil-test' import { timeTravel } from '@celo/dev-utils/ganache-test' import fs from 'fs' -import path from 'node:path' +import { decodeFunctionResult, encodeFunctionData, parseEther } from 'viem' import { stripAnsiCodesAndTxHashes, testLocallyWithNode } from '../../test-utils/cliUtils' import Execute from './execute' -import { decodeFunctionResult, encodeFunctionData, parseEther } from 'viem' process.env.NO_SYNCCHECK = 'true' @@ -196,6 +196,9 @@ testWithAnvilL2('governance:execute cmd', (provider) => { [ " ✔ 1 is in stage Execution ", ], + [ + " ✔ Proposal 1 is approved ", + ], [ " ✔ Proposal 1 is passing corresponding constitutional quorum ", ], @@ -217,4 +220,54 @@ testWithAnvilL2('governance:execute cmd', (provider) => { ] `) }) + + it('fails the approved check for an unapproved proposal in the Execution stage', async () => { + const kit = newKitFromProvider(provider) + const governanceWrapper = await kit.contracts.getGovernance() + const [, proposer, voter] = await kit.connection.getAccounts() + const minDeposit = (await governanceWrapper.minDeposit()).toFixed() + const lockedGold = await kit.contracts.getLockedGold() + const majorityOfVotes = (await lockedGold.getTotalLockedGold()).multipliedBy(0.6) + const dequeueFrequency = (await governanceWrapper.dequeueFrequency()).toNumber() + const proposalId = 1 + + await setCode(provider, PROXY_ADMIN_ADDRESS, TEST_TRANSACTIONS_BYTECODE) + + const proposeHash = await governanceWrapper.propose(PROPOSAL_TRANSACTIONS, 'URL', { + from: proposer, + value: minDeposit, + }) + await kit.connection.viemClient.waitForTransactionReceipt({ + hash: proposeHash as `0x${string}`, + }) + + const accountWrapper = await kit.contracts.getAccounts() + const createHash = await accountWrapper.createAccount({ from: voter }) + await kit.connection.viemClient.waitForTransactionReceipt({ hash: createHash as `0x${string}` }) + const lockHash = await lockedGold.lock({ from: voter, value: majorityOfVotes.toFixed() }) + await kit.connection.viemClient.waitForTransactionReceipt({ hash: lockHash as `0x${string}` }) + + await timeTravel(dequeueFrequency + 1, provider) + const dequeueHash = await governanceWrapper.dequeueProposalsIfReady({ from: proposer }) + await kit.connection.viemClient.waitForTransactionReceipt({ + hash: dequeueHash as `0x${string}`, + }) + + // NB: intentionally NOT approving the proposal, then vote + advance to Execution + const lockHash2 = await lockedGold.lock({ from: voter, value: minDeposit }) + await kit.connection.viemClient.waitForTransactionReceipt({ hash: lockHash2 as `0x${string}` }) + const voteHash = await governanceWrapper.vote(proposalId, 'Yes', { from: voter }) + await kit.connection.viemClient.waitForTransactionReceipt({ hash: voteHash as `0x${string}` }) + await timeTravel((await governanceWrapper.stageDurations()).Referendum.toNumber() + 1, provider) + + // The command must fail the on-chain approval precondition check rather than + // sending a tx that reverts with "Proposal not approved". + await expect( + testLocallyWithNode( + Execute, + ['--proposalID', proposalId.toString(), '--from', proposer], + provider + ) + ).rejects.toThrow(/checks didn't pass/i) + }) }) diff --git a/packages/cli/src/commands/governance/execute.ts b/packages/cli/src/commands/governance/execute.ts index ae1d360454..13a786d514 100644 --- a/packages/cli/src/commands/governance/execute.ts +++ b/packages/cli/src/commands/governance/execute.ts @@ -27,6 +27,7 @@ export default class Execute extends BaseCommand { await newCheckBuilder(this, account) .proposalExists(id) .proposalInStage(id, 'Execution') + .proposalIsApproved(id) .proposalIsPassing(id) .runChecks() diff --git a/packages/cli/src/commands/governance/propose.test.ts b/packages/cli/src/commands/governance/propose.test.ts index 0f88f0c03d..d6a12d1bfa 100644 --- a/packages/cli/src/commands/governance/propose.test.ts +++ b/packages/cli/src/commands/governance/propose.test.ts @@ -193,6 +193,8 @@ testWithAnvilL2( const proposalBefore = await governance.getProposal(1) expect(proposalBefore).toEqual([]) + const logMock = jest.spyOn(console, 'log') + await testLocallyWithNode( Propose, [ @@ -208,6 +210,13 @@ testWithAnvilL2( client ) + // The command must surface the newly created proposal id (decoded from + // the ProposalQueued event), not just the tx hash. + const loggedText = stripAnsiCodesFromNestedArray(logMock.mock.calls).flat().join('\n') + logMock.mockRestore() + expect(loggedText).toContain('ProposalQueued:') + expect(loggedText).toContain('proposalId: 1') + const proposal = await governance.getProposal(1) expect(proposal.length).toEqual(transactions.length) expect(proposal[0].to).toEqual(goldToken.address) @@ -251,6 +260,8 @@ testWithAnvilL2( const proposalBefore = await governance.getProposal(1) expect(proposalBefore).toEqual([]) + const logMock = jest.spyOn(console, 'log') + await testLocallyWithNode( Propose, [ @@ -269,6 +280,16 @@ testWithAnvilL2( client ) + // With a single-signer multisig the submit reaches threshold and the + // underlying governance.propose executes in the same receipt, so the + // command must surface BOTH the multisig submission id and the new + // proposal id (not just the tx hash). + const loggedText = stripAnsiCodesFromNestedArray(logMock.mock.calls).flat().join('\n') + logMock.mockRestore() + expect(loggedText).toContain('Submission:') + expect(loggedText).toContain('ProposalQueued:') + expect(loggedText).toContain('proposalId: 1') + const proposal = await governance.getProposal(1) expect(proposal.length).toEqual(transactions.length) expect(proposal[0].to).toEqual(goldToken.address) diff --git a/packages/cli/src/commands/governance/propose.ts b/packages/cli/src/commands/governance/propose.ts index e66518b4a2..9ea1b48091 100644 --- a/packages/cli/src/commands/governance/propose.ts +++ b/packages/cli/src/commands/governance/propose.ts @@ -1,9 +1,11 @@ -import { ProposalBuilder, proposalToJSON, ProposalTransactionJSON } from '@celo/governance' +import { governanceABI, multiSigABI } from '@celo/abis' import { proposalToParams } from '@celo/contractkit/lib/wrappers/Governance' +import { ProposalBuilder, ProposalTransactionJSON, proposalToJSON } from '@celo/governance' import { Flags } from '@oclif/core' import { BigNumber } from 'bignumber.js' import { readFileSync } from 'fs' import { BaseCommand } from '../../base' +import { withAnvilFork } from '../../utils/anvil-fork' import { newCheckBuilder } from '../../utils/checks' import { displayViemTx, printValueMapRecursive } from '../../utils/cli' import { CustomFlags } from '../../utils/command' @@ -39,8 +41,14 @@ export default class Propose extends BaseCommand { simulate: Flags.string({ required: false, description: - 'RPC URL of a forked node (e.g. anvil) to simulate the proposal against. Each proposal transaction is actually sent (not eth_call) from the Governance contract address, which the node must have unlocked (e.g. anvil --auto-impersonate). Replaces the default eth_call simulation. Useful for proposals where the success of one tx depends on a previous one succeeding.', - exclusive: ['force'], + 'RPC URL of a forked node (e.g. anvil) to simulate the proposal against. Each proposal transaction is actually sent (not eth_call) from the Governance contract address, which the node must have unlocked (e.g. anvil --auto-impersonate). Overrides the default bundled-anvil fork simulation. Useful for proposals where the success of one tx depends on a previous one succeeding.', + exclusive: ['force', 'noSimulate'], + }), + noSimulate: Flags.boolean({ + description: + 'Disable the default local fork simulation and fall back to independent per-transaction eth_call checks (which cannot see the effect of earlier transactions in the proposal).', + default: false, + exclusive: ['force', 'simulate'], }), noInfo: Flags.boolean({ description: 'Skip printing the proposal info', default: false }), descriptionURL: CustomFlags.proposalDescriptionURL({ @@ -127,9 +135,27 @@ export default class Propose extends BaseCommand { } if (!res.flags.force) { - const ok = res.flags.simulate - ? await simulateProposalOnRpc(proposal, res.flags.simulate, governance.address) - : await checkProposal(proposal, kit, governance.address) + let ok: boolean + if (res.flags.simulate) { + // Explicit external fork RPC. + ok = await simulateProposalOnRpc(proposal, res.flags.simulate, governance.address) + } else if (res.flags.noSimulate) { + // Opt-out: independent per-tx eth_call checks (legacy behaviour). + ok = await checkProposal(proposal, kit, governance.address) + } else { + // Default: spin up a bundled-anvil fork of the connected node and apply + // the proposal transactions sequentially, so transactions that depend on + // the effect of earlier ones (e.g. a method added by a prior upgrade tx) + // simulate correctly. Falls back to eth_call checks for non-http nodes. + const forkUrl = await this.getNodeUrl() + if (forkUrl?.startsWith('http')) { + ok = await withAnvilFork(forkUrl, (rpcUrl) => + simulateProposalOnRpc(proposal, rpcUrl, governance.address) + ) + } else { + ok = await checkProposal(proposal, kit, governance.address) + } + } if (!ok) { return } @@ -149,21 +175,35 @@ export default class Propose extends BaseCommand { proposeData, deposit.toFixed() ), - publicClient + publicClient, + // Surfaces the multisig transaction id. submitOrConfirmTransaction + // emits Submission on the first submit and Confirmation when a later + // signer confirms an already-submitted tx; surface both. When the + // multisig reaches its threshold, the underlying governance.propose + // executes in the same receipt, so also surface the new proposal id + // (ProposalQueued.proposalId) from that path. + { + abi: [...multiSigABI, ...governanceABI], + displayEventName: ['Submission', 'Confirmation', 'ProposalQueued'], + } ) } else { await performSafeTransaction( (await this.getKit()).connection.currentProvider, proposer, account, - safeTransactionMetadata(proposeData, governance.address, deposit.toFixed()) + safeTransactionMetadata(proposeData, governance.address, deposit.toFixed()), + // surfaces the new proposal id when the Safe execution reaches threshold + { abi: governanceABI, displayEventName: 'ProposalQueued' } ) } } else { await displayViemTx( 'proposeTx', governance.propose(proposal, res.flags.descriptionURL, { value: deposit.toFixed() }), - publicClient + publicClient, + // surfaces the new proposal id (ProposalQueued.proposalId) + { abi: governanceABI, displayEventName: 'ProposalQueued' } ) } } diff --git a/packages/cli/src/commands/governance/revokeupvote.ts b/packages/cli/src/commands/governance/revokeupvote.ts index 39ed4f3d85..d75a9338b0 100644 --- a/packages/cli/src/commands/governance/revokeupvote.ts +++ b/packages/cli/src/commands/governance/revokeupvote.ts @@ -1,3 +1,4 @@ +import { governanceABI } from '@celo/abis' import { BaseCommand } from '../../base' import { newCheckBuilder } from '../../utils/checks' import { displayViemTx } from '../../utils/cli' @@ -25,6 +26,9 @@ export default class RevokeUpvote extends BaseCommand { // TODO(nategraf): Check whether there are upvotes to revoke before sending transaction. const governance = await kit.contracts.getGovernance() const account = await (await kit.contracts.getAccounts()).voteSignerToAccount(signer) - await displayViemTx('revokeUpvoteTx', governance.revokeUpvote(account), publicClient) + await displayViemTx('revokeUpvoteTx', governance.revokeUpvote(account), publicClient, { + abi: governanceABI, + displayEventName: 'ProposalUpvoteRevoked', + }) } } diff --git a/packages/cli/src/commands/governance/upvote.ts b/packages/cli/src/commands/governance/upvote.ts index 680f1aabf4..d8538e7bbd 100644 --- a/packages/cli/src/commands/governance/upvote.ts +++ b/packages/cli/src/commands/governance/upvote.ts @@ -1,3 +1,4 @@ +import { governanceABI } from '@celo/abis' import { PublicCeloClient } from '@celo/actions' import { GovernanceWrapper } from '@celo/contractkit/src/wrappers/Governance' import { Flags } from '@oclif/core' @@ -41,7 +42,10 @@ export default class Upvote extends BaseCommand { ) if (!consideredProposals.some((k) => k.id === id)) { - await displayViemTx('upvoteTx', governance.upvote(id, account), publicClient) + await displayViemTx('upvoteTx', governance.upvote(id, account), publicClient, { + abi: governanceABI, + displayEventName: 'ProposalUpvoted', + }) } else { console.info(chalk.green('Proposal was dequeued, no need to upvote it.')) } diff --git a/packages/cli/src/commands/governance/votePartially.ts b/packages/cli/src/commands/governance/votePartially.ts index a87be8dc5d..5483f1126a 100644 --- a/packages/cli/src/commands/governance/votePartially.ts +++ b/packages/cli/src/commands/governance/votePartially.ts @@ -1,3 +1,4 @@ +import { governanceABI } from '@celo/abis' import { Flags } from '@oclif/core' import chalk from 'chalk' import { BaseCommand } from '../../base' @@ -47,7 +48,8 @@ export default class VotePartially extends BaseCommand { await displayViemTx( 'voteTx', governance.votePartially(id, res.flags.yes ?? 0, res.flags.no ?? 0, res.flags.abstain ?? 0), - publicClient + publicClient, + { abi: governanceABI, displayEventName: ['ProposalVoted', 'ProposalVotedV2'] } ) } } diff --git a/packages/cli/src/commands/lockedcelo/withdraw.test.ts b/packages/cli/src/commands/lockedcelo/withdraw.test.ts new file mode 100644 index 0000000000..538615911c --- /dev/null +++ b/packages/cli/src/commands/lockedcelo/withdraw.test.ts @@ -0,0 +1,30 @@ +import { newKitFromProvider } from '@celo/contractkit' +import { testWithAnvilL2 } from '@celo/dev-utils/anvil-test' +import { LONG_TIMEOUT_MS, testLocallyWithNode } from '../../test-utils/cliUtils' +import Register from '../account/register' +import Lock from './lock' +import Unlock from './unlock' +import Withdraw from './withdraw' + +process.env.NO_SYNCCHECK = 'true' + +testWithAnvilL2('lockedcelo:withdraw cmd', (provider) => { + it( + 'exits without hanging when there are no available withdrawals', + async () => { + const kit = newKitFromProvider(provider) + const [account] = await kit.connection.getAccounts() + await testLocallyWithNode(Register, ['--from', account], provider) + await testLocallyWithNode(Lock, ['--from', account, '--value', '100'], provider) + // create a pending withdrawal that is NOT yet available (unlocking period not elapsed) + await testLocallyWithNode(Unlock, ['--from', account, '--value', '50'], provider) + + // Regression: withdraw used to spin in an infinite `while (!madeWithdrawal)` + // loop when nothing was available. It must return promptly instead. + await expect( + testLocallyWithNode(Withdraw, ['--from', account], provider) + ).resolves.toBeUndefined() + }, + LONG_TIMEOUT_MS + ) +}) diff --git a/packages/cli/src/commands/lockedcelo/withdraw.ts b/packages/cli/src/commands/lockedcelo/withdraw.ts index fdb38fb3d1..4e1b114440 100644 --- a/packages/cli/src/commands/lockedcelo/withdraw.ts +++ b/packages/cli/src/commands/lockedcelo/withdraw.ts @@ -27,18 +27,23 @@ export default class Withdraw extends BaseCommand { const currentTime = Math.round(new Date().getTime() / 1000) let madeWithdrawal = false - while (!madeWithdrawal) { + // Withdraw available pending withdrawals one at a time, re-fetching after + // each because withdrawing index i shifts the on-chain list. Stop once none + // are available (otherwise this spins forever when nothing is ready). + while (true) { const pendingWithdrawals = await lockedgold.getPendingWithdrawals(flags.from) - for (let i = 0; i < pendingWithdrawals.length; i++) { - const pendingWithdrawal = pendingWithdrawals[i] - if (pendingWithdrawal.time.isLessThan(currentTime)) { - console.log( - `Found available pending withdrawal of value ${pendingWithdrawal.value.toFixed()}, withdrawing` - ) - await displayViemTx('withdraw', lockedgold.withdraw(i), publicClient) - madeWithdrawal = true - } + const i = pendingWithdrawals.findIndex((w) => w.time.isLessThan(currentTime)) + if (i === -1) { + break } + console.log( + `Found available pending withdrawal of value ${pendingWithdrawals[i].value.toFixed()}, withdrawing` + ) + await displayViemTx('withdraw', lockedgold.withdraw(i), publicClient) + madeWithdrawal = true + } + if (!madeWithdrawal) { + console.log('No pending withdrawals are available for withdrawal yet.') } const remainingPendingWithdrawals = await lockedgold.getPendingWithdrawals(flags.from) for (const pendingWithdrawal of remainingPendingWithdrawals) { diff --git a/packages/cli/src/commands/multisig/approve.ts b/packages/cli/src/commands/multisig/approve.ts index 32281b190c..fa20490f72 100644 --- a/packages/cli/src/commands/multisig/approve.ts +++ b/packages/cli/src/commands/multisig/approve.ts @@ -1,3 +1,4 @@ +import { multiSigABI } from '@celo/abis' import { getMultiSigContract } from '@celo/actions/contracts/multisig' import { BaseCommand } from '../../base' import { newCheckBuilder } from '../../utils/checks' @@ -63,7 +64,8 @@ export default class ApproveMultiSig extends BaseCommand { await displayViemTx( `multisig: approving transaction (approval ${currentConfirmations + 1} of ${neededConfirmations})`, multisig.write.confirmTransaction([BigInt(txIndex)]), - clients.public + clients.public, + { abi: multiSigABI, displayEventName: 'Confirmation' } ) } } diff --git a/packages/cli/src/commands/releasecelo/locked-gold.ts b/packages/cli/src/commands/releasecelo/locked-gold.ts index bb2925c4b2..7fb720213a 100644 --- a/packages/cli/src/commands/releasecelo/locked-gold.ts +++ b/packages/cli/src/commands/releasecelo/locked-gold.ts @@ -65,7 +65,6 @@ export default class LockedCelo extends ReleaseGoldBaseCommand { const accounts = await kit.contracts.getAccounts() const totalValue = await this.releaseGoldWrapper.getRemainingUnlockedBalance() const remaining = totalValue.minus(lockValue) - console.log('remaining', remaining.toFixed()) if ( !flags.yes && remaining.lt(new BigNumber(2e18)) && @@ -100,22 +99,27 @@ export default class LockedCelo extends ReleaseGoldBaseCommand { await checkBuilder.runChecks() const currentTime = Math.round(new Date().getTime() / 1000) let madeWithdrawal = false - while (!madeWithdrawal) { + // Withdraw available pending withdrawals one at a time, re-fetching after + // each because withdrawing index i shifts the on-chain list. Stop once none + // are available (otherwise this spins forever when nothing is ready). + while (true) { const pendingWithdrawals = await lockedGold.getPendingWithdrawals(contractAddress) - for (let i = 0; i < pendingWithdrawals.length; i++) { - const pendingWithdrawal = pendingWithdrawals[i] - if (pendingWithdrawal.time.isLessThan(currentTime)) { - console.log( - `Found available pending withdrawal of value ${pendingWithdrawal.value.toFixed()}, withdrawing` - ) - await displayViemTx( - 'lockedGoldWithdraw', - this.releaseGoldWrapper.withdrawLockedGold(i), - publicClient - ) - madeWithdrawal = true - } + const i = pendingWithdrawals.findIndex((w) => w.time.isLessThan(currentTime)) + if (i === -1) { + break } + console.log( + `Found available pending withdrawal of value ${pendingWithdrawals[i].value.toFixed()}, withdrawing` + ) + await displayViemTx( + 'lockedGoldWithdraw', + this.releaseGoldWrapper.withdrawLockedGold(i), + publicClient + ) + madeWithdrawal = true + } + if (!madeWithdrawal) { + console.log('No pending withdrawals are available for withdrawal yet.') } const remainingPendingWithdrawals = await lockedGold.getPendingWithdrawals(contractAddress) for (const pendingWithdrawal of remainingPendingWithdrawals) { diff --git a/packages/cli/src/commands/rewards/show.test.ts b/packages/cli/src/commands/rewards/show.test.ts index edf287e3b8..0c3f19c5c9 100644 --- a/packages/cli/src/commands/rewards/show.test.ts +++ b/packages/cli/src/commands/rewards/show.test.ts @@ -190,6 +190,32 @@ testWithAnvilL2('rewards:show cmd', (provider) => { }) }) + describe('--slashing', () => { + test('flag is accepted and slashing penalties are shown', async () => { + const lockedGoldMock = jest.spyOn(LockedGoldWrapper.prototype, 'getAccountsSlashed') + lockedGoldMock.mockImplementation(async () => [ + { + slashed: KNOWN_DEVCHAIN_VALIDATOR, + epochNumber: 1, + penalty: new BigNumber(2), + reporter: '', + reward: new BigNumber(10), + }, + ]) + + // Regression: `--slashing` used to be an undeclared flag, so the command + // errored with "Nonexistent flag: --slashing". It must now parse cleanly + // and surface the slashing section. + await expect( + testLocallyWithNode(Show, ['--validator', KNOWN_DEVCHAIN_VALIDATOR, '--slashing'], provider) + ).resolves.toBeUndefined() + const info = stripAnsiCodesFromNestedArray(infoMock.mock.calls).flat().map(String) + expect(info).toContain('Slashing penalties and rewards:') + expect(lockedGoldMock).toHaveBeenCalled() + lockedGoldMock.mockRestore() + }) + }) + describe('--voter', () => { test('invalid', async () => { await expect( diff --git a/packages/cli/src/commands/rewards/show.ts b/packages/cli/src/commands/rewards/show.ts index 25f9d30793..2e232e6f97 100644 --- a/packages/cli/src/commands/rewards/show.ts +++ b/packages/cli/src/commands/rewards/show.ts @@ -32,6 +32,10 @@ export default class Show extends BaseCommand { group: CustomFlags.address({ description: 'Validator Group to show rewards for', }), + slashing: Flags.boolean({ + default: false, + description: 'Show slashing penalties and rewards', + }), epochs: Flags.integer({ default: 1, diff --git a/packages/cli/src/commands/validatorgroup/commission.ts b/packages/cli/src/commands/validatorgroup/commission.ts index e3aaa666be..36e00fcd1c 100644 --- a/packages/cli/src/commands/validatorgroup/commission.ts +++ b/packages/cli/src/commands/validatorgroup/commission.ts @@ -49,7 +49,7 @@ export default class ValidatorGroupCommission extends BaseCommand { .addCheck('Commission is in range [0,1]', () => commission.gte(0) && commission.lte(1)) .isSignerOrAccount() .canSignValidatorTxs() - // .signerAccountIsValidatorGroup() + .signerAccountIsValidatorGroup() .runChecks() const tx = validators.setNextCommissionUpdate(commission) @@ -58,7 +58,7 @@ export default class ValidatorGroupCommission extends BaseCommand { await newCheckBuilder(this, res.flags.from) .isSignerOrAccount() .canSignValidatorTxs() - // .signerAccountIsValidatorGroup() + .signerAccountIsValidatorGroup() .hasACommissionUpdateQueued() .hasCommissionUpdateDelayPassed() .runChecks() diff --git a/packages/cli/src/packages-to-be/validators.test.ts b/packages/cli/src/packages-to-be/validators.test.ts new file mode 100644 index 0000000000..eb603553a5 --- /dev/null +++ b/packages/cli/src/packages-to-be/validators.test.ts @@ -0,0 +1,62 @@ +import { + meetsValidatorBalanceRequirements, + meetsValidatorGroupBalanceRequirements, +} from './validators' + +// resolveAddress hits the on-chain registry; stub it so the unit test only +// exercises the requirement-comparison logic. +jest.mock('@celo/actions', () => ({ + resolveAddress: jest.fn(async (_client: unknown, name: string) => + name === 'LockedGold' + ? '0x0000000000000000000000000000000000000001' + : '0x0000000000000000000000000000000000000002' + ), +})) + +// requirement tuple is [value, duration]; the bug compared duration (index 1) +// against locked gold instead of value (index 0). +const VALUE = 10_000n * 10n ** 18n // 10k CELO required +const DURATION = 5_184_000n // ~60 days, in seconds — far smaller than VALUE + +function clientWithLockedGold(lockedGold: bigint) { + return { + readContract: jest.fn(async ({ functionName }: { functionName: string }) => { + if (functionName === 'getAccountTotalLockedGold') { + return lockedGold + } + // getValidatorLockedGoldRequirements / getGroupLockedGoldRequirements + return [VALUE, DURATION] + }), + } as any +} + +describe('meetsValidatorBalanceRequirements', () => { + const addr = '0x00000000000000000000000000000000000000aa' as const + + it('returns true only when locked gold meets the requirement VALUE (not the duration)', async () => { + expect(await meetsValidatorBalanceRequirements(clientWithLockedGold(VALUE), addr)).toBe(true) + expect(await meetsValidatorBalanceRequirements(clientWithLockedGold(VALUE - 1n), addr)).toBe( + false + ) + }) + + it('regression: an amount above duration but below value must NOT pass', async () => { + // With the old `[1]` (duration) comparison this returned true; it must be false. + expect(await meetsValidatorBalanceRequirements(clientWithLockedGold(DURATION), addr)).toBe( + false + ) + }) +}) + +describe('meetsValidatorGroupBalanceRequirements', () => { + const addr = '0x00000000000000000000000000000000000000bb' as const + + it('compares against the requirement VALUE, not the duration', async () => { + expect(await meetsValidatorGroupBalanceRequirements(clientWithLockedGold(VALUE), addr)).toBe( + true + ) + expect(await meetsValidatorGroupBalanceRequirements(clientWithLockedGold(DURATION), addr)).toBe( + false + ) + }) +}) diff --git a/packages/cli/src/packages-to-be/validators.ts b/packages/cli/src/packages-to-be/validators.ts index b5d1d2345f..3c7f85a786 100644 --- a/packages/cli/src/packages-to-be/validators.ts +++ b/packages/cli/src/packages-to-be/validators.ts @@ -62,7 +62,7 @@ export const meetsValidatorBalanceRequirements = async ( functionName: 'getValidatorLockedGoldRequirements', }) - return validatorLockedGoldRequirements[1] <= accountTotalLockedGold + return validatorLockedGoldRequirements[0] <= accountTotalLockedGold } export const meetsValidatorGroupBalanceRequirements = async ( @@ -81,7 +81,7 @@ export const meetsValidatorGroupBalanceRequirements = async ( functionName: 'getGroupLockedGoldRequirements', }) - return validatorLockedGoldRequirements[1] <= accountTotalLockedGold + return validatorLockedGoldRequirements[0] <= accountTotalLockedGold } /* diff --git a/packages/cli/src/utils/anvil-fork.ts b/packages/cli/src/utils/anvil-fork.ts new file mode 100644 index 0000000000..8641a1e4ac --- /dev/null +++ b/packages/cli/src/utils/anvil-fork.ts @@ -0,0 +1,76 @@ +import { type Anvil, createAnvil } from '@viem/anvil' +import { type AddressInfo, createServer } from 'net' + +/** + * Ask the OS for a free TCP port on loopback. Avoids colliding with @viem/anvil's + * default 8545 (which createAnvil uses unless a port is given, and does not retry + * on bind failure) when another anvil/devchain is already running locally. + */ +async function getFreePort(): Promise { + return new Promise((resolve, reject) => { + const server = createServer() + server.unref() + server.on('error', reject) + server.listen(0, '127.0.0.1', () => { + const port = (server.address() as AddressInfo).port + server.close(() => resolve(port)) + }) + }) +} + +// Maps the running platform to the @foundry-rs/anvil prebuilt-binary package. +const PLATFORM_PACKAGES: Record = { + 'darwin-arm64': '@foundry-rs/anvil-darwin-arm64', + 'darwin-x64': '@foundry-rs/anvil-darwin-amd64', + 'linux-x64': '@foundry-rs/anvil-linux-amd64', + 'linux-arm64': '@foundry-rs/anvil-linux-arm64', + 'win32-x64': '@foundry-rs/anvil-win32-amd64', +} + +/** + * Resolve the path to the bundled anvil binary for the current platform. + * Returns undefined when no prebuilt binary is available, in which case + * @viem/anvil falls back to an `anvil` on the PATH (e.g. a foundry install). + */ +export function resolveAnvilBinary(): string | undefined { + const pkg = PLATFORM_PACKAGES[`${process.platform}-${process.arch}`] + if (!pkg) { + return undefined + } + for (const binPath of [`${pkg}/bin/anvil`, `${pkg}/bin/anvil.exe`]) { + try { + return require.resolve(binPath) + } catch { + // try the next candidate + } + } + return undefined +} + +/** + * Start a local anvil instance forking `forkUrl`, run `fn` against its RPC URL, + * and always stop the instance afterwards. Auto-impersonation is enabled so + * callers can send transactions from arbitrary accounts (e.g. the Governance + * contract) without unlocking them. + */ +export async function withAnvilFork( + forkUrl: string, + fn: (rpcUrl: string) => Promise +): Promise { + const anvil: Anvil = createAnvil({ + forkUrl, + autoImpersonate: true, + anvilBinary: resolveAnvilBinary(), + host: '127.0.0.1', + // Without this @viem/anvil defaults to 8545 and does not fall back on bind + // failure, so a locally-running node/anvil would break the simulation. + port: await getFreePort(), + }) + + await anvil.start() + try { + return await fn(`http://${anvil.host}:${anvil.port}`) + } finally { + await anvil.stop() + } +} diff --git a/packages/cli/src/utils/checks.ts b/packages/cli/src/utils/checks.ts index cddacb3f75..d4b4510749 100644 --- a/packages/cli/src/utils/checks.ts +++ b/packages/cli/src/utils/checks.ts @@ -10,9 +10,9 @@ import { getFeeCurrencyDirectoryContract, } from '@celo/actions/contracts/feecurrency-directory' import { + GovernanceContract, getGovernanceContract, getProposalStage, - GovernanceContract, } from '@celo/actions/contracts/governance' import { getLockedCeloContract, LockedCeloContract } from '@celo/actions/contracts/locked-celo' import { getValidatorsContract, ValidatorsContract } from '@celo/actions/contracts/validators' @@ -275,6 +275,12 @@ class CheckBuilder { this.withGovernance((governance) => governance.read.isProposalPassing([BigInt(proposalID)])) ) + proposalIsApproved = (proposalID: string | bigint) => + this.addCheck( + `Proposal ${proposalID} is approved`, + this.withGovernance((governance) => governance.read.isApproved([BigInt(proposalID)])) + ) + hotfixNotExecuted = (hash: Buffer) => this.addCheck( `Hotfix 0x${hash.toString('hex')} is not already executed`, diff --git a/packages/cli/src/utils/cli.ts b/packages/cli/src/utils/cli.ts index 65520f663f..2f819a97a8 100644 --- a/packages/cli/src/utils/cli.ts +++ b/packages/cli/src/utils/cli.ts @@ -1,3 +1,4 @@ +import { PublicCeloClient } from '@celo/actions' import { CeloTx } from '@celo/connect' import { LockedGoldRequirements } from '@celo/contractkit/lib/wrappers/Validators' import { Errors, ux } from '@oclif/core' @@ -5,19 +6,76 @@ import { TransactionResult as SafeTransactionResult } from '@safe-global/types-k import BigNumber from 'bignumber.js' import chalk from 'chalk' import humanizeDuration from 'humanize-duration' - -import { PublicCeloClient } from '@celo/actions' import { Abi, Address, ContractEventName, - decodeEventLog, DecodeEventLogReturnType, + decodeEventLog, formatEther, + Hex, } from 'viem' const CLIError = Errors.CLIError +type DecodeEventsOpts = { + abi: abi + displayEventName: ContractEventName | ContractEventName[] +} + +// Decode the receipt logs against the given abi and pretty-print every log that +// matches one of the requested event names. Shared by displayViemTx (viem +// receipts) and displaySafeTx (Safe/ethers receipts). +function decodeAndPrintEvents( + logs: readonly { data: Hex; topics: readonly Hex[] }[], + decodeEventsOpts: DecodeEventsOpts +) { + const { abi, displayEventName } = decodeEventsOpts + if (!displayEventName || !logs.length) { + return + } + const eventNames = typeof displayEventName === 'string' ? [displayEventName] : displayEventName + + const decodedLogs = logs.map((log) => { + let decodedLog: DecodeEventLogReturnType | undefined + try { + decodedLog = eventNames + .map((eventName) => { + try { + return decodeEventLog({ + abi, + data: log.data, + topics: log.topics as [signature: Hex, ...args: Hex[]] | [], + eventName, + }) + } catch (e) { + // unknown event, it's a-ok + } + }) + .filter(Boolean) + .at(0) as DecodeEventLogReturnType | undefined + } catch (e) { + // log not decodable with this abi; leave decodedLog undefined + } + return decodedLog + }) + + const filteredLogs = decodedLogs.filter((decodedLog): decodedLog is DecodeEventLogReturnType => { + if (!decodedLog) { + return false + } + return ( + (typeof displayEventName === 'string' && decodedLog.eventName === displayEventName) || + (displayEventName as string[]).includes(decodedLog.eventName) + ) + }) + + filteredLogs.forEach(({ eventName, args }) => { + console.log(chalk.magenta.bold(`${eventName}:`)) + printValueMap(args!, chalk.magenta) + }) +} + export async function displayTx(name: string, txObj: any, tx?: Omit) { ux.action.start(`Sending Transaction: ${name}`) const result = await txObj.send(tx) @@ -25,7 +83,11 @@ export async function displayTx(name: string, txObj: any, tx?: Omit( + name: string, + safeTxResult: SafeTransactionResult, + decodeEventsOpts?: abi extends Abi ? DecodeEventsOpts : never +) { ux.action.start(`Sending Transaction: ${name}`) try { @@ -47,6 +109,13 @@ export async function displaySafeTx(name: string, safeTxResult: SafeTransactionR const receipt = await (safeTxResult.transactionResponse as any).wait() printValueMap({ txHash: receipt.transactionHash }) + + // When the Safe reaches its threshold the underlying call (e.g. + // governance.propose) executes in this same receipt, so surface its + // events too (the receipt is ethers-shaped: { logs: { topics, data } }). + if (decodeEventsOpts && Array.isArray(receipt.logs)) { + decodeAndPrintEvents(receipt.logs, decodeEventsOpts) + } } ux.action.stop() @@ -76,51 +145,8 @@ export async function displayViemTx { - let decodedLog: DecodeEventLogReturnType | undefined - try { - const eventNames = - typeof displayEventName === 'string' ? [displayEventName] : displayEventName - - decodedLog = eventNames - .map((eventName) => { - try { - return decodeEventLog({ - abi, - data: log.data, - topics: log.topics, - eventName, - }) - } catch (e) { - // unknown event, it's a-ok - } - }) - .filter(Boolean) - .at(0) - } catch (e) {} - - return decodedLog - }) - - const filteredLogs = decodedLogs.filter( - (decodedLog): decodedLog is DecodeEventLogReturnType => { - if (!decodedLog) { - return false - } - return ( - (typeof displayEventName === 'string' && decodedLog.eventName === displayEventName) || - displayEventName.includes(decodedLog.eventName) - ) - } - ) - - filteredLogs.forEach(({ eventName, args }) => { - console.log(chalk.magenta.bold(`${eventName}:`)) - printValueMap(args!, chalk.magenta) - }) + if (decodeEventsOpts) { + decodeAndPrintEvents(logs, decodeEventsOpts) } ux.action.stop() diff --git a/packages/cli/src/utils/safe.ts b/packages/cli/src/utils/safe.ts index 5e8b983268..1ea55d020b 100644 --- a/packages/cli/src/utils/safe.ts +++ b/packages/cli/src/utils/safe.ts @@ -3,6 +3,7 @@ import { type Provider } from '@celo/connect' import { CeloProvider } from '@celo/connect/lib/celo-provider' import Safe from '@safe-global/protocol-kit' import { MetaTransactionData, TransactionResult } from '@safe-global/types-kit' +import { Abi, ContractEventName } from 'viem' import { displaySafeTx } from './cli' export const createSafe = async ( @@ -33,11 +34,17 @@ export const safeTransactionMetadata = ( } } -export const performSafeTransaction = async ( +export const performSafeTransaction = async ( provider: Provider, safeAddress: StrongAddress, safeSigner: StrongAddress, - txData: MetaTransactionData + txData: MetaTransactionData, + // When the Safe execution reaches its threshold the underlying call executes + // in the same receipt; pass abi/event names to surface those events (e.g. the + // governance ProposalQueued proposal id). + decodeEventsOpts?: abi extends Abi + ? { abi: abi; displayEventName: ContractEventName | ContractEventName[] } + : never ) => { const safe = await createSafe(provider, safeSigner, safeAddress) const approveTxPromise = await createApproveSafeTransactionIfNotApproved(safe, txData, safeSigner) @@ -49,7 +56,7 @@ export const performSafeTransaction = async ( const executeTxPromise = await createExecuteSafeTransactionIfThresholdMet(safe, txData) if (executeTxPromise) { - await displaySafeTx('executeTx', executeTxPromise) + await displaySafeTx('executeTx', executeTxPromise, decodeEventsOpts) } } diff --git a/packages/dev-utils/src/anvil-binary.ts b/packages/dev-utils/src/anvil-binary.ts new file mode 100644 index 0000000000..392f2a2142 --- /dev/null +++ b/packages/dev-utils/src/anvil-binary.ts @@ -0,0 +1,67 @@ +import { accessSync, constants } from 'node:fs' +import { delimiter, join } from 'node:path' + +/** + * Resolve the `anvil` binary to use for the test harness. + * + * The devchain state snapshot (`@celo/devchain-anvil`) is generated for the + * foundry version pinned by CI, and newer anvil releases cannot parse it. When + * a package depends on the npm-distributed anvil (e.g. `@foundry-rs/anvil`, + * which celocli bundles for runtime forking), yarn creates a + * `node_modules/.bin/anvil` shim that shadows the foundry install on PATH. That + * shim is a different (incompatible) anvil version, so we must skip it and pick + * the system/foundry anvil instead. + * + * Honors `CELO_TEST_ANVIL_BINARY` as an explicit override; otherwise returns + * the first `anvil` on PATH that is NOT inside a `node_modules/.bin` directory, + * falling back to `'anvil'` (let @viem/anvil resolve it) when none is found. + */ +export function resolveSystemAnvilBinary(): string { + const override = process.env.CELO_TEST_ANVIL_BINARY + if (override) { + return override + } + + const isWindows = process.platform === 'win32' + const exeNames = isWindows ? ['anvil.exe', 'anvil.cmd', 'anvil'] : ['anvil'] + + // Prefer foundry's standard install location. This is where `foundryup` + // installs locally and where the `foundry-toolchain` GitHub Action installs in + // CI ($HOME/.foundry/bin), so it reliably points at the snapshot-compatible + // version regardless of how PATH is ordered by the package manager. + const home = process.env.HOME || process.env.USERPROFILE + if (home) { + for (const exe of exeNames) { + const candidate = join(home, '.foundry', 'bin', exe) + try { + accessSync(candidate, constants.X_OK) + return candidate + } catch { + // not installed here, fall through to PATH scan + } + } + } + + // Fallback: first `anvil` on PATH that is not a package-manager bin shim. + // Skip node_modules/.bin and yarn's temporary wrapper dirs (e.g. `.../xfs-*`, + // `.yarn/`), which may resolve to a bundled, snapshot-incompatible anvil. + const isShimDir = (dir: string) => + dir.includes('node_modules') || /[\\/](xfs-[^\\/]*|\.yarn)([\\/]|$)/.test(dir) + + for (const dir of (process.env.PATH ?? '').split(delimiter)) { + if (!dir || isShimDir(dir)) { + continue + } + for (const exe of exeNames) { + const candidate = join(dir, exe) + try { + accessSync(candidate, constants.X_OK) + return candidate + } catch { + // not here, keep looking + } + } + } + + return 'anvil' +} diff --git a/packages/dev-utils/src/anvil-test.ts b/packages/dev-utils/src/anvil-test.ts index 7e684d2fb5..bd699d4edc 100644 --- a/packages/dev-utils/src/anvil-test.ts +++ b/packages/dev-utils/src/anvil-test.ts @@ -2,13 +2,14 @@ import { StrongAddress } from '@celo/base' import { Provider } from '@celo/connect' import { Anvil, CreateAnvilOptions, createAnvil } from '@viem/anvil' import BigNumber from 'bignumber.js' +import { resolveSystemAnvilBinary } from './anvil-binary' import { + jsonRpcCall, TEST_BALANCE, TEST_BASE_FEE, TEST_GAS_LIMIT, TEST_GAS_PRICE, TEST_MNEMONIC, - jsonRpcCall, testWithProvider, } from './test-utils' @@ -38,6 +39,7 @@ function createInstance(stateFilePath: string, chainId?: number): Anvil { const options: CreateAnvilOptions = { port, loadState: stateFilePath, + anvilBinary: resolveSystemAnvilBinary(), mnemonic: TEST_MNEMONIC, balance: TEST_BALANCE, gasPrice: TEST_GAS_PRICE, diff --git a/packages/dev-utils/src/viem/anvil-test.ts b/packages/dev-utils/src/viem/anvil-test.ts index 5065e785b5..bbb6158f12 100644 --- a/packages/dev-utils/src/viem/anvil-test.ts +++ b/packages/dev-utils/src/viem/anvil-test.ts @@ -1,13 +1,13 @@ import { StrongAddress } from '@celo/base' -import { Anvil, createAnvil, CreateAnvilOptions } from '@viem/anvil' +import { Anvil, CreateAnvilOptions, createAnvil } from '@viem/anvil' import { Account, Address, Client, createTestClient, Hex, - http, HttpTransport, + http, PublicActions, publicActions, RpcSchema, @@ -16,6 +16,7 @@ import { walletActions, } from 'viem' import { celo, celoSepolia } from 'viem/chains' +import { resolveSystemAnvilBinary } from '../anvil-binary' import { ANVIL_PORT, DEFAULT_OWNER_ADDRESS } from '../anvil-test' import { TEST_BALANCE, @@ -48,6 +49,7 @@ function createInstance(opts?: { chainId?: number; forkUrl?: string; forkBlockNu const port = ANVIL_PORT + (process.pid - process.ppid) + instanceCounter++ const options: CreateAnvilOptions = { port, + anvilBinary: resolveSystemAnvilBinary(), mnemonic: TEST_MNEMONIC, balance: TEST_BALANCE, gasPrice: TEST_GAS_PRICE, diff --git a/packages/sdk/explorer/src/block-explorer.ts b/packages/sdk/explorer/src/block-explorer.ts index 17a16daa2d..0decb7fdf6 100644 --- a/packages/sdk/explorer/src/block-explorer.ts +++ b/packages/sdk/explorer/src/block-explorer.ts @@ -13,6 +13,7 @@ import { mapFromPairs, obtainKitContractDetails, } from './base' +import { abiToContractMapping, fetchAbiFromExplorer } from './explorer' import { fetchMetadata, tryGetProxyImplementation } from './sourcify' const debug = debugFactory('kit:explorer:block') @@ -243,6 +244,60 @@ export class BlockExplorer { } } + /** + * Returns the ContractMapping for the contract at that address by looking up + * its verified ABI on the chain's block explorer (Blockscout / Celoscan). + * + * Useful when Sourcify does not have the contract (or its v1 API is + * unavailable) but the contract is verified on the canonical celo explorer. + * @param address + * @returns The ContractMapping for the contract at that address, or undefined + */ + getContractMappingFromExplorer = async ( + address: string + ): Promise => { + const cached = this.addressMapping.get(address) + if (cached) { + return cached + } + const fetched = await fetchAbiFromExplorer(this.kit.connection, address) + if (!fetched) { + return undefined + } + const mapping = abiToContractMapping(fetched.name, address, fetched.abi) + this.addressMapping.set(address, mapping) + return mapping + } + + /** + * Like getContractMappingFromExplorer, but treats the address as a proxy and + * resolves the implementation ABI. Honors proxyImplementationOverride, so a + * proxy upgraded earlier in the same proposal resolves to its new + * implementation's ABI. + * @param address + * @returns The ContractMapping for the contract at that address, or undefined + */ + getContractMappingFromExplorerAsProxy = async ( + address: string + ): Promise => { + let implAddress = await tryGetProxyImplementation(this.kit.connection, address) + if (this.proxyImplementationOverride.has(address)) { + implAddress = this.proxyImplementationOverride.get(address) + } + if (implAddress) { + const contractMapping = await this.getContractMappingFromExplorer(implAddress) + if (contractMapping) { + return { + ...contractMapping, + details: { + ...contractMapping.details, + address, // Show the proxy address + }, + } + } + } + } + /** * Uses all of the strategies available to find a contract mapping that contains * the given method selector. @@ -258,6 +313,8 @@ export class BlockExplorer { this.getContractMappingFromCore, this.getContractMappingFromSourcify, this.getContractMappingFromSourcifyAsProxy, + this.getContractMappingFromExplorer, + this.getContractMappingFromExplorerAsProxy, ] ): Promise { const mappings = await Promise.all( diff --git a/packages/sdk/explorer/src/explorer.ts b/packages/sdk/explorer/src/explorer.ts new file mode 100644 index 0000000000..9a6c5480c3 --- /dev/null +++ b/packages/sdk/explorer/src/explorer.ts @@ -0,0 +1,88 @@ +/** + * Block-explorer (Blockscout / Etherscan-compatible) helpers for querying a + * verified contract's ABI when Sourcify does not have it. + * + * Celo core contracts are canonically verified on Celoscan / Blockscout, so this + * is a more reliable ABI source than Sourcify for celo addresses (and works while + * Sourcify's v1 API is being sunset). + */ +import { ABIDefinition, AbiItem, Address, Connection } from '@celo/connect' +import fetch from 'cross-fetch' +import { toFunctionSelector } from 'viem' +import { ContractMapping, mapFromPairs } from './base' +import { formatAbiInputType } from './sourcify' + +/** + * Keyless Blockscout API base URL per chain id. Blockscout exposes the + * Etherscan-compatible `getsourcecode` action and holds Celo's verifications. + */ +const BLOCKSCOUT_API_BY_CHAIN: Record = { + 42220: 'https://celo.blockscout.com/api', + 11142220: 'https://celo-sepolia.blockscout.com/api', +} + +/** + * Build a ContractMapping (selector -> ABI item) from a raw ABI, mirroring the + * Sourcify Metadata path so downstream decoding behaves identically. + */ +export function abiToContractMapping( + name: string, + address: Address, + abi: AbiItem[] +): ContractMapping { + const fnMapping = mapFromPairs( + abi + .filter((item) => item.type === 'function') + .map((item) => { + const sig = `${item.name}(${(item.inputs || []) + .map((i) => formatAbiInputType(i)) + .join(',')})` + return { ...item, signature: toFunctionSelector(sig) } as ABIDefinition + }) + .map((item): [string, ABIDefinition] => [item.signature, item]) + ) + return { + details: { name: name || 'Unknown', address, jsonInterface: abi, isCore: false }, + fnMapping, + } +} + +/** + * Fetch a verified contract's ABI from the chain's Blockscout instance. + * Returns null when the chain is unsupported, the request fails, or the + * contract is not verified. + */ +export async function fetchAbiFromExplorer( + connection: Connection, + address: string +): Promise<{ name: string; abi: AbiItem[] } | null> { + let chainId: number + try { + chainId = await connection.viemClient.getChainId() + } catch { + return null + } + const base = BLOCKSCOUT_API_BY_CHAIN[chainId] + if (!base) { + return null + } + try { + const resp = await fetch(`${base}?module=contract&action=getsourcecode&address=${address}`) + if (!resp.ok) { + return null + } + const data = await resp.json() + const result = Array.isArray(data?.result) ? data.result[0] : undefined + const rawAbi: string | undefined = result?.ABI + if (!rawAbi || rawAbi === 'Contract source code not verified') { + return null + } + const abi = JSON.parse(rawAbi) as AbiItem[] + if (!Array.isArray(abi) || abi.length === 0) { + return null + } + return { name: result?.ContractName || 'Unknown', abi } + } catch { + return null + } +} diff --git a/packages/sdk/explorer/src/sourcify.test.ts b/packages/sdk/explorer/src/sourcify.test.ts index 8547652705..7845999295 100644 --- a/packages/sdk/explorer/src/sourcify.test.ts +++ b/packages/sdk/explorer/src/sourcify.test.ts @@ -1,7 +1,7 @@ -import * as crypto from 'crypto' import { Address, Connection, Provider } from '@celo/connect' +import * as crypto from 'crypto' import { toFunctionSelector } from 'viem' -import { Metadata, fetchMetadata, tryGetProxyImplementation } from './sourcify' +import { fetchMetadata, Metadata, tryGetProxyImplementation } from './sourcify' // This is taken from protocol/contracts/build/Account.json const CONTRACT_METADATA = require('../fixtures/contract.metadata.json') @@ -33,48 +33,42 @@ describe('sourcify helpers', () => { }) describe('fetchMetadata()', () => { - describe('when a full match exists', () => { - it('returns the metadata from the full match', async () => { - fetchMock.get( - `https://repo.sourcify.dev/contracts/full_match/42220/${address}/metadata.json`, - {} - ) + const v2Url = (addr: Address) => + `https://sourcify.dev/server/v2/contract/42220/${addr}?fields=metadata` + + describe('when an exact match exists', () => { + it('returns the metadata', async () => { + fetchMock.get(v2Url(address), { match: 'exact_match', metadata: {} }) const metadata = await fetchMetadata(connection, address) expect(metadata).toBeInstanceOf(Metadata) }) }) - describe('when a full match does not exist', () => { - describe('but a partial match exists', () => { - it('returns the metadata from the partial match', async () => { - fetchMock - .get( - `https://repo.sourcify.dev/contracts/full_match/42220/${address}/metadata.json`, - 400 - ) - .get( - `https://repo.sourcify.dev/contracts/partial_match/42220/${address}/metadata.json`, - {} - ) - const metadata = await fetchMetadata(connection, address) - expect(metadata).toBeInstanceOf(Metadata) - }) + describe('when only a (partial) match exists', () => { + it('returns the metadata when not strict', async () => { + fetchMock.get(v2Url(address), { match: 'match', metadata: {} }) + const metadata = await fetchMetadata(connection, address) + expect(metadata).toBeInstanceOf(Metadata) }) - describe('and a partial match does not exist', () => { - it('is null', async () => { - fetchMock - .get( - `https://repo.sourcify.dev/contracts/full_match/42220/${address}/metadata.json`, - 400 - ) - .get( - `https://repo.sourcify.dev/contracts/partial_match/42220/${address}/metadata.json`, - 400 - ) - const metadata = await fetchMetadata(connection, address) - expect(metadata).toEqual(null) - }) + it('is null when strict (exact match required)', async () => { + fetchMock.get(v2Url(address), { match: 'match', metadata: {} }) + const metadata = await fetchMetadata(connection, address, true) + expect(metadata).toEqual(null) + }) + }) + + describe('when the contract is not verified', () => { + it('is null on a 404 response', async () => { + fetchMock.get(v2Url(address), 404) + const metadata = await fetchMetadata(connection, address) + expect(metadata).toEqual(null) + }) + + it('is null when the response has no match', async () => { + fetchMock.get(v2Url(address), { match: null, metadata: null }) + const metadata = await fetchMetadata(connection, address) + expect(metadata).toEqual(null) }) }) }) diff --git a/packages/sdk/explorer/src/sourcify.ts b/packages/sdk/explorer/src/sourcify.ts index 80a79d4426..874a7aa204 100644 --- a/packages/sdk/explorer/src/sourcify.ts +++ b/packages/sdk/explorer/src/sourcify.ts @@ -10,9 +10,9 @@ * // do something with it. * } */ -import { ABIDefinition, AbiItem, AbiInput, Address, Connection } from '@celo/connect' -import { getAddress, toFunctionSelector } from 'viem' +import { ABIDefinition, AbiInput, AbiItem, Address, Connection } from '@celo/connect' import fetch from 'cross-fetch' +import { getAddress, toFunctionSelector } from 'viem' import { ContractMapping, mapFromPairs } from './base' /** @@ -28,9 +28,9 @@ function abiItemToSignatureString(item: AbiItem): string { } /** ABI input that may have tuple components (runtime ABI data from Solidity) */ -type AbiInputWithComponents = AbiInput & { components?: readonly AbiInputWithComponents[] } +export type AbiInputWithComponents = AbiInput & { components?: readonly AbiInputWithComponents[] } -function formatAbiInputType(input: AbiInputWithComponents): string { +export function formatAbiInputType(input: AbiInputWithComponents): string { if (input.type === 'tuple' && input.components) { return `(${input.components.map((c: AbiInput) => formatAbiInputType(c)).join(',')})` } @@ -194,49 +194,46 @@ export class Metadata { } } +/** Sourcify v2 match types. `exact_match` is the former "full" match; `match` + * is the former "partial" match. https://docs.sourcify.dev/docs/exact-match-vs-match/ */ +type SourcifyMatch = 'exact_match' | 'match' | null + +interface SourcifyV2Response { + match?: SourcifyMatch + metadata?: MetadataResponse +} + +const SOURCIFY_SERVER_URL = 'https://sourcify.dev/server' + /** - * Fetch the sourcify response and instantiate a Metadata wrapper class around it. - * Try a full_match but fallback to partial_match when not strict. + * Fetch the contract metadata from Sourcify and instantiate a Metadata wrapper + * class around it. Uses the Sourcify v2 API (the v1 repo API has been sunset). * @param connection @celo/connect instance * @param contract the address of the contract to query - * @param strict only allow full matches https://docs.sourcify.dev/docs/full-vs-partial-match/ + * @param strict only allow exact matches https://docs.sourcify.dev/docs/exact-match-vs-match/ * @returns Metadata or null */ export async function fetchMetadata( connection: Connection, contract: Address, strict = false -): Promise { - const fullMatchMetadata = await querySourcify(connection, 'full_match', contract) - if (fullMatchMetadata !== null) { - return fullMatchMetadata - } else if (strict) { - return null - } else { - return querySourcify(connection, 'partial_match', contract) - } -} - -/** - * Fetch the sourcify response and instantiate a Metadata wrapper class around it. - * @param connection @celo/connect instance - * @param matchType what type of match to query for https://docs.sourcify.dev/docs/full-vs-partial-match/ - * @param contract the address of the contract to query - * @returns Metadata or null - */ -async function querySourcify( - connection: Connection, - matchType: 'full_match' | 'partial_match', - contract: Address ): Promise { const chainID = await connection.viemClient.getChainId() const resp = await fetch( - `https://repo.sourcify.dev/contracts/${matchType}/${chainID}/${contract}/metadata.json` + `${SOURCIFY_SERVER_URL}/v2/contract/${chainID}/${contract}?fields=metadata` ) - if (resp.ok) { - return new Metadata(connection, contract, await resp.json()) + // 404 (and any non-2xx) means the contract isn't verified on Sourcify. + if (!resp.ok) { + return null + } + const data = (await resp.json()) as SourcifyV2Response + if (!data || !data.match || !data.metadata) { + return null + } + if (strict && data.match !== 'exact_match') { + return null } - return null + return new Metadata(connection, contract, data.metadata) } /** diff --git a/packages/sdk/governance/src/proposal-builder.test.ts b/packages/sdk/governance/src/proposal-builder.test.ts index d4261f34da..d5e6acc10a 100644 --- a/packages/sdk/governance/src/proposal-builder.test.ts +++ b/packages/sdk/governance/src/proposal-builder.test.ts @@ -4,6 +4,7 @@ import { CeloContract, ContractKit, newKitFromProvider } from '@celo/contractkit import { testWithAnvilL2 } from '@celo/dev-utils/anvil-test' import { encodeFunctionData } from 'viem' import { ProposalBuilder } from './proposal-builder' + testWithAnvilL2('ProposalBuilder', (provider) => { let kit: ContractKit let proposalBuilder: ProposalBuilder @@ -54,6 +55,32 @@ testWithAnvilL2('ProposalBuilder', (provider) => { }) }) + describe('fromJsonTx proxy repoint logging', () => { + it('logs the proxy identifier (contract) instead of undefined for core-contract repoints', async () => { + const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}) + const newImplementationAddress = '0x471ece3750da237f93b8e339c536989b8978a438' + // A core-contract repoint is keyed by `contract` (no `address`); the log + // used to print `undefined is a proxy, repointing to ...`. + try { + await proposalBuilder.fromJsonTx({ + contract: CeloContract.GoldToken, + function: '_setImplementation', + args: [newImplementationAddress], + value: '0', + } as any) + } catch { + // building the call may fail (the proxy method isn't on the impl ABI); + // the repoint log fires before that and is what we're asserting. + } + const logged = logSpy.mock.calls.map((c) => String(c[0])) + logSpy.mockRestore() + expect(logged.some((l) => l.includes('undefined is a proxy'))).toBe(false) + expect( + logged.some((l) => l === `GoldToken is a proxy, repointing to ${newImplementationAddress}`) + ).toBe(true) + }) + }) + describe('setRegistryAddition', () => { it('sets and gets registry addition', () => { const contract = CeloContract.CeloToken @@ -120,6 +147,57 @@ testWithAnvilL2('ProposalBuilder', (provider) => { expect(result.value).toBe('0') expect(result.input).toBeDefined() }) + + it('resolves a method missing from the bundled ABI via the repointed implementation', async () => { + // Simulate an earlier proposal tx that repointed the GoldToken proxy to a + // new implementation which adds a method not present in the bundled ABI. + const newMethodAbi: AbiItem = { + name: 'setMaxVoterRewardCommission', + type: 'function', + inputs: [{ name: 'commission', type: 'uint256' }], + outputs: [], + } + proposalBuilder.externalCallProxyRepoint.set( + CeloContract.GoldToken, + '0x471ece3750da237f93b8e339c536989b8978a438' + ) + // The new method's ABI would come from the impl's verified metadata. + proposalBuilder.lookupExternalMethodABI = async () => newMethodAbi + + const result = await proposalBuilder.buildCallToCoreContract({ + contract: CeloContract.GoldToken, + function: 'setMaxVoterRewardCommission', + args: ['1000000000000000000000000'], + value: '0', + }) + + expect(result.value).toBe('0') + // selector for setMaxVoterRewardCommission(uint256) + expect(result.input.startsWith('0x')).toBe(true) + expect(result.input.length).toBeGreaterThan(10) + }) + + it('falls back to a raw signature when the method is unknown but fully specified', async () => { + // No verified ABI available, but the JSON provides the full signature. + proposalBuilder.externalCallProxyRepoint.set( + CeloContract.GoldToken, + '0x471ece3750da237f93b8e339c536989b8978a438' + ) + proposalBuilder.lookupExternalMethodABI = async () => null + + const abi = await proposalBuilder.resolveCoreMethodABIFromRepoint( + { + contract: CeloContract.GoldToken, + function: 'setMaxVoterRewardCommission(uint256)', + args: ['1000000000000000000000000'], + value: '0', + }, + '0x471ece3750da237f93b8e339c536989b8978a438' + ) + + expect(abi).not.toBeNull() + expect(abi!.name).toBe('setMaxVoterRewardCommission') + }) }) describe('addJsonTx', () => { diff --git a/packages/sdk/governance/src/proposal-builder.ts b/packages/sdk/governance/src/proposal-builder.ts index 91de6c2d2e..de78cae605 100644 --- a/packages/sdk/governance/src/proposal-builder.ts +++ b/packages/sdk/governance/src/proposal-builder.ts @@ -1,31 +1,30 @@ import { AbiItem, signatureToAbiDefinition } from '@celo/connect' import { coerceArgsForAbi } from '@celo/connect/lib/viem-abi-coder' -import { toChecksumAddress } from '@celo/utils/lib/address' import { CeloContract, ContractKit, + getInitializeAbiOfImplementation, RegisteredContracts, SET_AND_INITIALIZE_IMPLEMENTATION_ABI, - getInitializeAbiOfImplementation, setImplementationOnProxy, } from '@celo/contractkit' import { stripProxy } from '@celo/contractkit/lib/base' import { ProposalTransaction } from '@celo/contractkit/lib/wrappers/Governance' import { fetchMetadata, tryGetProxyImplementation } from '@celo/explorer/lib/sourcify' -import { isValidAddress } from '@celo/utils/lib/address' +import { isValidAddress, toChecksumAddress } from '@celo/utils/lib/address' import { isNativeError } from 'util/types' import { encodeFunctionData } from 'viem' +import { bigintReplacer } from './json-utils' import { ExternalProposalTransactionJSON, - ProposalTransactionJSON, - ProposalTxParams, - RegistryAdditions, isProxySetAndInitFunction, isProxySetFunction, isRegistryRepoint, + ProposalTransactionJSON, + ProposalTxParams, + RegistryAdditions, registryRepointArgs, } from './proposals' -import { bigintReplacer } from './json-utils' /** * Builder class to construct proposals from JSON or transaction objects. @@ -166,6 +165,41 @@ export class ProposalBuilder { return res } + // Resolve a core-contract method ABI that is absent from the bundled ABI by + // looking it up on the implementation an earlier proposal tx repointed the + // proxy to (tracked in externalCallProxyRepoint, keyed by contract name or + // proxy address). Falls back to the on-chain proxy implementation and finally + // to a raw signature (e.g. `function: "setX(uint256)"`). + resolveCoreMethodABIFromRepoint = async ( + tx: ProposalTransactionJSON, + proxyAddress: string + ): Promise => { + // Repoints may be keyed by the proxy contract name (e.g. `ValidatorsProxy`), + // the bare contract name (`Validators`), or the proxy address. Match on the + // proxy-stripped name or the address so any of those forms resolves. + const wantedName = stripProxy(tx.contract as CeloContract) + let repointed: string | undefined + for (const [key, value] of this.externalCallProxyRepoint) { + if ( + stripProxy(key as CeloContract) === wantedName || + key.toLowerCase() === proxyAddress.toLowerCase() + ) { + repointed = value + break + } + } + const impl = repointed ?? (await tryGetProxyImplementation(this.kit.connection, proxyAddress)) + + let methodABI: AbiItem | null = null + if (impl) { + methodABI = await this.lookupExternalMethodABI(impl, tx as ExternalProposalTransactionJSON) + } + if (methodABI === null && tx.function.includes('(')) { + methodABI = signatureToAbiDefinition(tx.function) + } + return methodABI + } + buildCallToCoreContract = async (tx: ProposalTransactionJSON): Promise => { // Account for canonical registry addresses from current proposal const address = @@ -185,16 +219,23 @@ export class ProposalBuilder { const contract = await this.kit._contracts.getContract(tx.contract, address) const methodName = tx.function - const abiItem = (contract.abi as AbiItem[]).find( + let abiItem = (contract.abi as AbiItem[]).find( (item) => item.type === 'function' && item.name === methodName ) + if (!abiItem) { + // The bundled (static) ABI may be older than the implementation this + // proposal upgrades the proxy to. An earlier tx in this same proposal can + // repoint the proxy to a new implementation that adds this method, so try + // to resolve the method ABI from that new implementation before failing. + abiItem = (await this.resolveCoreMethodABIFromRepoint(tx, address)) ?? undefined + } if (!abiItem) { throw new Error(`Method ${methodName} not found in ABI for ${tx.contract}`) } const coercedArgs = abiItem.inputs ? coerceArgsForAbi(abiItem.inputs, tx.args) : tx.args const data = encodeFunctionData({ abi: [abiItem], - functionName: methodName, + functionName: abiItem.name, args: coercedArgs, }) return this.fromEncodedTx(data, { to: address, value: tx.value }) @@ -214,8 +255,9 @@ export class ProposalBuilder { } if (isProxySetAndInitFunction(tx) || isProxySetFunction(tx)) { - console.log(tx.address + ' is a proxy, repointing to ' + tx.args[0]) - this.externalCallProxyRepoint.set(tx.address || (tx.contract as string), tx.args[0] as string) + const proxyId = tx.address || (tx.contract as string) + console.log(proxyId + ' is a proxy, repointing to ' + tx.args[0]) + this.externalCallProxyRepoint.set(proxyId, tx.args[0] as string) } const strategies = [this.buildCallToCoreContract, this.buildCallToExternalContract] diff --git a/yarn.lock b/yarn.lock index 2ff4ce44d1..ecf19a742a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1773,6 +1773,7 @@ __metadata: "@celo/wallet-hsm-azure": "npm:^8.0.4" "@celo/wallet-ledger": "npm:^8.0.4" "@celo/wallet-local": "npm:^8.0.4" + "@foundry-rs/anvil": "npm:1.7.1" "@ledgerhq/hw-transport-node-hid": "npm:^6.28.5" "@oclif/core": "npm:^3.27.0" "@oclif/plugin-autocomplete": "npm:^3.2.0" @@ -1791,6 +1792,7 @@ __metadata: "@types/ledgerhq__hw-transport-node-hid": "npm:^4.22.5" "@types/node": "npm:^18.7.16" "@types/prompts": "npm:^1.1.1" + "@viem/anvil": "npm:^0.0.9" bignumber.js: "npm:9.0.0" chalk: "npm:^2.4.2" command-exists: "npm:^1.2.9" @@ -3124,6 +3126,77 @@ __metadata: languageName: node linkType: hard +"@foundry-rs/anvil-darwin-amd64@npm:1.7.1": + version: 1.7.1 + resolution: "@foundry-rs/anvil-darwin-amd64@npm:1.7.1" + bin: + anvil: bin/anvil + conditions: os=darwin & cpu=x64 + languageName: node + linkType: hard + +"@foundry-rs/anvil-darwin-arm64@npm:1.7.1": + version: 1.7.1 + resolution: "@foundry-rs/anvil-darwin-arm64@npm:1.7.1" + bin: + anvil: bin/anvil + conditions: os=darwin & cpu=arm64 + languageName: node + linkType: hard + +"@foundry-rs/anvil-linux-amd64@npm:1.7.1": + version: 1.7.1 + resolution: "@foundry-rs/anvil-linux-amd64@npm:1.7.1" + bin: + anvil: bin/anvil + conditions: os=linux & cpu=x64 + languageName: node + linkType: hard + +"@foundry-rs/anvil-linux-arm64@npm:1.7.1": + version: 1.7.1 + resolution: "@foundry-rs/anvil-linux-arm64@npm:1.7.1" + bin: + anvil: bin/anvil + conditions: os=linux & cpu=arm64 + languageName: node + linkType: hard + +"@foundry-rs/anvil-win32-amd64@npm:1.7.1": + version: 1.7.1 + resolution: "@foundry-rs/anvil-win32-amd64@npm:1.7.1" + bin: + anvil: bin/anvil.exe + conditions: os=win32 & cpu=x64 + languageName: node + linkType: hard + +"@foundry-rs/anvil@npm:1.7.1": + version: 1.7.1 + resolution: "@foundry-rs/anvil@npm:1.7.1" + dependencies: + "@foundry-rs/anvil-darwin-amd64": "npm:1.7.1" + "@foundry-rs/anvil-darwin-arm64": "npm:1.7.1" + "@foundry-rs/anvil-linux-amd64": "npm:1.7.1" + "@foundry-rs/anvil-linux-arm64": "npm:1.7.1" + "@foundry-rs/anvil-win32-amd64": "npm:1.7.1" + dependenciesMeta: + "@foundry-rs/anvil-darwin-amd64": + optional: true + "@foundry-rs/anvil-darwin-arm64": + optional: true + "@foundry-rs/anvil-linux-amd64": + optional: true + "@foundry-rs/anvil-linux-arm64": + optional: true + "@foundry-rs/anvil-win32-amd64": + optional: true + bin: + anvil: bin.mjs + checksum: 4c0c237a117a1a3694259f1f43f3e796034624f07bbb24debbe45f1103f586a647f3bc761cb7a3d2de1be615908c0189a28ebb9f36607c4b2ebc6720c7ea1da9 + languageName: node + linkType: hard + "@gerrit0/mini-shiki@npm:^3.2.2": version: 3.2.3 resolution: "@gerrit0/mini-shiki@npm:3.2.3"