hyperlane-xyz / hyperlane-monorepo

The home for Hyperlane core contracts, sdk packages, and other infrastructure
https://hyperlane.xyz
Other
312 stars 342 forks source link

fix(sdk): mark contracts as proxies #4123

Closed paulbalaji closed 2 months ago

paulbalaji commented 2 months ago

Problem

Sometimes when deploying and verifying contracts, we are able to verify both the proxy contract and implementation contract but miss the step of marking the proxy as a proxy.

Solution

Look into why we fail to mark some contracts as proxy, and also make it more fault tolerant of explorer API issues

a good starting point to investigate: https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/c5ab4425a7699f1e112f5164368f94c91bbdc7eb/typescript/sdk/src/deploy/verify/ContractVerifier.ts#L198-L204

avious00 commented 2 months ago

@nbayindirli does this also cover general verification messages https://www.notion.so/hyperlanexyz/deployed-contracts-are-not-verified-193673098724483797a54381029916fc

nbayindirli commented 2 months ago

although not confirmed, my hunch here is that we're not explicitly setting the expectedimplementation field on our verifyproxycontract request

nbayindirli commented 2 months ago

example output from proxy request on BSC:

📝 Verifying proxy at 0x170FAfC9958276a49486B27fAD3Ca7aEdBcedbc3...
url: URL {
  href: 'https://api.bscscan.com/api',
  origin: 'https://api.bscscan.com',
  protocol: 'https:',
  username: '',
  password: '',
  host: 'api.bscscan.com',
  hostname: 'api.bscscan.com',
  port: '',
  pathname: '/api',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}
init: {
  method: 'POST',
  headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
  body: URLSearchParams {
    'module' => 'contract',
    'action' => 'verifyproxycontract',
    'address' => '0x170FAfC9958276a49486B27fAD3Ca7aEdBcedbc3',
    'apikey' => 'ZZJIDRMAKBWG2H4E6E1UJN6YQR2F993ARZ' }
}
response: Response {
  size: 0,
  timeout: 0,
  [Symbol(Body internals)]: {
    body: Gunzip {
      _writeState: [Uint32Array],
      _events: [Object],
      _readableState: [ReadableState],
      _writableState: [WritableState],
      allowHalfOpen: true,
      _maxListeners: undefined,
      _eventsCount: 5,
      bytesWritten: 0,
      _handle: [Zlib],
      _outBuffer: <Buffer 7b 22 73 74 61 74 75 73 22 3a 22 31 22 2c 22 6d 65 73 73 61 67 65 22 3a 22 4f 4b 22 2c 22 72 65 73 75 6c 74 22 3a 22 71 74 7a 72 77 35 7a 70 37 34 62 ... 16334 more bytes>,
      _outOffset: 0,
      _chunkSize: 16384,
      _defaultFlushFlag: 2,
      _finishFlushFlag: 2,
      _defaultFullFlushFlag: 3,
      _info: undefined,
      _maxOutputLength: 9007199254740991,
      _level: -1,
      _strategy: 0,
      [Symbol(shapeMode)]: true,
      [Symbol(kCapture)]: false,
      [Symbol(kCallback)]: null,
      [Symbol(kError)]: null
    },
    disturbed: false,
    error: null
  },
  [Symbol(Response internals)]: {
    url: 'https://api.bscscan.com/api',
    status: 200,
    statusText: 'OK',
    headers: Headers { [Symbol(map)]: [Object: null prototype] },
    counter: 0
  }
}
nbayindirli commented 2 months ago

Root Cause

for context: verified can be an 'exact match' (includes constructor args) or 'similar match' (bytecode match but no constructor args) a proxy can be verified without being marked as a proxy. if not marked, "Read as Proxy" and "Write as Proxy" will not be present on the scanner UI to mark a proxy, the implementation must already be verified as an exact match (same goes for UI)

root cause: in our verifier, we always check to see if the impl isAlreadyVerified(). when there is a similar match, the response is identical to an exact match. as a result, we cannot mark the proxy of any impl with a similar match (if we check isAlreadyVerified)

examples:

fix: we can remove the isAlreadyVerified check, but this isn't great for deployment runtime. we can try to run the check for impls w/o proxy parents, although this is just a subset of the same issue

bc we do not anticipate many identical deployments to occur on the same network, I am alright with removing the check altogether.

nbayindirli commented 2 months ago

fixed by https://github.com/hyperlane-xyz/hyperlane-monorepo/pull/4165