Conversation
|
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 (4)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAllows broadcasts to try a direct API broadcast with a provided or prefetched access token first; if no token or the token-based broadcast fails with an auth-related error, the flow may fall back to an adapter-provided Changes
Sequence DiagramsequenceDiagram
participant Client
participant Broadcast as Broadcast Mutation
participant API as Direct API
participant Adapter as Platform Adapter
participant HS as HiveSigner
Client->>Broadcast: broadcastTransaction(ops, keyType)
alt Token present or prefetched
Broadcast->>API: attemptDirectBroadcast(token, ops)
alt Direct broadcast succeeds
API-->>Broadcast: TransactionConfirmation
Broadcast-->>Client: Success
else Direct broadcast fails (auth)
API--XBroadcast: Auth Error
alt Adapter supports HiveSigner
Broadcast->>Adapter: broadcastWithHiveSigner(username, ops, keyType)
Adapter->>HS: open HiveSigner UI
HS-->>Adapter: TransactionConfirmation
Adapter-->>Broadcast: TransactionConfirmation
Broadcast-->>Client: Success
end
end
else No token available
alt Adapter supports HiveSigner
Broadcast->>Adapter: broadcastWithHiveSigner(username, ops, keyType)
Adapter->>HS: open HiveSigner UI
HS-->>Adapter: TransactionConfirmation
Adapter-->>Broadcast: TransactionConfirmation
Broadcast-->>Client: Success
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/sdk/src/modules/core/mutations/use-broadcast-mutation.ts (1)
370-377: Minor inefficiency: redundant token fetch when token is unavailable.When
getAccessTokenreturnsnull/undefined,prefetchedTokenremainsundefined. InbroadcastWithMethod(line 105-107),fetchedToken !== undefinedcauses a second call togetAccessToken.Consider storing the result unconditionally to signal "already checked":
♻️ Suggested fix to avoid redundant fetch
// Pre-fetch token if available (will be reused in broadcast) const token = await adapter.getAccessToken(username); - if (token) { - prefetchedToken = token; // Store for reuse - } + prefetchedToken = token ?? null; // Store result (including null) to avoid re-fetch // When no token but adapter exists, don't skip — // broadcastWithMethod can fall back to adapter.broadcastWithHiveSigner🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/modules/core/mutations/use-broadcast-mutation.ts` around lines 370 - 377, The current flow redundantly calls adapter.getAccessToken again in broadcastWithMethod when the initial check returned null/undefined because prefetchedToken is only set when a token exists; instead always assign the result of adapter.getAccessToken(username) to prefetchedToken (even if null) in the pre-fetch block so broadcastWithMethod can inspect prefetchedToken to know the adapter has already been queried; update the logic around prefetchedToken and the pre-fetching code that calls adapter.getAccessToken to store the result unconditionally (references: adapter.getAccessToken, prefetchedToken, broadcastWithMethod).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/sdk/src/modules/core/mutations/use-broadcast-mutation.ts`:
- Around line 370-377: The current flow redundantly calls adapter.getAccessToken
again in broadcastWithMethod when the initial check returned null/undefined
because prefetchedToken is only set when a token exists; instead always assign
the result of adapter.getAccessToken(username) to prefetchedToken (even if null)
in the pre-fetch block so broadcastWithMethod can inspect prefetchedToken to
know the adapter has already been queried; update the logic around
prefetchedToken and the pre-fetching code that calls adapter.getAccessToken to
store the result unconditionally (references: adapter.getAccessToken,
prefetchedToken, broadcastWithMethod).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18b8392b-65a9-41f0-aaa3-748150b10cfe
⛔ Files ignored due to path filters (6)
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 (1)
packages/sdk/src/modules/core/mutations/use-broadcast-mutation.ts
Summary by CodeRabbit