-
Notifications
You must be signed in to change notification settings - Fork 529
fix: improve cctp v2 offchain lookup service #7334
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
base: main
Are you sure you want to change the base?
Changes from all commits
a50243d
718e33f
343f322
5460f69
b250527
d90eaa9
f7551fd
c349dfb
3a179af
c16fbe0
8b47c30
8868af5
dd9482d
a8252a6
89e3515
5ef9e97
272d808
81a4ad6
87515bc
92872d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { utils } from 'ethers'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AbstractCcipReadIsm__factory } from '@hyperlane-xyz/core'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { WithAddress } from '@hyperlane-xyz/utils'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { WithAddress, ensure0x } from '@hyperlane-xyz/utils'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { HyperlaneCore } from '../../core/HyperlaneCore.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IsmType, OffchainLookupIsmConfig } from '../types.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -58,11 +58,11 @@ export class OffchainLookupMetadataBuilder implements MetadataBuilder { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| const url = urlTemplate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .replace('{sender}', sender) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .replace('{data}', callDataHex); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let res: Response; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let responseJson: any; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (urlTemplate.includes('{data}')) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const res = await fetch(url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseJson = await res.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res = await fetch(url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const signature = await signer.signMessage( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| utils.arrayify( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -73,20 +73,29 @@ export class OffchainLookupMetadataBuilder implements MetadataBuilder { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const res = await fetch(url, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res = await fetch(url, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| method: 'POST', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { 'Content-Type': 'application/json' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body: JSON.stringify({ sender, data: callDataHex, signature }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseJson = await res.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const rawHex = responseJson.data as string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return rawHex.startsWith('0x') ? rawHex : `0x${rawHex}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error: any) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.core.logger.warn( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `CCIP-read metadata fetch failed for ${url}: ${error}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // try next URL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const responseJson = await res.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!res.ok) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.core.logger.warn( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Server at ${url} responded with error: ${responseJson.error}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // try next URL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ensure0x(responseJson.data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+90
to
99
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mind the mud when parsing responses. If a server answers with plain text or malformed JSON, - const responseJson = await res.json();
- if (!res.ok) {
- this.core.logger.warn(
- `Server at ${url} responded with error: ${responseJson.error}`,
- );
- // try next URL
- continue;
- } else {
- return ensure0x(responseJson.data);
- }
+ let responseJson: any;
+ try {
+ responseJson = await res.json();
+ } catch (error) {
+ this.core.logger.warn(
+ `Server at ${url} returned non-JSON payload: ${error}`,
+ );
+ continue;
+ }
+ if (!res.ok) {
+ this.core.logger.warn(
+ `Server at ${url} responded with error: ${responseJson?.error}`,
+ );
+ continue;
+ }
+ return ensure0x(responseJson.data);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle undefined
delayReasonto avoid confusing error messages.Right now, if
delayReasonis undefined (which it can be for normal pending states), you'll get an error message like "CCTP attestation is pending due to undefined". Not exactly helpful when you're trying to figure out what went wrong.Consider this approach:
json.messages.forEach((message) => { if (message.attestation === 'PENDING') { - const errorString = `CCTP attestation is pending due to ${message.delayReason}`; + const errorString = message.delayReason + ? `CCTP attestation is pending due to ${message.delayReason}` + : 'CCTP attestation is still pending'; switch (message.delayReason) { case 'insufficient_fee': case 'amount_above_max': case 'insufficient_allowance_available': PrometheusMetrics.logUnhandledError(this.serviceName); + break; } logger.error(context, errorString); throw new Error(errorString); } });📝 Committable suggestion
🤖 Prompt for AI Agents