fix: use babylon-proto-ts for btc staking#1515
Conversation
necipsagiro
requested review from
gbarkhatov,
jeremy-babylonlabs,
jonybur and
jrwbabylonlab
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the codebase from using useBbnQuery with legacy RPC provider setup to the new createRPCClient from babylon-proto-ts v1.13.0, removing the need for custom RPC error handling and provider components.
- Replaced custom RPC provider and error handling with direct
createRPCClientusage - Updated query method names to align with new client API (
btcTipQuery→btcTipHeightQuery,babyTipQuery→babyTipHeightQuery) - Simplified query implementations by removing manual proto client setup
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updates babylon-proto-ts to version 1.13.0 |
| src/ui/common/hooks/client/rpc/queries/useBbnQuery.ts | Replaces custom proto client setup with createRPCClient and simplifies query logic |
| src/ui/common/hooks/services/useTransactionService.ts | Updates to use new btcTipHeightQuery name |
| src/ui/common/hooks/services/useStakingManagerService.ts | Updates to use new babyTipHeightQuery name |
| src/ui/common/state/BalanceState.tsx | Removes RPC error handling properties from state interface and implementation |
| src/ui/common/providers.tsx | Removes BbnRpcProvider from provider hierarchy |
| src/ui/common/context/rpc/BbnRpcProvider.tsx | Removes entire RPC provider component |
| src/ui/common/hooks/client/rpc/useRpcErrorHandler.ts | Removes RPC error handler hook |
| tests/app/state/BalanceState.test.tsx | Removes RPC error handling tests and mock setup |
| tests/app/hooks/services/useTransactionService.test.tsx | Updates test mocks to use new query names |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return height; | ||
| }, | ||
| enabled: Boolean(tmClient && connected), | ||
| enabled: connected, |
There was a problem hiding this comment.
Should we add !isGeoBlocked && !isHealthcheckLoading control here too?
There was a problem hiding this comment.
I think it's fine. The getting block height is not a blocking operation which had to be guarded by the geo-blocking.
| { cause: error as Error }, | ||
| ); | ||
| } | ||
| const { baby } = await createRPCClient({ url: BBN_RPC_URL }); |
There was a problem hiding this comment.
Question: Do we really need to create so many RPC client or this can be a singleton instance and shared across different query clients?
jrwbabylonlab
left a comment
jrwbabylonlab
left a comment
There was a problem hiding this comment.
It looks like there are some React issues — the above error is thrown after waiting a few minutes. This is likely coming from periodic polling of endpoints, though I don’t see any corresponding errors in the network tab.
The second issue is that we can no longer perform staking transactions. It seems that a change in the underlying client method has broken the proof-of-possession logic.
Happy to zoom with you to walk through some of the business logic behind this to help debug
|
@necipsagiro please ping me once the above comments from @jrwbabylonlab are resolved, this is a very sensitive piece of code |
Migrated useBbnQuery to createRPCClient, dropped BbnRpcProvider/useRpcErrorHandler and updated BalanceState and tests.
Closes: #1502