-
Notifications
You must be signed in to change notification settings - Fork 78
Deploy EcoFacet to production #1416
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
Conversation
WalkthroughAdds EcoFacet entries and diamond facet registrations across many networks, updates config/eco.json with a new worldchain key and Tron portal format, appends extensive deployment log records, adds a Tron deploy-and-register EcoFacet script, and refactors Tron facet-registration logic to use normalized hex comparisons and zero-address handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)script/**/*.ts📄 CodeRabbit inference engine (conventions.md)
Files:
🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-09-22T00:52:26.172ZApplied to files:
📚 Learning: 2025-09-09T10:39:26.383ZApplied to files:
📚 Learning: 2024-11-01T11:53:57.162ZApplied to files:
📚 Learning: 2025-09-16T01:39:54.099ZApplied to files:
📚 Learning: 2024-11-26T01:01:18.499ZApplied to files:
📚 Learning: 2024-09-27T07:10:15.586ZApplied to files:
📚 Learning: 2024-11-26T01:04:16.637ZApplied to files:
📚 Learning: 2025-04-21T03:17:53.443ZApplied to files:
📚 Learning: 2025-04-21T03:17:53.443ZApplied to files:
📚 Learning: 2024-11-26T01:14:58.163ZApplied to files:
🧬 Code graph analysis (2)script/deploy/tron/deploy-and-register-allbridge-facet.ts (1)
script/deploy/tron/deploy-and-register-symbiosis-facet.ts (1)
🔇 Additional comments (12)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
script/deploy/tron/utils.ts (1)
963-968: Verify ADD fallback behavior on selector check errors.The fallback to ADD behavior when
facetAddress(selector)fails is reasonable for handling unregistered selectors, but may mask legitimate errors (e.g., network issues, contract call failures).Consider logging the error details for debugging:
} catch (error) { + consola.warn( + `Failed to check selector ${selector}: ${error instanceof Error ? error.message : String(error)}` + ) consola.debug( `Could not check selector ${selector}, assuming ADD needed` ) selectorsToAdd.push(selector) }script/deploy/tron/deploy-and-register-eco-facet.ts (3)
86-89: Consider reusing the TronWeb instance from deployer.A TronWeb instance is already created inside
TronContractDeployer(line 79). Creating another instance here is redundant and uses extra memory. If the deployer exposed itstronWebproperty publicly, you could reuse it.If refactoring the deployer is not feasible right now, this duplication is acceptable but consider exposing
deployer.tronWebas a public getter inTronContractDeployerfor future optimization.
93-108: Add validation for eco.json and portal address.Two improvements:
- The code loads
config/eco.jsonwithout checking if the file exists, which could throw an unclear error.- The portal address is converted to hex (line 104) without validating it's a properly formatted Tron address.
Apply this diff to add validation:
+ // Validate eco.json exists + const ecoConfigFile = Bun.file('config/eco.json') + if (!(await ecoConfigFile.exists())) { + throw new Error('config/eco.json not found') + } + - const ecoConfig = await Bun.file('config/eco.json').json() + const ecoConfig = await ecoConfigFile.json() const tronEcoConfig = ecoConfig.tron if (!tronEcoConfig) throw new Error('Tron configuration not found in config/eco.json') const portalTron = tronEcoConfig.portal if (!portalTron) throw new Error('Eco portal not found for tron in config/eco.json') + // Validate portal is a valid Tron address + if (!portalTron.startsWith('T') || portalTron.length !== 34) { + throw new Error(`Invalid Tron portal address: ${portalTron}`) + } + const portal = tronAddressToHex(portalTron, tronWeb)
125-134: Consider verifying constructor args when reusing existing deployment.When reusing an existing EcoFacet deployment, the script doesn't verify that the deployed contract was initialized with the same portal address. If the portal address in
config/eco.jsonchanged since the last deployment, reusing the old deployment could cause issues.Consider adding a verification step or at minimum a warning:
if (exists && !shouldRedeploy && address) { consola.warn(`Reusing existing EcoFacet at ${address}`) consola.warn(`Ensure the existing deployment was initialized with portal: ${portalTron}`) const shouldVerify = await consola.prompt('Continue with existing deployment?', { type: 'confirm', initial: true, }) if (!shouldVerify) { process.exit(0) } facetAddress = address // ... rest of the code }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
config/eco.json(1 hunks)deployments/_deployments_log_file.json(1 hunks)deployments/arbitrum.json(1 hunks)deployments/base.json(1 hunks)deployments/celo.json(1 hunks)deployments/ink.json(1 hunks)deployments/mainnet.json(1 hunks)deployments/optimism.json(1 hunks)deployments/polygon.json(1 hunks)deployments/sonic.json(1 hunks)deployments/tron.diamond.json(1 hunks)deployments/tron.json(1 hunks)deployments/unichain.json(1 hunks)deployments/worldchain.json(1 hunks)script/deploy/tron/deploy-and-register-eco-facet.ts(1 hunks)script/deploy/tron/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.ts
📄 CodeRabbit inference engine (conventions.md)
script/**/*.ts: All TypeScript scripts must follow .eslintrc.cjs, use async/await, try/catch error handling, proper logging, environment variables, correct typings, use citty for CLI parsing, consola for logging, validate env vars via getEnvVar(), and exit with appropriate codes
Prefer existing helper functions over reimplementation (e.g., getDeployments, getProvider, getWalletFromPrivateKeyInDotEnv, sendTransaction, ensureBalanceAndAllowanceToDiamond, getUniswapData*)
Always use proper TypeChain types (e.g., ILiFi.BridgeDataStruct) and never use any for bridge data or contract structures
Before/after changes: verify imports and types exist, ensure typechain is generated, keep changes comprehensive and consistent, run TS compilation, remove unused imports, and ensure function signatures match usage
Scripts must execute with bunx tsx
Files:
script/deploy/tron/utils.tsscript/deploy/tron/deploy-and-register-eco-facet.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Applied to files:
deployments/unichain.jsondeployments/base.jsondeployments/arbitrum.jsondeployments/mainnet.jsondeployments/polygon.jsondeployments/worldchain.jsondeployments/optimism.json
📚 Learning: 2025-09-25T07:47:30.735Z
Learnt from: ezynda3
PR: lifinance/contracts#1324
File: src/Facets/EcoFacet.sol:306-336
Timestamp: 2025-09-25T07:47:30.735Z
Learning: In EcoFacet (src/Facets/EcoFacet.sol), the team intentionally uses variable-length validation for nonEVMReceiver addresses (up to 44 bytes) to support cross-ecosystem bridging with different address formats, rather than enforcing fixed 32-byte raw pubkeys.
Applied to files:
deployments/unichain.jsondeployments/base.jsondeployments/ink.jsondeployments/arbitrum.jsondeployments/mainnet.jsondeployments/polygon.jsondeployments/worldchain.jsondeployments/optimism.json
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : Facet contracts must implement: internal _startBridge, swapAndStartBridgeTokensVia{FacetName}, and startBridgeTokensVia{FacetName}
Applied to files:
deployments/unichain.jsondeployments/sonic.json
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Applied to files:
deployments/arbitrum.jsondeployments/mainnet.jsondeployments/sonic.jsondeployments/worldchain.json
📚 Learning: 2025-09-16T01:39:54.099Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.099Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Applied to files:
script/deploy/tron/utils.ts
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Applied to files:
script/deploy/tron/utils.tsdeployments/sonic.jsondeployments/worldchain.jsondeployments/celo.json
📚 Learning: 2025-01-28T14:29:00.823Z
Learnt from: ezynda3
PR: lifinance/contracts#924
File: script/deploy/zksync/utils/UpdateScriptBase.sol:112-178
Timestamp: 2025-01-28T14:29:00.823Z
Learning: The suggestion to modify `buildDiamondCut` function in `UpdateScriptBase.sol` to handle selectors from multiple old facets differently was deemed unnecessary by the maintainer.
Applied to files:
script/deploy/tron/utils.ts
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
PR: lifinance/contracts#1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-05-28T17:33:33.959Z
Learnt from: mirooon
PR: lifinance/contracts#1170
File: deployments/_deployments_log_file.json:28060-28072
Timestamp: 2025-05-28T17:33:33.959Z
Learning: Deployment log files like `deployments/_deployments_log_file.json` are historical records that document the actual versions deployed at specific times. They should not be updated to match current contract versions - they must accurately reflect what was deployed when the deployment occurred.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-01-09T04:17:46.190Z
Learnt from: ezynda3
PR: lifinance/contracts#914
File: deployments/_deployments_log_file.json:26902-26916
Timestamp: 2025-01-09T04:17:46.190Z
Learning: Updates to _deployments_log_file.json can represent backfilling of historical deployment data, not just new deployments. Such updates don't require new audits.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-09-30T03:52:27.281Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#812
File: deployments/_deployments_log_file.json:1914-1927
Timestamp: 2024-09-30T03:52:27.281Z
Learning: Duplicate entries in `deployments/_deployments_log_file.json` that are outdated do not require changes.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-12-04T01:59:34.045Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#832
File: deployments/_deployments_log_file.json:23712-23720
Timestamp: 2024-12-04T01:59:34.045Z
Learning: In `deployments/_deployments_log_file.json`, duplicate deployment entries for the same version and address may occur because they correspond to deployments on different networks. These entries are acceptable and should not be flagged as duplicates in future reviews.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-05-28T17:33:10.529Z
Learnt from: mirooon
PR: lifinance/contracts#1170
File: deployments/_deployments_log_file.json:28706-28717
Timestamp: 2025-05-28T17:33:10.529Z
Learning: Deployment log files (like `_deployments_log_file.json`) are historical records that should not be updated to match current contract versions. They should accurately reflect the versions that were actually deployed at specific timestamps.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-09-09T10:39:26.383Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1357
File: deployments/lens.diamond.json:48-51
Timestamp: 2025-09-09T10:39:26.383Z
Learning: In the lifinance/contracts repository, when deployment JSON files show address changes (like AcrossFacetV3 address updates in deployments/*.diamond.json), the corresponding _deployments_log_file.json updates may be handled in separate PRs rather than the same PR that updates the diamond configuration files.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-09-15T12:33:51.069Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1377
File: deployments/_deployments_log_file.json:5607-5619
Timestamp: 2025-09-15T12:33:51.069Z
Learning: The deployments/_deployments_log_file.json is structured as network → environment → version → array of deployment records. Always understand the file hierarchy before attempting validation, and target specific paths rather than trying to validate all objects in the JSON.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-09-27T07:10:15.586Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#812
File: deployments/polygon.diamond.json:4-11
Timestamp: 2024-09-27T07:10:15.586Z
Learning: In `deployments/polygon.diamond.json`, it's acceptable for certain facets to have empty names and versions when specified by the developer.
Applied to files:
deployments/polygon.json
📚 Learning: 2025-09-22T00:52:26.172Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1388
File: deployments/hyperevm.diamond.json:72-75
Timestamp: 2025-09-22T00:52:26.172Z
Learning: In diamond configuration files (deployments/*.diamond.json), it's acceptable to have multiple versions of the same facet (e.g., GlacisFacet v1.0.0 and v1.1.0) deployed at different contract addresses. This is intentional design for version coexistence, gradual migration, or backward compatibility purposes.
Applied to files:
deployments/tron.diamond.json
🧬 Code graph analysis (2)
script/deploy/tron/utils.ts (1)
script/deploy/tron/constants.ts (1)
ZERO_ADDRESS(45-45)
script/deploy/tron/deploy-and-register-eco-facet.ts (4)
script/deploy/tron/utils.ts (12)
getEnvironment(131-138)displayNetworkInfo(1233-1258)tronAddressToHex(1120-1122)confirmDeployment(1147-1185)checkExistingDeployment(665-695)getContractVersion(289-310)deployContractWithLogging(699-753)printDeploymentSummary(1190-1228)getContractAddress(221-240)getFacetSelectors(610-660)displayRegistrationInfo(1263-1283)registerFacetToDiamond(880-1052)script/demoScripts/utils/demoScriptHelpers.ts (2)
getEnvVar(590-596)getPrivateKeyForEnvironment(917-923)script/deploy/tron/types.ts (2)
ITronDeploymentConfig(1-12)IDeploymentResult(66-73)script/deploy/tron/TronContractDeployer.ts (1)
TronContractDeployer(18-529)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (20)
script/deploy/tron/utils.ts (3)
934-937: Improved address handling with raw value reading.The change to read
currentFacetAddressRawand explicitly convert toStringprovides better control over address format handling, which is appropriate for cross-chain deployment utilities.
950-962: LGTM: Robust address normalization for comparison.The normalized hex comparison using
tronWeb.address.toHex()andtoLowerCase()ensures consistent address matching regardless of input format (base58 vs hex). The logic correctly identifies when selectors are already registered to the target facet versus needing replacement.
944-945: TRON_ZERO_ADDRESS matches Tron’s standard hex zero address (“41” + 40 zeros).deployments/unichain.json (1)
38-39: LGTM: EcoFacet deployment entry added.The EcoFacet entry has been correctly added with proper JSON formatting (trailing comma on the previous entry).
deployments/sonic.json (1)
33-34: LGTM: EcoFacet deployment entry added.The EcoFacet entry has been correctly added with proper JSON formatting.
deployments/polygon.json (1)
69-70: LGTM: EcoFacet deployment entry added.The EcoFacet entry has been correctly added. Note that this uses the same address as the Sonic deployment (0x499E4A6585df0638508CA45A6Bb41B9BD385030C), which is expected if EcoFacet is deployed using deterministic deployment (CREATE2).
deployments/worldchain.json (1)
34-35: LGTM: EcoFacet deployment entry added.The EcoFacet entry has been correctly added with the consistent address used across other networks (0x499E4A6585df0638508CA45A6Bb41B9BD385030C).
deployments/optimism.json (1)
66-67: LGTM: EcoFacet deployment entry added.The EcoFacet entry has been correctly added with consistent formatting and address.
deployments/arbitrum.json (1)
68-69: LGTM: EcoFacet deployment entry added.The EcoFacet entry has been correctly added. The consistent address (0x499E4A6585df0638508CA45A6Bb41B9BD385030C) across all networks (arbitrum, optimism, polygon, sonic, worldchain, unichain) suggests deterministic deployment using CREATE2, which is a good practice for cross-chain contract deployments.
config/eco.json (1)
29-34: Confirm portal address updates for Tron and Worldchain
- Verify
TBhZw2sb5DuqGXf3PcxMKDaqxtoZVUUtR7matches thePortalentry in deployments/tron.json and is live on Tron mainnet- Verify worldchain’s portal (
0x399Dbd5DF04f83103F77A58cBa2B7c4d3cdede97) intentionally mirrors other EVM chainsdeployments/ink.json (1)
33-34: EcoFacet added; syntax OK. Please confirm intended shared address.
- Trailing comma fix is correct.
- Verify 0x499E4A6585df0638508CA45A6Bb41B9BD385030C is the intended Ink deployment (same as other EVM networks).
Use the script in the base.json comment to compare addresses across networks.
deployments/tron.diamond.json (1)
55-58: EcoFacet v1.0.0 registration is correct
Single entry found in deployments/tron.diamond.json matching tron.json.deployments/tron.json (1)
19-20: Confirmed EcoFacet address alignment. TMLwFQAjLQqCJDUNVCVd9c9Q8LLqbmhaW8 is present in tron.diamond.json with Name "EcoFacet", Version "1.0.0".deployments/base.json (1)
57-58: Confirm EcoFacet address consistency
- Base, Mainnet, and Celo all use 0x499E4A6585df0638508CA45A6Bb41B9BD385030C; Ink’s address differs (expected for non-EVM)
- Ensure 0x499E4A6585df0638508CA45A6Bb41B9BD385030C matches the deployments log
deployments/mainnet.json (1)
76-77: EcoFacet entry added and logged correctly
EcoFacet appears in mainnet.json and _deployments_log_file.json with the correct address.deployments/celo.json (1)
33-34: EcoFacet address consistency confirmed; update public docs/runbooksThe address 0x499E4A6585df0638508CA45A6Bb41B9BD385030C matches all EVM deployments; update any public documentation or runbooks listing available facets per chain.
script/deploy/tron/deploy-and-register-eco-facet.ts (4)
1-32: LGTM!Imports are well-organized and follow the coding guidelines for TypeScript scripts. All dependencies (citty, consola, tronweb) and helper functions are properly imported.
34-67: LGTM!Environment setup and credential retrieval follow best practices with clear error messages and proper exit codes. The user-friendly error message on lines 59-64 is particularly helpful.
163-188: LGTM!Registration flow is well-structured with proper error handling. The script correctly retrieves the diamond address, gets selectors, displays info, and handles both success and failure cases appropriately.
190-219: LGTM!CLI command definition follows citty best practices and coding guidelines. The dryRun argument is properly defined, and the script orchestrates deployment and registration cleanly.
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script/deploy/tron/utils.ts (1)
197-203: Critical: deployment file path builds produce '.../tron..json' and '.../tron.diamond..json' in production.The extra dot before an empty suffix yields invalid filenames. Fix to choose the full filename suffix, not a prefix.
Apply:
@@ - const fileSuffix = - environment === EnvironmentEnum.production ? '' : 'staging.' - const deploymentFile = resolve( - process.cwd(), - `deployments/${network}.${fileSuffix}json` - ) + const fileSuffix = + environment === EnvironmentEnum.production ? 'json' : 'staging.json' + const deploymentFile = resolve( + process.cwd(), + `deployments/${network}.${fileSuffix}` + ) @@ - const fileSuffix = - environment === EnvironmentEnum.production ? '' : 'staging.' - const deploymentFile = resolve( - process.cwd(), - `deployments/${network}.${fileSuffix}json` - ) + const fileSuffix = + environment === EnvironmentEnum.production ? 'json' : 'staging.json' + const deploymentFile = resolve( + process.cwd(), + `deployments/${network}.${fileSuffix}` + ) @@ - const fileSuffix = - environment === EnvironmentEnum.production ? '' : 'staging.' - const diamondFile = resolve( - process.cwd(), - `deployments/${network}.diamond.${fileSuffix}json` - ) + const fileSuffix = + environment === EnvironmentEnum.production ? 'json' : 'staging.json' + const diamondFile = resolve( + process.cwd(), + `deployments/${network}.diamond.${fileSuffix}` + )Also applies to: 225-231, 250-256
🧹 Nitpick comments (3)
script/deploy/tron/utils.ts (2)
1410-1422: Optional: tighten Base58 regex for Tron addresses in parser.Current
[A-Za-z0-9]includes invalid Base58 chars (0,O,I,l). Use Base58 charset to reduce false matches.- const facetRegex = /\[([T][A-Za-z0-9]{33})\s+\[((?:0x[a-fA-F0-9]+\s*)*)\]\]/g + const facetRegex = /\[([T][1-9A-HJ-NP-Za-km-z]{33})\s+\[((?:0x[a-fA-F0-9]+\s*)*)\]\]/g
113-126: Optional: include stderr in shell command failures for better diagnostics.Currently only stdout is captured. Add stderr to error message/context.
export async function executeShellCommand(command: string): Promise<string> { - const proc = Bun.spawn(['bash', '-c', command], { + const proc = Bun.spawn(['bash', '-c', command], { cwd: process.cwd(), env: process.env, }) - const output = await new Response(proc.stdout).text() + const [output, err] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]) const exitCode = await proc.exited - if (exitCode !== 0) - throw new Error(`Command failed with exit code ${exitCode}: ${command}`) + if (exitCode !== 0) + throw new Error( + `Command failed (${exitCode}): ${command}\nSTDERR:\n${err.trim()}` + ) return output.trim() }deployments/_deployments_log_file.json (1)
43394-43526: Pre‑deploy checklist reminder (external/privileged calls + audit).Before merge/deploy, ensure:
- Arbitrary external calls are validated/restricted.
- Privileged/storage‑modifying calls are validated/restricted.
- New facet has at least one preliminary audit; record date and auditor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
config/eco.json(1 hunks)deployments/_deployments_log_file.json(1 hunks)deployments/arbitrum.json(1 hunks)deployments/base.json(1 hunks)deployments/celo.json(1 hunks)deployments/ink.json(1 hunks)deployments/mainnet.json(1 hunks)deployments/optimism.json(1 hunks)deployments/polygon.json(1 hunks)deployments/sonic.json(1 hunks)deployments/tron.diamond.json(1 hunks)deployments/tron.json(1 hunks)deployments/unichain.json(1 hunks)deployments/worldchain.json(1 hunks)script/deploy/tron/deploy-and-register-eco-facet.ts(1 hunks)script/deploy/tron/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.ts
📄 CodeRabbit inference engine (conventions.md)
script/**/*.ts: All TypeScript scripts must follow .eslintrc.cjs, use async/await, try/catch error handling, proper logging, environment variables, correct typings, use citty for CLI parsing, consola for logging, validate env vars via getEnvVar(), and exit with appropriate codes
Prefer existing helper functions over reimplementation (e.g., getDeployments, getProvider, getWalletFromPrivateKeyInDotEnv, sendTransaction, ensureBalanceAndAllowanceToDiamond, getUniswapData*)
Always use proper TypeChain types (e.g., ILiFi.BridgeDataStruct) and never use any for bridge data or contract structures
Before/after changes: verify imports and types exist, ensure typechain is generated, keep changes comprehensive and consistent, run TS compilation, remove unused imports, and ensure function signatures match usage
Scripts must execute with bunx tsx
Files:
script/deploy/tron/utils.tsscript/deploy/tron/deploy-and-register-eco-facet.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: mirooon
PR: lifinance/contracts#1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Applied to files:
deployments/arbitrum.jsondeployments/sonic.jsonscript/deploy/tron/utils.tsdeployments/worldchain.json
📚 Learning: 2025-09-25T07:47:30.735Z
Learnt from: ezynda3
PR: lifinance/contracts#1324
File: src/Facets/EcoFacet.sol:306-336
Timestamp: 2025-09-25T07:47:30.735Z
Learning: In EcoFacet (src/Facets/EcoFacet.sol), the team intentionally uses variable-length validation for nonEVMReceiver addresses (up to 44 bytes) to support cross-ecosystem bridging with different address formats, rather than enforcing fixed 32-byte raw pubkeys.
Applied to files:
deployments/arbitrum.jsondeployments/optimism.jsondeployments/unichain.jsondeployments/ink.jsondeployments/polygon.jsondeployments/mainnet.jsondeployments/worldchain.jsondeployments/base.json
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Applied to files:
deployments/arbitrum.jsondeployments/optimism.jsondeployments/unichain.jsondeployments/polygon.jsondeployments/mainnet.jsondeployments/worldchain.jsondeployments/base.json
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Applied to files:
deployments/sonic.jsonscript/deploy/tron/utils.tsdeployments/worldchain.json
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : Facet contracts must implement: internal _startBridge, swapAndStartBridgeTokensVia{FacetName}, and startBridgeTokensVia{FacetName}
Applied to files:
deployments/sonic.jsondeployments/unichain.json
📚 Learning: 2025-09-16T01:39:54.099Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.099Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Applied to files:
script/deploy/tron/utils.ts
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
PR: lifinance/contracts#1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-05-28T17:33:33.959Z
Learnt from: mirooon
PR: lifinance/contracts#1170
File: deployments/_deployments_log_file.json:28060-28072
Timestamp: 2025-05-28T17:33:33.959Z
Learning: Deployment log files like `deployments/_deployments_log_file.json` are historical records that document the actual versions deployed at specific times. They should not be updated to match current contract versions - they must accurately reflect what was deployed when the deployment occurred.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
PR: lifinance/contracts#1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, understand that _targetState.json tracks contract version numbers (not addresses) and _deployments_log_file.json contains deployment records with addresses that may not be organized in obvious network sections. Always verify the actual file structure before flagging missing entries.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-12-04T01:59:34.045Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#832
File: deployments/_deployments_log_file.json:23712-23720
Timestamp: 2024-12-04T01:59:34.045Z
Learning: In `deployments/_deployments_log_file.json`, duplicate deployment entries for the same version and address may occur because they correspond to deployments on different networks. These entries are acceptable and should not be flagged as duplicates in future reviews.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-09-09T10:39:26.383Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1357
File: deployments/lens.diamond.json:48-51
Timestamp: 2025-09-09T10:39:26.383Z
Learning: In the lifinance/contracts repository, when deployment JSON files show address changes (like AcrossFacetV3 address updates in deployments/*.diamond.json), the corresponding _deployments_log_file.json updates may be handled in separate PRs rather than the same PR that updates the diamond configuration files.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-05-28T17:33:10.529Z
Learnt from: mirooon
PR: lifinance/contracts#1170
File: deployments/_deployments_log_file.json:28706-28717
Timestamp: 2025-05-28T17:33:10.529Z
Learning: Deployment log files (like `_deployments_log_file.json`) are historical records that should not be updated to match current contract versions. They should accurately reflect the versions that were actually deployed at specific timestamps.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-09-30T03:52:27.281Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#812
File: deployments/_deployments_log_file.json:1914-1927
Timestamp: 2024-09-30T03:52:27.281Z
Learning: Duplicate entries in `deployments/_deployments_log_file.json` that are outdated do not require changes.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-01-09T04:17:46.190Z
Learnt from: ezynda3
PR: lifinance/contracts#914
File: deployments/_deployments_log_file.json:26902-26916
Timestamp: 2025-01-09T04:17:46.190Z
Learning: Updates to _deployments_log_file.json can represent backfilling of historical deployment data, not just new deployments. Such updates don't require new audits.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2025-09-22T00:52:26.172Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1388
File: deployments/hyperevm.diamond.json:72-75
Timestamp: 2025-09-22T00:52:26.172Z
Learning: In diamond configuration files (deployments/*.diamond.json), it's acceptable to have multiple versions of the same facet (e.g., GlacisFacet v1.0.0 and v1.1.0) deployed at different contract addresses. This is intentional design for version coexistence, gradual migration, or backward compatibility purposes.
Applied to files:
deployments/tron.diamond.json
🧬 Code graph analysis (2)
script/deploy/tron/utils.ts (1)
script/deploy/tron/constants.ts (1)
ZERO_ADDRESS(45-45)
script/deploy/tron/deploy-and-register-eco-facet.ts (5)
script/deploy/tron/utils.ts (12)
getEnvironment(131-138)validateBalance(1127-1142)tronAddressToHex(1120-1122)confirmDeployment(1147-1185)checkExistingDeployment(665-695)getContractVersion(289-310)deployContractWithLogging(699-753)printDeploymentSummary(1190-1228)getContractAddress(221-240)getFacetSelectors(610-660)displayRegistrationInfo(1263-1283)registerFacetToDiamond(880-1052)script/demoScripts/utils/demoScriptHelpers.ts (2)
getEnvVar(590-596)getPrivateKeyForEnvironment(917-923)script/deploy/tron/types.ts (2)
ITronDeploymentConfig(1-12)IDeploymentResult(66-73)script/deploy/tron/TronContractDeployer.ts (1)
TronContractDeployer(18-529)script/deploy/tron/constants.ts (1)
MIN_BALANCE_WARNING(14-14)
🪛 Gitleaks (8.28.0)
deployments/tron.json
[high] 20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (13)
deployments/tron.diamond.json (1)
55-58: EcoFacet mapping already present in deployments/tron.json — no changes required.Likely an incorrect or invalid review comment.
deployments/ink.json (1)
33-34: Confirm EcoFacet address consistency
The EcoFacet address in deployments/ink.json (0xad8fEeFf8BC2d19495A0D5B1E1e287bAA67cA0Db) matches deployments/unichain.json. Manually verify any deployment logs or target-state files reference this same address to prevent drift.script/deploy/tron/utils.ts (2)
4-4: Use named import for TronWeb v6.x
TronWeb v6.x switched to named exports;import { TronWeb } from 'tronweb'is correct.Likely an incorrect or invalid review comment.
934-962: Selector registration logic: robust hex normalization and zero-address checks — LGTM.Add a debug log or test call to confirm that
diamond.facetAddress(selector).call()returns a hex or base58 string compatible withtronWeb.address.toHex.deployments/mainnet.json (1)
76-77: EcoFacet mainnet address matches other primary networks — confirm this deterministic address is correct on-chain.deployments/worldchain.json (1)
34-35: Worldchain diamond.json missing EcoFacet mappingEcoFacet was added to deployments/worldchain.json but no corresponding entry was found in deployments/worldchain.diamond.json; ensure the facet is registered under the correct key in the diamond config.
config/eco.json (1)
29-33: Mixed-format portal addresses already supported
Solidity deploy scripts parse hex portals via stdJson.readAddress; Tron TS scripts convert Base58 via tronAddressToHex.script/deploy/tron/deploy-and-register-eco-facet.ts (2)
1-32: LGTM!The import structure and setup follow the coding guidelines correctly, using citty for CLI parsing, consola for logging, and importing appropriate helper functions from the utility modules.
200-219: LGTM!The CLI definition follows the coding guidelines correctly, using
defineCommandandrunMainfrom citty with a cleardryRunflag for safe testing.deployments/_deployments_log_file.json (4)
43395-43407: Log entries look consistent with prior schema.Addresses, constructor args, optimizer runs, and verified flags align with existing patterns. Duplicate addresses across different networks are acceptable for logs.
Based on learnings
43398-43405: Confirm deterministic address method vs empty SALT.Same ADDRESS appears across multiple chains while SALT is empty. If CREATE2 was used, consider recording the salt; if not, please confirm how deterministic addressing was achieved.
Based on learnings
Also applies to: 43428-43435, 43443-43450, 43473-43480, 43503-43510, 43518-43525
43395-43396: Verify timestamp convention (timezone/format).Timestamps are uniform; confirm they follow the repo’s standard timezone and format for this log.
Based on learnings
Also applies to: 43415-43416, 43445-43446, 43460-43461, 43475-43476, 43505-43506, 43520-43521
43394-43526: Add EcoFacet to diamond deployments and target state
EcoFacet is present in each deployments/.json but missing from all deployments/.diamond.json files and from script/deploy/_targetState.json. Add EcoFacet entries with version 1.0.0 and the corresponding addresses across those files.⛔ Skipped due to learnings
Learnt from: 0xDEnYO PR: lifinance/contracts#782 File: deployments/arbitrum.diamond.json:148-150 Timestamp: 2024-11-26T01:01:18.499Z Learning: In `deployments/*.diamond.json` configuration files, facets that are part of an open PR and not yet merged into the main branch may have missing `Name` and `Version` fields.Learnt from: 0xDEnYO PR: lifinance/contracts#1388 File: deployments/hyperevm.diamond.json:72-75 Timestamp: 2025-09-22T00:52:26.172Z Learning: In diamond configuration files (deployments/*.diamond.json), it's acceptable to have multiple versions of the same facet (e.g., GlacisFacet v1.0.0 and v1.1.0) deployed at different contract addresses. This is intentional design for version coexistence, gradual migration, or backward compatibility purposes.Learnt from: 0xDEnYO PR: lifinance/contracts#1381 File: deployments/arbitrum.json:65-69 Timestamp: 2025-09-16T01:39:54.099Z Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.Learnt from: 0xDEnYO PR: lifinance/contracts#782 File: deployments/worldchain.diamond.json:5-63 Timestamp: 2024-11-26T01:14:58.163Z Learning: When facet versions are set to empty strings in `deployments/*.diamond.json` files, do not report this issue, as it is already known and will be addressed separately.Learnt from: 0xDEnYO PR: lifinance/contracts#1109 File: deployments/worldchain.json:28-28 Timestamp: 2025-04-21T03:17:53.443Z Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.Learnt from: 0xDEnYO PR: lifinance/contracts#1109 File: deployments/worldchain.json:28-28 Timestamp: 2025-04-21T03:17:53.443Z Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.Learnt from: 0xDEnYO PR: lifinance/contracts#782 File: deployments/fantom.diamond.json:92-94 Timestamp: 2024-11-26T01:04:16.637Z Learning: In `deployments/*.diamond.json` files, it's acceptable for facets to have empty `Name` and `Version` fields, and these should not be flagged as issues.Learnt from: ezynda3 PR: lifinance/contracts#846 File: deployments/cronos.diamond.json:28-31 Timestamp: 2024-11-01T11:53:57.162Z Learning: In `deployments/cronos.diamond.json`, both `GenericSwapFacet` and `GenericSwapFacetV3` are distinct facets that should be included together, as they serve different purposes.Learnt from: 0xDEnYO PR: lifinance/contracts#1357 File: deployments/lens.diamond.json:48-51 Timestamp: 2025-09-09T10:39:26.383Z Learning: In the lifinance/contracts repository, when deployment JSON files show address changes (like AcrossFacetV3 address updates in deployments/*.diamond.json), the corresponding _deployments_log_file.json updates may be handled in separate PRs rather than the same PR that updates the diamond configuration files.Learnt from: 0xDEnYO PR: lifinance/contracts#1109 File: deployments/worldchain.diamond.json:84-85 Timestamp: 2025-04-21T03:15:12.236Z Learning: In deployment JSON files that contain "diamond" in their filename (located in the deployments folder), periphery contracts may have empty string values for their addresses. This indicates that the contract is not deployed on that particular chain and should not be flagged as an issue during code reviews.
…o deploy-eco-facet
Which Jira task belongs to this PR?
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)