Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Tracked by #10990
arthurgousset opened this issue Jun 7, 2024 · 8 comments
Assignees

Comments

@arthurgousset
Copy link
Member

arthurgousset commented Jun 7, 2024

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 arthurgousset changed the title Event missing ReleaseGold event is missing in foundry migrations ("ReleaseGoldInstanceCreated") Jun 7, 2024
@arthurgousset
Copy link
Member Author

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

@shazarre
Copy link
Contributor

shazarre commented Jun 7, 2024

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
Copy link
Member Author

arthurgousset commented Jun 11, 2024

@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
Copy link
Contributor

@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
Copy link
Member Author

arthurgousset commented Jun 11, 2024

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.

@arthurgousset arthurgousset self-assigned this Jun 11, 2024
@shazarre
Copy link
Contributor

@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
Copy link
Member Author

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
Copy link
Member Author

On that basis, I'll close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants