Feat: yield balances migration#516
Conversation
Fall back to the yield reward rate when a position balance has no personalized campaign APY so the APY composition still appears for matching campaign yields.
🦋 Changeset detectedLatest commit: fda10e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughMigration to a Yield API fetch client and domain model: adds a YieldApi client/provider, replaces many Changes
Sequence Diagram(s)sequenceDiagram
participant Widget as Widget App
participant Provider as YieldApiClientProvider
participant FetchClient as OpenAPI Fetch Client
participant YieldAPI as Yield API Server
participant DB as Backend
rect rgba(200,150,255,0.5)
Note over Widget,Provider: Client initialization
Widget->>Provider: useYieldApiFetchClient()
Provider->>Provider: createYieldApiFetchClient(apiKey, url, hooks)
Provider-->>Widget: YieldApiFetchClient & OpenAPI client
end
rect rgba(150,200,255,0.5)
Note over Widget,YieldAPI: Fetch yield opportunity
Widget->>FetchClient: GET /v1/yields/{yieldId}
FetchClient->>YieldAPI: HTTP GET
YieldAPI->>DB: Resolve yield + mechanics
DB-->>YieldAPI: YieldApiYieldDto
YieldAPI-->>FetchClient: HTTP 200 + body
FetchClient->>Widget: parsed data (geo-block / rich-error handled)
end
rect rgba(150,255,200,0.5)
Note over Widget,YieldAPI: Create action (preview/submit)
Widget->>FetchClient: POST /v1/actions/{enter|exit|manage} (YieldCreateActionDto)
FetchClient->>YieldAPI: HTTP POST
YieldAPI->>DB: Create action + build txs
DB-->>YieldAPI: ActionDto + Transactions
YieldAPI-->>FetchClient: Action response
FetchClient-->>Widget: ActionDto for preview/submit
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/widget/src/pages/details/earn-page/components/select-yield-section/use-animate-yield-percent.ts (1)
43-45:⚠️ Potential issue | 🟠 MajorString placeholder path is ignored in final return.
On Line 43, when
perRewardis"- %", the function still returnsestimatedRewards.extract()?.percentage, so the new placeholder may never be shown (and can becomeundefinedafter DTO migration).💡 Proposed fix
- return typeof perReward === "string" || config.env.isTestMode - ? estimatedRewards.extract()?.percentage - : transformedMotionValue; + return typeof perReward === "string" + ? perReward + : config.env.isTestMode + ? estimatedRewards.extract()?.percentage + : transformedMotionValue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/pages/details/earn-page/components/select-yield-section/use-animate-yield-percent.ts` around lines 43 - 45, The current return expression returns estimatedRewards.extract()?.percentage when perReward is a string placeholder like "- %", hiding the placeholder and possibly yielding undefined; change the logic in use-animate-yield-percent so that if typeof perReward === "string" you return perReward directly (not estimatedRewards.extract()?.percentage), otherwise preserve the existing test-mode and motion-value behavior; also guard the estimatedRewards.extract()?.percentage with a nullish fallback (e.g., ?? perReward or a safe default) to avoid returning undefined after DTO migration.packages/widget/tests/use-cases/select-opportunity.test.tsx (1)
117-129:⚠️ Potential issue | 🟡 MinorReplace wildcard matcher with
legacyApiRoute()for/v1/tokensendpoint.This endpoint is the legacy API version used for token retrieval (per
use-default-tokens.ts). Using the route helper maintains consistency with other endpoint mocking in this test (lines 65, 135-141) and prevents potential host/base-URL wiring issues.Suggested change
- http.get("*/v1/tokens", async () => { + http.get(legacyApiRoute("/v1/tokens"), async () => { await delay(); return HttpResponse.json([ { token, availableYields: [ "ethereum-eth-lido-staking", "ethereum-eth-stakewise-staking", "ethereum-eth-reth-staking", ], }, ]); }),Note: This pattern appears in multiple test setup files (external-provider, deep-links-flow, staking-flow, gas-warning-flow, etc.). Consider applying the same fix across the test suite for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/tests/use-cases/select-opportunity.test.tsx` around lines 117 - 129, Replace the wildcard http.get matcher for the "/v1/tokens" endpoint with the helper legacyApiRoute() so the test uses the same legacy API route as use-default-tokens.ts; locate the http.get(...) call that returns HttpResponse.json([...]) and change its first argument to legacyApiRoute("/v1/tokens") to avoid host/base-URL wiring issues and match other mocks in this suite.packages/widget/src/providers/sk-wallet/errors.ts (1)
30-30:⚠️ Potential issue | 🟡 MinorRemove dead code: unused
SendTransactionErrorinstantiation.This line creates an error instance that's never used or assigned. It appears to be leftover debug/test code.
🗑️ Proposed fix
- -new SendTransactionError();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/providers/sk-wallet/errors.ts` at line 30, The line instantiating SendTransactionError is dead code and should be removed: delete the standalone `new SendTransactionError();` expression in the errors module so the class is only defined/used where intended; ensure no other code relies on that unused instantiation and run tests/typechecks to confirm no references to the unused instance remain (look for the SendTransactionError symbol in the same file or nearby exports).packages/widget/src/pages/position-details/hooks/use-unstake-machine.ts (1)
334-337:⚠️ Potential issue | 🟠 Major
assigncall has no effect in error handler.On line 335,
assign(({ context }) => ({ ...context, error }))is called but its return value is not used. In XState,assignreturns an action object that needs to be executed as part of the machine's actions array, not called imperatively. The error won't be stored in context.🐛 Proposed fix
The error assignment should be handled via the
__SUBMIT_ERROR__event's actions in the machine definition (around line 265), or the error should be passed with the event:.caseOf({ Right() { self.send({ type: "__SUBMIT_SUCCESS__" }); }, Left(error) { - assign(({ context }) => ({ ...context, error })); - self.send({ type: "__SUBMIT_ERROR__" }); + self.send({ type: "__SUBMIT_ERROR__", error }); }, }),Then update the event type and add an action handler:
- | { type: "__SUBMIT_ERROR__" }, + | { type: "__SUBMIT_ERROR__"; error?: Error },And in the
onhandler for__SUBMIT_ERROR__:__SUBMIT_ERROR__: { target: ".error", + actions: assign(({ event }) => ({ + error: Maybe.fromNullable(event.error), + })), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/pages/position-details/hooks/use-unstake-machine.ts` around lines 334 - 337, The assign call inside Left is not applied because assign returns an action object instead of mutating context; update Left to include the error in the event payload (send an event with type "__SUBMIT_ERROR__" and the error) and then move the assign logic into the machine's event handler for "__SUBMIT_ERROR__" (add an action that assigns error into context via assign in the machine definition near the existing __SUBMIT_ERROR__ handling), or alternatively replace the send call with executing the assign action directly as part of the actions array; reference: Left, assign, self.send, __SUBMIT_ERROR__, and the machine definition where __SUBMIT_ERROR__ actions are declared.
🟡 Minor comments (10)
packages/widget/src/components/molecules/amount-toggle/index.tsx-62-62 (1)
62-62:⚠️ Potential issue | 🟡 MinorRemove or justify this unrelated export reordering.
The exports were reordered to alphabetical order (
AmountbeforeRoot), but no ESLint or Prettier configuration enforces this convention. This change appears unrelated to the PR's stated objective of yield balances migration and lacks explanation. Either revert this change or clarify its purpose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/components/molecules/amount-toggle/index.tsx` at line 62, The change reordered the named exports to "Amount, Root" which is unrelated to the PR; revert the export ordering to the original sequence (export { Root, Amount }) in the amount-toggle module or add a short inline comment explaining why alphabetical order is required and reference a style rule, or add a lint/prettier rule that enforces alphabetical exports; update the export statement for Amount and Root accordingly (symbols: Amount, Root) so the diff only contains intentional changes.packages/widget/.env.test-1-4 (1)
1-4:⚠️ Potential issue | 🟡 MinorReorder keys to clear dotenv-linter warnings.
Current ordering triggers static analysis warnings; reordering keeps CI/lint output clean.
Based on learnings: Confirm nothing is broken with lint command which checks lint/type errors after making changes.Suggested reorder
-VITE_API_URL=https://api.stakek.it/ -VITE_YIELDS_API_URL=https://api.yield.xyz/ -VITE_API_KEY=vitest-api-key MODE=test +VITE_API_KEY=vitest-api-key +VITE_API_URL=https://api.stakek.it/ +VITE_YIELDS_API_URL=https://api.yield.xyz/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/.env.test` around lines 1 - 4, Reorder the environment variable keys in the packages/widget/.env.test file to satisfy dotenv-linter (e.g., sort keys alphabetically or follow the project's expected key order such that VITE_API_KEY, VITE_API_URL, VITE_YIELDS_API_URL, MODE are in the corrected order); update the .env.test file accordingly and then run the project's lint command (the same lint/type-check step used in CI) to confirm no lint or type errors were introduced.packages/widget/src/pages-dashboard/activity/activity-details.page.tsx-89-89 (1)
89-89:⚠️ Potential issue | 🟡 MinorConfirm that
integrationIdprop receives and semantically represents yield identifiers.The change is functionally correct:
selectedAction.yieldIdis passed tointegrationId, which is then used as a yield identifier inisEthenaUsdeStaking()(compares against "ethena-usde-staking") anduseYieldOpportunity(). However, the prop naming creates a semantic inconsistency—the parameter is namedintegrationIdbut receives and is treated as a yield ID throughout downstream code. This works because both represent the same string identifier in this codebase, but the naming should be clarified or unified for maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/pages-dashboard/activity/activity-details.page.tsx` at line 89, The prop integrationId is being passed selectedAction.yieldId but is treated as a yield identifier downstream (see isEthenaUsdeStaking() and useYieldOpportunity()), causing a semantic mismatch; update the component API so the prop name matches its meaning: either rename the prop integrationId to yieldId where it’s declared/consumed or change the call site to pass selectedAction.yieldId into a yieldId prop, and update all usages of integrationId inside the component and in calls to isEthenaUsdeStaking() and useYieldOpportunity() to use yieldId so naming is unified and self-explanatory.packages/widget/src/translation/French/translations.json-163-163 (1)
163-163:⚠️ Potential issue | 🟡 MinorFix French typography in APY composition label.
Line 163 should use
Jusqu’à {{value}}(with apostrophe/accent), notJusqu'a {{value}}.✏️ Proposed fix
- "up_to": "Jusqu'a {{value}}" + "up_to": "Jusqu’à {{value}}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/translation/French/translations.json` at line 163, Update the French translation value for the "up_to" key in translations.json to use the correct typographic apostrophe and accented "à": replace "Jusqu'a {{value}}" with "Jusqu’à {{value}}" (use the proper right single quotation mark + "à") so the label reads correctly in French.packages/widget/src/translation/French/translations.json-407-407 (1)
407-407:⚠️ Potential issue | 🟡 MinorFix accent in personalized APY label.
Line 407 should be
APY personnalisé.✏️ Proposed fix
- "personalized_apy": "APY personnalise", + "personalized_apy": "APY personnalisé",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/translation/French/translations.json` at line 407, Update the French translation value for the key "personalized_apy" in translations.json to use the correct accent: change the string "APY personnalise" to "APY personnalisé" so the label displays the proper French accent.packages/widget/src/hooks/api/use-tokens-prices.ts-18-27 (1)
18-27:⚠️ Potential issue | 🟡 MinorFix duplicate token in
tokenList: second entry should reference yield's base token.The comment on line 9 documents "Requested Token + Yield base token + Yield gas fee token", but line 23 incorrectly uses
val.tokentwice:tokenList: [val.token, val.token, val.yieldDto.mechanics.gasFeeToken]The second entry should be the yield's base token, not the requested token:
tokenList: [val.token, val.yieldDto.token, val.yieldDto.mechanics.gasFeeToken]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/hooks/api/use-tokens-prices.ts` around lines 18 - 27, The tokenList currently contains a duplicate requested token in the priceRequestDto creation; update the mapping inside the useMemo (where Maybe.fromRecord({ yieldDto, token }) is mapped to the DTO) so tokenList becomes [val.token, val.yieldDto.token, val.yieldDto.mechanics.gasFeeToken] (i.e., replace the second val.token with the yield's base token val.yieldDto.token) to match the documented "Requested Token + Yield base token + Yield gas fee token".packages/widget/src/hooks/use-positions-data.ts-76-80 (1)
76-80:⚠️ Potential issue | 🟡 MinorFix validator-address fallback when
validatorsis an empty array.Current
??fallback skipsbalance.validatorwheneverbalance.validatorsis present but empty. That can drop addresses unexpectedly.Suggested fix
-const getBalanceValidatorAddresses = (balance: YieldBalanceDto) => - ( - balance.validators?.map((validator) => validator.address) ?? - (balance.validator?.address ? [balance.validator.address] : []) - ).filter(Boolean); +const getBalanceValidatorAddresses = (balance: YieldBalanceDto) => { + const fromValidators = ( + balance.validators?.map((validator) => validator.address) ?? [] + ).filter(Boolean); + + if (fromValidators.length > 0) return fromValidators; + return balance.validator?.address ? [balance.validator.address] : []; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/hooks/use-positions-data.ts` around lines 76 - 80, getBalanceValidatorAddresses uses the nullish coalescing operator (??) which treats an empty validators array as present and therefore skips the fallback to balance.validator; change the expression to check for a non-empty validators array (e.g., use a length check or logical OR) so that when balance.validators is an empty array it falls back to balance.validator.address; update the function getBalanceValidatorAddresses to use something like (balance.validators && balance.validators.length ? balance.validators.map(v => v.address) : (balance.validator?.address ? [balance.validator.address] : [])) and keep the final .filter(Boolean) intact.packages/widget/src/pages/position-details/hooks/use-stake-exit-request-dto.ts-46-53 (1)
46-53:⚠️ Potential issue | 🟡 MinorPotential issue:
providerIdmay be included when undefined.When
b.validator?.providerIdis truthy (line 48), the returned object includes bothproviderIdandvalidatorAddress. However, ifb.validator.addressis undefined, you'll be includingvalidatorAddress: undefinedin the DTO, which may cause issues downstream.🛡️ Suggested defensive fix
return List.find( (b) => !!b.validator?.providerId, val.stakedOrLiquidBalances ).map((b) => ({ providerId: b.validator?.providerId, - validatorAddress: b.validator?.address, + ...(b.validator?.address && { validatorAddress: b.validator.address }), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/pages/position-details/hooks/use-stake-exit-request-dto.ts` around lines 46 - 53, The code in use-stake-exit-request-dto uses List.find on val.stakedOrLiquidBalances when isYieldIntegrationAggregator(val.integrationData) is true but may return an object with validatorAddress undefined; change the mapping logic so you only include providerId/validatorAddress when validator.address is defined (e.g., check b.validator?.address before returning the DTO), or return undefined/null instead of an object with validatorAddress: undefined; update the mapping in the List.find result to guard on b.validator?.address so downstream consumers never receive validatorAddress set to undefined.packages/widget/src/providers/yield-api-client-provider/index.tsx-84-91 (1)
84-91:⚠️ Potential issue | 🟡 MinorPotential excessive client recreation due to
i18ndependency.The
i18nobject fromuseTranslation()may change reference on every render or language change. SincecreateYieldApiFetchClientis called insideuseMemowithi18nas a dependency, this could cause the fetch client to be recreated more often than necessary. Consider using a ref or extracting only the stable parts ofi18nneeded for error handling.♻️ Suggested fix using a ref for stable i18n reference
export const YieldApiClientProvider = ({ children }: PropsWithChildren) => { const { apiKey, yieldsApiUrl } = useSettings(); const { i18n } = useTranslation(); const url = yieldsApiUrl ?? config.env.yieldsApiUrl; + const i18nRef = useRef(i18n); + i18nRef.current = i18n; const clients = useMemo(() => { - const fetchClient = createYieldApiFetchClient({ apiKey, i18n, url }); + const fetchClient = createYieldApiFetchClient({ + apiKey, + i18n: i18nRef.current, + url + }); return { fetchClient, queryClient: createClient(fetchClient), }; - }, [apiKey, i18n, url]); + }, [apiKey, url]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/providers/yield-api-client-provider/index.tsx` around lines 84 - 91, The useMemo block recreates fetchClient whenever the entire i18n object reference changes; update createYieldApiFetchClient usage in the useMemo so it depends only on stable values (e.g., i18n.language or a stable translator function) or keep i18n in a ref and read from that ref inside error handlers—specifically, modify the useMemo that calls createYieldApiFetchClient and createClient so its dependency array uses apiKey, url and a stable i18n token (or remove i18n and use an i18nRef where i18nRef.current = i18n) to avoid recreating fetchClient unnecessarily.packages/widget/src/pages/position-details/hooks/use-position-details.ts-192-200 (1)
192-200:⚠️ Potential issue | 🟡 MinorPotential division by zero in share conversion calculation.
While there's a filter for
yb.shareAmount && yb.amount(line 192), ifyb.amountis"0"(a truthy string), the division on line 197-199 would result inInfinity. Consider adding a numeric check.🛡️ Proposed fix to guard against zero division
.filter((yb) => yb.shareAmount && yb.amount && !yb.token.isPoints) + .filter((yb) => new BigNumber(yb.amount ?? 0).isGreaterThan(0)) .forEach((yb) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/widget/src/pages/position-details/hooks/use-position-details.ts` around lines 192 - 200, The division can produce Infinity when yb.amount is a non-empty "0" string; update the conversion logic in use-position-details (the block that filters over yb and calls acc.set with defaultFormattedNumber(new BigNumber(yb.shareAmount).dividedBy(new BigNumber(yb.amount)))) to coerce yb.amount to a numeric value and guard against zero before dividing (e.g., parse/convert yb.amount to a Number or BigNumber and skip or set a safe fallback when it equals 0), ensuring the value passed to defaultFormattedNumber is valid; keep references to yb.shareAmount, yb.amount, defaultFormattedNumber, BigNumber, acc.set, and yb.token.symbol/yb.shareToken?.symbol when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae3c0135-c881-4a62-96ea-2a41e0fe728e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (223)
.changeset/lemon-drinks-prove.md.github/workflows/ci.yml.gitignoreAGENTS.mdmise.tomlpackage.jsonpackages/widget/.env.testpackages/widget/package.jsonpackages/widget/public/mockServiceWorker.jspackages/widget/script.tspackages/widget/src/common/check-gas-amount.tspackages/widget/src/common/delay-api-requests.tspackages/widget/src/common/get-gas-mode-value.tspackages/widget/src/common/private-api.tspackages/widget/src/components/atoms/image-fallback/index.tsxpackages/widget/src/components/atoms/image-fallback/styles.css.tspackages/widget/src/components/atoms/image/index.tsxpackages/widget/src/components/atoms/token-icon/index.tsxpackages/widget/src/components/atoms/token-icon/network-icon-image/index.tsxpackages/widget/src/components/atoms/token-icon/provider-icon/index.tsxpackages/widget/src/components/atoms/token-icon/token-icon-container/hooks/use-variant-token-urls.tspackages/widget/src/components/atoms/token-icon/token-icon-container/index.tsxpackages/widget/src/components/atoms/token-icon/token-icon-image/index.tsxpackages/widget/src/components/atoms/virtual-list/index.tsxpackages/widget/src/components/molecules/amount-toggle/index.tsxpackages/widget/src/components/molecules/reward-rate-breakdown/index.tsxpackages/widget/src/components/molecules/reward-token-details/index.tsxpackages/widget/src/components/molecules/select-opportunity-list-item/index.tsxpackages/widget/src/components/molecules/select-validator/index.tsxpackages/widget/src/components/molecules/select-validator/meta-info.tsxpackages/widget/src/components/molecules/select-validator/select-validator-list.tsxpackages/widget/src/components/molecules/select-yield/index.tsxpackages/widget/src/config/index.tspackages/widget/src/domain/index.tspackages/widget/src/domain/types/action.tspackages/widget/src/domain/types/addresses.tspackages/widget/src/domain/types/errors.tspackages/widget/src/domain/types/fees.tspackages/widget/src/domain/types/init-params.tspackages/widget/src/domain/types/pending-action.tspackages/widget/src/domain/types/positions.tspackages/widget/src/domain/types/price.tspackages/widget/src/domain/types/reward-rate.tspackages/widget/src/domain/types/rewards.tspackages/widget/src/domain/types/settings.tspackages/widget/src/domain/types/stake.tspackages/widget/src/domain/types/token-balance.tspackages/widget/src/domain/types/tokens.tspackages/widget/src/domain/types/transaction.tspackages/widget/src/domain/types/tron.tspackages/widget/src/domain/types/validators.tspackages/widget/src/domain/types/wallets/generic-wallet.tspackages/widget/src/domain/types/yield-api.tspackages/widget/src/domain/types/yields.tspackages/widget/src/hooks/api/use-activity-actions.tspackages/widget/src/hooks/api/use-default-tokens.tspackages/widget/src/hooks/api/use-multi-yields.tspackages/widget/src/hooks/api/use-prices.tspackages/widget/src/hooks/api/use-token-balances-scan.tspackages/widget/src/hooks/api/use-tokens-prices.tspackages/widget/src/hooks/api/use-yield-balances-scan.tspackages/widget/src/hooks/api/use-yield-opportunity/get-yield-opportunity.tspackages/widget/src/hooks/api/use-yield-opportunity/index.tspackages/widget/src/hooks/api/use-yield-validators.tspackages/widget/src/hooks/navigation/use-dashboard-tabs-page-match.tspackages/widget/src/hooks/navigation/use-unstake-or-pending-action-params.tspackages/widget/src/hooks/use-base-token.tspackages/widget/src/hooks/use-estimated-rewards.tspackages/widget/src/hooks/use-force-max-amount.tspackages/widget/src/hooks/use-gas-fee-token.tspackages/widget/src/hooks/use-gas-warning-check.tspackages/widget/src/hooks/use-geo-block.tspackages/widget/src/hooks/use-handle-deep-links.tspackages/widget/src/hooks/use-init-params.tspackages/widget/src/hooks/use-init-query-params.tspackages/widget/src/hooks/use-max-min-yield-amount.tspackages/widget/src/hooks/use-position-balance-by-type.tspackages/widget/src/hooks/use-position-balances.tspackages/widget/src/hooks/use-positions-data.tspackages/widget/src/hooks/use-provider-details.tspackages/widget/src/hooks/use-reward-token-details/get-reward-token-symbols.tsxpackages/widget/src/hooks/use-reward-token-details/index.tspackages/widget/src/hooks/use-rewards-summary.tspackages/widget/src/hooks/use-rich-errors.tspackages/widget/src/hooks/use-staked-or-liquid-balance.tspackages/widget/src/hooks/use-summary.tsxpackages/widget/src/hooks/use-under-maintenance.tspackages/widget/src/hooks/use-yield-meta-info.tsxpackages/widget/src/hooks/use-yield-type.tspackages/widget/src/pages-dashboard/activity/action-list-item/index.tsxpackages/widget/src/pages-dashboard/activity/activity-details.page.tsxpackages/widget/src/pages-dashboard/activity/activity.page.tsxpackages/widget/src/pages-dashboard/activity/position-balances.tsxpackages/widget/src/pages-dashboard/overview/earn-page/utila-select-validator-section.tsxpackages/widget/src/pages-dashboard/overview/earn-page/utila-select-validator-trigger.tsxpackages/widget/src/pages-dashboard/overview/positions/components/positions-list-item.tsxpackages/widget/src/pages-dashboard/overview/positions/hooks/use-position-list-item.tspackages/widget/src/pages-dashboard/position-details/components/position-details-actions.tsxpackages/widget/src/pages-dashboard/position-details/components/position-details-info.tsxpackages/widget/src/pages-dashboard/position-details/components/provider-details.tsxpackages/widget/src/pages-dashboard/position-details/components/top-header.tsxpackages/widget/src/pages-dashboard/rewards/components/positions-list-item.tsxpackages/widget/src/pages/complete/hooks/use-activity-complete.hook.tspackages/widget/src/pages/complete/hooks/use-complete.hook.tspackages/widget/src/pages/complete/pages/activity-complete.page.tsxpackages/widget/src/pages/complete/pages/common.page.tsxpackages/widget/src/pages/complete/pages/pending-complete.page.tsxpackages/widget/src/pages/complete/pages/stake-complete.page.tsxpackages/widget/src/pages/complete/pages/unstake-complete.page.tsxpackages/widget/src/pages/complete/state/index.tsxpackages/widget/src/pages/components/meta-info/index.tsxpackages/widget/src/pages/details/activity-page/components/action-list-item/index.tsxpackages/widget/src/pages/details/activity-page/components/list-item-bullet/index.tsxpackages/widget/src/pages/details/activity-page/hooks/use-action-list-item.tspackages/widget/src/pages/details/activity-page/hooks/use-activity-page.tsxpackages/widget/src/pages/details/activity-page/state/activity-page.context.tsxpackages/widget/src/pages/details/activity-page/types.tspackages/widget/src/pages/details/earn-page/components/extra-args-selection/index.tsxpackages/widget/src/pages/details/earn-page/components/select-provider/index.tsxpackages/widget/src/pages/details/earn-page/components/select-token-section/index.tsxpackages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsxpackages/widget/src/pages/details/earn-page/components/select-validator-section/index.tsxpackages/widget/src/pages/details/earn-page/components/select-validator-section/select-validator-trigger.tsxpackages/widget/src/pages/details/earn-page/components/select-validator-section/use-select-validator.tspackages/widget/src/pages/details/earn-page/components/select-yield-section/index.tsxpackages/widget/src/pages/details/earn-page/components/select-yield-section/select-opportunity.tsxpackages/widget/src/pages/details/earn-page/components/select-yield-section/select-yield-reward-details.tsxpackages/widget/src/pages/details/earn-page/components/select-yield-section/staked-via.tsxpackages/widget/src/pages/details/earn-page/components/select-yield-section/use-animate-yield-percent.tspackages/widget/src/pages/details/earn-page/state/earn-page-context.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsxpackages/widget/src/pages/details/earn-page/state/types.tspackages/widget/src/pages/details/earn-page/state/use-get-init-yield.tspackages/widget/src/pages/details/earn-page/state/use-get-token-balances-map.tspackages/widget/src/pages/details/earn-page/state/use-init-token.tspackages/widget/src/pages/details/earn-page/state/use-init-yield.tspackages/widget/src/pages/details/earn-page/state/use-pending-action-deep-link.tspackages/widget/src/pages/details/earn-page/state/use-stake-enter-request-dto.tspackages/widget/src/pages/details/earn-page/state/use-token-balance.tspackages/widget/src/pages/details/earn-page/state/use-track-state-events.tspackages/widget/src/pages/details/earn-page/state/utils.tspackages/widget/src/pages/details/earn-page/types.tspackages/widget/src/pages/details/positions-page/components/positions-list-item.tsxpackages/widget/src/pages/details/positions-page/hooks/use-position-list-item.tspackages/widget/src/pages/details/positions-page/hooks/use-positions.tspackages/widget/src/pages/position-details/components/amount-block.tsxpackages/widget/src/pages/position-details/components/position-balances.tsxpackages/widget/src/pages/position-details/components/provider-details.tsxpackages/widget/src/pages/position-details/components/static-action-block.tsxpackages/widget/src/pages/position-details/hooks/use-pending-actions.tspackages/widget/src/pages/position-details/hooks/use-position-details.tspackages/widget/src/pages/position-details/hooks/use-stake-exit-request-dto.tspackages/widget/src/pages/position-details/hooks/use-unstake-machine.tspackages/widget/src/pages/position-details/hooks/use-validator-addresses-handling.tspackages/widget/src/pages/position-details/hooks/utils.tspackages/widget/src/pages/position-details/position-details.page.tsxpackages/widget/src/pages/position-details/state/index.tsxpackages/widget/src/pages/position-details/state/types.tspackages/widget/src/pages/position-details/state/utils.tspackages/widget/src/pages/review/hooks/use-action-review.hook.tspackages/widget/src/pages/review/hooks/use-fees.tspackages/widget/src/pages/review/hooks/use-pending-review.hook.tspackages/widget/src/pages/review/hooks/use-stake-review.hook.tspackages/widget/src/pages/review/hooks/use-unstake-review.hook.tspackages/widget/src/pages/review/pages/action-review.page.tsxpackages/widget/src/pages/review/pages/common-page/common.page.tsxpackages/widget/src/pages/review/pages/common-page/components/review-top-section.tsxpackages/widget/src/pages/review/pages/pending-review.page.tsxpackages/widget/src/pages/review/pages/unstake-review.page.tsxpackages/widget/src/pages/steps/hooks/use-steps-machine.hook.tspackages/widget/src/pages/steps/hooks/use-steps.hook.tspackages/widget/src/pages/steps/pages/activity-steps.page.tsxpackages/widget/src/pages/steps/pages/common.page.tsxpackages/widget/src/pages/steps/pages/pending-steps.page.tsxpackages/widget/src/pages/steps/pages/stake-steps.page.tsxpackages/widget/src/pages/steps/pages/tx-state.tsxpackages/widget/src/pages/steps/pages/unstake-steps.page.tsxpackages/widget/src/providers/activity-provider/index.tsxpackages/widget/src/providers/api/get-enabled-networks.tspackages/widget/src/providers/cosmos/config.tspackages/widget/src/providers/enter-stake-store/index.tsxpackages/widget/src/providers/ethereum/config.tspackages/widget/src/providers/exit-stake-store/index.tsxpackages/widget/src/providers/index.tsxpackages/widget/src/providers/misc/solana-connector.tspackages/widget/src/providers/pending-action-store/index.tsxpackages/widget/src/providers/settings/types.tspackages/widget/src/providers/sk-wallet/errors.tspackages/widget/src/providers/sk-wallet/index.tsxpackages/widget/src/providers/sk-wallet/use-additional-addresses.tspackages/widget/src/providers/substrate/config.tspackages/widget/src/providers/wagmi/index.tspackages/widget/src/providers/yield-api-client-provider/actions.tspackages/widget/src/providers/yield-api-client-provider/index.tsxpackages/widget/src/providers/yield-api-client-provider/request-helpers.tspackages/widget/src/providers/yield-api-client-provider/types.tspackages/widget/src/translation/English/translations.jsonpackages/widget/src/translation/French/translations.jsonpackages/widget/src/types/yield-api-schema.d.tspackages/widget/src/utils/formatters.tspackages/widget/src/utils/index.tspackages/widget/src/utils/mappers.tspackages/widget/src/utils/region-iso-3166-codes.tspackages/widget/src/worker.tspackages/widget/tests/components/atoms/image.test.tsxpackages/widget/tests/components/atoms/token-icon-image.test.tsxpackages/widget/tests/fixtures/index.tspackages/widget/tests/mocks/api-routes.tspackages/widget/tests/use-cases/deep-links-flow/param-validation.test.tsxpackages/widget/tests/use-cases/deep-links-flow/setup.tspackages/widget/tests/use-cases/external-provider/setup.tspackages/widget/tests/use-cases/gas-warning-flow/gas-warning-flow.test.tsxpackages/widget/tests/use-cases/gas-warning-flow/setup.tspackages/widget/tests/use-cases/geo-block.test.tsxpackages/widget/tests/use-cases/renders-initial-page.test.tsxpackages/widget/tests/use-cases/select-opportunity.test.tsxpackages/widget/tests/use-cases/sk-wallet.test.tsxpackages/widget/tests/use-cases/staking-flow/setup.tspackages/widget/tests/use-cases/staking-flow/staking-flow.test.tsxpackages/widget/tests/use-cases/trust-incentive-apy/setup.tspackages/widget/tests/use-cases/trust-incentive-apy/trust-incentive-apy.test.tsxpackages/widget/tests/use-cases/under-maintenance.test.tsxpackages/widget/vite/vite.config.base.ts
💤 Files with no reviewable changes (7)
- packages/widget/src/hooks/navigation/use-dashboard-tabs-page-match.ts
- packages/widget/src/components/atoms/image-fallback/styles.css.ts
- packages/widget/src/hooks/use-base-token.ts
- packages/widget/src/components/atoms/image-fallback/index.tsx
- packages/widget/src/common/get-gas-mode-value.ts
- packages/widget/src/hooks/use-gas-fee-token.ts
- packages/widget/script.ts
| <Box | ||
| {...imgProps} | ||
| src={src ?? generatedFallbackSrc} | ||
| as="img" | ||
| onError={onError} |
There was a problem hiding this comment.
Treat empty src values as missing.
Line 41 only falls back for null/undefined. If a caller passes "" for a missing logo, this will render a broken image instead of the monogram fallback.
Suggested fix
- src={src ?? generatedFallbackSrc}
+ src={src?.trim() ? src : generatedFallbackSrc}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/src/components/atoms/image/index.tsx` around lines 39 - 43,
The src fallback only handles null/undefined but not empty strings, so update
the image props in the Box component (the src prop used with
generatedFallbackSrc) to treat empty or whitespace-only strings as missing;
replace the nullish fallback with a check like "use src if it is a non-empty
string (trimmed), otherwise use generatedFallbackSrc" so the monogram fallback
renders when callers pass "" or whitespace for src (also keep onError and
existing imgProps unchanged).
| const tokenMappingResult = Maybe.fromNullable(tokenIconMapping) | ||
| .chainNullable((mapping) => { | ||
| if (typeof mapping === "function") { | ||
| return mapping(token); | ||
| return mapping(token as Parameters<typeof mapping>[0]); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tokenIconMapping type and settings contract =="
rg -n -C4 '\btokenIconMapping\b' packages/widget/src
echo
echo "== TokenDto / YieldTokenDto definitions =="
fd 'tokens.ts' packages/widget/src/domain/types -x sed -n '1,240p' {}
echo
echo "== Hook implementation =="
fd 'use-variant-token-urls.ts' packages/widget/src -x sed -n '1,140p' {}Repository: stakekit/widget
Length of output: 7550
Update the settings contract to reflect that tokenIconMapping callbacks must handle both TokenDto and YieldTokenDto.
The cast at Line 42 (mapping(token as Parameters<typeof mapping>[0])) hides a type mismatch: the settings type defines tokenIconMapping callbacks to accept only TokenDto, but the hook now accepts TokenDto | YieldTokenDto. Integrators' callbacks written before YieldTokenDto support may fail at runtime when passed a yield token. Either widen the callback type in the settings contract to (token: TokenDto | YieldTokenDto) => string, or normalize the token shape before calling the callback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/widget/src/components/atoms/token-icon/token-icon-container/hooks/use-variant-token-urls.ts`
around lines 39 - 43, The tokenIconMapping callback is currently typed to accept
only TokenDto but use-variant-token-urls now passes TokenDto | YieldTokenDto
(see Maybe.fromNullable(...).chainNullable and the mapping(token as
Parameters<typeof mapping>[0]) cast); remove this unsafe cast by either widening
the settings contract so tokenIconMapping is declared as (token: TokenDto |
YieldTokenDto) => string, or normalize/convert a YieldTokenDto to the TokenDto
shape inside useVariantTokenUrls before calling mapping; update the
tokenIconMapping type in the settings contract and any call sites (or implement
a small normalizer in useVariantTokenUrls) so callbacks always receive the
declared token shape without runtime type mismatches.
| <Image | ||
| data-rk="token-logo" | ||
| containerProps={{ hw: tokenLogoHw, "data-rk": "token-logo" }} | ||
| wrapperProps={{ hw: tokenLogoHw, "data-rk": "token-logo" }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/widget/src/components/atoms -name "*.tsx" -o -name "*.ts" | grep -E "(token-icon-image|image)" | head -20Repository: stakekit/widget
Length of output: 344
🏁 Script executed:
cat -n packages/widget/src/components/atoms/token-icon/token-icon-image/index.tsxRepository: stakekit/widget
Length of output: 757
🏁 Script executed:
cat -n packages/widget/src/components/atoms/image/index.tsxRepository: stakekit/widget
Length of output: 2347
🏁 Script executed:
rg "Image" packages/widget/src/components/atoms -A 5 -B 1 | grep -E "(data-rk|Image|wrapperProps)" | head -40Repository: stakekit/widget
Length of output: 3604
🏁 Script executed:
fd "tsconfig" packages/widget | head -5Repository: stakekit/widget
Length of output: 125
🏁 Script executed:
cat packages/widget/tsconfig.json | head -30Repository: stakekit/widget
Length of output: 276
🏁 Script executed:
cat /dev/null | head -0 && cat ../../tsconfig.json | grep -E "(strict|jsx)" | head -10Repository: stakekit/widget
Length of output: 111
🏁 Script executed:
rg "data-rk" packages/widget/src/components/atoms/token-icon-image -A 3 -B 3Repository: stakekit/widget
Length of output: 153
🏁 Script executed:
cat tsconfig.json | grep -E "(strict|jsx)"Repository: stakekit/widget
Length of output: 103
🏁 Script executed:
rg "data-rk" packages/widget/src/components/atoms/token-icon -A 2 -B 2Repository: stakekit/widget
Length of output: 1247
🏁 Script executed:
rg "export const Image" packages/widget/src/components/atoms/image -A 20 | head -30Repository: stakekit/widget
Length of output: 1613
🏁 Script executed:
# Check if there are any component usages with extraneous props on Image
rg "<Image" packages/widget/src -A 5 | grep -E "(<Image|data-rk)" | head -20Repository: stakekit/widget
Length of output: 1570
Remove the extraneous data-rk prop from <Image>.
The prop on line 18 is not part of ImageProps and will fail strict TypeScript type-checking. The same attribute is already defined in wrapperProps on line 19.
Suggested fix
<Image
- data-rk="token-logo"
wrapperProps={{ hw: tokenLogoHw, "data-rk": "token-logo" }}
imgProps={{ hw: tokenLogoHw }}
src={mainUrl ?? fallbackUrl}
fallbackName={name}
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/src/components/atoms/token-icon/token-icon-image/index.tsx`
around lines 17 - 19, Remove the extraneous top-level prop named `data-rk`
passed to the `Image` component (the `Image` JSX in token-icon-image/index.tsx);
it is not part of `ImageProps` and duplicates the same `data-rk` already present
in `wrapperProps`. Edit the `Image` element to drop the `data-rk` attribute so
only `wrapperProps={{ hw: tokenLogoHw, "data-rk": "token-logo" }}` remains,
ensuring TypeScript strict props validation passes.
| <Box key={virtualItem.key} data-index={virtualItem.index}> | ||
| {itemContent(virtualItem.index, data[virtualItem.index])} | ||
| </Box> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does the @tanstack/react-virtualversion used by this repo still require wiringvirtualizer.measureElementthrough each row'sreftogether withdata-index for dynamically sized rows?
💡 Result:
Yes, the @tanstack/react-virtual library (latest version 3.13.24 as of April 2026) still requires wiring virtualizer.measureElement through each row's ref along with the data-index attribute for dynamically sized rows. This is the standard and recommended pattern for dynamic measurement in all official documentation, examples, and recent issues. Official TanStack Virtual docs explicitly state: "You are responsible for calling this [measureElement] in your virtualizer markup when the component is rendered (eg. using something like React's ref callback prop) also adding data-index" with example:
@tanstack/react-virtual) for dynamic row heights show identical pattern: <tr data-index={virtualRow.index} ref={(node) => rowVirtualizer.measureElement(node)} ... > Recent GitHub issues from 2025 (e.g., #1063, #1036 on v3.13.x) confirm this remains required, with bugs specifically referencing the need for data-index and measureElement. A 2024 PR (#737) added optional virtualItem.measureElement for special cases (e.g., Lit integration where node.isConnected is unreliable), but the primary API for React remains virtualizer.measureElement + data-index. No changelog entries or releases indicate this pattern has changed in v3.x up to 3.13.24. Blogs and tutorials from 2026 (e.g., viprasol.com) continue using this exact approach for dynamic rows.
Citations:
- 1: https://tanstack.com/table/alpha/docs/framework/react/examples/virtualized-rows-experimental
- 2: https://tanstack.com/table/v8/docs/framework/react/examples/virtualized-rows
- 3: why the change from virtualItem.measureElement -> virtualizer.measureElement + data-index attr? TanStack/virtual#617
- 4: https://tanstack.com/virtual/latest/docs/api/virtualizer
- 5: no measeure element height for useWindowVirtualizer() TanStack/virtual#664
- 6: https://tanstack.com/virtual/latest/docs/framework/react/examples
- 7: https://viprasol.com/blog/react-virtual-table/
- 8: https://www.npmjs.com/package/@tanstack/react-virtual
- 9: Measurements of virtual items don't get re-calculated when lanes are changed (with
measureElement) TanStack/virtual#1063 - 10: https://deepwiki.com/TanStack/virtual/4.2-variable-and-dynamic-sizing
Restore row measurement for variable-height items.
Lines 107 and 232 remove the only call site that lets TanStack Virtual capture actual row heights. measureElement is the required API for the virtualizer to track actual row heights; without it, getTotalSize() and item offsets remain estimate-based. When row heights differ from estimateSize, this produces mispositioned items and unreliable end-of-list detection. The current version (v3.13.24) requires wiring this ref with data-index on each rendered row.
Suggested fix
- <Box key={virtualItem.key} data-index={virtualItem.index}>
+ <Box
+ key={virtualItem.key}
+ data-index={virtualItem.index}
+ ref={rowVirtualizer.measureElement}
+ >
{itemContent(virtualItem.index, data[virtualItem.index])}
</Box>- <Box data-index={virtualItem.index} key={virtualItem.index}>
+ <Box
+ data-index={virtualItem.index}
+ key={virtualItem.index}
+ ref={rowVirtualizer.measureElement}
+ >
{type === "child"
? itemContent(item.index, item.parentIndex)
: groupContent(item.index)}
</Box>Also applies to: 232-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/src/components/atoms/virtual-list/index.tsx` around lines 107
- 109, The virtualizer lost its measureElement wiring so variable-height rows
are using estimates; restore the required ref on each rendered row so TanStack
Virtual can capture actual heights by passing the element and index to
measureElement. In the component that renders rows (where Box and virtualItem
are used and itemContent is called), add a ref that calls measureElement(el,
virtualItem.index) (or the equivalent measureElement API from your virtualizer
instance) on the Box rendering each virtualItem, keeping the existing data-index
attribute so the virtualizer can correlate elements to indexes.
| const getTokenGetTokensQueryKey = (params?: TokenGetTokensParams) => | ||
| ["/v1/tokens", ...(params ? [params] : [])] as const; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'getTokenGetTokensQueryKey|enabledYieldsOnly|queryKey:' packages/widget/src/hooks/api/use-default-tokens.ts -C2Repository: stakekit/widget
Length of output: 1040
🏁 Script executed:
cat -n packages/widget/src/hooks/api/use-default-tokens.tsRepository: stakekit/widget
Length of output: 2290
Include enabledYieldsOnly in the query key to avoid stale cache on setting changes.
The queryFn depends on enabledYieldsOnly (line 23), but the query key only varies by network. When toggling the tokensForEnabledYieldsOnly setting, the cache key remains identical, causing stale data to be returned. This affects both useDefaultTokens (line 18) and getDefaultTokens (line 35–37).
Proposed fix
-const getTokenGetTokensQueryKey = (params?: TokenGetTokensParams) =>
+const getTokenGetTokensQueryKey = (
+ params?: Pick<TokenGetTokensParams, "network" | "enabledYieldsOnly">
+) =>
["/v1/tokens", ...(params ? [params] : [])] as const;
return useQuery({
- queryKey: getTokenGetTokensQueryKey({ network: network ?? undefined }),
+ queryKey: getTokenGetTokensQueryKey({
+ network: network ?? undefined,
+ enabledYieldsOnly: !!tokensForEnabledYieldsOnly || undefined,
+ }),
queryFn: async () =>
(
await queryFn({
network: network ?? undefined,
enabledYieldsOnly: !!tokensForEnabledYieldsOnly,
})
).unsafeCoerce(),
@@
queryKey: getTokenGetTokensQueryKey({
network: params.network ?? undefined,
+ enabledYieldsOnly: params.enabledYieldsOnly || undefined,
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/src/hooks/api/use-default-tokens.ts` around lines 10 - 11,
The query key generator getTokenGetTokensQueryKey currently only includes
TokenGetTokensParams (network) so toggling tokensForEnabledYieldsOnly
(enabledYieldsOnly) doesn't change the key and returns stale cache; update
getTokenGetTokensQueryKey to include enabledYieldsOnly (or accept a params shape
that contains enabledYieldsOnly) so the key becomes ["/v1/tokens", params?,
enabledYieldsOnly] (or equivalent), and update both callers useDefaultTokens and
getDefaultTokens to pass the enabledYieldsOnly value into the key generation so
the query key varies when the setting changes.
| }) => { | ||
| const { t } = useTranslation(); | ||
|
|
||
| const nameOrAddress = providerDetails.name ?? providerDetails ?? ""; |
There was a problem hiding this comment.
Bug: providerDetails is an object, not a string.
The fallback providerDetails.name ?? providerDetails will result in [object Object] being used as nameOrAddress when name is undefined, since providerDetails is the destructured props object.
This should likely be:
🐛 Proposed fix
- const nameOrAddress = providerDetails.name ?? providerDetails ?? "";
+ const nameOrAddress = providerDetails.name ?? providerDetails.address ?? "";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const nameOrAddress = providerDetails.name ?? providerDetails ?? ""; | |
| const nameOrAddress = providerDetails.name ?? providerDetails.address ?? ""; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/src/pages/position-details/components/provider-details.tsx`
at line 42, The current assignment to nameOrAddress uses providerDetails
directly (const nameOrAddress = providerDetails.name ?? providerDetails ?? "";),
but providerDetails is an object so that falls back to "[object Object]"; change
the fallback to a scalar property (e.g., use providerDetails.address or
providerDetails.id) instead of the whole object — update the expression to
something like providerDetails.name ?? providerDetails.address ?? "" so
nameOrAddress is always a string; locate the constant nameOrAddress in
provider-details.tsx and replace the fallback accordingly.
| const actionPreviewQuery = useQuery({ | ||
| enabled: !!exitRequest, | ||
| queryKey: ["unstake-review-action-preview", exitRequest.requestDto], | ||
| retry: false, | ||
| queryFn: () => | ||
| createExitAction({ | ||
| fetchClient: yieldApiFetchClient, | ||
| requestDto: exitRequest.requestDto, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Handle action-preview failures explicitly.
This new preview query drives both stakeExitTxGas and the gas warning state, but its error path is ignored. If createExitAction fails, stakeExitTxGas becomes empty, gasCheckLoading flips to false, and the review can proceed with no fee/warning data instead of surfacing the failure.
Also applies to: 47-59, 185-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/src/pages/review/hooks/use-unstake-review.hook.ts` around
lines 36 - 45, The action-preview query (actionPreviewQuery) that calls
createExitAction currently ignores errors, which leaves stakeExitTxGas empty and
gasCheckLoading false so the review can proceed without fee/warning data; update
the queryFn/error handling to explicitly handle failures by capturing the error
(query's onError or error state), set a distinct error flag/state (e.g.,
actionPreviewError) and ensure stakeExitTxGas and gasCheckLoading reflect the
failed state (prevent proceeding or show an explicit error UI/message), and
propagate that error into any dependent logic that reads stakeExitTxGas or
gasCheckLoading (also apply the same explicit error handling pattern to the
other similar queries/usages referenced by the comment).
| if ( | ||
| recentConnectorId && | ||
| recentConnectorId === solanaWallet.adapter.name | ||
| ) { | ||
| await solanaWallet.adapter.autoConnect(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @solana/wallet-adapter(the version used in this repo), canautoConnect()reject, and is catching errors recommended for non-throwing auth checks likeisAuthorized?
💡 Result:
Yes, autoConnect can reject with a Promise that may throw WalletError subclasses (e.g., WalletNotConnectedError, WalletNotSelectedError, WalletConnectionError) based on wallet state and availability. Catching errors is recommended even for non-throwing auth checks like wallet connection status, as useWallet provides connected and publicKey booleans/states that are synchronous and reliable for authorization without async calls; however, any manual connect or autoConnect calls (e.g., in custom logic) should be wrapped in try-catch to handle uncaught promise rejections, as seen in GitHub issues where errors like WalletNotSelectedError occur without handling.
Citations:
- 1: https://github.com/solana-labs/wallet-adapter/blob/master/packages/core/base/src/adapter.ts
- 2: Uncaught (in promise) WalletNotSelectedError anza-xyz/wallet-adapter#743
- 3: WalletConnectionError: Unexpected error at StandardWalletAdapter._StandardWalletAdapter_connect anza-xyz/wallet-adapter#833
- 4: An internal error has occurred anza-xyz/wallet-adapter#613
- 5: https://github.com/solana-labs/wallet-adapter/blob/master/packages/core/react/src/WalletProvider.tsx
🏁 Script executed:
cat -n packages/widget/src/providers/misc/solana-connector.ts | head -150 | tail -60Repository: stakekit/widget
Length of output: 2131
Guard autoConnect() errors in isAuthorized
autoConnect() can reject with WalletError subclasses. Unhandled rejection causes isAuthorized to throw instead of returning false, breaking the reconnect/auth probe flow. Wrap the call in try-catch and fail closed.
Suggested fix
if (
recentConnectorId &&
recentConnectorId === solanaWallet.adapter.name
) {
- await solanaWallet.adapter.autoConnect();
+ if (!solanaWallet.adapter.connected) {
+ try {
+ await solanaWallet.adapter.autoConnect();
+ } catch {
+ return false;
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| recentConnectorId && | |
| recentConnectorId === solanaWallet.adapter.name | |
| ) { | |
| await solanaWallet.adapter.autoConnect(); | |
| } | |
| if ( | |
| recentConnectorId && | |
| recentConnectorId === solanaWallet.adapter.name | |
| ) { | |
| if (!solanaWallet.adapter.connected) { | |
| try { | |
| await solanaWallet.adapter.autoConnect(); | |
| } catch { | |
| return false; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/src/providers/misc/solana-connector.ts` around lines 111 -
116, In isAuthorized, guard the call to solanaWallet.adapter.autoConnect() so
any rejection (WalletError subclasses) is caught and handled: wrap the await
solanaWallet.adapter.autoConnect() invocation in a try/catch, return false (fail
closed) when an error is caught and optionally log the error; keep the existing
logic that only attempts autoConnect when recentConnectorId && recentConnectorId
=== solanaWallet.adapter.name.
| export const yieldApiYieldFixture = ( | ||
| overrides?: Partial<YieldApiYieldDto> | ||
| ): YieldApiYieldDto => | ||
| ({ | ||
| ...getYieldV2ControllerGetYieldByIdResponseMock(), | ||
| rewardRate: yieldRewardRateFixture(), | ||
| ...overrides, | ||
| }) as YieldApiYieldDto; |
There was a problem hiding this comment.
yieldApiYieldFixture is still returning a legacy payload shape.
This helper currently just rebrands getYieldV2ControllerGetYieldByIdResponseMock() with a cast, so fields like mechanics, inputTokens, outputToken, network, and providerId stay missing unless every caller remembers to patch them in. That makes the new fixture brittle and can hide schema drift in tests. Please build it from explicit Yield API defaults or route it through yieldApiYieldFixtureFromLegacy(...) instead of relying on the cast.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/tests/fixtures/index.ts` around lines 72 - 79,
yieldApiYieldFixture currently returns a legacy-shaped payload by simply casting
getYieldV2ControllerGetYieldByIdResponseMock(), leaving fields like mechanics,
inputTokens, outputToken, network, and providerId absent; replace the cast
approach by either (a) constructing the fixture from explicit Yield API defaults
(set mechanics, inputTokens, outputToken, network, providerId, rewardRate, etc.)
in the body of yieldApiYieldFixture, merging in overrides, or (b) delegate to
the existing helper yieldApiYieldFixtureFromLegacy(...) so the legacy mock is
normalized into the new schema before applying overrides; update
yieldApiYieldFixture to use one of these approaches and ensure all required
YieldApiYieldDto fields are present.
| const requestFn = vitest.fn(async ({ method }: { method: string }) => { | ||
| await waitForMs(100); | ||
|
|
||
| switch (method) { | ||
| case "eth_chainId": | ||
| return avalanche.id; | ||
| case "eth_requestAccounts": | ||
| return [account]; | ||
| default: | ||
| throw new Error("unhandled method"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's verify the file and lines mentioned
cd packages/widget/tests/use-cases/trust-incentive-apy
wc -l setup.tsRepository: stakekit/widget
Length of output: 72
🏁 Script executed:
# Read the specific lines in question
sed -n '345,355p' packages/widget/tests/use-cases/trust-incentive-apy/setup.tsRepository: stakekit/widget
Length of output: 363
🏁 Script executed:
# Let's also see broader context around this to understand the setup
sed -n '340,365p' packages/widget/tests/use-cases/trust-incentive-apy/setup.tsRepository: stakekit/widget
Length of output: 591
🏁 Script executed:
# Find the avalanche object definition to check what type avalanche.id is
rg "avalanche\s*=" packages/widget/tests/use-cases/trust-incentive-apy/setup.ts -A 5Repository: stakekit/widget
Length of output: 41
🏁 Script executed:
# Also search in related chain definition files
fd -i "chain\|avalanche" packages/widget/tests --type f | head -20Repository: stakekit/widget
Length of output: 41
🏁 Script executed:
# Search for avalanche import or definition in the setup file
head -100 packages/widget/tests/use-cases/trust-incentive-apy/setup.ts | rg -i "avalanche|import"Repository: stakekit/widget
Length of output: 457
🏁 Script executed:
# Search in the entire packages/widget/tests directory
rg "export.*avalanche|const avalanche" packages/widget/tests --type ts -B 2 -A 2Repository: stakekit/widget
Length of output: 3131
🏁 Script executed:
# Check for common chain definitions, including avalanche
rg "avalanche|chainId" packages/widget/tests/use-cases/trust-incentive-apy/setup.ts | head -20Repository: stakekit/widget
Length of output: 329
🌐 Web query:
JSON-RPC eth_chainId response format EIP-1193 specification
💡 Result:
The JSON-RPC eth_chainId method returns the current chain ID as a hexadecimal string (e.g., "0x1" for Ethereum mainnet). This is defined in EIP-695 and referenced in EIP-1193 for Ethereum Provider JavaScript APIs. Example JSON-RPC response: { "jsonrpc": "2.0", "id": 1, "result": "0x1" } In EIP-1193 providers (e.g., window.ethereum), calling provider.request({ method: 'eth_chainId' }) returns the same hexadecimal string. The chainId property and chainChanged event also use this hex string format per eth_chainId.
Citations:
- 1: https://www.quicknode.com/docs/ethereum/eth_chainId
- 2: https://docs.chainrpc.io/chainrpc/networks/ethereum/json-rpc-methods/eth_chainid
- 3: https://eips.ethereum.org/EIPS/eip-695
- 4: https://docs.metamask.io/services/reference/ethereum/json-rpc-methods/eth_chainid/
- 5: https://eips.ethereum.org/EIPS/eip-1193
- 6: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1193.md
Return a hex chain id from the wallet mock.
eth_chainId responses are hex strings per the JSON-RPC specification (EIP-695, EIP-1193), not numbers. The mock currently returns avalanche.id as a numeric value, which differs from production wallets and can hide parsing bugs in the connector flow.
Suggested fix
switch (method) {
case "eth_chainId":
- return avalanche.id;
+ return `0x${avalanche.id.toString(16)}`;
case "eth_requestAccounts":
return [account];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const requestFn = vitest.fn(async ({ method }: { method: string }) => { | |
| await waitForMs(100); | |
| switch (method) { | |
| case "eth_chainId": | |
| return avalanche.id; | |
| case "eth_requestAccounts": | |
| return [account]; | |
| default: | |
| throw new Error("unhandled method"); | |
| } | |
| const requestFn = vitest.fn(async ({ method }: { method: string }) => { | |
| await waitForMs(100); | |
| switch (method) { | |
| case "eth_chainId": | |
| return `0x${avalanche.id.toString(16)}`; | |
| case "eth_requestAccounts": | |
| return [account]; | |
| default: | |
| throw new Error("unhandled method"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/widget/tests/use-cases/trust-incentive-apy/setup.ts` around lines
345 - 355, The wallet mock `requestFn` currently returns `avalanche.id` (a
number) for the "eth_chainId" case; update the "eth_chainId" branch in the
`requestFn` (the vitest.fn mock) to return a hex string per JSON-RPC (e.g.
convert `avalanche.id` to a "0x..." hex string) instead of a numeric value so
the connector receives the same format as real wallets.
petar-omni
force-pushed
the
feat/yield-balances-migration
branch
from
dfbc459 to
bf2f956
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mise.toml (1)
2-3: Addengines.nodeto package.json to enforce Node version outsidemiseNode 24 is pinned in
mise.tomland CI, but contributors who bypassmise(or use different package managers) won't have this constraint enforced. Addengines.nodetopackage.jsonfor consistency across all tooling paths.Proposed change
{ "packageManager": "pnpm@10.33.2", + "engines": { + "node": "24.x" + } }After applying this change, run
npm run lint(orpnpm lint) locally to confirm no regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mise.toml` around lines 2 - 3, The repo pins Node 24 in mise.toml but lacks a corresponding engines.node entry in package.json; add an "engines": { "node": ">=24 <25" } (or the exact range you want) to package.json to enforce Node version for npm/pnpm users, commit that change, and run the linter (npm run lint or pnpm lint) to ensure no lint/regression issues; locate package.json and update its top-level "engines" field (or add it if missing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mise.toml`:
- Around line 2-3: The repo pins Node 24 in mise.toml but lacks a corresponding
engines.node entry in package.json; add an "engines": { "node": ">=24 <25" } (or
the exact range you want) to package.json to enforce Node version for npm/pnpm
users, commit that change, and run the linter (npm run lint or pnpm lint) to
ensure no lint/regression issues; locate package.json and update its top-level
"engines" field (or add it if missing).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67a6cca9-3034-4aed-b846-34d0bde63fb8
📒 Files selected for processing (2)
mise.tomlpackage.json
✅ Files skipped from review due to trivial changes (1)
- package.json
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores