Convert op, optimistic commenting fixes #613
Conversation
📝 WalkthroughWalkthroughAdds a Convert wallet operation and related UI/translation, reworks React Query cache keys for discussions, removes legacy wallet cache migration logic, centralizes SDK base URL resolution via ConfigManager.getValidatedBaseUrl, adds convertHbd mutation and wiring, and bumps package versions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletOp as Wallet Operation Dispatcher
participant Convert as convertHbd
participant Auth as Auth Context / Broadcaster
participant HiveClient
participant HiveSigner
User->>WalletOp: Trigger Convert action
WalletOp->>Convert: call convertHbd(payload, auth?)
alt payload.type == "key"
Convert->>HiveClient: broadcast via sendOperations(key)
HiveClient-->>Convert: response
else payload.type == "keychain" or "hiveauth"
Convert->>Auth: use broadcaster to broadcast(operation,"active")
alt broadcaster available
Auth-->>Convert: response
else no broadcaster
Convert-->>WalletOp: throw error
end
else (default: hive-signer)
Convert->>HiveSigner: sendOperation(operation, callbackUrl)
HiveSigner-->>User: redirect to wallet for approval
User-->>Convert: completes via callback
end
Convert-->>WalletOp: result
WalletOp-->>User: surface outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/api/mutations/create-reply.ts (1)
6-6: Remove unused import.
QueryIdentifiersis imported on line 6 but never used in the file. The code now uses explicit string-based keys instead.
🤖 Fix all issues with AI agents
In
@packages/sdk/src/modules/accounts/queries/get-referrals-infinite-query-options.ts:
- Around line 13-14: The URL construction can throw when CONFIG.privateApiHost
is falsy and window is undefined because baseUrl becomes an empty string; update
the logic around baseUrl and the new URL(...) call in
get-referrals-infinite-query-options.ts to guard against an empty baseUrl (e.g.,
compute a validated baseUrl or throw a descriptive error before calling new URL)
and include the same guard wherever the pattern repeats
(get-waves-by-tag-query-options.ts, get-waves-following-query-options.ts, etc.);
centralize this into a shared helper function (e.g., getValidatedBaseUrl or
resolveApiBaseUrl) that checks CONFIG.privateApiHost, falls back to
window.location.origin when available, and otherwise throws a clear error so new
URL(...) is never called with an empty base.
In
@packages/sdk/src/modules/posts/queries/get-waves-trending-tags-query-options.ts:
- Around line 15-16: The code builds `baseUrl` then calls `new
URL("/private-api/waves/trending/tags", baseUrl)`, but when
CONFIG.privateApiHost is falsy and window is undefined `baseUrl` becomes an
empty string and `new URL()` throws; update the logic around the `baseUrl`/`url`
creation to ensure a valid base is used on server side (e.g., if
CONFIG.privateApiHost is falsy and typeof window === 'undefined' fallback to a
safe origin like 'http://localhost' or construct an absolute URL elsewhere),
validate `baseUrl` before calling `new URL()`, and use the same variable names
(`baseUrl`, `CONFIG.privateApiHost`, `window`, `url`, `new URL`) so the fix is
easy to locate.
🧹 Nitpick comments (4)
packages/sdk/src/modules/posts/queries/get-waves-following-query-options.ts (1)
26-27: SamebaseUrlfallback issue, though mitigated by try-catch.The empty string fallback for
baseUrlwould throw when constructing the URL. Here, the try-catch block (lines 25-63) catches the error and returns an empty array, preventing a crash. However, this silently hides potential configuration issues in SSR environments.The console.error on line 61 will log the error, but consider adding explicit validation for better debuggability, especially since this pattern is duplicated across multiple query options files.
packages/sdk/src/modules/posts/queries/get-waves-by-tag-query-options.ts (1)
21-22: Consider extracting the baseUrl initialization into a shared utility function.The URL construction pattern is duplicated across 4 files:
get-waves-by-tag-query-options.tsget-waves-following-query-options.tsget-waves-trending-tags-query-options.tsget-referrals-infinite-query-options.tsThe try-catch block (lines 20-51) properly handles errors. If this pattern is intentional and likely to be reused elsewhere, extracting it into a utility function would reduce duplication and make future updates easier.
packages/wallets/src/modules/assets/hive/mutations/convert.ts (2)
26-31: Redundant operation payload reconstruction.The
operationvariable is already constructed at line 24 but the "key" path reconstructs it inline at line 29. This duplicates logic and can lead to inconsistencies if one is updated without the other.Proposed refactor to reuse operation variable
if (payload.type === "key" && "key" in payload) { - const { key, type, ...params } = payload; - return CONFIG.hiveClient.broadcast.sendOperations( - [["convert", { ...params, owner: params.from, requestid }]], - key - ); + const { key } = payload; + return CONFIG.hiveClient.broadcast.sendOperations([operation], key); } else if (payload.type === "keychain" || payload.type === "hiveauth") {
40-42: Hardcoded callback URL and empty callback handler.The callback URL is hardcoded to
https://ecency.com/@${payload.from}/wallet. Consider using a configurable base URL from CONFIG for portability across environments. Additionally, the empty callback function() => {}silently ignores the result—consider adding error handling or at least logging.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
packages/sdk/dist/browser/index.jsis excluded by!**/dist/**packages/sdk/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.cjsis excluded by!**/dist/**packages/sdk/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.mjsis excluded by!**/dist/**packages/sdk/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/browser/index.d.tsis excluded by!**/dist/**packages/wallets/dist/browser/index.jsis excluded by!**/dist/**packages/wallets/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.cjsis excluded by!**/dist/**packages/wallets/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.mjsis excluded by!**/dist/**packages/wallets/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (16)
apps/web/src/api/mutations/create-reply.tsapps/web/src/api/mutations/delete-comment.tsapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-operation-helpers.tsxapps/web/src/app/client-init.tsxapps/web/src/features/i18n/locales/en-US.jsonapps/web/src/features/shared/discussion/discussion-item.tsxapps/web/src/features/wallet/wallet-operations-dialog.tsxpackages/sdk/package.jsonpackages/sdk/src/modules/accounts/queries/get-referrals-infinite-query-options.tspackages/sdk/src/modules/posts/queries/get-waves-by-tag-query-options.tspackages/sdk/src/modules/posts/queries/get-waves-following-query-options.tspackages/sdk/src/modules/posts/queries/get-waves-trending-tags-query-options.tspackages/wallets/package.jsonpackages/wallets/src/modules/assets/hive/mutations/convert.tspackages/wallets/src/modules/assets/hive/mutations/index.tspackages/wallets/src/modules/wallets/mutations/wallet-operation.ts
💤 Files with no reviewable changes (1)
- apps/web/src/app/client-init.tsx
🧰 Additional context used
🧬 Code graph analysis (9)
packages/sdk/src/modules/accounts/queries/get-referrals-infinite-query-options.ts (3)
packages/sdk/dist/node/index.mjs (14)
baseUrl(1027-1027)baseUrl(2118-2118)baseUrl(2156-2156)baseUrl(2194-2194)url(1028-1028)url(1191-1193)url(2119-2119)url(2157-2157)url(2195-2195)url(4023-4023)url(4061-4061)url(4070-4070)url(5040-5042)url(5060-5060)packages/sdk/dist/node/index.cjs (6)
baseUrl(1052-1052)baseUrl(2143-2143)baseUrl(2181-2181)baseUrl(2219-2219)url(1053-1053)url(1216-1218)packages/sdk/dist/browser/index.js (4)
url(4061-4061)url(4070-4070)url(5040-5042)url(5060-5060)
packages/sdk/src/modules/posts/queries/get-waves-following-query-options.ts (3)
packages/sdk/dist/node/index.mjs (14)
baseUrl(1027-1027)baseUrl(2118-2118)baseUrl(2156-2156)baseUrl(2194-2194)url(1028-1028)url(1191-1193)url(2119-2119)url(2157-2157)url(2195-2195)url(4023-4023)url(4061-4061)url(4070-4070)url(5040-5042)url(5060-5060)packages/sdk/dist/node/index.cjs (6)
baseUrl(1052-1052)baseUrl(2143-2143)baseUrl(2181-2181)baseUrl(2219-2219)url(1053-1053)url(1216-1218)packages/sdk/dist/browser/index.js (4)
url(4061-4061)url(4070-4070)url(5040-5042)url(5060-5060)
apps/web/src/features/wallet/wallet-operations-dialog.tsx (2)
packages/wallets/dist/browser/index.d.ts (1)
AssetOperation(2031-2031)packages/wallets/dist/index.d.ts (1)
AssetOperation(1643-1643)
apps/web/src/features/shared/discussion/discussion-item.tsx (1)
packages/sdk/dist/browser/index.d.ts (1)
SortOrder(3181-3181)
apps/web/src/api/mutations/delete-comment.ts (2)
packages/sdk/src/modules/posts/types/entry.ts (1)
Entry(46-89)packages/sdk/dist/browser/index.d.ts (1)
SortOrder(3181-3181)
packages/sdk/src/modules/posts/queries/get-waves-by-tag-query-options.ts (2)
packages/sdk/dist/node/index.cjs (6)
baseUrl(1052-1052)baseUrl(2143-2143)baseUrl(2181-2181)baseUrl(2219-2219)url(1053-1053)url(1216-1218)packages/sdk/dist/browser/index.js (4)
url(4061-4061)url(4070-4070)url(5040-5042)url(5060-5060)
packages/sdk/src/modules/posts/queries/get-waves-trending-tags-query-options.ts (2)
packages/sdk/dist/node/index.mjs (14)
baseUrl(1027-1027)baseUrl(2118-2118)baseUrl(2156-2156)baseUrl(2194-2194)url(1028-1028)url(1191-1193)url(2119-2119)url(2157-2157)url(2195-2195)url(4023-4023)url(4061-4061)url(4070-4070)url(5040-5042)url(5060-5060)packages/sdk/dist/node/index.cjs (6)
baseUrl(1052-1052)baseUrl(2143-2143)baseUrl(2181-2181)baseUrl(2219-2219)url(1053-1053)url(1216-1218)
packages/wallets/src/modules/assets/hive/mutations/convert.ts (1)
packages/wallets/dist/node/index.cjs (8)
auth(400-400)requestid(1581-1581)operationPayload(1310-1315)operationPayload(1338-1344)operationPayload(1366-1371)operationPayload(1393-1397)key(411-411)key(416-416)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-operation-helpers.tsx (1)
packages/wallets/dist/browser/index.d.ts (1)
AssetOperation(2031-2031)
🔇 Additional comments (16)
packages/wallets/package.json (1)
4-4: LGTM!Version bump aligns with the new convert mutation feature and related wallet operation changes.
packages/sdk/package.json (1)
4-4: LGTM!Version bump aligns with the URL construction improvements in the SDK query options.
packages/wallets/src/modules/assets/hive/mutations/index.ts (1)
10-10: LGTM!The new export follows the established barrel export pattern in this file.
packages/wallets/src/modules/wallets/mutations/wallet-operation.ts (2)
23-24: LGTM on the import.The
convertHbdimport is correctly added to support the new Convert operation.
40-46: Asset refresh logic is correct - no changes needed.The Convert operation correctly refreshes both HBD and HIVE balances in the
onSuccesshandler. Since converting HBD produces HIVE, the current implementation at lines 130-131 properly handles both asset refreshes.apps/web/src/features/wallet/wallet-operations-dialog.tsx (1)
106-117: LGTM with note on balance handling for Convert operationThe addition of
AssetOperation.Convertto the operations list and its exclusion from theshowMemorequirement are both correct. Convert operations don't require a memo field per the HBD conversion operation specification.However, the WalletOperationsTransfer component's balance calculation for Convert is worth reviewing: Convert operations currently fall through to the default balance case (total account balance) rather than explicitly using liquidBalance like Transfer and PowerUp operations. Since Convert converts HBD, it should likely display the liquid HBD balance. This may work functionally but could display incorrect balance amounts to users.
apps/web/src/features/i18n/locales/en-US.json (1)
3036-3036: LGTM!The new translation entry for the Convert operation follows the existing naming pattern and is consistent with other wallet operation translations.
apps/web/src/api/mutations/create-reply.ts (4)
68-81: LGTM!The cache key update is consistent with the new composite key pattern. The blockchain success handler correctly replaces the optimistic entry and maintains proper cache state.
91-102: LGTM!Error handling correctly removes the optimistic entry from cache using the same key structure, maintaining consistency.
146-156: LGTM!The
onMutateoptimistic update uses the same key pattern, ensuring cache operations are aligned across the mutation lifecycle.
172-183: LGTM!The
onErrorhandler appropriately uses optional chaining foractiveUser?.usernamesince this is an error recovery path where activeUser state might be uncertain.apps/web/src/api/mutations/delete-comment.ts (2)
9-9: LGTM!The
SortOrderimport is correctly added to support the new composite cache key structure.
33-57: Cache key alignment is correct despite confusing parameter naming.The
parentparameter passed touseDeleteCommentin thediscussion-itemcontext is actually the root post (parent={root}), so the cache key construction correctly targets the same discussion thread cache used bycreate-reply.tsanddiscussion-item.tsx. No fix is required.apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-operation-helpers.tsx (2)
14-14: LGTM!The new icon mappings are well-chosen:
UilUserMinussemantically represents undelegation (removing a user relationship)UilArrowsHAltfor Convert aligns with Swap, as both operations involve asset transformationAlso applies to: 28-30
41-55: LGTM!The label function correctly handles the new
Convertoperation via the fallback path, which resolves toprofile-wallet.operations.convert- matching the translation added inen-US.json.apps/web/src/features/shared/discussion/discussion-item.tsx (1)
175-182: Cache key mismatch between sort order in parent component and hardcoded sort in cache read.The
discussion-item.tsxhardcodesSortOrder.createdin the cache key, but the parentDiscussioncomponent can change the sort order dynamically viasetOrder(). When a user changes the sort order, the query cache key changes (e.g., toSortOrder.trending), butdiscussion-item.tsxstill attempts to read cache with the hardcodedSortOrder.created, resulting in cache misses.The username fallback (
activeUser?.username ?? root.author) is intentional and working correctly—it matches the fallback behavior ingetDiscussionsQueryOptionswhich usesobserver || entry?.author.Likely an incorrect or invalid review comment.
packages/sdk/src/modules/accounts/queries/get-referrals-infinite-query-options.ts
Outdated
Show resolved
Hide resolved
packages/sdk/src/modules/posts/queries/get-waves-trending-tags-query-options.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sdk/src/modules/hive-engine/requests.ts (2)
28-29: Consider usingnew URL()for consistency.This uses string interpolation (
${baseUrl}/private-api/engine-api) while other endpoints in this file use thenew URL()constructor (e.g., lines 285, 311). For consistency and safer URL handling, consider aligning with thenew URL()pattern.♻️ Optional refactor for consistency
async function engineRpc<T>(payload: Record<string, unknown>): Promise<T> { const fetchApi = getBoundFetch(); const baseUrl = ConfigManager.getValidatedBaseUrl(); - const response = await fetchApi(`${baseUrl}/private-api/engine-api`, { + const url = new URL("/private-api/engine-api", baseUrl); + const response = await fetchApi(url.toString(), { method: "POST", body: JSON.stringify(payload), headers: ENGINE_RPC_HEADERS, });
332-334: Consider URL-encoding the username parameter.The
usernameis interpolated directly into the URL path without encoding. While Hive usernames typically follow strict character rules, defensively encoding would prevent issues with edge cases.♻️ Safer URL construction
export async function getHiveEngineUnclaimedRewards<T = Record<string, unknown>>( username: string ): Promise<Record<string, T>> { const fetchApi = getBoundFetch(); const baseUrl = ConfigManager.getValidatedBaseUrl(); + const url = new URL(`/private-api/engine-reward-api/${encodeURIComponent(username)}`, baseUrl); + url.searchParams.set("hive", "1"); const response = await fetchApi( - `${baseUrl}/private-api/engine-reward-api/${username}?hive=1` + url.toString() );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/sdk/dist/browser/index.d.tsis excluded by!**/dist/**packages/sdk/dist/browser/index.jsis excluded by!**/dist/**packages/sdk/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.cjsis excluded by!**/dist/**packages/sdk/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.mjsis excluded by!**/dist/**packages/sdk/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (7)
packages/sdk/src/modules/accounts/queries/get-referrals-infinite-query-options.tspackages/sdk/src/modules/core/config.tspackages/sdk/src/modules/hive-engine/requests.tspackages/sdk/src/modules/posts/queries/get-promoted-posts-query-options.tspackages/sdk/src/modules/posts/queries/get-waves-by-tag-query-options.tspackages/sdk/src/modules/posts/queries/get-waves-following-query-options.tspackages/sdk/src/modules/posts/queries/get-waves-trending-tags-query-options.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/sdk/src/modules/posts/queries/get-waves-trending-tags-query-options.ts
- packages/sdk/src/modules/accounts/queries/get-referrals-infinite-query-options.ts
🧰 Additional context used
🧬 Code graph analysis (4)
packages/sdk/src/modules/core/config.ts (1)
packages/sdk/dist/browser/index.d.ts (1)
CONFIG(3194-3194)
packages/sdk/src/modules/posts/queries/get-promoted-posts-query-options.ts (3)
packages/sdk/dist/node/index.mjs (19)
baseUrl(1037-1037)baseUrl(1201-1201)baseUrl(2127-2127)baseUrl(2165-2165)baseUrl(2203-2203)baseUrl(4844-4844)baseUrl(5050-5050)baseUrl(5069-5069)baseUrl(5085-5085)url(1038-1038)url(1202-1202)url(2128-2128)url(2166-2166)url(2204-2204)url(4032-4032)url(4070-4070)url(4079-4079)url(5051-5051)url(5070-5070)packages/sdk/dist/browser/index.js (8)
baseUrl(4844-4844)baseUrl(5050-5050)baseUrl(5069-5069)baseUrl(5085-5085)url(4070-4070)url(4079-4079)url(5051-5051)url(5070-5070)packages/sdk/dist/node/index.cjs (5)
baseUrl(1062-1062)baseUrl(1226-1226)baseUrl(2152-2152)url(1063-1063)url(1227-1227)
packages/sdk/src/modules/posts/queries/get-waves-following-query-options.ts (2)
packages/sdk/dist/browser/index.js (8)
baseUrl(4844-4844)baseUrl(5050-5050)baseUrl(5069-5069)baseUrl(5085-5085)url(4070-4070)url(4079-4079)url(5051-5051)url(5070-5070)packages/sdk/dist/node/index.cjs (5)
baseUrl(1062-1062)baseUrl(1226-1226)baseUrl(2152-2152)url(1063-1063)url(1227-1227)
packages/sdk/src/modules/hive-engine/requests.ts (4)
packages/sdk/dist/node/index.mjs (19)
baseUrl(1037-1037)baseUrl(1201-1201)baseUrl(2127-2127)baseUrl(2165-2165)baseUrl(2203-2203)baseUrl(4844-4844)baseUrl(5050-5050)baseUrl(5069-5069)baseUrl(5085-5085)url(1038-1038)url(1202-1202)url(2128-2128)url(2166-2166)url(2204-2204)url(4032-4032)url(4070-4070)url(4079-4079)url(5051-5051)url(5070-5070)packages/sdk/dist/browser/index.js (8)
baseUrl(4844-4844)baseUrl(5050-5050)baseUrl(5069-5069)baseUrl(5085-5085)url(4070-4070)url(4079-4079)url(5051-5051)url(5070-5070)packages/sdk/dist/node/index.cjs (5)
baseUrl(1062-1062)baseUrl(1226-1226)baseUrl(2152-2152)url(1063-1063)url(1227-1227)packages/sdk/dist/browser/index.d.ts (1)
ConfigManager(3194-3194)
🔇 Additional comments (5)
packages/sdk/src/modules/posts/queries/get-waves-following-query-options.ts (1)
2-2: LGTM! Consistent adoption of centralized base URL resolution.The change from direct CONFIG usage to
ConfigManager.getValidatedBaseUrl()aligns with the broader refactor across the SDK. Thenew URL()constructor properly handles path resolution against the base URL.Also applies to: 26-27
packages/sdk/src/modules/posts/queries/get-waves-by-tag-query-options.ts (1)
2-2: LGTM! Consistent with the centralized URL resolution pattern.The migration to
ConfigManager.getValidatedBaseUrl()follows the same pattern as other waves-related query options, maintaining consistency across the SDK.Also applies to: 21-22
packages/sdk/src/modules/posts/queries/get-promoted-posts-query-options.ts (1)
1-1: LGTM! Consistent migration to ConfigManager.The import update and URL construction follow the established pattern across the SDK modules.
Also applies to: 11-12
packages/sdk/src/modules/hive-engine/requests.ts (1)
284-285: LGTM! Proper URL construction withnew URL().Both
getHiveEngineTokenTransactionsandgetHiveEngineTokenMetricscorrectly use thenew URL()constructor andsearchParams.set()for query parameters, which properly handles URL encoding.Also applies to: 310-311
packages/sdk/src/modules/core/config.ts (1)
67-78: Review comment is incorrect—CONFIG.privateApiHost is set to an empty string in production.The review overlooks that
setPrivateApiHost("")is called in production environments (apps/web/src/core/sdk-init.ts, line 22-23) based onisMainProductionlogic. An empty string is falsy in JavaScript, so the conditionif (CONFIG.privateApiHost)on line 68 evaluates to false in production, allowing thewindow.location.originand SSR fallback branches to execute as intended.The fallback logic is working correctly and is reachable in production scenarios.
Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.