celo-org / celo-monorepo

Official repository for core projects comprising the Celo platform
https://celo.org
Apache License 2.0
697 stars 370 forks source link

ReleaseGold event is missing in foundry migrations ("ReleaseGoldInstanceCreated") #11026

Closed arthurgousset closed 3 months ago

arthurgousset commented 4 months ago

From @shazarre:

When running releasecelo tests I was getting an error that an event (I think connected to the releasecelo contract creation) wasn’t found ReleaseGoldInstanceCreated(address,address)

Source: Slack

arthurgousset commented 4 months ago

@shazarre could you please link to the test that is failing in celo-org/developer-tooling because of this missing event. Thanks!

shazarre commented 4 months ago

Output for failing tests:

 FAIL  src/commands/releasecelo/admin-revoke.test.ts
  releasegold:admin-revoke cmd
    ✕ will revoke (8 ms)
    ✕ will rescue all cUSD balance (2 ms)
    ✕ will refund and finalize (2 ms)
    #when account exists with locked celo
      ✕ will unlock all gold (3 ms)
      #when account has authorized a vote signer
        ✕ will rotate vote signer (2 ms)
        #when account has voted
          ✕ will revoke governance votes and upvotes (2 ms)

  ● releasegold:admin-revoke cmd › will revoke

    Error: contract could not be found matching signature ReleaseGoldInstanceCreated(address,address)

      37 |   })
      38 |   if (logs.length === 0) {
    > 39 |     throw Error(`Error: contract could not be found matching signature ${eventSignature}`)
         |           ^
      40 |   }
      41 |   const logIndex = filter?.index ?? 0
      42 |   if (!filter?.expectedData) {

      at getContractFromEvent (../dev-utils/src/ganache-test.ts:39:11)
      at Object.<anonymous> (src/commands/releasecelo/admin-revoke.test.ts:32:23)

(same for other tests)

The tests can be found here, but seems like it's actually from the setup function call that's failing. Seems it's expecting a ReleaseGold contract deployed in the devchain already.

arthurgousset commented 3 months ago

@shazarre: I'm trying to reproduce the error.

Here are the steps I'm following:

# On `master` branch
$ git status
On branch master
Your branch is up to date with 'origin/master'.

# Install all dependencies
$ yarn

# Build all packages
$ yarn build

# Build the CLI package explicitly
$ yarn workspace @celo/celocli build

# Run CLI tests
$ yarn workspace @celo/celocli test

PASS  src/commands/releasecelo/admin-revoke.test.ts

Unless I'm missing something, the test (admin-revoke.test.ts) is passing on my local master branch.

Here is the shell output when filtered for "releasegold":

Sending Transaction: releasegold: revokeBeneficiary... done
Sending Transaction: releasegold: refundAndFinalize... done
Sending Transaction: releasegold: revokeBeneficiary... done
Sending Transaction: releasegold: rescueCUSD... done
Sending Transaction: releasegold: refundAndFinalize... done
Sending Transaction: releasegold: revokeBeneficiary... done
Sending Transaction: releasegold: refundAndFinalize... done
Sending Transaction: releasegold: revokeBeneficiary... done
Sending Transaction: releasegold: unlockAllGold... done
Sending Transaction: releasegold: revokeBeneficiary... done
Sending Transaction: releasegold: unlockAllGold... done
Sending Transaction: releasegold: revokeBeneficiary... done
Sending Transaction: releasegold: unlockAllGold... done
      SendTransaction: releasegold: revokeBeneficiary
      SendTransaction: releasegold: refundAndFinalize
      ReleaseGoldInstanceDestroyed:
      SendTransaction: releasegold: revokeBeneficiary
      SendTransaction: releasegold: rescueCUSD
      SendTransaction: releasegold: refundAndFinalize
      ReleaseGoldInstanceDestroyed:
      SendTransaction: releasegold: revokeBeneficiary
      SendTransaction: releasegold: refundAndFinalize
      ReleaseGoldInstanceDestroyed:
      SendTransaction: releasegold: revokeBeneficiary
      SendTransaction: releasegold: unlockAllGold
      SendTransaction: releasegold: revokeBeneficiary
      SendTransaction: releasegold: unlockAllGold
      SendTransaction: releasegold: revokeBeneficiary
      SendTransaction: releasegold: unlockAllGold

It looks like all ReleaseGold related tests are passing.

shazarre commented 3 months ago

@arthurgousset you'd have to migrate those tests to anvil

basically you change testWithGanache to testWithAnvil and you need to replace all occurrences of testLocally with testLocallyWithWeb3 (and pass web3 as the last param)

right now those tests are still being run against ganache, so they'll all pass on CI

arthurgousset commented 3 months ago

Could you please create a branch with these changes and add step-by-step instructions to reproduce the bug, so anyone can easily checkout the branch, and reproduce the bug locally. I'll pause on this until that's done.

shazarre commented 3 months ago

@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
arthurgousset commented 3 months ago

Re: ReleaseGold instances are missing on the devchain

After talking with @soloseng and @pahor167, we agreed that the devchain will not include pre-deployed ReleaseGold contracts. Instead, the CLI test suite should deploy ReleaseGold contracts during set-up, which can then be used to test revoking and other functionality.

Re: Reserve tests are failing because of MultiSig error

I'll dig into this as part of this issue #11025.

arthurgousset commented 3 months ago

On that basis, I'll close this issue.