Skip to content

Conversation

@mathiasmoeller
Copy link
Member

Which Jira task belongs to this PR?

https://lifi.atlassian.net/browse/LF-16130

Why did I implement it this way?

Added new signatures, will run more tests before publishing this

Checklist before requesting a review

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

  • I have checked that any arbitrary calls to external contracts are validated and or restricted
  • I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
  • I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

@mathiasmoeller mathiasmoeller added the WIP Work in progress label Nov 7, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

Walkthrough

Configuration files updated with new DEX addresses across multiple networks in config/dexs.json and a new function selector added to config/sigs.json. These are data-only changes to whitelisted addresses and selectors with no logic or API modifications.

Changes

Cohort / File(s) Change Summary
DEX Address Configuration
config/dexs.json
Added new DEX address 0x72229141d4b016682d3618ece47c046f30da4ad1 to mainnet and 0x328de900860816d29d1367f6903a24d8ed40c997 to multiple networks (apechain, berachain, opbnb, base, mantle, moonbeam, etc.)
Signature Selector Configuration
config/sigs.json
Added new selector entry 0xf380b676 to the selectors array

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Configuration-only changes with no logic, control flow, or API modifications
  • Simple array element additions following existing patterns
  • No risk of functional impact; purely data augmentation

Possibly related PRs

Suggested labels

AuditNotRequired

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description includes the required Jira task reference and explanation, but all pre-review and reviewer checklists are unchecked, indicating incomplete preparation. Complete the pre-review checklist items (especially self-review and tests) and have the reviewer verify security and audit requirements before merging this draft PR.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main changes: adding Lido-related signatures and contract addresses to configuration files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch lido-staking

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between d10c553 and c5365f1.

📒 Files selected for processing (2)
  • config/dexs.json (4 hunks)
  • config/sigs.json (1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/blast.json:27-27
Timestamp: 2024-10-04T09:06:19.927Z
Learning: Audit references for older contracts deployed over 2 months ago, including `LiFiDEXAggregator.sol`, will be added early next year (estimated).
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/blast.json:27-27
Timestamp: 2024-10-09T03:47:21.269Z
Learning: Audit references for older contracts deployed over 2 months ago, including `LiFiDEXAggregator.sol`, will be added early next year (estimated).
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1237
File: conventions.md:56-60
Timestamp: 2025-07-03T01:44:43.968Z
Learning: Always consult the conventions.md file for the latest rules and conventions when reviewing PRs or code changes in the lifinance/contracts repository. Make suggestions when code changes do not match the documented conventions in this file.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/avalanche.diamond.json:105-105
Timestamp: 2024-10-04T09:17:19.275Z
Learning: In the `lifinance/contracts` repository, contracts may have different addresses across networks. Do not check if contracts have the same addresses across all networks in future reviews.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/avalanche.diamond.json:105-105
Timestamp: 2024-10-09T03:47:21.269Z
Learning: In the `lifinance/contracts` repository, contracts may have different addresses across networks. Do not check if contracts have the same addresses across all networks in future reviews.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 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.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1136
File: config/dexs.json:0-0
Timestamp: 2025-08-28T02:02:56.965Z
Learning: Different networks in config/dexs.json naturally and appropriately have different DEX addresses. Do not flag different addresses across different networks as issues - only flag if the same network's addresses appear to have changed unintentionally between branches.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 836
File: config/dexs.json:396-397
Timestamp: 2024-10-21T01:27:47.808Z
Learning: In `config/dexs.json`, it's acceptable for a whitelisted DEX address to be the same across multiple networks.
Learnt from: ezynda3
Repo: lifinance/contracts PR: 861
File: config/dexs.json:748-752
Timestamp: 2024-11-21T08:39:29.530Z
Learning: In the 'worldchain' network, the addresses `0x50D5a8aCFAe13Dceb217E9a071F6c6Bd5bDB4155`, `0x8f023b4193a6b18C227B4a755f8e28B3D30Ef9a1`, and `0x603a538477d44064eA5A5d8C345b4Ff6fca1142a` are used as DEXs and should be included in `config/dexs.json`.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 846
File: config/dexs.json:296-300
Timestamp: 2024-11-04T02:25:07.478Z
Learning: In `config/dexs.json`, it's expected that some addresses appear multiple times across different networks.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1201
File: config/dexs.json:536-537
Timestamp: 2025-06-11T08:55:44.396Z
Learning: In lifinance/contracts, the addresses stored in config/dexs.json are handled case-insensitively, so they do not need to be written in EIP-55 checksummed format.
Learnt from: ezynda3
Repo: lifinance/contracts PR: 937
File: config/dexs.json:95-99
Timestamp: 2025-01-21T11:04:46.116Z
Learning: In the dexs.json configuration, utility contracts (like FeeCollector, LiFuelFeeCollector, TokenWrapper) can be treated as DEX contracts since they are handled similarly in the system's logic.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/scroll.diamond.json:82-82
Timestamp: 2024-10-09T03:47:21.269Z
Learning: In the `deployments/*.diamond.json` and `deployments/*.json` files, the `LiFiDEXAggregator` contract may intentionally have the same contract address across multiple networks. This is acceptable and should not be flagged as an issue in future code reviews.
Learnt from: ezynda3
Repo: lifinance/contracts PR: 861
File: config/global.json:146-146
Timestamp: 2024-11-21T08:34:30.300Z
Learning: The project is deprecating `safeAddresses` and `safeApiUrls` in `global.json` and moving these configurations to `config/networks.json` for network configurations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 782
File: deployments/base.diamond.json:148-148
Timestamp: 2024-11-25T09:05:43.045Z
Learning: In deployment configuration files (e.g., `deployments/base.diamond.json`), empty addresses for contracts like `Permit2Proxy` may be placeholders and will be updated after approvals are released from the multisig safe.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 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.
📚 Learning: 2024-12-03T10:59:31.056Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 807
File: config/sigs.json:150-151
Timestamp: 2024-12-03T10:59:31.056Z
Learning: Facet selectors are not added to the `config/sigs.json` file.

Applied to files:

  • config/sigs.json
📚 Learning: 2024-11-21T08:39:29.530Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 861
File: config/dexs.json:748-752
Timestamp: 2024-11-21T08:39:29.530Z
Learning: In the 'worldchain' network, the addresses `0x50D5a8aCFAe13Dceb217E9a071F6c6Bd5bDB4155`, `0x8f023b4193a6b18C227B4a755f8e28B3D30Ef9a1`, and `0x603a538477d44064eA5A5d8C345b4Ff6fca1142a` are used as DEXs and should be included in `config/dexs.json`.

Applied to files:

  • config/dexs.json
📚 Learning: 2025-08-28T02:02:56.965Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1136
File: config/dexs.json:0-0
Timestamp: 2025-08-28T02:02:56.965Z
Learning: Different networks in config/dexs.json naturally and appropriately have different DEX addresses. Do not flag different addresses across different networks as issues - only flag if the same network's addresses appear to have changed unintentionally between branches.

Applied to files:

  • config/dexs.json
📚 Learning: 2024-11-04T02:25:07.478Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 846
File: config/dexs.json:296-300
Timestamp: 2024-11-04T02:25:07.478Z
Learning: In `config/dexs.json`, it's expected that some addresses appear multiple times across different networks.

Applied to files:

  • config/dexs.json
📚 Learning: 2024-10-21T01:27:47.808Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 836
File: config/dexs.json:396-397
Timestamp: 2024-10-21T01:27:47.808Z
Learning: In `config/dexs.json`, it's acceptable for a whitelisted DEX address to be the same across multiple networks.

Applied to files:

  • config/dexs.json
📚 Learning: 2025-06-11T08:55:44.396Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1201
File: config/dexs.json:536-537
Timestamp: 2025-06-11T08:55:44.396Z
Learning: In lifinance/contracts, the addresses stored in config/dexs.json are handled case-insensitively, so they do not need to be written in EIP-55 checksummed format.

Applied to files:

  • config/dexs.json
📚 Learning: 2025-01-21T11:04:46.116Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 937
File: config/dexs.json:95-99
Timestamp: 2025-01-21T11:04:46.116Z
Learning: In the dexs.json configuration, utility contracts (like FeeCollector, LiFuelFeeCollector, TokenWrapper) can be treated as DEX contracts since they are handled similarly in the system's logic.

Applied to files:

  • config/dexs.json
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/scroll.diamond.json:82-82
Timestamp: 2024-10-09T03:47:21.269Z
Learning: In the `deployments/*.diamond.json` and `deployments/*.json` files, the `LiFiDEXAggregator` contract may intentionally have the same contract address across multiple networks. This is acceptable and should not be flagged as an issue in future code reviews.

Applied to files:

  • config/dexs.json
🔇 Additional comments (3)
config/sigs.json (1)

190-191: Verify selector type per established conventions.

A previous review (PR #807) established that facet selectors should not be added to config/sigs.json. Before merging, please confirm that 0xf380b676 is an external contract function selector (e.g., from Lido) rather than a facet selector. If this is indeed a Lido contract function selector being whitelisted, that's acceptable—please clarify the purpose in the PR description.

config/dexs.json (2)

124-125: Verify rollout completeness across all target networks.

The new DEX address 0x328de900860816d29d1367f6903a24d8ed40c997 is being added to multiple networks. Confirm that all intended networks have been updated. Based on the AI summary, updates should include apechain, berachain, opbnb, base, mantles, and moonbeam—verify that the provided file snippet includes all necessary networks and no unintended networks are affected.

Also, given this is positioned as Lido-related (per PR title), confirm this address corresponds to a legitimate Lido contract or whitelisted Lido-compatible DEX.

Also applies to: 256-257, 757-758, 966-967


124-125: Address formatting and consistency look good.

The lowercase address format is acceptable per project conventions (PR #1201). The consistent placement of the new address after existing entries maintains data structure integrity across all affected networks.

Also applies to: 256-257, 757-758, 966-967

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WIP Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants