-
Notifications
You must be signed in to change notification settings - Fork 13
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
### Description
running `celocli network:contracts` would lead to a cryptic error
```
Error: execution reverted
at decoratedCallback (/Users/martinvol/celo/developer-tooling/packages/sdk/connect/lib/utils/rpc-caller.js:109:23)
at /Users/martinvol/celo/developer-tooling/node_modules/web3-core/node_modules/web3-providers-http/lib/index.js:131:13
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
Error: execution reverted
```
adding try/catch around some calls lead to the discovery that the call
to fetch the version number was failing for GovernanceSlasher contract.
A chat with @martinvol revealed there was an issue and we should display
version as 1.1.0.0 if none could be fetched.
#### Other changes
### Tested
added to and improve the contracts-l2 tests
### How to QA
run. `celocli network:contracts` command and expect to see a table to
contracts with versions
### Related issues
<!-- start pr-codex -->
---
## PR-Codex overview
This PR focuses on fixing a bug related to the `GovernanceSlasher`
contract's version retrieval, enhancing error handling when fetching
contract information, and updating tests to accommodate these changes.
### Detailed summary
- Fixed bug in version retrieval for `GovernanceSlasher`.
- Added error handling in `contracts.ts` when fetching contract version.
- Updated test cases in `contracts-l2.test.ts` to handle scenarios where
version retrieval fails.
- Mocked `ICeloVersionedContract` for better test isolation.
- Adjusted snapshot tests to reflect new behavior and outputs.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your
question}`
<!-- end pr-codex -->- @celo/wallet-remote@7.0.0
- @celo/wallet-remote@7.0.0-beta.1
- @celo/wallet-local@7.0.0
- @celo/wallet-local@7.0.0-beta.1
- @celo/wallet-ledger@7.0.0
- @celo/wallet-ledger@7.0.0-beta.1
- @celo/wallet-hsm-gcp@7.0.0
- @celo/wallet-hsm-gcp@7.0.0-beta.1
- @celo/wallet-hsm-azure@7.0.0
- @celo/wallet-hsm-azure@7.0.0-beta.1
- @celo/wallet-hsm-aws@7.0.0
- @celo/wallet-hsm-aws@7.0.0-beta.1
- @celo/wallet-hsm@7.0.0
- @celo/wallet-hsm@7.0.0-beta.1
- @celo/wallet-base@7.0.0
- @celo/wallet-base@7.0.0-beta.1
- @celo/viem-account-ledger@1.1.0
- @celo/viem-account-ledger@1.1.0-beta.1
- @celo/utils@8.0.1
- @celo/transactions-uri@5.0.13
- @celo/phone-utils@6.0.5
- @celo/network-utils@5.0.7
- @celo/metadata-claims@1.0.1
- @celo/keystores@5.0.13
- @celo/governance@5.1.5
- @celo/governance@5.1.5-beta.1
- @celo/explorer@5.0.14
- @celo/dev-utils@0.0.8
- @celo/cryptographic-utils@5.1.2
- @celo/contractkit@9.0.1
- @celo/contractkit@9.0.1-beta.2
- @celo/contractkit@9.0.1-beta.1
- @celo/connect@6.1.1
- @celo/celocli@6.1.0
- @celo/celocli@6.1.0-beta.2
- @celo/celocli@6.1.0-beta.1
- @celo/base@7.0.1
Showing
4 changed files
with
272 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@celo/celocli': patch | ||
| --- | ||
|
|
||
| Fix bug with GovernanceSlasher missing version causing failure. defend against exceptions when printing contracts info |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,64 @@ | ||
| import { newICeloVersionedContract } from '@celo/abis/web3/ICeloVersionedContract' | ||
| import { testWithAnvilL2 } from '@celo/dev-utils/lib/anvil-test' | ||
| import write from '@oclif/core/lib/cli-ux/write' | ||
| import { testLocallyWithWeb3Node } from '../../test-utils/cliUtils' | ||
| import Contracts from './contracts' | ||
| process.env.NO_SYNCCHECK = 'true' | ||
| jest.mock('@celo/abis/web3/ICeloVersionedContract') | ||
|
|
||
| testWithAnvilL2('network:contracts', (web3) => { | ||
| test('runs', async () => { | ||
| const spy = jest.spyOn(write, 'stdout') | ||
| await testLocallyWithWeb3Node(Contracts, ['--output', 'json'], web3) | ||
| expect(spy.mock.calls).toMatchSnapshot() | ||
| describe('when version can be obtained', () => { | ||
| beforeEach(() => { | ||
| jest.unmock('@celo/abis/web3/ICeloVersionedContract') | ||
| jest.resetModules() | ||
| const actual = jest.requireActual('@celo/abis/web3/ICeloVersionedContract') | ||
| // @ts-expect-error | ||
| newICeloVersionedContract.mockImplementation(actual.newICeloVersionedContract) | ||
| }) | ||
| test('runs', async () => { | ||
| const spy = jest.spyOn(write, 'stdout') | ||
| const warnSpy = jest.spyOn(console, 'warn') | ||
| expect(warnSpy.mock.calls).toMatchInlineSnapshot(`[]`) | ||
| await testLocallyWithWeb3Node(Contracts, ['--output', 'json'], web3) | ||
| expect(spy.mock.calls).toMatchSnapshot() | ||
| }) | ||
| }) | ||
| describe('when version cant be obtained', () => { | ||
| beforeEach(() => { | ||
| // @ts-expect-error | ||
| newICeloVersionedContract.mockImplementation((_, address) => { | ||
| return { | ||
| methods: { | ||
| getVersionNumber: jest.fn().mockImplementation(() => { | ||
| // fake governance slasher | ||
| if (address === '0x76C05a43234EB2804aa83Cd40BA10080a43d07AE') { | ||
| return { call: jest.fn().mockRejectedValue(new Error('Error: execution reverted')) } | ||
| } else { | ||
| // return a fake normal version | ||
| return { call: jest.fn().mockResolvedValue([1, 2, 3, 4]) } | ||
| } | ||
| }), | ||
| }, | ||
| } | ||
| }) | ||
| }) | ||
| afterEach(() => { | ||
| jest.clearAllMocks() | ||
| jest.resetModules() | ||
| }) | ||
| it('still prints rest of contracts', async () => { | ||
| const spy = jest.spyOn(write, 'stdout') | ||
| const warnSpy = jest.spyOn(console, 'warn') | ||
|
|
||
| await testLocallyWithWeb3Node(Contracts, ['--output', 'json'], web3) | ||
| expect(warnSpy.mock.calls).toMatchInlineSnapshot(` | ||
| [ | ||
| [ | ||
| "Failed to get version for GovernanceSlasher at 0xDDA88a8ebeaaB19d2a58374D8c72200AFAF94bB4", | ||
| ], | ||
| ] | ||
| `) | ||
| expect(spy.mock.calls).toMatchSnapshot() // see the file for the snapshot | ||
| }) | ||
| }) | ||
| }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters