refactor(contractkit): migrate to viem-native contract interaction [2/8]#767
refactor(contractkit): migrate to viem-native contract interaction [2/8]#767pahor167 wants to merge 1 commit into
Conversation
- Migrate BaseWrapper from proxySend/proxyCall to contract.write/contract.read - Add strongly-typed generic ABIs to BaseWrapper and all 36 wrappers (~110 any eliminated) - Replace web3-contract-cache.ts with contract-factory-cache.ts - Delete PromiEventStub.ts - Remove web3 dependency from kit.ts and mini-kit.ts - Replace CeloTransactionObject with direct tx hash + waitForTransactionReceipt
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d1623cf9a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (url.endsWith('.ipc')) { | ||
| try { | ||
| const net = require('net') | ||
| web3 = new Web3(new Web3.providers.IpcProvider(url, net)) | ||
| } catch (e) { | ||
| console.error('.ipc only works in environments with native net module') | ||
| } | ||
| web3 = new Web3(url) | ||
| } else if (url.toLowerCase().startsWith('http')) { | ||
| web3 = new Web3(new Web3.providers.HttpProvider(url, options)) | ||
| return new SimpleIpcProvider(url, net) | ||
| } else { | ||
| web3 = new Web3(url) | ||
| return new SimpleHttpProvider(url, options) |
There was a problem hiding this comment.
getProviderForKit now routes every non-.ipc URL through SimpleHttpProvider, so ws:///wss:// RPC endpoints are treated as HTTP and break at runtime. The previous implementation delegated non-HTTP URLs to new Web3(url), which supported websocket providers; this change removes that path and will fail for deployments configured with websocket-only nodes.
Useful? React with 👍 / 👎.
| address: this.contract.address, | ||
| event: eventAbi as any, | ||
| fromBlock, | ||
| toBlock, | ||
| }) |
There was a problem hiding this comment.
getPastEvents no longer passes options.filter (or other indexed-arg constraints) into getLogs, so callers that rely on filtered event queries will now receive unfiltered logs. That changes behavior from the previous contract.getPastEvents(event, options) path and can return incorrect results and much larger log sets for the same query.
Useful? React with 👍 / 👎.
| } catch { | ||
| // Event decoding may fail for proxy contracts; return empty gracefully | ||
| return [] |
There was a problem hiding this comment.
The blanket catch converts any RPC/decode failure into [], which makes node outages, malformed responses, and decode bugs look identical to “no events found.” This silently corrupts downstream logic that uses event history for state derivation, because failures are no longer observable to callers.
Useful? React with 👍 / 👎.
…es (#780) ## Summary Combined branch containing the full web3 → viem migration (the union of the stacked PRs #766–#773) plus 24 commits of review fixes from a deep multi-agent audit of the cumulative diff (~350 files). ### Migration (from the original stack) - `@celo/connect`, `@celo/utils`: web3 replaced with viem-native primitives (#766) - `@celo/contractkit`: viem-native contract interaction across all wrappers (#767) - `@celo/explorer`, `@celo/governance`, wallets, dev-utils (#768) - `@celo/celocli`: all command groups migrated (#769–#773) ### Review fixes on top (highlights, 38 findings fixed) Funds/correctness critical: - wallet-base: tx quantities above 2^53 were silently truncated when hex-encoding (`1 CELO + 1 wei` signed as exactly 1 CELO); now BigInt-backed with a regression test - connect: `defaultFeeCurrency` was silently ignored for every `contract.write`; dynamic chain now carries the Celo serializers/formatters so CIP-64 serializes correctly - contractkit: IPC provider hung forever (parsed only on socket `end`); ws:// / wss:// node URLs fell through to the HTTP provider; HTTP error statuses resolved `undefined` - contractkit: `setAccount` without proof-of-possession always threw (`'0x0'` as bytes32); `Attestations.getPendingWithdrawals` parameter order restored to match the contract - cli: `releasecelo:admin-revoke` lost the `{from: voteSigner}` override → revokes reverted with an authorized vote signer - connect: fee-math radix bug inflated `maxFeePerGas` for string inputs High: - tuple-blind ABI signatures produced wrong selectors/topic hashes (ABI enrichment, `methodIds`, log-explorer event matching) - `BaseWrapper.getPastEvents` swallowed RPC errors as "no events"; unknown event names now throw (web3 parity) - Election historical `blockNumber` queries were silently ignored (voter rewards at past epochs) - governance CLI commands no longer displayed on-chain event confirmations (ProposalExecuted / HotfixExecuted / HotfixApproved / Deposit) — restored, including the silently-dropped test snapshots - `isValidChecksumAddress` accepted all-lowercase addresses; sourcify proxy implementation address checksummed again - BigInt-safe JSON-RPC serialization in production providers Plus test-semantics restorations (weakened assertions, mocked-method-under-test), test-infra robustness (receipt waits, strict `evm_revert`), changeset corrections (contractkit correctly marked major with full breaking-change list), and documentation of behavior changes. Full audit trail (62 findings: 38 fixed, 12 refuted with evidence, the rest classified pre-existing/wontfix) lives in `.omc/ralph/pr-review-progress.md` on the working machine; findings are also traceable through the individual commit messages. ## Test plan - [x] Full workspace build green (all 23 packages) - [x] Full test suite: all packages green locally except known env-flaky timeouts under full parallel load (actions/keystores) and a pre-existing `election:activate --wait` snapshot-order flake that reproduces on the unmodified stack HEAD - [x] Anvil-backed suites green: contractkit 258/258, celocli 337 passed - [x] Live smoke tests: IPC provider against `anvil --ipc`, WebSocket provider against `wss://forno.celo.org/ws` - [ ] CI green on this PR --------- Co-authored-by: Pavel Hornak <pavel.hornak@clabs.co> Co-authored-by: Paul Lange <palango@gmx.de>
1 participant
Stack
Changes
BaseWrapperfromproxySend/proxyCalltocontract.write/contract.readanyeliminated)web3-contract-cache.tswithcontract-factory-cache.tsPromiEventStub.tskit.tsandmini-kit.tsCeloTransactionObjectwith direct tx hash +waitForTransactionReceiptPart of the web3 → viem migration. No CI requirement on this PR.
PR-Codex overview
This PR focuses on refactoring the
contractkitpackage to replaceweb3dependencies with a new connection model. It improves type safety, updates method calls, and enhances transaction handling, aligning with the new architecture.Detailed summary
web3dependency and related files.SolidityValueand other types.newKitFromWeb3tonewKitFromProvider.viemClientfor receipt waiting.