[TASK-7669] feat: implement rpc fallback mechanism#630
[TASK-7669] feat: implement rpc fallback mechanism#630kushagrasarathe wants to merge 2 commits intodevelopfrom
Conversation
- add fallback to public RPC when Infura fails - add test coverage for RPC fallback - mock necessary dependencies for testing
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces changes to the blockchain provider utility functions and testing setup. The modifications include mocking the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Learnings (1)src/components/Request/Pay/Views/Initial.view.tsx (3)🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
kushagrasarathe
requested review from
Hugo0
and removed request for
jjramirezn
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/utils/chains.utils.ts (1)
29-35: Test the public RPC connection before returning the providerCurrently, when falling back to the public RPC, the connection is not tested before returning the provider. Similar to the default provider, consider testing the connection with
getNetwork()to ensure the public RPC is functional.Apply this diff to test the fallback provider:
34- return peanut.getDefaultProvider(publicRpcUrl) + const fallbackProvider = new peanut.JsonRpcProvider(publicRpcUrl) + await fallbackProvider.getNetwork() // Test the connection + return fallbackProviderThis ensures that if the public RPC is also unavailable, the function will throw an error appropriately.
jest.setup.ts (1)
17-19: Nitpick: Consolidateconsole.warnmocking in the global console objectFor consistency, include
warn: jest.fn(),in the initialglobal.consoleassignment instead of reassigningglobal.console.warnseparately. This keeps all console mocks in one place and makes the setup cleaner.Apply this diff:
global.console = { ...console, // Uncomment to ignore a specific log level // log: jest.fn(), debug: jest.fn(), + warn: jest.fn(), // error: jest.fn(), } -// mock console.warn -global.console.warn = jest.fn()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
jest.setup.ts(1 hunks)src/utils/__tests__/chains.utils.test.ts(1 hunks)src/utils/chains.utils.ts(2 hunks)
| throw new Error(`No public RPC URL found for chain ${chainId}`) | ||
| } | ||
|
|
||
| return peanut.getDefaultProvider(publicRpcUrl) |
There was a problem hiding this comment.
Critical Issue: peanut.getDefaultProvider may not accept an RPC URL
In the fallback scenario, you're calling peanut.getDefaultProvider(publicRpcUrl). However, peanut.getDefaultProvider might not accept an RPC URL as a parameter—it may expect a chain ID or network name instead. This could cause the fallback to fail or return an incorrect provider.
To ensure proper functionality, verify that peanut.getDefaultProvider can accept an RPC URL. If it doesn't, use the appropriate method to create a provider from an RPC URL. Additionally, test the connection after obtaining the fallback provider to confirm it's operational.
Apply this diff to correct the code:
34 return peanut.getDefaultProvider(publicRpcUrl)
+ const fallbackProvider = new peanut.JsonRpcProvider(publicRpcUrl)
+ await fallbackProvider.getNetwork() // Test the connection
+ return fallbackProviderEnsure that peanut.JsonRpcProvider or the correct constructor is used based on the Peanut SDK's API.
Committable suggestion skipped: line range outside the PR's diff.
| default: { http: ['some-url'] }, | ||
| }, | ||
| } | ||
| jest.spyOn(chains, 'find').mockReturnValueOnce(chainWithoutPublicRpc) |
There was a problem hiding this comment.
Critical Issue: Cannot use jest.spyOn on array prototype methods
You're attempting to mock chains.find using jest.spyOn(chains, 'find'). However, find is a prototype method of JavaScript arrays, and jest.spyOn cannot be used on built-in prototype methods. This means the mock won't work as intended.
To fix this, replace the chains array with a custom one for this test or remock the module to provide the desired chain configuration.
Apply this diff to adjust your test setup:
- jest.spyOn(chains, 'find').mockReturnValueOnce(chainWithoutPublicRpc)
+ // Replace the chains array with one that has the chain without public RPC
+ (chains as any) = [
+ chainWithoutPublicRpc,
+ ]Alternatively, remock the chains module:
+ jest.mock('@/constants/chains.consts', () => ({
+ chains: [
+ chainWithoutPublicRpc,
+ ],
+ }))Ensure you reset the module after the test to avoid affecting other tests:
afterEach(() => {
jest.resetModules()
})
| // Try default (Infura) provider first | ||
| try { | ||
| const defaultProvider = await peanut.getDefaultProvider(chainId) | ||
| await defaultProvider.getNetwork() // Test the connection | ||
| return defaultProvider | ||
| } catch (error) { |
There was a problem hiding this comment.
Q: Im confused - getDefaultProvider finds a public provider first, no?
| it('should successfully return provider when Infura RPC works', async () => { | ||
| // setup successful infura response | ||
| mockProvider.getNetwork.mockResolvedValueOnce({ chainId: 1 }) | ||
| ;(peanut.getDefaultProvider as jest.Mock).mockResolvedValueOnce(mockProvider) | ||
|
|
||
| const provider = await getChainProvider('1') | ||
|
|
||
| expect(provider).toBe(mockProvider) | ||
| expect(peanut.getDefaultProvider).toHaveBeenCalledWith('1') | ||
| expect(mockProvider.getNetwork).toHaveBeenCalled() | ||
| }) |
There was a problem hiding this comment.
Nit: Ideally we can test for infura being in the rpc url - that way we're sure we got an infura RPC
| senderAddress: string | ||
| }) { | ||
| const provider = getChainProvider(tokenData.chainId) || (await peanut.getDefaultProvider(tokenData.chainId)) | ||
| const provider = (await getChainProvider(tokenData.chainId)) || (await peanut.getDefaultProvider(tokenData.chainId)) |
There was a problem hiding this comment.
Q: idg this, isn't getChainProvider just a wrapper of getDefaultProvider right now? What logic does this line make?
There was a problem hiding this comment.
valid, dumb me, fixing this 🫠
2 participants
implements an automatic fallback mechanism for RPC providers
when the default infura rpc fails, automatically tries to use public rpc endpoints
getChainProviderfnSummary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor
getChainProviderfunction to be asynchronous.