feat: oracle bridge upgrade on admin networks #4817
Conversation
Signed-off-by: Reinis Martinsons <[email protected]>
Reinis-FRP
requested review from
mrice32,
chrismaree,
md0x and
daywiss
as code owners
| }, | ||
| "devDependencies": { | ||
| "ts-node": "^10.9.2", | ||
| "typescript": "^5.8.3" |
There was a problem hiding this comment.
@safe-global/types-kit was not compatible with older typescipt from the root of the workspace.
| (e) => | ||
| e.returnValues.calls === | ||
| [{ to: contractsByNetId[network.chainId].registry.options.address, data: registerContractData }] | ||
| const relayedRegisterContractEvent = relayedTransactions.find((e) => |
There was a problem hiding this comment.
The newer typescript version errored to build this as it does not allow comparison of object references
| data: BytesLike; | ||
| }[] = []; | ||
| const changeImplementationAddressTx = await finder.populateTransaction.changeImplementationAddress( | ||
| ethersUtils.formatBytes32String("Oracle"), |
There was a problem hiding this comment.
nit: use interfaceName.Oracle from
const { interfaceName } = require("@uma/common");
There was a problem hiding this comment.
totally forgot we had it, will fix
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the Oracle bridge contract for admin networks by updating deployment addresses, multisig payload generation, and verification scripts. It updates relevant scripts to use the shared interfaceName for Oracle references, adds new admin proposal payload and verification scripts, and introduces a type generation tool.
Reviewed Changes
Copilot reviewed 9 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/scripts/src/utils/gnosisPayload.ts | Adds functions to build and simulate safe payloads for multisig execution |
| packages/scripts/src/admin-proposals/upgrade-oo-request-bridging/*.ts | Updates contract address calls to use interfaceName.Oracle and adjusts payload logic |
| packages/scripts/src/admin-proposals/upgrade-oo-request-bridging-admin/*.ts | Adds new scripts for payload generation and verification on admin chains |
| packages/scripts/src/admin-proposals/registerContract.js | Uses deep equality (isEqual) for event comparison in contract registration |
| packages/scripts/generate-types.ts | Introduces a script to generate ethers-v5 typechain files |
Files not reviewed (6)
- packages/core/networks/100.json: Language not supported
- packages/core/networks/1116.json: Language not supported
- packages/core/networks/1514.json: Language not supported
- packages/core/networks/43114.json: Language not supported
- packages/scripts/.gitignore: Language not supported
- packages/scripts/package.json: Language not supported
Comments suppressed due to low confidence (2)
packages/scripts/src/utils/gnosisPayload.ts:231
- Consider adding unit tests for the safe payload simulation functions (simulateSafePayload, approveTransaction, executeTransaction) to cover error scenarios and edge cases.
export async function simulateSafePayload(provider: JsonRpcProvider, payload: GnosisPayload, version: SafeVersion): Promise<void> {
packages/scripts/src/admin-proposals/registerContract.js:429
- [nitpick] The deep equality check using 'isEqual' assumes a consistent order of calls in the event data. Consider adding a comment or adjusting the logic to handle potential changes in ordering.
isEqual(e.returnValues.calls, [ { to: contractsByNetId[network.chainId].registry.options.address, data: registerContractData } ])
| for (const owner of owners.slice(0, threshold)) { | ||
| const signer = await getImpersonatedSigner(provider, owner, 1); | ||
|
|
||
| process.stdout.write(`Approving transaction from ${owner}...`); |
There was a problem hiding this comment.
[nitpick] Using process.stdout for logging may lead to issues in non-interactive environments; consider using an environment-agnostic logging framework instead.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
the script is expected to be run only interactively, so it should be fine to use process.stdout
Motivation
The Oracle bridge contract upgrade from #4816 should be rolled out also to admin controlled networks.
Summary
Updates contract deployment address and scripts for generating and verifying multisig payload for upgrading Oracle bridging contracts on admin controlled networks.
Testing
Check a box to describe how you tested these changes and list the steps for reviewers to test.
Issue(s)
Fixes #XXXX