Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix initializing existing mailbox with failing gas estimation #3099

Merged

Conversation

aostrun
Copy link
Contributor

@aostrun aostrun commented Dec 22, 2023

Description

I had an issue with the hyperlane deploy core script and it was caused by trying to initialize the existing mailbox contract. The deploy script already has an error handling for that specific reason, but it doesn't work if the call fails during the dry run when estimating gas. In that case, the returned error looks like this:

{
  "reason": "cannot estimate gas; transaction may fail or may require manual gas limit",
  "code": "UNPREDICTABLE_GAS_LIMIT",
  "error": {
    "reason": "execution reverted: Initializable: contract is already initialized",
    "code": "UNPREDICTABLE_GAS_LIMIT",
    "method": "estimateGas",
    "transaction": {
      "from": "0xD4643110b33D92783A5bF29D41b2d14a2Db69016",
      "gasPrice": {
        "type": "BigNumber",
        "hex": "0x0b165100c3"
      },
      "to": "0xD4C4582cB9B0e63B180fEFefAE78dB486B736299",
      "data": "0xf8c8765e000000000000000000000000d4643110b33d92783a5bf29d41b2d14a2db690160000000000000000000000006ccb17924989f8a7091c685ac8b45d2ff5692282000000000000000000000000ae7a25753380e9c0f9a8887de1caef75d81351fc000000000000000000000000f97c7ab2700da42c5fb86d20ef728be545ce09e9",
      "accessList": null
    },
    "error": {
      "reason": "processing response error",
      "code": "SERVER_ERROR",
      "body": "{\"jsonrpc\":\"2.0\",\"id\":83,\"error\":{\"code\":3,\"message\":\"execution reverted: Initializable: contract is already initialized\",\"data\":\"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002e496e697469616c697a61626c653a20636f6e747261637420697320616c726561647920696e697469616c697a6564000000000000000000000000000000000000\"}}\n",
      "error": {
        "code": 3,
        "data": "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002e496e697469616c697a61626c653a20636f6e747261637420697320616c726561647920696e697469616c697a6564000000000000000000000000000000000000"
      },
      "requestBody": "{\"method\":\"eth_estimateGas\",\"params\":[{\"gasPrice\":\"0xb165100c3\",\"from\":\"0xd4643110b33d92783a5bf29d41b2d14a2db69016\",\"to\":\"0xd4c4582cb9b0e63b180fefefae78db486b736299\",\"data\":\"0xf8c8765e000000000000000000000000d4643110b33d92783a5bf29d41b2d14a2db690160000000000000000000000006ccb17924989f8a7091c685ac8b45d2ff5692282000000000000000000000000ae7a25753380e9c0f9a8887de1caef75d81351fc000000000000000000000000f97c7ab2700da42c5fb86d20ef728be545ce09e9\"}],\"id\":83,\"jsonrpc\":\"2.0\"}",
      "requestMethod": "POST",
      "url": "https://rpc.qtestnet.org"
    }
  },
  "tx": {
    "data": "0xf8c8765e000000000000000000000000d4643110b33d92783a5bf29d41b2d14a2db690160000000000000000000000006ccb17924989f8a7091c685ac8b45d2ff5692282000000000000000000000000ae7a25753380e9c0f9a8887de1caef75d81351fc000000000000000000000000f97c7ab2700da42c5fb86d20ef728be545ce09e9",
    "to": {},
    "from": "0xD4643110b33D92783A5bF29D41b2d14a2Db69016",
    "gasPrice": {
      "type": "BigNumber",
      "hex": "0x0b165100c3"
    },
    "type": 0,
    "nonce": {},
    "gasLimit": {},
    "chainId": {}
  }
}

Here we can see that the real error reason for the error is nested in e.error.reason, but the current error handling for calling initialize on an already initialized mailbox contract only handles situations where the error reason is included in e.message which is not available when the transaction fails during the gas estimation phase.

} catch (e: any) {
if (
!e.message.includes('already initialized') &&
// Some RPC providers dont return the revert reason (nor allow ethers to parse it), so we have to check the message
!e.message.includes('Reverted 0x08c379a')
) {
throw e;
}
this.logger('Mailbox already initialized');

Drive-by changes

Add additional check to handle a situation when the transaction fails during the gas estimation phase:

// Handle situation where the gas estimation fails on the call function,
// then the real error reason is not available in `e.message`, but rather in `e.error.reason`
!e.error?.reason?.includes('already initialized')

Related issues

No related issue, I discovered the issue during my unsuccessful tries of invoking the core deployment script.

Backward compatibility

Yes

Testing

Manual

Copy link

changeset-bot bot commented Dec 22, 2023

⚠️ No Changeset found

Latest commit: 8734170

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

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution

@jmrossy jmrossy merged commit 0e60c22 into hyperlane-xyz:main Dec 22, 2023
9 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants