chore(infra): verify that admin addresses for proxied contracts are checked by check-deploy #4171

Mo-Hussain · 2024-07-22T09:51:26Z

Description

This PR verifies that check-deploy checks 1) if the actual admin addresses for proxied contracts match the config b) if the ownable contracts (proxyAdmin for mailbox and mailbox) are in fact owned by the safes defined in config
Logging is adding to the PR to demonstrate that 1 is checked
2 is verified by changing the config in owners to raise a violation

Core

$ LOG_LEVEL=debug LOG_FORMAT=pretty yarn tsx ./scripts/check-deploy.ts -e mainnet3 -m core -c ethereum
Checking admins addresses for proxied contracts for ethereum
Checking contract: mailbox Expected admin address: 0x75EE15Ee1B4A75Fa3e2fDF5DF3253c25599cc659 Actual admin address: 0x75EE15Ee1B4A75Fa3e2fDF5DF3253c25599cc659

Checking proxy for ethereum with name Mailbox proxy and address 0xc005dc82818d67AF737725bD4bf75435d065D239
Checking ownership of contracts for ethereum with owner 0x0DA0C3e52C977Ed3cBc641fF02DD271c3ED55aFe
Ownable contracts: validatorAnnounce,proxyAdmin,mailbox
Checking ownership for validatorAnnounce, address 0xCe74905e51497b4adD3639366708b821dcBcff96 expected owner 0x3e0A78A330F2b97059A4D507ca9d8292b65B6FB5, actual owner 0xa7ECcdb9Be08178f896c26b7BbD8C3D4E844d9Ba
Checking ownership for proxyAdmin, address 0x75EE15Ee1B4A75Fa3e2fDF5DF3253c25599cc659 expected owner 0x0DA0C3e52C977Ed3cBc641fF02DD271c3ED55aFe, actual owner 0x3965AC3D295641E452E0ea896a086A9cD7C6C5b6
Checking ownership for mailbox, address 0xc005dc82818d67AF737725bD4bf75435d065D239 expected owner 0x0DA0C3e52C977Ed3cBc641fF02DD271c3ED55aFe, actual owner 0x3965AC3D295641E452E0ea896a086A9cD7C6C5b6
┌─────────┬────────────┬────────┬─────────────────────┬───────────┬──────────────┬──────────────────────────────────────────────┬──────────────────────────────────────────────┐
│ (index) │ chain      │ remote │ name                │ type      │ subType      │ actual                                       │ expected                                     │
├─────────┼────────────┼────────┼─────────────────────┼───────────┼──────────────┼──────────────────────────────────────────────┼──────────────────────────────────────────────┤
│ 0       │ 'ethereum' │        │                     │ 'Mailbox' │ 'DefaultIsm' │ '0xA02D4C8C566811a82a918173178029eBF6c361D3' │ [Object]                                     │
│ 1       │ 'ethereum' │        │ 'validatorAnnounce' │ 'Owner'   │              │ '0xa7ECcdb9Be08178f896c26b7BbD8C3D4E844d9Ba' │ '0x3e0A78A330F2b97059A4D507ca9d8292b65B6FB5' │
│ 2       │ 'ethereum' │        │ 'proxyAdmin'        │ 'Owner'   │              │ '0x3965AC3D295641E452E0ea896a086A9cD7C6C5b6' │ '0x0DA0C3e52C977Ed3cBc641fF02DD271c3ED55aFe' │
│ 3       │ 'ethereum' │        │ 'mailbox'           │ 'Owner'   │              │ '0x3965AC3D295641E452E0ea896a086A9cD7C6C5b6' │ '0x0DA0C3e52C977Ed3cBc641fF02DD271c3ED55aFe' │
└─────────┴────────────┴────────┴─────────────────────┴───────────┴──────────────┴──────────────────────────────────────────────┴──────────────────────────────────────────────┘

Error: Checking core deploy yielded 3 violations
    at check (file:///Users/mo/dev/hyperlane/hyperlane-monorepo/typescript/infra/scripts/check-deploy.ts:1:4262)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Warp

$ LOG_LEVEL=debug LOG_FORMAT=pretty yarn tsx ./scripts/deploy.ts -e mainnet3 -f ethereum -m warp
Deploying staticAggregationIsm to ethereum 
Deploying messageIdMultisigIsm to ethereum 
Recovered 2 of 3 address set on ethereum: 0xD32513263Eb3190c4DA126fe418864904D9F4A32
Deploying merkleRootMultisigIsm to ethereum 
Recovered 2 of 3 address set on ethereum: 0x054B75b89DA7a428C2BF1f6b5df33944167B785C
Recovered 1 of 2 address set on ethereum: 0x9c764dF6bdd6B716CfadEAa348E1dB146694F67F
Checking ownership of contracts for ethereum with owner 0x3e0A78A330F2b97059A4D507ca9d8292b65B6FB5
Ownable contracts: collateral
Checking ownership for collateral, address 0x8b4192B9Ad1fCa440A5808641261e5289e6de95D expected owner 0x3e0A78A330F2b97059A4D507ca9d8292b65B6FB5, actual owner 0xa7ECcdb9Be08178f896c26b7BbD8C3D4E844d9Ba
┌─────────┬────────────┬────────┬──────────────┬─────────┬─────────┬──────────────────────────────────────────────┬──────────────────────────────────────────────┐
│ (index) │ chain      │ remote │ name         │ type    │ subType │ actual                                       │ expected                                     │
├─────────┼────────────┼────────┼──────────────┼─────────┼─────────┼──────────────────────────────────────────────┼──────────────────────────────────────────────┤
│ 0       │ 'ethereum' │        │ 'collateral' │ 'Owner' │         │ '0xa7ECcdb9Be08178f896c26b7BbD8C3D4E844d9Ba' │ '0x3e0A78A330F2b97059A4D507ca9d8292b65B6FB5' │
└─────────┴────────────┴────────┴──────────────┴─────────┴─────────┴──────────────────────────────────────────────┴──────────────────────────────────────────────┘

Drive-by changes

Related issues

Backward compatibility

Testing

changeset-bot · 2024-07-22T09:51:29Z

⚠️ No Changeset found

Latest commit: 8320017

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Mo-Hussain · 2024-07-22T09:53:12Z

typescript/infra/config/environments/mainnet3/owners.ts

@@ -25,7 +25,7 @@ export function localAccountRouters(): ChainMap<Address> {
 export const safes: ChainMap<Address | undefined> = {
  mantapacific: '0x03ed2D65f2742193CeD99D48EbF1F1D6F12345B6', // does not have a UI
  celo: '0x879038d6Fc9F6D5e2BA73188bd078486d77e1156',
-  ethereum: '0x3965AC3D295641E452E0ea896a086A9cD7C6C5b6',
+  ethereum: '0x0DA0C3e52C977Ed3cBc641fF02DD271c3ED55aFe',


Changed this to a known safe address on ethereum

Mo-Hussain · 2024-07-22T09:54:47Z

typescript/sdk/src/deploy/HyperlaneAppChecker.ts

@@ -90,6 +95,12 @@ export abstract class HyperlaneAppChecker<
        if (await isProxy(provider, contract.address)) {
          // Check the ProxiedContract's admin matches expectation
          const actualAdmin = await proxyAdmin(provider, contract.address);
+          this.logger.debug(


Checking admin addresses for proxied contracts here

Mo-Hussain · 2024-07-22T09:57:19Z

typescript/sdk/src/deploy/HyperlaneAppChecker.ts

    const ownableContracts = await this.ownables(chain);
+    this.logger.debug(`Ownable contracts: ${Object.keys(ownableContracts)}`);
    for (const [name, contract] of Object.entries(ownableContracts)) {


Check if ownable contracts are owned by addresses defined in config

codecov · 2024-07-22T10:12:46Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (7fdd395) to head (8320017).
Report is 1 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #4171   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          1       1           
  Lines         14      14           
=====================================
  Misses        14      14

Components	Coverage Δ
core	`∅ <ø> (∅)`
hooks	`∅ <ø> (∅)`
isms	`∅ <ø> (∅)`
token	`∅ <ø> (∅)`
middlewares	`∅ <ø> (∅)`

…hecked by check-deploy

Mo-Hussain · 2024-07-22T10:48:54Z

typescript/infra/config/environments/mainnet3/owners.ts

@@ -44,7 +44,7 @@ export const safes: ChainMap<Address | undefined> = {
  mode: '0xaCD1865B262C89Fb0b50dcc8fB095330ae8F35b5',
 };

-export const DEPLOYER = '0xa7ECcdb9Be08178f896c26b7BbD8C3D4E844d9Ba';


Changed deployer key to throw violation for warp

Mo-Hussain self-assigned this Jul 22, 2024

Mo-Hussain commented Jul 22, 2024

View reviewed changes

chore(infra): verify that admin addresses for proxied contracts are c…

8320017

…hecked by check-deploy

Mo-Hussain force-pushed the mo/verify-proxy-contracts branch from 1b056a8 to 8320017 Compare July 22, 2024 10:48

Mo-Hussain commented Jul 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(infra): verify that admin addresses for proxied contracts are checked by check-deploy #4171

chore(infra): verify that admin addresses for proxied contracts are checked by check-deploy #4171

Mo-Hussain commented Jul 22, 2024 •

edited

Loading

changeset-bot bot commented Jul 22, 2024 •

edited

Loading

Mo-Hussain Jul 22, 2024

Mo-Hussain Jul 22, 2024

Mo-Hussain Jul 22, 2024

codecov bot commented Jul 22, 2024 •

edited

Loading

Mo-Hussain Jul 22, 2024

chore(infra): verify that admin addresses for proxied contracts are checked by check-deploy #4171

Are you sure you want to change the base?

chore(infra): verify that admin addresses for proxied contracts are checked by check-deploy #4171

Conversation

Mo-Hussain commented Jul 22, 2024 • edited Loading

Description

Core

Warp

Drive-by changes

Related issues

Backward compatibility

Testing

changeset-bot bot commented Jul 22, 2024 • edited Loading

⚠️ No Changeset found

Mo-Hussain Jul 22, 2024

Choose a reason for hiding this comment

Mo-Hussain Jul 22, 2024

Choose a reason for hiding this comment

Mo-Hussain Jul 22, 2024

Choose a reason for hiding this comment

codecov bot commented Jul 22, 2024 • edited Loading

Codecov Report

Mo-Hussain Jul 22, 2024

Choose a reason for hiding this comment

Mo-Hussain commented Jul 22, 2024 •

edited

Loading

changeset-bot bot commented Jul 22, 2024 •

edited

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading