Add ReserveSpenderMultiSig to the foundry migrations

[arthurgousset]

From @shazarre:

reserve tests are failing with MultiSig not (yet) registered

 ● reserve:transfergold cmd › can transferGold with multisig option

    MultiSig not (yet) registered

Source: Slack

[shazarre]

Exact output is:

 FAIL  src/commands/reserve/transfergold.test.ts
  reserve:transfergold cmd
    ✓ transferGold fails if spender not passed in (723 ms)
    ✕ can transferGold with multisig option (45 ms)

  ● reserve:transfergold cmd › can transferGold with multisig option

    MultiSig not (yet) registered

      41 |       debug('Fetched address %s', address)
      42 |       if (!address || address === NULL_ADDRESS) {
    > 43 |         throw new UnregisteredError(contract)
         |               ^
      44 |       }
      45 |       this.cache.set(contract, address as StrongAddress)
      46 |     }

while the test can be found here.

[arthurgousset]

For reference, the test is:

test('can transferGold with multisig option', async () => {
    const initialBalance = await goldToken.balanceOf(accounts[9])
    await testLocally(TransferGold, [
      '--from',
      accounts[0],
      '--value',
      transferAmt.toString(10),
      '--to',
      accounts[9],
      '--useMultiSig',
    ])
    await testLocally(TransferGold, [
      '--from',
      accounts[7],
      '--value',
      transferAmt.toString(10),
      '--to',
      accounts[9],
      '--useMultiSig',
    ])
    expect(await goldToken.balanceOf(accounts[9])).toEqual(initialBalance.plus(transferAmt))
  })

[arthurgousset]

There is a comment relating to this in the migrations:

celo-monorepo/packages/protocol/migrations_sol/Migration.s.sol

Lines 332 to 333 in b10b77a

	function migrateReserve(string memory json) public {
	// Reserve spend multisig not migrated

[arthurgousset]

Looks like this might be the related ganache migration:

celo-monorepo/packages/protocol/migrations_ts/07_reserve_spender_multisig.ts

Lines 11 to 32 in b10b77a

	const initializeArgs = async (): Promise<any[]> => {
	return [
	config.reserveSpenderMultiSig.signatories,
	config.reserveSpenderMultiSig.numRequiredConfirmations,
	config.reserveSpenderMultiSig.numInternalRequiredConfirmations,
	]
	}
	module.exports = deploymentForProxiedContract<ReserveSpenderMultiSigInstance>(
	web3,
	artifacts,
	CeloContractName.ReserveSpenderMultiSig,
	initializeArgs,
	async (reserveSpenderMultiSig: ReserveSpenderMultiSigInstance) => {
	await transferOwnershipOfProxy(
	CeloContractName.ReserveSpenderMultiSig,
	reserveSpenderMultiSig.address,
	ArtifactsSingleton.getInstance(MENTO_PACKAGE)
	)
	},
	MENTO_PACKAGE
	)

The relevant configs are:

celo-monorepo/packages/protocol/migrationsConfig.js

Lines 163 to 167 in b10b77a

	reserveSpenderMultiSig: {
	signatories: [network.from],
	numRequiredConfirmations: 1,
	numInternalRequiredConfirmations: 1,
	},

[arthurgousset]

Here is a similar MultiSig deployment:

In foundry:

celo-monorepo/packages/protocol/migrations_sol/Migration.s.sol

Lines 787 to 808 in b10b77a

	function migrateGovernanceApproverMultiSig(string memory json) public {
	address[] memory owners = new address[](1);
	owners[0] = deployerAccount;
	uint256 required = abi.decode(json.parseRaw(".governanceApproverMultiSig.required"), (uint256));
	uint256 internalRequired = abi.decode(
	json.parseRaw(".governanceApproverMultiSig.internalRequired"),
	(uint256)
	);
	// This adds the multisig to the registry, which is not a case in mainnet but it's useful to keep a reference
	// of the deployed contract
	deployProxiedContract(
	"GovernanceApproverMultiSig",
	abi.encodeWithSelector(
	IGovernanceApproverMultiSigInitializer.initialize.selector,
	owners,
	required,
	internalRequired
	)
	);
	}

celo-monorepo/packages/protocol/migrations_sol/migrationsConfig.json

Lines 132 to 136 in b10b77a

	"governanceApproverMultiSig": {
	"required": 1,
	"internalRequired": 1,
	"governanceApproverMultiSig": true
	},

In ganache:

celo-monorepo/packages/protocol/migrations_ts/23_governance_approver_multisig.ts

Lines 9 to 29 in b10b77a

	const initializeArgs = async (): Promise<any[]> => {
	return [
	config.governanceApproverMultiSig.signatories,
	config.governanceApproverMultiSig.numRequiredConfirmations,
	config.governanceApproverMultiSig.numInternalRequiredConfirmations,
	]
	}
	module.exports = deploymentForProxiedContract<GovernanceApproverMultiSigInstance>(
	web3,
	artifacts,
	CeloContractName.GovernanceApproverMultiSig,
	initializeArgs,
	async (governanceApproverMultiSig: GovernanceApproverMultiSigInstance) => {
	await transferOwnershipOfProxy(
	CeloContractName.GovernanceApproverMultiSig,
	governanceApproverMultiSig.address,
	artifacts
	)
	}
	)

celo-monorepo/packages/protocol/migrationsConfig.js

Lines 114 to 120 in b10b77a

	governanceApproverMultiSig: {
	signatories: [network.from],
	numRequiredConfirmations: 1,
	numInternalRequiredConfirmations: 1,
	useMultiSig: true,
	},
	grandaMento: {

[arthurgousset]

Working on the PR here:

• feat: add ReserveSpenderMultiSig to anvil migrations #11028

[arthurgousset]

For visibility @shazarre, I marked this PR as ready for review and asked for a review from the primitive team:

• feat: add ReserveSpenderMultiSig to anvil migrations #11028

If and when it's merged to master, the new devchain will automatically publish, and you can let me know if the CLI test passes as expected.

[arthurgousset]

@shazarre this is done, and the new @celo/devchain-anvil is currently being published (see CI).

Let me know if the CLI test passes as expected.

[arthurgousset]

@arthurgousset created a branch that has failing examples for both releasecelo and reserve

to test it:

git checkout shazarre/Migrate_reserve_and_releasecelo_tests_to_anvil
yarn && yarn build
yarn workspace @celo/celocli run test

Originally posted by @shazarre in #11026 (comment)

[arthurgousset]

When I run the tests as described above, I get this error:

 FAIL  src/commands/reserve/transfergold.test.ts
  ● Console

    console.log
      Running Checks:

      at CheckBuilder.runChecks (src/utils/checks.ts:491:13)

    console.log
         ✘  0x5409ED021D9299bf6814279A6A1411A7e866A631 is a reserve spender

      at CheckBuilder.runChecks (src/utils/checks.ts:498:15)

    console.log
         ✘  0x91c987bf62D25945dB517BDAa840A6c661374402 is another reserve address

[arthurgousset]

It looks like the CLI doesn't find any spenders using the getSpenders() helper function:

async getSpenders(): Promise<Address[]> {
    const spendersAdded = (
      await this.getPastEvents('SpenderAdded', {
        fromBlock: 0,
        toBlock: 'latest',
      })
    ).map((eventlog: EventLog) => eventlog.returnValues.spender)
    const spendersRemoved = (
      await this.getPastEvents('SpenderRemoved', {
        fromBlock: 0,
        toBlock: 'latest',
      })
    ).map((eventlog: EventLog) => eventlog.returnValues.spender)
    return spendersAdded.filter((spender) => !spendersRemoved.includes(spender))
  }
}

Source: contractkit/src/wrappers/Reserve.ts

const spenders = useMultiSig ? await reserve.getSpenders() : []
/////////////////////// DEBUGGING ///////////////////////
console.log('spenders[] are:', spenders)
/////////////////////// DEBUGGING ///////////////////////

Source: cli/src/commands/reserve/transfergold.ts

git checkout shazarre/Migrate_reserve_and_releasecelo_tests_to_anvil
yarn && yarn build
yarn workspace @celo/celocli run test

console.log
  spenders[] are: []

  at TransferGold.run (src/commands/reserve/transfergold.ts:36:13)

On the devchain, the SpenderAdded event is emitted:

cast logs \
"SpenderAdded(address indexed spender)" \
--from-block 0 --to-block latest \
--rpc-url=http://127.0.0.1:8546
- address: 0x0a887500E327375378edF7b7d0E85F0378b8677a
  blockHash: 0xdd3366edde3433c624cf059c190e864470ee5ed423fe57393fabffc52020c242
  blockNumber: 43
  data: 0x
  logIndex: 0
  removed: false
  topics: [
  	0x3139419c41cdd7abca84fa19dd21118cd285d3e2ce1a9444e8161ce9fa62fdcd
  	0x0000000000000000000000001a888d93eeacf683c68e07ad58aa43f2a5742490
  ]
  transactionHash: 0xbf4e79a92f7c6a1be5451d1489e70278efd89c9c15c3497e968527d854f6063c
  transactionIndex: 0

The ReserveSpenderMultiSig address is 0x1A888D93eeAcF683c68E07Ad58Aa43f2A5742490 as expected from the log and the contract call.

cast call \
"0x0a887500E327375378edF7b7d0E85F0378b8677a" \
"isSpender(address)(bool)" \
"0x1A888D93eeAcF683c68E07Ad58Aa43f2A5742490" \
--rpc-url=http://127.0.0.1:8546
true

[arthurgousset]

Minimal reproducible example to check if the bug is related to the JSON dump or NPM version

1. Make a test directory

   mkdir ~/Documents/celo-org/devchain-anvil-installation
   cd ~/Documents/celo-org/devchain-anvil-installation

2. Install the @celo/devchain-anvil package

   npm install --save-dev @celo/devchain-anvil

3. Start up anvil with the JSON state from the package

   anvil --state node_modules/@celo/devchain-anvil/devchain.json

4. (separate terminal) Find the Reserve address in the Registry

   cast call \
   "0x000000000000000000000000000000000000ce10" \
   "getAddressForStringOrDie(string calldata identifier)(address)" \
   "Reserve" \
   --rpc-url=http://127.0.0.1:8545
   0xe5cC39E5404c451604A85a4CD9686d918D18577e

5. (separate terminal) Find the ReserveSpenderMultiSig address in the Registry

   cast call \
   "0x000000000000000000000000000000000000ce10" \
   "getAddressForStringOrDie(string calldata identifier)(address)" \
   "ReserveSpenderMultiSig" \
   --rpc-url=http://127.0.0.1:8545
   0x909158d6b441312D6813CBea21D466C66ef94280

6. (separate terminal) Check that 0x1A888D93eeAcF683c68E07Ad58Aa43f2A5742490 is a spender ✅

   cast call \
   "0xe5cC39E5404c451604A85a4CD9686d918D18577e" \
   "isSpender(address)(bool)" \
   "0x909158d6b441312D6813CBea21D466C66ef94280" \
   --rpc-url=http://127.0.0.1:8545
   true

7. But, we found that logs are not included in the JSON dump. That means helpers that use events, cannot be used anymore with Anvil. ❌

   cast logs \
   "SpenderAdded(address indexed spender)" \
   --from-block 0 --to-block latest \
   --rpc-url=http://127.0.0.1:8545

   Anvil cannot dump events into a JSON. This is a known limitation:

   • Add anvil --dump-event foundry-rs/foundry#5906

[arthurgousset]

That means the contractkit/src/wrappers/Reserve.ts helper function won't work with Anvil.

But, there is an (anvil) devchain specific fix, which is to get the ReserveSpenderMultiSig address from the Registry.
The contract is in the Registry on the devchain for ease of reference, but it's not available on Mainnet.

[arthurgousset]

I'm closing this issue on that basis.