Conversation
WalkthroughThis PR updates relay URL construction patterns, migrates test networks from Goerli to Sepolia, introduces slug-based chain resolution in bridge helpers, adds status tracking functionality via a new getStatus function, and extends the NativeTokens enum with a "plasma" member. Includes formatting consistency improvements across admin scripts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/native-bridge-helpers/optimism/l1Tol2Relay.ts (1)
18-19:⚠️ Potential issue | 🟡 MinorUpdate to Sepolia to match other Optimism bridge helpers.
Lines 18-19 use deprecated Goerli networks while
l2tol1Relay.tsandop-stack-native-withdrawals.tsin the same directory have migrated to Sepolia. UpdateHardhatChainName.GOERLItoHardhatChainName.SEPOLIAandHardhatChainName.OPTIMISM_GOERLItoHardhatChainName.OPTIMISM_SEPOLIA, or add a comment explaining why this script requires Goerli.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/native-bridge-helpers/optimism/l1Tol2Relay.ts` around lines 18 - 19, Update the network constants used by this relay: replace HardhatChainName.GOERLI with HardhatChainName.SEPOLIA for localChain and replace HardhatChainName.OPTIMISM_GOERLI with HardhatChainName.OPTIMISM_SEPOLIA for remoteChain in the l1Tol2Relay.ts module (identifiers: localChain, remoteChain, HardhatChainName). If you intentionally need Goerli, instead add a clear comment above these declarations explaining why Goerli is required and note the discrepancy with the other Optimism bridge helpers.scripts/native-bridge-helpers/optimism/l2tol1Relay.ts (1)
15-29:⚠️ Potential issue | 🟠 MajorChainId is the wrong key for
hardhatChainNameToSlug.Line 15-29 indexes
hardhatChainNameToSlugwithChainId(numeric). The mapping is keyed byHardhatChainName, so this resolves toundefinedandgetJsonRpcUrlwill throw at runtime. This should use HardhatChainName/ChainSlug for RPC resolution and keep ChainId only for the messenger.🛠️ Suggested fix
-const l1ChainId = ChainId.SEPOLIA; -const l2ChainId = ChainId.OPTIMISM_SEPOLIA; +const l1ChainId = ChainId.SEPOLIA; +const l2ChainId = ChainId.OPTIMISM_SEPOLIA; +const l1ChainName = HardhatChainName.SEPOLIA; +const l2ChainName = HardhatChainName.OPTIMISM_SEPOLIA; @@ -const l1Provider = new providers.JsonRpcProvider( - getJsonRpcUrl(hardhatChainNameToSlug[l1ChainId]) -); +const l1Provider = new providers.JsonRpcProvider( + getJsonRpcUrl(hardhatChainNameToSlug[l1ChainName]) +); @@ - l2SignerOrProvider: new providers.JsonRpcProvider( - getJsonRpcUrl(hardhatChainNameToSlug[l2ChainId]) - ), + l2SignerOrProvider: new providers.JsonRpcProvider( + getJsonRpcUrl(hardhatChainNameToSlug[l2ChainName]) + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/native-bridge-helpers/optimism/l2tol1Relay.ts` around lines 15 - 29, The code incorrectly indexes hardhatChainNameToSlug with numeric l1ChainId/l2ChainId (causing undefined RPC URLs); replace those lookups with the corresponding HardhatChainName/chain-slug keys (e.g., compute or use existing variables like l1ChainSlug and l2ChainSlug) when calling getJsonRpcUrl for l1Provider and the l2SignerOrProvider, while keeping numeric l1ChainId and l2ChainId passed to CrossChainMessenger; update the references to hardhatChainNameToSlug in l1Provider and the l2SignerOrProvider creation so they use the slug/name keys instead of the numeric chain IDs.scripts/deploy/utils/relayer.ts (1)
51-63:⚠️ Potential issue | 🟠 Major
getStatusmust return a string consistently; handle undefinedtxIdand encode the URL parameter.Line 51-63 returns objects (
response?.dataor{ hash: "" }) instead of strings, and doesn't handleundefinedtxId. When callers stringify the result, objects become[object Object], andundefinedtxId produces/status?txId=undefined. The catch block also returnsundefinedinstead of a fallback value.The function is called at line 140 as
const txHash = await getStatus(response?.txId), whereresponse?.txIdcan be undefined. The result is used in a URL template, so it must always be a string or empty string.Changes needed:
- Accept optional
txIdand return""if missing- Consistently return strings (extract
txHashorhashfrom response data, not the object itself)- Return
""on error or exception, neverundefined- Encode the txId in the URL to prevent injection
Suggested fix
-export const getStatus = async (txId: string) => { +export const getStatus = async (txId?: string) => { try { + if (!txId) return ""; const response = await axiosGet( - `${await getRelayUrl(mode)}/status?txId=${txId}` + `${await getRelayUrl(mode)}/status?txId=${encodeURIComponent(txId)}` ); - if (response?.success) return response?.data; + if (response?.success) { + const data = response?.data; + return (typeof data === "string" ? data : data?.txHash || data?.hash) || ""; + } else { console.log("error in relaying tx", response); - return { hash: "" }; + return ""; } } catch (error) { console.log("uncaught error", error); + return ""; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy/utils/relayer.ts` around lines 51 - 63, Update getStatus to accept an optional txId and always return a string: if txId is falsy return ""; encode txId with encodeURIComponent when building the URL (use getRelayUrl and axiosGet as currently referenced); when parsing the response extract the hash string (e.g., response.data.hash or response.data.txHash) and return that string, not the whole object; on non-success or in the catch block return an empty string "" so callers never receive undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/deploy/helpers/send-msg/utils.ts`:
- Around line 132-148: The code calls getStatus(response?.txId) and builds
tracking URLs even when response or response.txId is missing; update the flow in
this function to check for a valid response.txId (reference response and txId)
immediately after receiving the relay response and before calling getStatus or
printing tracking info, bail out or log an error/return if txId is falsy, and
only then call getStatus(response.txId) and use getAPIBaseURL(mode) with
chainSlug to print the tracking URL; ensure no further status lookup or URL
construction occurs when txId is undefined.
In `@scripts/deploy/utils/relayer.ts`:
- Around line 36-48: relayTx and getStatus currently swallow errors and return
inconsistent shapes (e.g., { hash: "" } or undefined), causing callers that
expect response.txId (e.g., getStatus(response?.txId)) to operate on invalid
values; update relayTx and getStatus to either (a) throw a descriptive error on
API failure/exception (include response error details) so callers can bail out,
or (b) return a consistent nullable result type (e.g., null or an object with an
explicit error field) instead of { hash: "" } and ensure callers check for
null/error before calling getStatus; locate and change implementations of
relayTx and getStatus to follow one consistent approach and bubble up or return
explicit errors.
In `@scripts/native-bridge-helpers/optimism/op-stack-native-withdrawals.ts`:
- Line 6: The issue: numeric ChainId enum values (e.g., ChainId.SEPOLIA) are
being passed to hardhatChainNameToSlug which expects HardhatChainName string
keys, causing undefined lookups for hardhatChainNameToSlug[l1Chain] and
hardhatChainNameToSlug[l2Chain]. Fix by changing the l1Chain/l2Chain variables
(the ones used for hardhatChainNameToSlug and getJsonRpcUrl calls) to use the
HardhatChainName enum/string values instead of ChainId; if you still need
numeric ChainId for CrossChainMessenger (used later in CrossChainMessenger
construction), create separate variables (e.g., l1ChainId/l2ChainId) typed as
ChainId and pass those to CrossChainMessenger while using the HardhatChainName
variables for RPC lookup.
In `@scripts/native-bridge-helpers/polygon/l2tol1Relay.ts`:
- Around line 93-100: The code instantiates SocketRelaySigner for l1Signer and
l2Signer using process.env.RELAYER_URL_DEV with a non-null assertion; remove the
`!` usage and validate the env var first (e.g., read into a local const
relayerUrl = process.env.RELAYER_URL_DEV), check that relayerUrl is defined and
non-empty, and if not throw or exit with a clear error message indicating
RELAYER_URL_DEV is missing; then construct the relay endpoint as
`${relayerUrl}/relay` when creating SocketRelaySigner for l1Signer and l2Signer.
---
Outside diff comments:
In `@scripts/deploy/utils/relayer.ts`:
- Around line 51-63: Update getStatus to accept an optional txId and always
return a string: if txId is falsy return ""; encode txId with encodeURIComponent
when building the URL (use getRelayUrl and axiosGet as currently referenced);
when parsing the response extract the hash string (e.g., response.data.hash or
response.data.txHash) and return that string, not the whole object; on
non-success or in the catch block return an empty string "" so callers never
receive undefined.
In `@scripts/native-bridge-helpers/optimism/l1Tol2Relay.ts`:
- Around line 18-19: Update the network constants used by this relay: replace
HardhatChainName.GOERLI with HardhatChainName.SEPOLIA for localChain and replace
HardhatChainName.OPTIMISM_GOERLI with HardhatChainName.OPTIMISM_SEPOLIA for
remoteChain in the l1Tol2Relay.ts module (identifiers: localChain, remoteChain,
HardhatChainName). If you intentionally need Goerli, instead add a clear comment
above these declarations explaining why Goerli is required and note the
discrepancy with the other Optimism bridge helpers.
In `@scripts/native-bridge-helpers/optimism/l2tol1Relay.ts`:
- Around line 15-29: The code incorrectly indexes hardhatChainNameToSlug with
numeric l1ChainId/l2ChainId (causing undefined RPC URLs); replace those lookups
with the corresponding HardhatChainName/chain-slug keys (e.g., compute or use
existing variables like l1ChainSlug and l2ChainSlug) when calling getJsonRpcUrl
for l1Provider and the l2SignerOrProvider, while keeping numeric l1ChainId and
l2ChainId passed to CrossChainMessenger; update the references to
hardhatChainNameToSlug in l1Provider and the l2SignerOrProvider creation so they
use the slug/name keys instead of the numeric chain IDs.
| value: value?.toString(), | ||
| gasLimit: gasLimit?.toString(), | ||
| gasPrice: gasPrice?.toString(), | ||
| type, | ||
| chainSlug, | ||
| }); | ||
| console.log( | ||
| `Track message here: ${getAPIBaseURL( | ||
| mode | ||
| )}/messages-from-tx?srcChainSlug=${chainSlug}&srcTxHash=${response?.hash}` | ||
| ); | ||
|
|
||
| console.log(`Tx Id: ${response?.txId}`); | ||
| const txHash = await getStatus(response?.txId); | ||
|
|
||
| if (txHash) { | ||
| console.log( | ||
| `Track message here: ${getAPIBaseURL( | ||
| mode | ||
| )}/messages-from-tx?srcChainSlug=${chainSlug}&srcTxHash=${txHash}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Guard against missing txId before status lookup.
Line 139-148 calls getStatus(response?.txId) even when the relay response failed or is undefined, producing /status?txId=undefined and misleading tracking logs. This should bail out early if txId is missing.
As per coding guidelines, Check for resource leaks, race conditions, and unhandled edge cases.
🛠️ Suggested fix
- console.log(`Tx Id: ${response?.txId}`);
- const txHash = await getStatus(response?.txId);
+ if (!response?.txId) {
+ console.log("Relay did not return txId; skipping status lookup");
+ return;
+ }
+ console.log(`Tx Id: ${response.txId}`);
+ const txHash = await getStatus(response.txId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| value: value?.toString(), | |
| gasLimit: gasLimit?.toString(), | |
| gasPrice: gasPrice?.toString(), | |
| type, | |
| chainSlug, | |
| }); | |
| console.log( | |
| `Track message here: ${getAPIBaseURL( | |
| mode | |
| )}/messages-from-tx?srcChainSlug=${chainSlug}&srcTxHash=${response?.hash}` | |
| ); | |
| console.log(`Tx Id: ${response?.txId}`); | |
| const txHash = await getStatus(response?.txId); | |
| if (txHash) { | |
| console.log( | |
| `Track message here: ${getAPIBaseURL( | |
| mode | |
| )}/messages-from-tx?srcChainSlug=${chainSlug}&srcTxHash=${txHash}` | |
| ); | |
| } | |
| value: value?.toString(), | |
| gasLimit: gasLimit?.toString(), | |
| gasPrice: gasPrice?.toString(), | |
| type, | |
| chainSlug, | |
| }); | |
| if (!response?.txId) { | |
| console.log("Relay did not return txId; skipping status lookup"); | |
| return; | |
| } | |
| console.log(`Tx Id: ${response.txId}`); | |
| const txHash = await getStatus(response.txId); | |
| if (txHash) { | |
| console.log( | |
| `Track message here: ${getAPIBaseURL( | |
| mode | |
| )}/messages-from-tx?srcChainSlug=${chainSlug}&srcTxHash=${txHash}` | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/deploy/helpers/send-msg/utils.ts` around lines 132 - 148, The code
calls getStatus(response?.txId) and builds tracking URLs even when response or
response.txId is missing; update the flow in this function to check for a valid
response.txId (reference response and txId) immediately after receiving the
relay response and before calling getStatus or printing tracking info, bail out
or log an error/return if txId is falsy, and only then call
getStatus(response.txId) and use getAPIBaseURL(mode) with chainSlug to print the
tracking URL; ensure no further status lookup or URL construction occurs when
txId is undefined.
| let response = await axiosPost( | ||
| `${await getRelayUrl(mode)}/relay`, | ||
| body, | ||
| config | ||
| ); | ||
| if (response?.success) return response?.data; | ||
| else { | ||
| console.log("error in relaying tx", response); | ||
| return { hash: "" }; | ||
| } | ||
| } catch (error) { | ||
| console.log("uncaught error", error); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and display the relayer.ts file
find . -name "relayer.ts" -type f | head -20Repository: SocketDotTech/socket-DL
Length of output: 100
🏁 Script executed:
# Once found, examine the full file
cat -n scripts/deploy/utils/relayer.tsRepository: SocketDotTech/socket-DL
Length of output: 2078
🏁 Script executed:
# Search for all usages of relayTx and getStatus to understand what callers expect
rg -n "relayTx|getStatus" --type ts --type js -B 2 -A 2 scripts srcRepository: SocketDotTech/socket-DL
Length of output: 2762
🏁 Script executed:
# Also check for txId or txHash usage near relayTx calls
rg -n "txId|txHash|hash" --type ts --type js -B 3 -A 3 | grep -A 5 -B 5 "relayTx\|getStatus"Repository: SocketDotTech/socket-DL
Length of output: 1391
Both relayTx and getStatus swallow errors and return inconsistent shapes, breaking downstream error handling.
Lines 36-48 and 51-63 return response?.data on success or { hash: "" } on API failure, or implicitly undefined on exceptions. Callers at line 139-140 expect response?.txId, but on failure get an empty hash object or undefined. When relayTx fails, line 140 calls getStatus(response?.txId) with undefined, which silently produces /status?txId=undefined.
This violates the coding guideline to check for edge cases and unhandled errors. Either throw errors so callers can bail out, or return a consistent nullable type with explicit error signals.
🛠️ Suggested fix
-export const relayTx = async (params: RequestObj) => {
+export const relayTx = async (
+ params: RequestObj
+): Promise<{ txId?: string; hash?: string } | null> => {
try {
@@
- if (response?.success) return response?.data;
- else {
- console.log("error in relaying tx", response);
- return { hash: "" };
- }
+ if (!response?.success) {
+ console.log("error in relaying tx", response);
+ return null;
+ }
+ return response.data;
} catch (error) {
console.log("uncaught error", error);
+ throw error;
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/deploy/utils/relayer.ts` around lines 36 - 48, relayTx and getStatus
currently swallow errors and return inconsistent shapes (e.g., { hash: "" } or
undefined), causing callers that expect response.txId (e.g.,
getStatus(response?.txId)) to operate on invalid values; update relayTx and
getStatus to either (a) throw a descriptive error on API failure/exception
(include response error details) so callers can bail out, or (b) return a
consistent nullable result type (e.g., null or an object with an explicit error
field) instead of { hash: "" } and ensure callers check for null/error before
calling getStatus; locate and change implementations of relayTx and getStatus to
follow one consistent approach and bubble up or return explicit errors.
| import { CrossChainMessenger, MessageStatus } from "@eth-optimism/sdk"; | ||
| import { getJsonRpcUrl } from "../../constants"; | ||
| import { ChainId } from "../../../src"; | ||
| import { ChainId, hardhatChainNameToSlug } from "../../../src"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the file in question
cat -n scripts/native-bridge-helpers/optimism/op-stack-native-withdrawals.ts | head -50Repository: SocketDotTech/socket-DL
Length of output: 2353
🏁 Script executed:
# Find and examine the enum definitions
echo "=== Looking for ChainId enum ==="
rg "enum ChainId" src/ -A 15 | head -30
echo "=== Looking for HardhatChainName enum ==="
rg "enum HardhatChainName|type HardhatChainName|const HardhatChainName" src/ -A 15 | head -30Repository: SocketDotTech/socket-DL
Length of output: 1762
🏁 Script executed:
# Find the hardhatChainNameToSlug definition
echo "=== Looking for hardhatChainNameToSlug ==="
rg "hardhatChainNameToSlug" src/ -B 2 -A 10 | head -50Repository: SocketDotTech/socket-DL
Length of output: 1704
🏁 Script executed:
# Check for any implicit conversions or mappings
echo "=== Checking for ChainId to HardhatChainName mappings ==="
rg "chainId.*hardhat|HardhatChainName.*ChainId" src/ -i | head -20
echo "=== Check getJsonRpcUrl signature ==="
rg "getJsonRpcUrl" src/ -B 2 -A 5 | head -30Repository: SocketDotTech/socket-DL
Length of output: 281
🏁 Script executed:
# Verify ChainId values are indeed numeric
cat src/enums/chainId.tsRepository: SocketDotTech/socket-DL
Length of output: 1632
Type mismatch: ChainId enum values (numeric) passed to hardhatChainNameToSlug which expects HardhatChainName enum values (strings).
ChainId.SEPOLIA is 11155111 but hardhatChainNameToSlug is keyed by HardhatChainName.SEPOLIA which is "sepolia". The lookups on lines 24 and 27 (hardhatChainNameToSlug[l1Chain] and hardhatChainNameToSlug[l2Chain]) will return undefined, causing getJsonRpcUrl to fail.
Change lines 15-16 to use HardhatChainName instead of ChainId:
-const l1Chain = ChainId.SEPOLIA;
-const l2Chain = ChainId.LYRA_TESTNET;
+const l1Chain = HardhatChainName.SEPOLIA;
+const l2Chain = HardhatChainName.LYRA_TESTNET;If ChainId values are needed elsewhere (e.g., for CrossChainMessenger on lines 47-48), use separate variables with ChainId and pass the appropriate one to each constructor parameter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/native-bridge-helpers/optimism/op-stack-native-withdrawals.ts` at
line 6, The issue: numeric ChainId enum values (e.g., ChainId.SEPOLIA) are being
passed to hardhatChainNameToSlug which expects HardhatChainName string keys,
causing undefined lookups for hardhatChainNameToSlug[l1Chain] and
hardhatChainNameToSlug[l2Chain]. Fix by changing the l1Chain/l2Chain variables
(the ones used for hardhatChainNameToSlug and getJsonRpcUrl calls) to use the
HardhatChainName enum/string values instead of ChainId; if you still need
numeric ChainId for CrossChainMessenger (used later in CrossChainMessenger
construction), create separate variables (e.g., l1ChainId/l2ChainId) typed as
ChainId and pass those to CrossChainMessenger while using the HardhatChainName
variables for RPC lookup.
| const l1Signer = new SocketRelaySigner( | ||
| l1Provider, | ||
| process.env.RELAYER_URL_DEV! | ||
| `${process.env.RELAYER_URL_DEV!}/relay` | ||
| ); | ||
| const l2Signer = new SocketRelaySigner( | ||
| l2Provider, | ||
| process.env.RELAYER_URL_DEV! | ||
| `${process.env.RELAYER_URL_DEV!}/relay` | ||
| ); |
There was a problem hiding this comment.
Missing validation for RELAYER_URL_DEV environment variable.
The non-null assertion (!) hides potential undefined values. If RELAYER_URL_DEV is not set, the URL becomes "undefined/relay", causing confusing runtime errors from axios.
Proposed fix
+const relayerUrl = process.env.RELAYER_URL_DEV;
+if (!relayerUrl) {
+ throw new Error("RELAYER_URL_DEV environment variable is required");
+}
+
const l1Signer = new SocketRelaySigner(
l1Provider,
- `${process.env.RELAYER_URL_DEV!}/relay`
+ `${relayerUrl}/relay`
);
const l2Signer = new SocketRelaySigner(
l2Provider,
- `${process.env.RELAYER_URL_DEV!}/relay`
+ `${relayerUrl}/relay`
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const l1Signer = new SocketRelaySigner( | |
| l1Provider, | |
| process.env.RELAYER_URL_DEV! | |
| `${process.env.RELAYER_URL_DEV!}/relay` | |
| ); | |
| const l2Signer = new SocketRelaySigner( | |
| l2Provider, | |
| process.env.RELAYER_URL_DEV! | |
| `${process.env.RELAYER_URL_DEV!}/relay` | |
| ); | |
| const relayerUrl = process.env.RELAYER_URL_DEV; | |
| if (!relayerUrl) { | |
| throw new Error("RELAYER_URL_DEV environment variable is required"); | |
| } | |
| const l1Signer = new SocketRelaySigner( | |
| l1Provider, | |
| `${relayerUrl}/relay` | |
| ); | |
| const l2Signer = new SocketRelaySigner( | |
| l2Provider, | |
| `${relayerUrl}/relay` | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/native-bridge-helpers/polygon/l2tol1Relay.ts` around lines 93 - 100,
The code instantiates SocketRelaySigner for l1Signer and l2Signer using
process.env.RELAYER_URL_DEV with a non-null assertion; remove the `!` usage and
validate the env var first (e.g., read into a local const relayerUrl =
process.env.RELAYER_URL_DEV), check that relayerUrl is defined and non-empty,
and if not throw or exit with a clear error message indicating RELAYER_URL_DEV
is missing; then construct the relay endpoint as `${relayerUrl}/relay` when
creating SocketRelaySigner for l1Signer and l2Signer.
1 participant
Summary by CodeRabbit
New Features
Chores
Style