Conversation
cruzdanilo
requested review from
dieguezguille,
franm91,
itofarina and
nfmelendez
as code owners
🦋 Changeset detectedLatest commit: 078eeac The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
PR SummaryMedium Risk Overview Modernizes build tooling and workflows: bumps Node to Contracts/dev tooling adjustments: updates OpenZeppelin + Written by Cursor Bugbot for commit 9ba9e88. This will update automatically on new commits. Configure here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBulk dependency and CI/tooling upgrades, Expo/OneSignal app.config edits, a metro web timer patch, ERC1967Proxy inline initialization in contract tests, protocol address reindexing in test harness/server, and multiple small UI style and theme/logic tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test harness (Protocol.s.sol)
participant EVM as EVM / Anvil
participant Proxy as ERC1967Proxy (constructor)
participant Impl as Contract implementation
Test->>EVM: deploy Impl (new Impl)
Test->>EVM: create ERC1967Proxy(address(new Impl...), abi.encodeCall(Impl.initialize,args))
EVM->>Proxy: create proxy with impl address + init calldata
Proxy->>Impl: delegatecall initialize(calldata) during proxy creation
Impl-->>Proxy: initialization completes (state set)
Test->>EVM: run post-deploy setup calls (setInterestRateModel, enableMarket, label)
EVM-->>Test: deployment and setup complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on a comprehensive upgrade of project dependencies across all modules, ensuring the codebase remains current with the latest library versions. It includes updates to core frameworks like Expo and React Native, various development tools, and smart contract-related packages. Additionally, minor refinements have been made to contract deployment logic, build configurations, and UI styling to enhance stability and maintainability. Highlights
Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #877 +/- ##
==========================================
+ Coverage 72.21% 72.24% +0.03%
==========================================
Files 227 227
Lines 8388 8387 -1
Branches 2699 2708 +9
==========================================
+ Hits 6057 6059 +2
+ Misses 2109 2106 -3
Partials 222 222
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
cruzdanilo
force-pushed
the
upgrade
branch
2 times, most recently
from
6cb9a05 to
3f2a415
Compare
dieguezguille
force-pushed
the
payments
branch
14 times, most recently
from
fb0e1dd to
5c192fc
Compare
cruzdanilo
force-pushed
the
upgrade
branch
2 times, most recently
from
2f2c4c8 to
a5d595a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| repayMarket, | ||
| denyExchanges, | ||
| externalAsset ? !!externalAssetAvailable : !!repayMarketAvailable, |
There was a problem hiding this comment.
🟡 Query key uses full volatile repayMarket object instead of stable repayMarket?.asset address, causing excessive cache invalidation
The routeFrom and route query keys were changed from repayMarket?.asset (a stable contract address string) to repayMarket (the full market object). The repayMarket object is derived from positions?.find(...) at src/components/pay/Repay.tsx:159, which spreads the full market data including volatile fields like floatingDepositAssets, usdPrice, etc. These fields change on every market data refresh. Since React Query uses deep structural comparison for keys, any change to these volatile fields invalidates the cached route data and triggers a new LiFi API fetch.
For the route query specifically (line 252), unlike routeFrom which uses placeholderData: keepPreviousData, there is no placeholder — so route becomes undefined during each refetch. This causes downstream effects: maxAmountIn becomes undefined (src/components/pay/Repay.tsx:282), the crossRepayPropose simulation is disabled, and the confirm button can flicker between enabled/disabled states. The queryFn only actually uses repayMarket.asset (a constant address), so the key should use repayMarket?.asset instead of the full object.
Adding `denyExchanges` to the key is correct
Note: the addition of denyExchanges to both query keys is a valid bug fix — without it, denying an exchange wouldn't trigger an immediate refetch. The issue is only with replacing repayMarket?.asset with repayMarket.
| repayMarket, | |
| denyExchanges, | |
| externalAsset ? !!externalAssetAvailable : !!repayMarketAvailable, | |
| repayMarket?.asset, | |
| denyExchanges, | |
| externalAsset ? !!externalAssetAvailable : !!repayMarketAvailable, |
Was this helpful? React with 👍 or 👎 to provide feedback.
| isFetching: isRouteFetching, | ||
| } = useQuery({ | ||
| queryKey: ["lifi", "route", mode, account, selectedAsset.address, repayMarket?.asset, maxRepay], | ||
| queryKey: ["lifi", "route", mode, account, selectedAsset.address, repayMarket, denyExchanges, maxRepay], |
There was a problem hiding this comment.
🟡 Second route query key also uses full volatile repayMarket object instead of repayMarket?.asset
Same issue as the routeFrom query key: the route query key at line 252 uses the full repayMarket object instead of repayMarket?.asset. This is worse here because this query does NOT use placeholderData: keepPreviousData, so every time the market data refreshes and any volatile field in repayMarket changes, route becomes undefined during the refetch. This causes the swap route data, maxAmountIn, and crossRepayPropose to all become unavailable, potentially flickering the UI and temporarily disabling the confirm button.
| queryKey: ["lifi", "route", mode, account, selectedAsset.address, repayMarket, denyExchanges, maxRepay], | |
| queryKey: ["lifi", "route", mode, account, selectedAsset.address, repayMarket?.asset, denyExchanges, maxRepay], |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
closes #853
Summary by CodeRabbit
Chores
Bug Fixes
Configuration