feat: replace blocknative gas price with rpc node#7646
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
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:
WalkthroughRemoves the Blocknative gas price API integration (endpoints, API keys, parsing logic, and CI secrets) and replaces it with Blockscout oracle endpoints plus an ChangesBlocknative → Blockscout + RPC Gas Migration
CancellationModal UI Refinement
Sequence Diagram(s)sequenceDiagram
participant App
participant GasFeeApi
participant BlockscoutOracle
participant WagmiRPC as Wagmi RPC (eth_gasPrice)
App->>GasFeeApi: getGasPrices(chainId)
alt Blockscout URL configured
GasFeeApi->>BlockscoutOracle: fetchData(url)
alt fetch succeeds
BlockscoutOracle-->>GasFeeApi: {fast, average, slow} in gwei
GasFeeApi->>GasFeeApi: parseData → GasFeeEndpointResponse
else fetch fails or error
GasFeeApi->>WagmiRPC: getGasPrice(reownWagmiConfig, {chainId})
WagmiRPC-->>GasFeeApi: bigint gasPrice
GasFeeApi->>GasFeeApi: uniform price for all confidence levels
end
else No URL (chain unsupported by Blockscout)
GasFeeApi->>WagmiRPC: getGasPrice(reownWagmiConfig, {chainId})
WagmiRPC-->>GasFeeApi: bigint gasPrice
GasFeeApi->>GasFeeApi: uniform price for all confidence levels
end
GasFeeApi-->>App: GasFeeEndpointResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Deploying swap-dev with
|
| Latest commit: |
0c4daef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://37d7720a.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://remove-blocknative.swap-dev-5u6.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cowswap-frontend/src/api/gasPrices/index.ts`:
- Around line 51-69: The fetchData method does not validate HTTP response status
codes, and the parseData method does not validate that required price keys
contain valid finite numbers, allowing malformed responses to silently return
null values instead of triggering error handling. Add a response.ok check in the
fetchData method to throw an error for non-2xx status codes. In the parseData
method, add validation logic before the loop that iterates over PRICE_KEYS to
ensure each key exists in the input json object and contains a valid finite
number; throw an error with a descriptive message if validation fails, so that
the try-catch in getGasPrices can properly trigger the RPC fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3b81567-aa14-4801-81f6-ae529ee0ea6a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/workflows/ci.yml.github/workflows/ipfs.ymlapps/cowswap-frontend/package.jsonapps/cowswap-frontend/src/api/gasPrices/index.tsapps/cowswap-frontend/src/common/updaters/CancelReplaceTxUpdater.tsxapps/cowswap-frontend/src/cow-react/sentry/beforeSend.tslibs/common-const/src/common.tslibs/wallet/src/index.ts
💤 Files with no reviewable changes (3)
- .github/workflows/ci.yml
- apps/cowswap-frontend/src/common/updaters/CancelReplaceTxUpdater.tsx
- .github/workflows/ipfs.yml
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
On chains that blockscout is present, I see the request succeeding. However, the on-chain cancellation does not show the tx cost:
On chain where it's not supported, I see the requests failing:
Is it something we should configure on our side (CoW Swap/Cf-pages) or theirs?
|
Same issue as for @alfetopito : I don't see the gas estimation (however, I did not see it before either) |
Deploying explorer-dev with
|
| Latest commit: |
0c4daef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://28fe9929.explorer-dev-dxz.pages.dev |
| Branch Preview URL: | https://remove-blocknative.explorer-dev-dxz.pages.dev |
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
I cannot load balances on BNB, Polygon, Avlanche, Linea, Ink and Gnosis in this PR
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx`:
- Around line 102-107: Remove the console.log statement that is currently
logging the 'CCCCC' debug message with the isOnChainType, showMore,
txCostAmount, and nativeCurrency variables in the ModalTopContent component.
This debug logging fires on every render and should be removed before the code
is merged to production.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f947e217-1763-47f5-a838-ca9033c17f3b
📒 Files selected for processing (1)
apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx
It appears to be not related to the current PR |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx`:
- Line 50: Remove the console.log(StyledNotificationBanner) statement from the
module scope in ModalTopContent.tsx. This debug logging statement executes when
the module is imported and produces unnecessary noise in production logs without
providing user-facing value. Simply delete this line entirely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2488d102-6668-43e8-b416-e3a4d9102613
📒 Files selected for processing (1)
apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx
… remove/blocknative # Conflicts: # pnpm-lock.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx (1)
43-52: ⚡ Quick winExtract styled elements to a
.styled.tsmodule for this component.Lines 43-52 keep style declarations inline in the
.tsxfile; this diverges from the frontend convention and makes style reuse/import direction checks harder to enforce consistently.As per coding guidelines,
apps/cowswap-frontend/**/*.{ts,tsx}should “Import styles asimport * as styledEl from './X.styled'.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx` around lines 43 - 52, The StyledNotificationBanner styled component is currently defined inline in the ModalTopContent.tsx file, which violates the frontend convention of keeping styles in separate .styled.ts modules. Extract the StyledNotificationBanner definition from the .tsx file and move it to a new ModalTopContent.styled.ts file, then import it back into the component using the pattern import * as styledEl from './ModalTopContent.styled' to follow the coding guidelines and maintain consistency with the project's style organization.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx`:
- Around line 140-143: The StyledNotificationBanner div (containing the tx cost
information) is currently nested inside a paragraph element opened at line 134,
which violates HTML semantics and can cause React DOM warnings. Move the
StyledNotificationBanner block outside and after the closing paragraph tag to
ensure proper HTML structure. The paragraph element should only contain inline
text content, while the StyledNotificationBanner should be a sibling element at
the same nesting level as the paragraph.
---
Nitpick comments:
In `@apps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsx`:
- Around line 43-52: The StyledNotificationBanner styled component is currently
defined inline in the ModalTopContent.tsx file, which violates the frontend
convention of keeping styles in separate .styled.ts modules. Extract the
StyledNotificationBanner definition from the .tsx file and move it to a new
ModalTopContent.styled.ts file, then import it back into the component using the
pattern import * as styledEl from './ModalTopContent.styled' to follow the
coding guidelines and maintain consistency with the project's style
organization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 10110408-cc87-4091-815d-4c2145b9bbf6
📒 Files selected for processing (4)
apps/cowswap-frontend/src/common/hooks/useNotificationState.tsapps/cowswap-frontend/src/common/pure/CancellationModal/ModalTopContent.tsxapps/cowswap-frontend/src/common/pure/CancellationModal/styled.tsxapps/cowswap-frontend/src/legacy/components/NotificationBanner/index.tsx
💤 Files with no reviewable changes (3)
- apps/cowswap-frontend/src/legacy/components/NotificationBanner/index.tsx
- apps/cowswap-frontend/src/common/pure/CancellationModal/styled.tsx
- apps/cowswap-frontend/src/common/hooks/useNotificationState.ts
|
@alfetopito @elena-zh fixed the displaying issue. This was a weird one, because it was not reproducible locally. |
|
|
||
| useEffect(() => { | ||
| if (isClosed !== null) { | ||
| setIsActive((state) => !state) |
There was a problem hiding this comment.
This was a key issue.
Locally, it was triggering twice and ending up with isActive=true.
But not locally it was setting it to false.
There was a problem hiding this comment.
But then, is the notification banner being removed completely? Wasn't it in use at all?
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
⚠️ AI Review (Codex GPT-5, worked 3m): remove remaining Blocknative build env
Finding: Prod Vercel builds still receive the obsolete Blocknative key
- Location:
.github/workflows/vercel.yml:61 - This PR removes the Blocknative gas-price consumer and already drops
REACT_APP_BLOCKNATIVE_API_KEYfromci.ymlandipfs.yml, butvercel.ymlstill injects it into production builds. - The tracked frontend
.envalso still documentsREACT_APP_BLOCKNATIVE_API_KEY, so future local/prod setup still looks like it needs a Blocknative credential even though the app now uses Blockscout/RPC.
Suggested fix
- Remove
REACT_APP_BLOCKNATIVE_API_KEYfrom.github/workflows/vercel.yml. - Remove the stale Blocknative section from
apps/cowswap-frontend/.env.
Review scope and related context
I did not repeat existing review threads, which already cover:
- Blockscout malformed JSON validation in
apps/cowswap-frontend/src/api/gasPrices/index.ts. - Debug logs and invalid HTML nesting in
ModalTopContent.tsx/NotificationBanner. - The notification active-state discussion from the author follow-up.
Automated validation was not run locally because this worktree has no node_modules; the finding is based on current PR-head diff and config inspection.
🤖 Prompt for AI agents
Verify this finding against current PR head. If still valid, remove the remaining Blocknative env wiring only:
- .github/workflows/vercel.yml: remove REACT_APP_BLOCKNATIVE_API_KEY from the build env.
- apps/cowswap-frontend/.env: remove the stale Blocknative key comments.
Keep the change scoped and re-run targeted workflow/config checks if available.
Generated using the pr-review skill from the CoW Protocol skills repo.
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
comment that does not create a thread
|
@shoom3301 Build failed? #7646 (comment) I ran $pr-qa skill but it complains:
|
|
@fairlighteth fixed the build |
🧪 Scoped browser QA passed: Cloudflare build URL renders at latest PR head
How to retest
Commands + setup
Generated using the |
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
Manually tested the limit order steps and works correctly.
4 participants
Summary
Fixes #7579
Since Blocknative API is going to be shat down, we have to remove it from the app.
As an alternative I added eth_gasPrice RPC method: https://wagmi.sh/core/api/actions/getGasPrice
It's being used as a fallback in case if
blockscoutAPI failed.To Test
For every network:
Summary by CodeRabbit