fix: use fallback HTTP transport in delegatedEoa mode#502
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces a memoized Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 pillarx-debug with
|
| Latest commit: |
9fa9257
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d5ce5b41.pillarx-debug.pages.dev |
| Branch Preview URL: | https://fix-delegated-eoa-signing.pillarx-debug.pages.dev |
Deploying x with
|
| Latest commit: |
9fa9257
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1adfb229.x-e62.pages.dev |
| Branch Preview URL: | https://fix-delegated-eoa-signing.x-e62.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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/apps/perps/hooks/useHyperliquid.ts (3)
194-194:⚠️ Potential issue | 🔴 CriticalMissing
clientTransportin dependency array causes stale closure.The
checkSetupStatuscallback usesclientTransport(lines 76, 90) but the dependency array is empty. Ifkitchanges andclientTransportis recomputed, this callback will continue using the stale transport value.🐛 Proposed fix
- }, []); + }, [clientTransport]);
245-245:⚠️ Potential issue | 🔴 CriticalMissing
clientTransportin dependency array.The
setupHyperliquidcallback usesclientTransport(lines 208, 214) but it's not listed in the dependency array, causing a stale closure.🐛 Proposed fix
- }, [address, checkSetupStatus]); + }, [address, checkSetupStatus, clientTransport]);
395-395:⚠️ Potential issue | 🔴 CriticalMissing
clientTransportin dependency array.The
executeCopyTradecallback usesclientTransport(lines 312, 318) but it's not listed in the dependency array.🐛 Proposed fix
- [address, setupStatus, loadBalance] + [address, setupStatus, loadBalance, clientTransport]
🤖 Fix all issues with AI agents
In `@src/apps/perps/hooks/useHyperliquid.ts`:
- Around line 45-54: The current useMemo fallback returns http() which cannot
sign and breaks EOA signing; change the clientTransport logic so that when
kit.getProvider() throws you do NOT return http(), but instead return
undefined/null and emit a debug log indicating the provider was unavailable;
update any WalletClient creation sites (where WalletClient is constructed for
address-only accounts and where signUserAction calls walletClient.signTypedData)
to use custom(provider) only when clientTransport is present and otherwise
construct the WalletClient so it relies on the injected signer/account (i.e., do
not attach a non-signing http transport); ensure you reference the
clientTransport variable, the useMemo/provider retrieval code
(kit.getProvider()), WalletClient instantiation sites, and
signUserAction/walletClient.signTypedData to make these changes.
github-actions
bot
temporarily deployed
to
Preview (fix/delegated-eoa-signing)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apps/perps/hooks/useHyperliquid.ts (1)
65-92:⚠️ Potential issue | 🟠 MajorDon’t drop the EOA address when transport is unavailable.
When
clientTransportisnull,targetAddressstaysnull, so Line 136 short‑circuits and user state/orders never load in delegatedEoa. You can still set the EOA address for read‑only fetches and only gate wallet‑client creation on transport availability.✅ Proposed fix
- if (eoa && clientTransport) { - targetAddress = eoa; - client = createWalletClient({ - account: eoa as `0x${string}`, - chain: arbitrum, - transport: clientTransport, - }); - } + if (eoa) { + targetAddress = eoa; + if (clientTransport) { + client = createWalletClient({ + account: eoa as `0x${string}`, + chain: arbitrum, + transport: clientTransport, + }); + } + }
🤖 Fix all issues with AI agents
In `@src/apps/perps/hooks/useHyperliquid.ts`:
- Around line 45-54: The callbacks setupHyperliquid and executeCopyTrade capture
clientTransport but don't list it in their dependency arrays, causing stale
closures if the provider becomes available later; update the dependency arrays
for setupHyperliquid (currently [address, checkSetupStatus]) to include
clientTransport, and for executeCopyTrade (currently [address, setupStatus,
loadBalance]) to include clientTransport as well; also consider adding
clientTransport to checkSetupStatus's dependencies (it currently has an empty
array) for consistency since checkSetupStatus uses clientTransport when invoked.
1 participant
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit