Feature: Bridge GoodDollar using OFT adapter via LayerZero Endpoint v2 #289
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- In
set-minter-burner-limits.ts, the env var handling is brittle:parseUnitswill throw when a limit is unset/empty, and later checks likeif (!newWeeklyMintLimit ...)and!== nullon BigNumbers don’t behave as intended; add explicitif (process.env.X != null)guards and compare BigNumbers with.eq()instead of truthy/falsy or null checks. - In
bridge-oft-token.ts, the destination network is hard-coded to the development networks (development-xdc/development-celo) even when running on production networks, which contradicts the comments and makes production bridging fragile; consider deriving the dest network name from the current network (prod vs dev) or making it configurable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `set-minter-burner-limits.ts`, the env var handling is brittle: `parseUnits` will throw when a limit is unset/empty, and later checks like `if (!newWeeklyMintLimit ...)` and `!== null` on BigNumbers don’t behave as intended; add explicit `if (process.env.X != null)` guards and compare BigNumbers with `.eq()` instead of truthy/falsy or null checks.
- In `bridge-oft-token.ts`, the destination network is hard-coded to the development networks (`development-xdc`/`development-celo`) even when running on production networks, which contradicts the comments and makes production bridging fragile; consider deriving the dest network name from the current network (prod vs dev) or making it configurable.
## Individual Comments
### Comment 1
<location> `scripts/multichain-deploy/oft/bridge-oft-token.ts:145-151` </location>
<code_context>
+ console.log(`\nRecipient on ${destNetwork}:`, recipient);
+
+ // Get destination network OFT adapter address
+ let destNetworkName: string;
+ if (isXDC) {
+ // Bridging to CELO - try production-celo first, then development-celo
+ destNetworkName = "development-celo";
+ } else {
+ // Bridging to XDC - try production-xdc first, then development-xdc
+ destNetworkName = "development-xdc";
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Destination network selection ignores production networks and contradicts the usage docs.
Hardcoding `development-celo` / `development-xdc` as the destination means `dao[destNetworkName]` will be wrong when running on `production-*` networks, so bridging from production will fail. Instead, derive `destNetworkName` from `networkName` (e.g., swapping `production-xdc` ↔ `production-celo` and `development-xdc` ↔ `development-celo`) and only fall back to the other environment if the primary lookup is missing.
</issue_to_address>
### Comment 2
<location> `scripts/multichain-deploy/oft/oft-deploy.ts:58-59` </location>
<code_context>
+ }
+ const NameService = await ethers.getContractAt("NameService", nameServiceAddress);
+
+ // Get owner - use Avatar if available, otherwise use deployer
+ const owner = root.address;
+
+ console.log("settings:", settings);
</code_context>
<issue_to_address>
**issue:** Adapter ownership is set to the deployer while logs/documentation imply it is the DAO avatar.
The adapter is deployed with `owner = root.address`, but later logs/descriptions treat the avatar as the owner (e.g. `"Owner set to DAO avatar:"`, `"OFT Adapter Owner (Avatar):"` using `avatarAddress`). This mismatch forces an extra `transfer-oft-adapter-ownership` step. Either construct the adapter with `avatarAddress` as `owner`, or keep the current flow and update logs/descriptions to state that the deployer is the initial owner and the avatar only becomes owner after running the transfer script.
</issue_to_address>
### Comment 3
<location> `scripts/multichain-deploy/oft/set-minter-burner-limits.ts:67-70` </location>
<code_context>
+
+ // Get new limits from environment variables or command line args
+ // Format: values should be in G$ (will be converted to wei)
+ const weeklyMintLimitStr = process.env.WEEKLY_MINT_LIMIT;
+ const monthlyMintLimitStr = process.env.MONTHLY_MINT_LIMIT;
+ const weeklyBurnLimitStr = process.env.WEEKLY_BURN_LIMIT;
+ const monthlyBurnLimitStr = process.env.MONTHLY_BURN_LIMIT;
+
+ const newWeeklyMintLimit = ethers.utils.parseUnits(weeklyMintLimitStr, 18);
</code_context>
<issue_to_address>
**issue:** Environment variables are assumed to be present; undefined values will cause parseUnits to throw.
Because `parseUnits` is called immediately on each env var, any unset value (e.g., `undefined`) will throw before the later "no limits specified" logic runs. To align behavior with the help text, gate `parseUnits` behind a defined/non-empty check and treat missing values as `null` / "no change" instead of letting it throw.
</issue_to_address>
### Comment 4
<location> `scripts/multichain-deploy/oft/set-minter-burner-limits.ts:72-75` </location>
<code_context>
+ const weeklyBurnLimitStr = process.env.WEEKLY_BURN_LIMIT;
+ const monthlyBurnLimitStr = process.env.MONTHLY_BURN_LIMIT;
+
+ const newWeeklyMintLimit = ethers.utils.parseUnits(weeklyMintLimitStr, 18);
+ const newMonthlyMintLimit = ethers.utils.parseUnits(monthlyMintLimitStr, 18);
+ const newWeeklyBurnLimit = ethers.utils.parseUnits(weeklyBurnLimitStr, 18);
+ const newMonthlyBurnLimit = ethers.utils.parseUnits(monthlyBurnLimitStr, 18);
+
+ // Check if any limits are being set
</code_context>
<issue_to_address>
**issue (bug_risk):** Boolean checks on BigNumber limits don’t behave as intended and prevent the "no limits specified" path.
Because `parseUnits` always returns a truthy `BigNumber` and never `null`, the later checks like `if (!newWeeklyMintLimit && !newMonthlyMintLimit && …)` and `if (newWeeklyMintLimit !== null)` will never behave as intended—the "no limits specified" path can’t be reached and all four limits are treated as set. To support unset vs set (and disabling with 0), keep the values as `null`/`undefined` until you know the env var is present, and only then call `parseUnits`, and use explicit `== null` checks instead of truthiness on `BigNumber`s.
</issue_to_address>
### Comment 5
<location> `scripts/multichain-deploy/oft/set-minter-burner-limits.ts:65-70` </location>
<code_context>
+ console.log("Weekly Burn Limit:", ethers.utils.formatEther(currentWeeklyBurnLimit), "G$");
+ console.log("Monthly Burn Limit:", ethers.utils.formatEther(currentMonthlyBurnLimit), "G$");
+
+ // Get new limits from environment variables or command line args
+ // Format: values should be in G$ (will be converted to wei)
+ const weeklyMintLimitStr = process.env.WEEKLY_MINT_LIMIT;
+ const monthlyMintLimitStr = process.env.MONTHLY_MINT_LIMIT;
+ const weeklyBurnLimitStr = process.env.WEEKLY_BURN_LIMIT;
+ const monthlyBurnLimitStr = process.env.MONTHLY_BURN_LIMIT;
+
+ const newWeeklyMintLimit = ethers.utils.parseUnits(weeklyMintLimitStr, 18);
</code_context>
<issue_to_address>
**nitpick:** The script mentions command-line args and the ability to disable limits, but only environment variables are actually supported.
The usage comments describe `--weekly-mint=...` flags and say “set to 0 or 'disable'” to turn off a limit, but the code only reads env vars and never parses CLI args or a `'disable'` value. This mismatch is likely to confuse operators. Please either add CLI and special-value handling to match the comments, or update the comments to reflect the current env-var-only behavior and supported value formats.
</issue_to_address>
### Comment 6
<location> `scripts/multichain-deploy/oft/grant-minter-role.ts:22` </location>
<code_context>
+ console.log("=== Grant MINTER_ROLE to GoodDollarMinterBurner ===");
+ console.log("Network:", networkName);
+ console.log("Signer:", signer.address);
+ console.log("Signer balance:", ethers.utils.formatEther(await ethers.provider.getBalance(signer.address)), "CELO");
+
+ // Get deployment info
</code_context>
<issue_to_address>
**nitpick (bug_risk):** Hardcoded "CELO" symbol in the balance log is inaccurate for non-CELO networks.
Since this script runs on both XDC and CELO, the log label should not always say "CELO". Please derive the symbol from the current network (or use a generic label like "native token") so the printed balance matches the actual network currency.
</issue_to_address>
### Comment 7
<location> `contracts/token/oft/GoodDollarMinterBurner.sol:175-184` </location>
<code_context>
+ * Only authorized operators (like OFT adapter) or the DAO avatar can call this.
+ * Enforces weekly and monthly burn limits if set.
+ */
+ function burn(address _from, uint256 _amount) external onlyOperators returns (bool) {
+ // Reset periods if needed
+ _resetWeeklyIfNeeded();
+ _resetMonthlyIfNeeded();
+
+ // Check weekly limit (0 means no limit)
+ if (weeklyBurnLimit > 0) {
+ require(weeklyBurned + _amount <= weeklyBurnLimit, "Weekly burn limit exceeded");
+ }
+
+ // Check monthly limit (0 means no limit)
+ if (monthlyBurnLimit > 0) {
+ require(monthlyBurned + _amount <= monthlyBurnLimit, "Monthly burn limit exceeded");
+ }
+
+ // Update counters
+ weeklyBurned += _amount;
+ monthlyBurned += _amount;
+
+ token.burnFrom(_from, _amount);
+ return true;
+ }
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider emitting events on mint/burn to aid observability and auditing.
Currently only configuration changes emit events, while `mint` and `burn`—the most sensitive operations—do not. Please add events for these actions (including `from/to`, `amount`, and the calling operator `msg.sender`) to improve on-chain monitoring, off-chain accounting, and post-incident analysis without altering core logic.
</issue_to_address>
### Comment 8
<location> `scripts/multichain-deploy/oft/bridge-oft-token.ts:41` </location>
<code_context>
+ ? parseInt(process.env.CELO_LZ_ENDPOINT_ID)
+ : EndpointId.CELO_V2_MAINNET; // Default CELO LayerZero endpoint ID
+
+const main = async () => {
+ const networkName = network.name;
+ const [sender] = await ethers.getSigners();
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring this script by extracting repeated logic into reusable helpers and a data-driven network configuration map to make `main` shorter and easier to extend.
You can reduce the complexity of this script without changing behavior by extracting some repeated/branch-heavy logic into small helpers and a data-driven network map.
### 1. Data-driven network configuration
Replace the ad-hoc `isXDC`/`isCELO` and `destNetworkName` branching with a simple config map:
```ts
const NETWORKS = {
"production-xdc": {
chain: "XDC",
eid: XDC_ENDPOINT_ID,
native: "XDC",
destNetworkName: "production-celo",
destChain: "CELO",
},
"development-xdc": {
chain: "XDC",
eid: XDC_ENDPOINT_ID,
native: "XDC",
destNetworkName: "development-celo",
destChain: "CELO",
},
"production-celo": {
chain: "CELO",
eid: CELO_ENDPOINT_ID,
native: "CELO",
destNetworkName: "production-xdc",
destChain: "XDC",
},
"development-celo": {
chain: "CELO",
eid: CELO_ENDPOINT_ID,
native: "CELO",
destNetworkName: "development-xdc",
destChain: "XDC",
},
} as const;
type NetworkKey = keyof typeof NETWORKS;
```
Then at the top of `main`:
```ts
const networkName = network.name as NetworkKey;
const cfg = NETWORKS[networkName];
if (!cfg) {
throw new Error(`Unsupported network: ${networkName}`);
}
const sourceNetwork = cfg.chain;
const destNetwork = cfg.destChain;
const sourceEndpointId = cfg.eid;
const destEndpointId = cfg.chain === "XDC" ? CELO_ENDPOINT_ID : XDC_ENDPOINT_ID;
const nativeTokenName = cfg.native;
const destNetworkName = cfg.destNetworkName;
```
This removes string `includes` and scattered ternaries, and makes it easy to add more networks.
### 2. Factor out allowance/approval pattern
The allowance check/approve/wait/log pattern is duplicated and then repeated for final verification. You can centralize it:
```ts
async function ensureAllowance(
token: Contract,
owner: string,
spender: string,
amount: ethers.BigNumber,
label: string,
) {
const current = await token.allowance(owner, spender);
console.log(`\nChecking ${label} allowance...`);
console.log(`Current ${label} allowance: ${ethers.utils.formatEther(current)} G$`);
if (current.lt(amount)) {
console.log(`\nApproving ${label}...`);
const tx = await token.approve(spender, amount);
await tx.wait();
console.log(`${label} approval confirmed:`, tx.hash);
} else {
console.log(`Sufficient ${label} allowance already set`);
}
}
```
Usage in `main`:
```ts
await ensureAllowance(token, sender.address, minterBurnerAddress, amount, "MinterBurner");
await ensureAllowance(token, sender.address, oftAdapterAddress, amount, "OFT adapter");
```
Final verification can also reuse a slimmer helper:
```ts
async function requireMinAllowance(
token: Contract,
owner: string,
spender: string,
amount: ethers.BigNumber,
label: string,
) {
const current = await token.allowance(owner, spender);
console.log(`Final ${label} allowance: ${ethers.utils.formatEther(current)} G$`);
if (current.lt(amount)) {
throw new Error(
`${label} allowance insufficient. Need ${ethers.utils.formatEther(
amount,
)} G$, have ${ethers.utils.formatEther(current)} G$`,
);
}
}
```
### 3. Simplify peer validation into a helper
The peer check logic (string lowercasing, hash-zero comparison, logging) can be wrapped in a focused helper:
```ts
async function assertPeerConfigured(
oftAdapter: Contract,
destEndpointId: number,
expectedAdapterAddress: string,
destNetwork: string,
) {
console.log(`\nChecking if ${destNetwork} peer is configured...`);
const destPeer = await oftAdapter.peers(destEndpointId);
const expectedPeer = ethers.utils.hexZeroPad(expectedAdapterAddress, 32);
console.log(`Current ${destNetwork} peer:`, destPeer);
console.log(`Expected ${destNetwork} peer (bytes32):`, expectedPeer);
const peerIsZero = destPeer === ethers.constants.HashZero;
const mismatch = destPeer.toLowerCase() !== expectedPeer.toLowerCase();
if (peerIsZero || mismatch) {
console.log(`\n⚠️ WARNING: ${destNetwork} peer is not configured correctly!`);
console.log("You need to set the peer before bridging. Run this command:");
console.log(` oftAdapter.setPeer(${destEndpointId}, "${expectedPeer}")`);
console.log("\nOr use the LayerZero wire command:");
console.log(` npx hardhat lz:oapp:wire --oapp-config layerzero.config.ts --network ${network.name}`);
throw new Error(
`NoPeer: ${destNetwork} peer (endpoint ${destEndpointId}) is not set. Expected: ${expectedAdapterAddress}`,
);
}
console.log(`✅ ${destNetwork} peer is configured correctly`);
}
```
Usage:
```ts
await assertPeerConfigured(oftAdapter, destEndpointId, destOFTAdapter, destNetwork);
```
### 4. Optional: isolate logging-heavy sections
To make `main` visually thinner, you can move verbose logging into helpers without changing the flow, e.g.:
```ts
function logSendSummary(
sourceNetwork: string,
destNetwork: string,
recipient: string,
amount: ethers.BigNumber,
txHash: string,
) {
console.log("\n=== Bridge Initiated Successfully ===");
console.log(`Bridging from ${sourceNetwork} to ${destNetwork}`);
console.log("Transaction hash:", txHash);
console.log(`Recipient on ${destNetwork}:`, recipient);
console.log("Amount:", ethers.utils.formatEther(amount), "G$");
console.log("\nYou can track the cross-chain message at:");
console.log(`https://layerzeroscan.com/tx/${txHash}`);
console.log(`\nNote: The tokens will arrive on ${destNetwork} after the LayerZero message is delivered.`);
console.log("This typically takes a few minutes.");
}
```
Then in the `try` block:
```ts
logSendSummary(sourceNetwork, destNetwork, recipient, amount, sendTx.hash);
```
These changes keep behavior and logs intact, but make `main` shorter, easier to follow, and safer to extend.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+145
to
+151
| let destNetworkName: string; | ||
| if (isXDC) { | ||
| // Bridging to CELO - try production-celo first, then development-celo | ||
| destNetworkName = "development-celo"; | ||
| } else { | ||
| // Bridging to XDC - try production-xdc first, then development-xdc | ||
| destNetworkName = "development-xdc"; |
There was a problem hiding this comment.
issue (bug_risk): Destination network selection ignores production networks and contradicts the usage docs.
Hardcoding development-celo / development-xdc as the destination means dao[destNetworkName] will be wrong when running on production-* networks, so bridging from production will fail. Instead, derive destNetworkName from networkName (e.g., swapping production-xdc ↔ production-celo and development-xdc ↔ development-celo) and only fall back to the other environment if the primary lookup is missing.
Comment on lines
+58
to
+59
| // Get owner - use Avatar if available, otherwise use deployer | ||
| const owner = root.address; |
There was a problem hiding this comment.
issue: Adapter ownership is set to the deployer while logs/documentation imply it is the DAO avatar.
The adapter is deployed with owner = root.address, but later logs/descriptions treat the avatar as the owner (e.g. "Owner set to DAO avatar:", "OFT Adapter Owner (Avatar):" using avatarAddress). This mismatch forces an extra transfer-oft-adapter-ownership step. Either construct the adapter with avatarAddress as owner, or keep the current flow and update logs/descriptions to state that the deployer is the initial owner and the avatar only becomes owner after running the transfer script.
Comment on lines
67
to
70
| const weeklyMintLimitStr = process.env.WEEKLY_MINT_LIMIT; | ||
| const monthlyMintLimitStr = process.env.MONTHLY_MINT_LIMIT; | ||
| const weeklyBurnLimitStr = process.env.WEEKLY_BURN_LIMIT; | ||
| const monthlyBurnLimitStr = process.env.MONTHLY_BURN_LIMIT; |
There was a problem hiding this comment.
issue: Environment variables are assumed to be present; undefined values will cause parseUnits to throw.
Because parseUnits is called immediately on each env var, any unset value (e.g., undefined) will throw before the later "no limits specified" logic runs. To align behavior with the help text, gate parseUnits behind a defined/non-empty check and treat missing values as null / "no change" instead of letting it throw.
Comment on lines
72
to
75
| const newWeeklyMintLimit = ethers.utils.parseUnits(weeklyMintLimitStr, 18); | ||
| const newMonthlyMintLimit = ethers.utils.parseUnits(monthlyMintLimitStr, 18); | ||
| const newWeeklyBurnLimit = ethers.utils.parseUnits(weeklyBurnLimitStr, 18); | ||
| const newMonthlyBurnLimit = ethers.utils.parseUnits(monthlyBurnLimitStr, 18); |
There was a problem hiding this comment.
issue (bug_risk): Boolean checks on BigNumber limits don’t behave as intended and prevent the "no limits specified" path.
Because parseUnits always returns a truthy BigNumber and never null, the later checks like if (!newWeeklyMintLimit && !newMonthlyMintLimit && …) and if (newWeeklyMintLimit !== null) will never behave as intended—the "no limits specified" path can’t be reached and all four limits are treated as set. To support unset vs set (and disabling with 0), keep the values as null/undefined until you know the env var is present, and only then call parseUnits, and use explicit == null checks instead of truthiness on BigNumbers.
Comment on lines
65
to
70
| // Get new limits from environment variables or command line args | ||
| // Format: values should be in G$ (will be converted to wei) | ||
| const weeklyMintLimitStr = process.env.WEEKLY_MINT_LIMIT; | ||
| const monthlyMintLimitStr = process.env.MONTHLY_MINT_LIMIT; | ||
| const weeklyBurnLimitStr = process.env.WEEKLY_BURN_LIMIT; | ||
| const monthlyBurnLimitStr = process.env.MONTHLY_BURN_LIMIT; |
There was a problem hiding this comment.
nitpick: The script mentions command-line args and the ability to disable limits, but only environment variables are actually supported.
The usage comments describe --weekly-mint=... flags and say “set to 0 or 'disable'” to turn off a limit, but the code only reads env vars and never parses CLI args or a 'disable' value. This mismatch is likely to confuse operators. Please either add CLI and special-value handling to match the comments, or update the comments to reflect the current env-var-only behavior and supported value formats.
| ? parseInt(process.env.CELO_LZ_ENDPOINT_ID) | ||
| : EndpointId.CELO_V2_MAINNET; // Default CELO LayerZero endpoint ID | ||
|
|
||
| const main = async () => { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring this script by extracting repeated logic into reusable helpers and a data-driven network configuration map to make main shorter and easier to extend.
You can reduce the complexity of this script without changing behavior by extracting some repeated/branch-heavy logic into small helpers and a data-driven network map.
1. Data-driven network configuration
Replace the ad-hoc isXDC/isCELO and destNetworkName branching with a simple config map:
const NETWORKS = {
"production-xdc": {
chain: "XDC",
eid: XDC_ENDPOINT_ID,
native: "XDC",
destNetworkName: "production-celo",
destChain: "CELO",
},
"development-xdc": {
chain: "XDC",
eid: XDC_ENDPOINT_ID,
native: "XDC",
destNetworkName: "development-celo",
destChain: "CELO",
},
"production-celo": {
chain: "CELO",
eid: CELO_ENDPOINT_ID,
native: "CELO",
destNetworkName: "production-xdc",
destChain: "XDC",
},
"development-celo": {
chain: "CELO",
eid: CELO_ENDPOINT_ID,
native: "CELO",
destNetworkName: "development-xdc",
destChain: "XDC",
},
} as const;
type NetworkKey = keyof typeof NETWORKS;Then at the top of main:
const networkName = network.name as NetworkKey;
const cfg = NETWORKS[networkName];
if (!cfg) {
throw new Error(`Unsupported network: ${networkName}`);
}
const sourceNetwork = cfg.chain;
const destNetwork = cfg.destChain;
const sourceEndpointId = cfg.eid;
const destEndpointId = cfg.chain === "XDC" ? CELO_ENDPOINT_ID : XDC_ENDPOINT_ID;
const nativeTokenName = cfg.native;
const destNetworkName = cfg.destNetworkName;This removes string includes and scattered ternaries, and makes it easy to add more networks.
2. Factor out allowance/approval pattern
The allowance check/approve/wait/log pattern is duplicated and then repeated for final verification. You can centralize it:
async function ensureAllowance(
token: Contract,
owner: string,
spender: string,
amount: ethers.BigNumber,
label: string,
) {
const current = await token.allowance(owner, spender);
console.log(`\nChecking ${label} allowance...`);
console.log(`Current ${label} allowance: ${ethers.utils.formatEther(current)} G$`);
if (current.lt(amount)) {
console.log(`\nApproving ${label}...`);
const tx = await token.approve(spender, amount);
await tx.wait();
console.log(`${label} approval confirmed:`, tx.hash);
} else {
console.log(`Sufficient ${label} allowance already set`);
}
}Usage in main:
await ensureAllowance(token, sender.address, minterBurnerAddress, amount, "MinterBurner");
await ensureAllowance(token, sender.address, oftAdapterAddress, amount, "OFT adapter");Final verification can also reuse a slimmer helper:
async function requireMinAllowance(
token: Contract,
owner: string,
spender: string,
amount: ethers.BigNumber,
label: string,
) {
const current = await token.allowance(owner, spender);
console.log(`Final ${label} allowance: ${ethers.utils.formatEther(current)} G$`);
if (current.lt(amount)) {
throw new Error(
`${label} allowance insufficient. Need ${ethers.utils.formatEther(
amount,
)} G$, have ${ethers.utils.formatEther(current)} G$`,
);
}
}3. Simplify peer validation into a helper
The peer check logic (string lowercasing, hash-zero comparison, logging) can be wrapped in a focused helper:
async function assertPeerConfigured(
oftAdapter: Contract,
destEndpointId: number,
expectedAdapterAddress: string,
destNetwork: string,
) {
console.log(`\nChecking if ${destNetwork} peer is configured...`);
const destPeer = await oftAdapter.peers(destEndpointId);
const expectedPeer = ethers.utils.hexZeroPad(expectedAdapterAddress, 32);
console.log(`Current ${destNetwork} peer:`, destPeer);
console.log(`Expected ${destNetwork} peer (bytes32):`, expectedPeer);
const peerIsZero = destPeer === ethers.constants.HashZero;
const mismatch = destPeer.toLowerCase() !== expectedPeer.toLowerCase();
if (peerIsZero || mismatch) {
console.log(`\n⚠️ WARNING: ${destNetwork} peer is not configured correctly!`);
console.log("You need to set the peer before bridging. Run this command:");
console.log(` oftAdapter.setPeer(${destEndpointId}, "${expectedPeer}")`);
console.log("\nOr use the LayerZero wire command:");
console.log(` npx hardhat lz:oapp:wire --oapp-config layerzero.config.ts --network ${network.name}`);
throw new Error(
`NoPeer: ${destNetwork} peer (endpoint ${destEndpointId}) is not set. Expected: ${expectedAdapterAddress}`,
);
}
console.log(`✅ ${destNetwork} peer is configured correctly`);
}Usage:
await assertPeerConfigured(oftAdapter, destEndpointId, destOFTAdapter, destNetwork);4. Optional: isolate logging-heavy sections
To make main visually thinner, you can move verbose logging into helpers without changing the flow, e.g.:
function logSendSummary(
sourceNetwork: string,
destNetwork: string,
recipient: string,
amount: ethers.BigNumber,
txHash: string,
) {
console.log("\n=== Bridge Initiated Successfully ===");
console.log(`Bridging from ${sourceNetwork} to ${destNetwork}`);
console.log("Transaction hash:", txHash);
console.log(`Recipient on ${destNetwork}:`, recipient);
console.log("Amount:", ethers.utils.formatEther(amount), "G$");
console.log("\nYou can track the cross-chain message at:");
console.log(`https://layerzeroscan.com/tx/${txHash}`);
console.log(`\nNote: The tokens will arrive on ${destNetwork} after the LayerZero message is delivered.`);
console.log("This typically takes a few minutes.");
}Then in the try block:
logSendSummary(sourceNetwork, destNetwork, recipient, amount, sendTx.hash);These changes keep behavior and logs intact, but make main shorter, easier to follow, and safer to extend.
Feature: Bridge GoodDollar using OFT adapter via LayerZero Endpoint v2
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
…date logging in deployment scripts
Contributor
|
Contributor
|
also lets move this to the GoodBridge repo |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements cross-chain bridging for GoodDollar (G$) tokens using LayerZero's Omnichain Fungible Token (OFT) v2 adapter. The implementation enables seamless token transfers between XDC and CELO networks.
About #288
How Has This Been Tested?
Bridge functionality is tested on XDC and CELO. You can find tx on layerzero scan.
https://layerzeroscan.com/tx/0x38e63cb7d4cfa1250197a5dc16f9d6e1977d0c0a4203e4af3e197ea6c39943c7
https://layerzeroscan.com/tx/0xa6c5bd9a381bfaba5136519cee047d4242b50232fbb4eddd27a110ba9c797061
Checklist: