Skip to content

Commit

Permalink
Fix base bridging and a race condition in key funder (#3012)
Browse files Browse the repository at this point in the history
### Description

Turns out bridging from L1 to Base has been broken the whole time :(
it's because the version of @eth-optimism/sdk we were using was 1.8.0
(according to yarn.lock) which was released in late 2022
https://github.com/ethereum-optimism/optimism/releases/tag/%40eth-optimism%2Fsdk%401.8.0,
far before Base was launched. So we'd always get this error in key
funder because we rely on the optimism SDK knowing about Base's chain
ID:
```
{"chain":"base","error":"Error: cannot get contract AddressManager for unknown L2 chain ID 8453, you must provide an address\n    at getOEContract (/hyperlane-monorepo/node_modules/@eth-optimism/sdk/src/utils/contracts.ts:58:11)\n    at getAllOEContracts (/hyperlane-monorepo/node_modules/@eth-optimism/sdk/src/utils/contracts.ts:121:46)\n    at new CrossChainMessenger (/hyperlane-monorepo/node_modules/@eth-optimism/sdk/src/cross-chain-messenger.ts:170:39)\n    at ContextFunder.bridgeToOptimism (/hyperlane-monorepo/typescript/infra/scripts/funding/fund-keys-from-deployer.ts:652:33)\n    at ContextFunder.bridgeToL2 (/hyperlane-monorepo/typescript/infra/scripts/funding/fund-keys-from-deployer.ts:637:23)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n    at async ContextFunder.bridgeIfL2 (/hyperlane-monorepo/typescript/infra/scripts/funding/fund-keys-from-deployer.ts:492:9)\n    at async gracefullyHandleError (/hyperlane-monorepo/typescript/infra/scripts/funding/fund-keys-from-deployer.ts:774:5)\n    at async /hyperlane-monorepo/typescript/infra/scripts/funding/fund-keys-from-deployer.ts:394:29\n    at async Promise.all (index 9)","level":"error","message":"Error bridging to L2"}
```

The fix was just to upgrade to a newer optimism SDK version

A mystery to me is that we'd get that log ^ but sometimes the key funder
pod would show as having ran successfully. My best guess here is there
was a race condition in `fund` when we had a variable local to the
function called `failureOccurred` that many promises which were
`Promise.all`'d would read and write to it via `failureOccurred ||=
await someFallibleFn()`. I changed the logic here to not have multiple
concurrent promises contend for the variable

Also made a small change to not include mantapacific in the list of
relayer keys for the Hyperlane context. It's not ever used, and had a
downstream effect of us trying to fund the Kathy key on mantapacific,
which we don't want to actually do

### Drive-by changes

n/a

### Related issues

n/a

### Backward compatibility

ye

### Testing

ran locally
  • Loading branch information
tkporter authored Dec 1, 2023
1 parent 7297c8d commit 9fc0866
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 322 deletions.
15 changes: 9 additions & 6 deletions typescript/infra/config/environments/mainnet3/chains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,17 @@ export const ethereumChainNames = Object.keys(
ethereumMainnetConfigs,
) as MainnetChains[];

// Remove mantapacific, as it's not considered a "blessed"
// chain. It's not included in the scraper domains table,
// and we don't relay to mantapacific on the Hyperlane or RC contexts.
const hyperlaneContextRelayChains = ethereumChainNames.filter(
(chainName) => chainName !== chainMetadata.mantapacific.name,
);

// Hyperlane & RC context agent chain names.
export const agentChainNames: AgentChainNames = {
// Run validators for all chains.
[Role.Validator]: supportedChainNames,
// Only run relayers for Ethereum chains at the moment.
[Role.Relayer]: ethereumChainNames,
// Remove mantapacific for now, as it's not included in the scraper domains table
[Role.Scraper]: ethereumChainNames.filter(
(chainName) => chainName !== chainMetadata.mantapacific.name,
),
[Role.Relayer]: hyperlaneContextRelayChains,
[Role.Scraper]: hyperlaneContextRelayChains,
};
2 changes: 1 addition & 1 deletion typescript/infra/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"@aws-sdk/client-kms": "3.48.0",
"@aws-sdk/client-s3": "^3.74.0",
"@cosmjs/amino": "^0.31.3",
"@eth-optimism/sdk": "^1.7.0",
"@eth-optimism/sdk": "^3.1.6",
"@ethersproject/experimental": "^5.7.0",
"@ethersproject/hardware-wallets": "^5.7.0",
"@ethersproject/providers": "^5.7.2",
Expand Down
29 changes: 20 additions & 9 deletions typescript/infra/scripts/funding/fund-keys-from-deployer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,10 @@ class ContextFunder {
// Funds all the roles in this.rolesToFund
// Returns whether a failure occurred.
async fund(): Promise<boolean> {
let failureOccurred = false;

const chainKeys = this.getChainKeys();
const promises = Object.entries(chainKeys).map(async ([chain, keys]) => {
const chainKeyEntries = Object.entries(chainKeys);
const promises = chainKeyEntries.map(async ([chain, keys]) => {
let failureOccurred = false;
if (keys.length > 0) {
if (!this.skipIgpClaim) {
failureOccurred ||= await gracefullyHandleError(
Expand All @@ -418,14 +418,25 @@ class ContextFunder {
const failure = await this.attemptToFundKey(key, chain);
failureOccurred ||= failure;
}
return failureOccurred;
});

try {
await Promise.all(promises);
} catch (e) {
error('Unhandled error when funding key', { error: format(e) });
failureOccurred = true;
}
// A failure occurred if any of the promises rejected or
// if any of them resolved with true, indicating a failure
// somewhere along the way
const failureOccurred = (await Promise.allSettled(promises)).reduce(
(failureAgg, result, i) => {
if (result.status === 'rejected') {
error('Funding promise for chain rejected', {
chain: chainKeyEntries[i][0],
error: format(result.reason),
});
return true;
}
return result.value || failureAgg;
},
false,
);

return failureOccurred;
}
Expand Down
Loading

0 comments on commit 9fc0866

Please sign in to comment.