feat(stores-eth): hybrid ERC20 balance query (Alchemy + batch eth_call)#1930
feat(stores-eth): hybrid ERC20 balance query (Alchemy + batch eth_call)#1930piatoss3612 wants to merge 16 commits intodevelopfrom
Conversation
- Introduced `ObservableQueryEthereumERC20BalancesBatchParent` to handle batch requests for ERC20 token balances. - Updated `ObservableQueryEthereumERC20BalanceImpl` to utilize the new batch querying mechanism. - Refactored `ObservableQueryThirdpartyERC20BalancesImplParent` to integrate with the batch parent for improved balance fetching. - Removed hardcoded balance handling for specific tokens in `RootStore`. - Added `ERC20BalanceBatchParentStore` to manage instances of batch queries efficiently. - Enhanced overall balance querying performance by reducing redundant calls.
- Remove orphaned forceNativeERC20Query callback in root.tsx - Surface batch RPC errors via aggregated BatchParent.error - Wait for pending debounced rebuild before resolving waitFreshResponse - Forward batch parent lifecycle state when balance is served from fallback - Refresh batch queries alongside Alchemy on imperative fetch when in fallback
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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.
Downstream readiness gates in hooks-evm/tx/{amount,fee}.ts test truthiness
of bal.response to decide whether to render a loading-block. With the batch
refactor the Child used to return undefined unconditionally, stranding EVM
send/fee flows in loading on Alchemy-unsupported chains and on fallback
paths when Alchemy errored. Synthesize a truthy response from the batch
parent's per-contract balance so gates progress once data arrives.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
hooks-evm readiness gates early-return on `response` before touching `balance`, so observation tracking keyed only on `balance` left native ERC20 contracts unregistered with the batch parent. Extend the attach to cover `response` as well, using a local refcount so duplicate observation doesn't double-register or tear down prematurely.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
isInBatch only flips to true after Alchemy responds and the reaction runs. The previous sequential branch took the non-batch path on the first refresh and resolved before the fallback batch eth_call fired, leaving callers with stale/ready(false) balances for missing tokens. Always await Alchemy first, then await batch if the reaction flipped the state in the meantime.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Two related correctness fixes for hybrid ERC20 balance queries: - Thirdparty Child's `response` getter was forwarding `parent.response` for every token once Alchemy answered, even for tokens not present in the Alchemy payload. hooks-evm readiness gates on `bal.response` then passed with `.ready(false)` balances, surfacing false insufficient- balance errors. Gate the response on batch data when the contract is handled via fallback. Also mark `isInBatch` observable so the computed re-evaluates when the reaction flips it. - BatchParent.waitFreshResponse only waited for a debounced rebuild when batchQueries was empty. Adding a contract to a non-empty parent equally requires a rebuild before resolving. Track the last-built key and wait via `when` until it matches the current refcount keys.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… error - Per-contract batch error: replaced BatchParent.error with getError(contract) that returns only the error of the chunk owning the contract. A failing chunk no longer contaminates siblings in successful chunks, which previously surfaced spurious `bal.error` warnings across the ERC20 list. - Alchemy-error fallback: the reaction that chose covered/missing held a stale `response` after an Alchemy failure (ObservableQuery retains the previous response on error, alchemyContractSet only updates on success), stranding tokens in the covered state. Prioritize `parent.error` → missing so affected tokens drop into batch fallback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…per-request batch errors - Imperative ERC20 fetch() outside a reactive observer no longer falls through: the Child now force-adds/removes the contract around waitFreshResponse so post-send refresh paths actually issue an eth_call even when nothing is observing balance/response yet. - ObservableJsonRpcBatchQuery exposes per-request errors via a new perRequestErrors map. Individual failures inside a batch response were previously dropped silently, leaving bad contract addresses stuck at ready(false) with no error signal. BatchParent.getError checks this map so per-contract error propagation works for both HTTP-level and per-request failures.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Extend observation tracking to the `error` getter so consumers that only read `bal.error` (e.g. validators that short-circuit on error) keep the batch fallback subscription alive. Both Child impls now register when any of balance/response/error is observed. - Once the batch fallback has actual data for a contract, suppress the lingering Alchemy error. Previously a transient Alchemy failure plus a successful batch refresh still surfaced `parent.error` to callers gating on `bal.error`.
Convergent findings from 4 parallel review rounds: - Thirdparty Child's imperative fetch() / waitFreshResponse / waitResponse now evaluate Alchemy coverage directly and force-register to the batch parent when the contract is missing, so non-reactive refresh paths (post-tx refresh, explicit fetch) actually exercise the fallback instead of resolving after Alchemy only. - BatchParent.getError now reads chunk ownership from a snapshot taken at rebuild time instead of recomputing index from the live refcount ordering, eliminating misattribution during the debounce window. - Child.error suppresses parent.error while batch fallback is still loading (no data, no batch error yet) so consumers stay in the loading state rather than warning on a transient Alchemy failure. - ObservableJsonRpcBatchQuery synthesizes a per-request error for any requested id missing from the batch response, preventing permanent loading on partial/dropped server responses. - Child.waitFreshResponse / waitResponse now return the effective response object, matching the existing query interface contract.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50b5b6d491
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sing Mirror the observation-driven reaction: if parent.error is set, the imperative awaitFallbackIfMissing() path must not trust a cached response and should drop into batch fallback, so fetch()/wait*() produce fresh data instead of silently returning the stale Alchemy balance.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 581a3e36ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous commit swept in an in-progress USDT approve(0) debug patch (`0xdeadbeef` tx data) that is unrelated to this PR. Restore the file to match develop.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53b9ef436e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…isInBatch isInBatch is only flipped by the observation-driven reaction, so imperative callers (fetch()/waitFreshResponse/waitResponse with no reactive observers) completed the batch fallback but still read parent.response / parent.error via the old gate, regressing to false loading or stale Alchemy state for tokens missing from Alchemy. Replace the isInBatch gate on response/error/isFetching with a shared alchemyCovers() predicate that inspects current data state, matching balance's existing behavior and mirroring the reaction's logic.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe82d22dd4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Previously the rebuild reaction tracked only the refcount set, so a runtime endpoint change (Keplr custom RPC configuration) left batchQueries bound to the stale URL until the contract set happened to change again. Include the current RPC URL in the reaction key and in waitFreshResponse's match predicate, and check refcount.size for the empty-contracts branch instead of relying on the key string.
|
@codex review |
|
리뷰 라운드가 너무 누적되고 commit churn이 커져 PR을 일단 닫습니다. 내부 점검/정리 후 새 PR로 올릴 예정입니다. (브랜치 |
1 participant
Summary
Alchemy
alchemy_getTokenBalances는maxCount: 100으로 호출되며 pageKey 페이지네이션을 하지 않아, 응답 첫 100개에 포함되지 못한 ERC20 토큰은 잔고가 조회되지 않는다. 이전에 하드코딩으로 우회했던 Base axlUSDC 케이스도 아마 같은 원인(응답 tail에 밀려 누락)으로 보인다. 이번 변경은 registry에 등록된 토큰 중 응답에 없는 것을 batcheth_callfallback으로 자동 처리한다.eth_callfallbackforceNativeERC20Queryprop 및 axlUSDC 하드코딩 제거 — hybrid가 자동 대체Details
ObservableQueryEthereumERC20BalancesBatchParent— refcountMap<contractLower, number>+ObservableJsonRpcBatchQuery[](chunks of 10). 200ms debounced reaction rebuild. chunk 쿼리에서 에러 집계.ERC20BalanceBatchParentStore—(chainId, address)단위 공유. Ethereum Registry + Thirdparty Registry가 같은 BatchParent를 주입받아 중복 쿼리 방지.fetch/waitFreshResponse/error/isFetching가 BatchParent 상태를 반영하도록 수정 — imperative refresh / 에러 표면화가 Alchemy 경로뿐 아니라 fallback 경로에서도 동작.ObservableJsonRpcBatchQuery+ chunk + reaction rebuild 패턴 재사용.Test plan
.ready(false)영구 stuck 없음