feat(governance): show how many approvers still need to approve

.changeset/multisig-read-batching.md

@@ -0,0 +1,13 @@
+---
+'@celo/contractkit': patch
+---
+
+Reduce the RPC load of MultiSigWrapper reads.
+
+- `getConfirmations` uses the contract's `getConfirmations` view instead of one
+  `confirmations` read per owner.
+- `getTransactionDataByContent` scans transactions newest-first in small batches and
+  stops at the first match, instead of fetching the multisig's entire history in one
+  burst (268 parallel requests on the mainnet approver multisig, enough to get
+  rate-limited). When several transactions share the same content, the most recent
+  one is now returned.

.changeset/remaining-approvals.md

@@ -0,0 +1,15 @@
+---
+'@celo/contractkit': minor
+'@celo/celocli': minor
+---
+
+Show how many approvers still need to approve.
+
+- `governance:show --proposalID` reports the confirmations the approver multisig is still
+  missing, both in the `approvals` map (`required` / `remaining`) and as a note.
+- `governance:show --hotfix` gained an `approvals` section with the approver and security
+  council progress towards their respective thresholds.
+- `multisig:show` reports `confirmations`, `confirmationsRequired` and
+  `confirmationsRemaining` per displayed transaction, using the internal threshold for
+  transactions the multisig sends to itself.
+- `GovernanceWrapper.getApprovalStatus` returns the new `required` and `remaining` fields.

packages/cli/src/commands/governance/show.test.ts

@@ -1,11 +1,18 @@
+import path from 'node:path'
+import { StrongAddress } from '@celo/base'
 import { newKitFromProvider } from '@celo/contractkit'
 import { unixSecondsTimestampToDateString } from '@celo/contractkit/lib/wrappers/BaseWrapper'
 import { Proposal } from '@celo/contractkit/lib/wrappers/Governance'
 import { testWithAnvilL2 } from '@celo/dev-utils/anvil-test'
 import { timeTravel } from '@celo/dev-utils/ganache-test'
 import fs from 'fs'
-import path from 'node:path'
-import { stripAnsiCodesAndTxHashes, testLocallyWithNode } from '../../test-utils/cliUtils'
+import { changeMultiSigOwner } from '../../test-utils/chain-setup'
+import {
+  stripAnsiCodesAndTxHashes,
+  stripAnsiCodesFromNestedArray,
+  testLocallyWithNode,
+} from '../../test-utils/cliUtils'
+import Approve from './approve'
 import Show from './show'
 
 process.env.NO_SYNCCHECK = 'true'
@@ -95,6 +102,8 @@ testWithAnvilL2('governance:show cmd', (provider) => {
         completion: 0 / 1
         confirmations: 
 
+        remaining: 1
+        required: 1
       approved: false
       metadata: 
         deposit: 100000000000000000000 (~1.000e+20)
@@ -118,6 +127,9 @@ testWithAnvilL2('governance:show cmd', (provider) => {
         No: 0 
         Yes: 100000000000000000000 (~1.000e+20)",
         ],
+        [
+          "Note: 1 more approver confirmation(s) needed to approve this proposal (0/1)",
+        ],
         [
           "Note: required is the minimal amount of yes + abstain votes needed to pass the proposal",
         ],
@@ -131,4 +143,33 @@ testWithAnvilL2('governance:show cmd', (provider) => {
       ]
     `)
   })
+  it('shows how many approvals a hotfix still needs', async () => {
+    const HOTFIX_HASH = '0xbf670baa773b342120e1af45433a465bbd6fa289a5cf72763d63d95e4e22482d'
+    const kit = newKitFromProvider(provider)
+    const [approver] = (await kit.connection.getAccounts()) as StrongAddress[]
+    // make an account we can send from a signatory of the approver multisig
+    await changeMultiSigOwner(kit, approver)
+
+    const logMock = jest.spyOn(console, 'log')
+    logMock.mockClear()
+    await testLocallyWithNode(Show, ['--hotfix', HOTFIX_HASH], provider)
+
+    const beforeApproval = stripAnsiCodesFromNestedArray(logMock.mock.calls).flat().join('\n')
+    expect(beforeApproval).toContain(
+      'Note: 1 more approver confirmation(s) needed to approve this hotfix (0/1)'
+    )
+
+    await testLocallyWithNode(
+      Approve,
+      ['--hotfix', HOTFIX_HASH, '--from', approver, '--useMultiSig'],
+      provider
+    )
+
+    logMock.mockClear()
+    await testLocallyWithNode(Show, ['--hotfix', HOTFIX_HASH], provider)
+
+    const afterApproval = stripAnsiCodesFromNestedArray(logMock.mock.calls).flat().join('\n')
+    expect(afterApproval).toContain('Note: the approver has approved this hotfix')
+    expect(afterApproval).toContain('this hotfix still needs the approval of the security council')
+  })
 })

packages/cli/src/commands/governance/show.ts

@@ -1,17 +1,43 @@
 import { ProposalBuilder, proposalToJSON } from '@celo/governance'
-import { hexToBytes } from 'viem'
 import { Flags } from '@oclif/core'
 import chalk from 'chalk'
 import { writeFileSync } from 'fs'
+import { hexToBytes } from 'viem'
 import { BaseCommand } from '../../base'
 import { newCheckBuilder } from '../../utils/checks'
 import { printValueMap, printValueMapRecursive } from '../../utils/cli'
 import { ViewCommmandFlags } from '../../utils/flags'
 import {
+  ApprovalProgress,
   addExistingProposalIDToBuilder,
   addExistingProposalJSONFileToBuilder,
+  getHotfixApprovalProgress,
 } from '../../utils/governance'
 
+function printApprovalNote(label: string, progress: ApprovalProgress) {
+  if (progress.approved) {
+    console.log(`Note: the ${label} has approved this hotfix`)
+    return
+  }
+
+  switch (progress.kind) {
+    case 'multisig':
+      console.log(
+        progress.remaining > 0
+          ? `Note: ${progress.remaining} more ${label} confirmation(s) needed to approve this hotfix (${progress.confirmations.length}/${progress.required})`
+          : `Note: the ${label} multisig has enough confirmations, the approval is pending execution`
+      )
+      return
+    case 'safe':
+      console.log(
+        `Note: this hotfix still needs the approval of the ${label} SAFE ${progress.address} (${progress.required} signature(s) required, collected offchain)`
+      )
+      return
+    default:
+      console.log(`Note: this hotfix still needs the approval of the ${label} ${progress.address}`)
+  }
+}
+
 export default class Show extends BaseCommand {
   static aliases = [
     'governance:show',
@@ -139,6 +165,15 @@ export default class Show extends BaseCommand {
         schedule,
       })
 
+      if (record.approvals && !record.approved) {
+        const { remaining, required, confirmations } = record.approvals
+        console.log(
+          remaining > 0
+            ? `Note: ${remaining} more approver confirmation(s) needed to approve this proposal (${confirmations.length}/${required})`
+            : 'Note: the approver multisig has enough confirmations, the approval is pending execution'
+        )
+      }
+
       if (Object.keys(requirements).length !== 0) {
         console.log(
           'Note: required is the minimal amount of yes + abstain votes needed to pass the proposal'
@@ -151,6 +186,18 @@ export default class Show extends BaseCommand {
       const hotfixBuf = Buffer.from(hexToBytes(hotfix as `0x${string}`))
       const record = await governance.getHotfixRecord(hotfixBuf)
       printValueMap(record)
+
+      if (!record.executed) {
+        const { approver, securityCouncil } = await getHotfixApprovalProgress(
+          await this.getPublicClient(),
+          governance,
+          hotfix,
+          record
+        )
+        printValueMapRecursive({ approvals: { approver, securityCouncil } })
+        printApprovalNote('approver', approver)
+        printApprovalNote('security council', securityCouncil)
+      }
     } else if (account) {
       const accounts = await kit.contracts.getAccounts()
       printValueMapRecursive(await governance.getVoter(await accounts.signerToAccount(account)))

packages/cli/src/commands/multisig/show.test.ts

@@ -1,8 +1,11 @@
+import { multiSigABI } from '@celo/abis'
 import { StrongAddress } from '@celo/base'
 import { ContractKit, newKitFromProvider } from '@celo/contractkit'
 import { testWithAnvilL2 } from '@celo/dev-utils/anvil-test'
+import { encodeFunctionData } from 'viem'
 import { stripAnsiCodesFromNestedArray, testLocallyWithNode } from '../../test-utils/cliUtils'
 import { createMultisig } from '../../test-utils/multisigUtils'
+import ApproveMultiSig from './approve'
 import ProposeMultiSig from './propose'
 import ShowMultiSig from './show'
 
@@ -113,13 +116,59 @@ testWithAnvilL2('multisig:show integration tests', (provider) => {
           1: 1000000000000000000 (~1e+18)
           2: 0x
           3: false
+          confirmations: 
+            0: 0x5409ED021D9299bf6814279A6A1411A7e866A631
+          confirmationsRemaining: 1 
+          confirmationsRequired: 2 
           data: null",
           ],
         ]
       `)
       expect(result).toBeUndefined()
     })
 
+    it('shows how many confirmations a transaction still needs', async () => {
+      // a dedicated multisig keeps the transaction indexes independent of the other tests.
+      // 3 signatures are required in general, but only 2 for transactions the multisig sends
+      // to itself, which is what this transaction does
+      const isolatedMultisig = await createMultisig(kit, [owner1, owner2, owner3], 3, 2)
+      const addOwner = encodeFunctionData({
+        abi: multiSigABI,
+        functionName: 'addOwner',
+        args: [accounts[5]],
+      })
+      const txId = '0'
+
+      await testLocallyWithNode(
+        ProposeMultiSig,
+        [isolatedMultisig, '--from', owner1, '--to', isolatedMultisig, '--data', addOwner],
+        provider
+      )
+
+      const logMock = jest.spyOn(console, 'log')
+      logMock.mockClear()
+      await testLocallyWithNode(ShowMultiSig, [isolatedMultisig, '--tx', txId], provider)
+
+      // proposing confirms on behalf of the proposer, so 1 of the 2 internally required is missing
+      const beforeApproval = stripAnsiCodesFromNestedArray(logMock.mock.calls).flat().join('\n')
+      expect(beforeApproval).toContain('confirmationsRequired: 2')
+      expect(beforeApproval).toContain('confirmationsRemaining: 1')
+
+      await testLocallyWithNode(
+        ApproveMultiSig,
+        ['--from', owner2, '--for', isolatedMultisig, '--tx', txId],
+        provider
+      )
+
+      logMock.mockClear()
+      await testLocallyWithNode(ShowMultiSig, [isolatedMultisig, '--tx', txId], provider)
+
+      const afterApproval = stripAnsiCodesFromNestedArray(logMock.mock.calls).flat().join('\n')
+      expect(afterApproval).toContain('confirmationsRemaining: 0')
+      expect(afterApproval).toContain(owner1)
+      expect(afterApproval).toContain(owner2)
+    })
+
     it('shows raw transaction data', async () => {
       const logMock = jest.spyOn(console, 'log')
 

packages/cli/src/commands/multisig/show.ts

@@ -2,11 +2,12 @@ import { getMultiSigContract } from '@celo/actions/contracts/multisig'
 import { CeloContract } from '@celo/contractkit'
 import { newBlockExplorer } from '@celo/explorer/lib/block-explorer'
 import { Flags } from '@oclif/core'
-import { Address } from 'viem'
+import { Address, zeroAddress } from 'viem'
 import { BaseCommand } from '../../base'
 import { printValueMapRecursive } from '../../utils/cli'
 import { CustomArgs } from '../../utils/command'
 import { ViewCommmandFlags } from '../../utils/flags'
+import { getConfirmationProgress } from '../../utils/multisig-utils'
 
 export default class ShowMultiSig extends BaseCommand {
   static description = 'Shows information about multi-sig contract'
@@ -44,30 +45,58 @@ export default class ShowMultiSig extends BaseCommand {
 
     const multisig = await getMultiSigContract(clients, multisigAddress)
     const txCount = await multisig.read.getTransactionCount([true, true])
+    const [required, internalRequired] = await Promise.all([
+      multisig.read.required(),
+      multisig.read.internalRequired(),
+    ])
     const explorer = await newBlockExplorer(await this.getKit())
     await explorer.updateContractDetailsMapping(CeloContract.MultiSig, multisigAddress)
-    const process = async (txdata: Awaited<ReturnType<typeof multisig.read.transactions>>) => {
-      if (raw) return txdata
-      return { ...txdata, data: await explorer.tryParseTxInput(txdata[0], txdata[2]) }
+    const confirmationStatus = async (txId: bigint, destination: Address, executed: boolean) => {
+      if (destination === zeroAddress) {
+        // transaction does not exist, there is nothing to confirm
+        return {}
+      }
+      const progress = await getConfirmationProgress(
+        multisig.read,
+        txId,
+        { destination, executed },
+        { required, internalRequired },
+        multisigAddress
+      )
+      // prefixed keys: a bare `required` next to the multisig-wide
+      // 'Required confirmations' lines would be ambiguous
+      return {
+        confirmations: progress.confirmations,
+        confirmationsRequired: progress.required,
+        confirmationsRemaining: progress.remaining,
+      }
+    }
+    const process = async (
+      txId: bigint,
+      txdata: Awaited<ReturnType<typeof multisig.read.transactions>>
+    ) => {
+      const [destination, , input, executed] = txdata
+      const withConfirmations = {
+        ...txdata,
+        ...(await confirmationStatus(txId, destination, executed)),
+      }
+      if (raw) return withConfirmations
+      return { ...withConfirmations, data: await explorer.tryParseTxInput(destination, input) }
     }
     const txinfo =
       tx !== undefined
-        ? await process(await multisig.read.transactions([BigInt(tx)]))
+        ? await process(BigInt(tx), await multisig.read.transactions([BigInt(tx)]))
         : all
           ? await Promise.all(
-              (
-                await Promise.all(
-                  (
-                    await multisig.read.getTransactionIds([BigInt(0), txCount, true, true])
-                  ).map((tx) => multisig.read.transactions([tx]))
-                )
-              ).map(process)
+              (await multisig.read.getTransactionIds([BigInt(0), txCount, true, true])).map(
+                async (txId) => process(txId, await multisig.read.transactions([txId]))
+              )
             )
           : txCount
     const info = {
       Owners: await multisig.read.getOwners(),
-      'Required confirmations': await multisig.read.required(),
-      'Required confirmations (internal)': await multisig.read.internalRequired(),
+      'Required confirmations': required,
+      'Required confirmations (internal)': internalRequired,
       Transactions: txinfo,
     }
     printValueMapRecursive(info)