fix: complete cleanup of PR #12 and PR #13#23
Conversation
Staged bugfixes applied: - sorobanTx.ts: use correct Horizon.Server and SorobanServer from rpc subpath - useAssignDispute.ts: use drawDispute (plugin shape) instead of non-existent assignDispute - BalanceCard.tsx: destructure isLoading/refetch from useTokenBalance; fix displayBalance shadowing - FaucetButton.tsx: use requestTokens/isRequesting from useFaucet hook - auth route: fix tokenType to 'signup' and comment cleanup - EvidenceView.tsx, PendingExecutionsDialog.tsx, PendingPaymentsDialog.tsx: add explicit any casts to fix implicit-any TS errors - DisputeListView.tsx: use Number(d.status) for robust status comparison - manage/page.tsx, my-votes/page.tsx: add explicit any on filter callbacks to fix TS BlockchainBalance: removed duplicate loading? field, keep only isLoading BlockchainHooks: added optional useSubmitEvidence hook type mock plugin: implemented useSubmitEvidence returning toast + true hooks.ts: added useSubmitEvidence proxy export with nullish fallback blockchain/types.ts: added Dispute interface export; hooks.ts re-exports it stellarPlugin withdraw: changed toast.info to toast.error with clearer message stellar-registry: removed unused dependency from contracts/slice/Cargo.toml storage.rs: added TODO comment above set_draft_queue for persistent storage migration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR refactors the blockchain plugin type contracts by simplifying the ChangesBlockchain Plugin Refactor and Evidence Submission
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/app/(evidence)/manage/page.tsxOops! Something went wrong! :( ESLint: 9.39.2 TypeError: Converting circular structure to JSON ... [truncated 453 characters] ... c/dist/eslintrc.cjs:3261:25) src/app/(voting)/my-votes/page.tsxOops! Something went wrong! :( ESLint: 9.39.2 TypeError: Converting circular structure to JSON ... [truncated 453 characters] ... c/dist/eslintrc.cjs:3261:25) src/app/api/auth/passkey/authenticate/route.tsOops! Something went wrong! :( ESLint: 9.39.2 TypeError: Converting circular structure to JSON ... [truncated 453 characters] ... c/dist/eslintrc.cjs:3261:25)
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.
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 (3)
src/hooks/soroban/useAssignDispute.ts (1)
11-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDead parameter:
categoryis accepted but never used.The
assignDisputefunction signature acceptscategory: string, but Line 31 only passesstakeAmounttodrawDispute. Thecategoryparameter has no effect on the operation.Either remove
categoryfrom the signature or pass it to the underlying hook if it's needed.🔧 Option A: Remove unused parameter
interface UseAssignDisputeReturn { - assignDispute: (category: string, stakeAmount: number | bigint) => Promise<boolean>; + assignDispute: (stakeAmount: number | bigint) => Promise<boolean>; isAssigning: boolean; } ... const assignDispute = useCallback( - async (category: string, stakeAmount: number | bigint): Promise<boolean> => { + async (stakeAmount: number | bigint): Promise<boolean> => {Also applies to: 20-21, 31-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/soroban/useAssignDispute.ts` around lines 11 - 14, The category parameter in the assignDispute function signature is not being used anywhere in the implementation. Remove the category parameter from the assignDispute function signature in the UseAssignDisputeReturn interface (the (category: string, stakeAmount: number | bigint) signature) and update the corresponding function implementation. Ensure that all calls to assignDispute are also updated to remove the category argument, since the underlying drawDispute call only receives stakeAmount.src/components/profile/PendingPaymentsDialog.tsx (1)
29-40:⚠️ Potential issue | 🟠 MajorPending payment filtering should normalize
statusbefore comparison to handle string values from blockchain data.The
Disputeinterface allowsstatusto benumber | string, but line 31 uses strict numeric comparisond.status !== 0, which incorrectly filters out disputes where status is the string"0". Other parts of the codebase (e.g.,DisputeListView.tsx) properly normalize withNumber(d.status)before comparing. This will hide required payment actions.Also applies to line 83.
Suggested fix
- return disputes.filter((d: any) => { + return disputes.filter((d: Dispute) => { + const status = Number(d.status); // Must be in Created status (waiting for funds) - if (d.status !== 0) return false; + if (status !== 0) return false;- {pendingDisputes.map((dispute: any) => ( + {pendingDisputes.map((dispute: Dispute) => (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/profile/PendingPaymentsDialog.tsx` around lines 29 - 40, The pending payment filter in the disputes.filter function uses strict numeric comparison (d.status !== 0) which fails when the status value from blockchain data is a string like "0" instead of a number, since the Dispute interface allows status to be either number or string. Normalize the status value by wrapping it with Number() before the comparison, so that both numeric and string status values are properly evaluated. Apply this same fix to both occurrences mentioned at line 31 and line 83.src/app/(voting)/my-votes/page.tsx (1)
28-33:⚠️ Potential issue | 🟠 MajorNormalize
statuscomparison in the tasks filter to handle string and numeric values.The
Disputeinterface explicitly allowsstatus: number | string, but line 32 uses strict equalityd.status === 2. If data arrives with status as"2"(valid per the type), those WITHDRAW tasks will be silently excluded from the UI. Additionally, usinganytype on line 29 and line 110 masks this type safety issue.Suggested fix
-const tasks = disputes.filter( - (d: any) => - d.phase === "VOTE" || - d.phase === "REVEAL" || - (d.phase === "WITHDRAW" && d.status === 2), -); +const tasks = disputes.filter((d: Dispute) => { + const status = Number(d.status); + return ( + d.phase === "VOTE" || + d.phase === "REVEAL" || + (d.phase === "WITHDRAW" && status === 2) + ); +});-{tasks.map((task: any, index: number) => ( +{tasks.map((task: Dispute, index: number) => (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(voting)/my-votes/page.tsx around lines 28 - 33, The tasks filter in the disputes.filter function uses strict equality d.status === 2 which only matches numeric values, but the Dispute interface allows status to be either a number or string. Update the status comparison to normalize both string and numeric values (such as using Number() conversion or loose equality) so that WITHDRAW tasks are included whether status arrives as "2" or 2. Additionally, replace the `any` type annotation on the parameter to enable proper type checking instead of masking the type safety issue.
🧹 Nitpick comments (3)
src/blockchain/types.ts (1)
175-190: 💤 Low valueType safety erosion: multiple hooks now return
any.The dispute query hooks (
useGetDispute,useDisputeList,useMyDisputes,useAllDisputes) and the newuseSubmitEvidencenow useanyfor dispute IDs and return types. While this provides flexibility, it removes compile-time guarantees for consumers.Consider using a union type or generic constraint for
disputeIdparameters (e.g.,disputeId: string | bigint) and defining minimal return type shapes for query hooks to preserve type safety while remaining flexible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/blockchain/types.ts` around lines 175 - 190, Replace the `any` type usage in the hook definitions to improve type safety. For the `disputeId` parameters in hooks like useSubmitEvidence, useVote, useReveal, and useGetDispute, use a union type such as `string | bigint` instead of `any`. For the return types of all the voting and dispute query hooks (useVote, useReveal, useSliceVoting, useJurorStats, useGetDispute, useDisputeList, useMyDisputes, useAllDisputes), define minimal but concrete return type shapes instead of returning `any`, ensuring each hook has a clear interface that documents what consumers can expect to receive.src/util/sorobanTx.ts (1)
86-86: 💤 Low valueTransaction timeout exceeds guideline default.
The timeout is set to 300 seconds, while the coding guidelines specify a default of 180 seconds. This may be intentional for long-running operations, but verify this aligns with expected transaction finality times.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/util/sorobanTx.ts` at line 86, The setTimeout call on the txBuilder object is configured with a timeout value of 300 seconds, which exceeds the coding guideline default of 180 seconds. Either update the timeout value to 180 seconds to align with guidelines, or if the longer timeout is intentional for this specific operation, add a comment explaining why it deviates from the standard guideline and what long-running operation justifies the extended timeout.Source: Coding guidelines
src/components/dispute-overview/EvidenceView.tsx (1)
240-240: ⚡ Quick winKeep
imageEvidenceitem typing instead ofany.Line 240 weakens evidence-shape safety by forcing
any; inferred typing here is already sufficient and safer.Suggested fix
-{imageEvidence.map((img: any) => ( +{imageEvidence.map((img) => (As per coding guidelines, "Evidence JSON must match the
DisputeUIinterface used by the frontend for correct decoding and rendering."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/dispute-overview/EvidenceView.tsx` at line 240, Remove the explicit `any` type annotation from the `img` parameter in the `imageEvidence.map()` callback on line 240. Instead of `(img: any)`, use `(img)` and let TypeScript infer the proper type from the `imageEvidence` array to maintain type safety and align with the `DisputeUI` interface requirements for evidence handling.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(evidence)/manage/page.tsx:
- Around line 28-31: Remove the `any` type cast from the dispute parameter in
the filter function and normalize the dispute status values to handle both
string and number types consistently. Instead of using `any`, properly type the
dispute parameter and implement status normalization (such as converting status
to a consistent type like number or string) before performing strict equality
comparisons in the subsequent filter and conditional checks to ensure values
like "0" are correctly matched against 0 and prevent logic bypasses where action
buttons fail to render.
In `@src/app/api/auth/passkey/authenticate/route.ts`:
- Around line 78-84: The generateLink function call is using type 'magiclink',
but the response is returning tokenType as 'signup', which creates a mismatch
when the client attempts to verify the token. Update the tokenType value in the
response (around line 112) to be 'magiclink' instead of 'signup' to match the
generated token type. Additionally, correct the comment on lines 78-79 to
accurately state that 'magiclink' type is being used rather than 'signup', so
the code and comments are consistent.
---
Outside diff comments:
In `@src/app/`(voting)/my-votes/page.tsx:
- Around line 28-33: The tasks filter in the disputes.filter function uses
strict equality d.status === 2 which only matches numeric values, but the
Dispute interface allows status to be either a number or string. Update the
status comparison to normalize both string and numeric values (such as using
Number() conversion or loose equality) so that WITHDRAW tasks are included
whether status arrives as "2" or 2. Additionally, replace the `any` type
annotation on the parameter to enable proper type checking instead of masking
the type safety issue.
In `@src/components/profile/PendingPaymentsDialog.tsx`:
- Around line 29-40: The pending payment filter in the disputes.filter function
uses strict numeric comparison (d.status !== 0) which fails when the status
value from blockchain data is a string like "0" instead of a number, since the
Dispute interface allows status to be either number or string. Normalize the
status value by wrapping it with Number() before the comparison, so that both
numeric and string status values are properly evaluated. Apply this same fix to
both occurrences mentioned at line 31 and line 83.
In `@src/hooks/soroban/useAssignDispute.ts`:
- Around line 11-14: The category parameter in the assignDispute function
signature is not being used anywhere in the implementation. Remove the category
parameter from the assignDispute function signature in the
UseAssignDisputeReturn interface (the (category: string, stakeAmount: number |
bigint) signature) and update the corresponding function implementation. Ensure
that all calls to assignDispute are also updated to remove the category
argument, since the underlying drawDispute call only receives stakeAmount.
---
Nitpick comments:
In `@src/blockchain/types.ts`:
- Around line 175-190: Replace the `any` type usage in the hook definitions to
improve type safety. For the `disputeId` parameters in hooks like
useSubmitEvidence, useVote, useReveal, and useGetDispute, use a union type such
as `string | bigint` instead of `any`. For the return types of all the voting
and dispute query hooks (useVote, useReveal, useSliceVoting, useJurorStats,
useGetDispute, useDisputeList, useMyDisputes, useAllDisputes), define minimal
but concrete return type shapes instead of returning `any`, ensuring each hook
has a clear interface that documents what consumers can expect to receive.
In `@src/components/dispute-overview/EvidenceView.tsx`:
- Line 240: Remove the explicit `any` type annotation from the `img` parameter
in the `imageEvidence.map()` callback on line 240. Instead of `(img: any)`, use
`(img)` and let TypeScript infer the proper type from the `imageEvidence` array
to maintain type safety and align with the `DisputeUI` interface requirements
for evidence handling.
In `@src/util/sorobanTx.ts`:
- Line 86: The setTimeout call on the txBuilder object is configured with a
timeout value of 300 seconds, which exceeds the coding guideline default of 180
seconds. Either update the timeout value to 180 seconds to align with
guidelines, or if the longer timeout is intentional for this specific operation,
add a comment explaining why it deviates from the standard guideline and what
long-running operation justifies the extended timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ca9552d-f271-43a7-8bea-9857c5a2a1d6
📒 Files selected for processing (18)
contracts/slice/Cargo.tomlcontracts/slice/src/storage.rssrc/app/(evidence)/manage/page.tsxsrc/app/(voting)/my-votes/page.tsxsrc/app/api/auth/passkey/authenticate/route.tssrc/blockchain/hooks.tssrc/blockchain/plugins/mock.tsxsrc/blockchain/plugins/stellar.tsxsrc/blockchain/types.tssrc/components/create/SelectParty.tsxsrc/components/dispute-overview/EvidenceView.tsxsrc/components/disputes/BalanceCard.tsxsrc/components/disputes/DisputeListView.tsxsrc/components/disputes/FaucetButton.tsxsrc/components/profile/PendingExecutionsDialog.tsxsrc/components/profile/PendingPaymentsDialog.tsxsrc/hooks/soroban/useAssignDispute.tssrc/util/sorobanTx.ts
💤 Files with no reviewable changes (1)
- contracts/slice/Cargo.toml
| (d: any) => | ||
| d.claimer.toLowerCase() === address.toLowerCase() || | ||
| d.defender.toLowerCase() === address.toLowerCase(), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file at the mentioned lines
cat -n src/app/\(evidence\)/manage/page.tsx | head -170Repository: sliceprotocol/slice-stellar
Length of output: 7099
🏁 Script executed:
# Find the Dispute type definition
fd -t f -e ts -e tsx | xargs rg "type Dispute|interface Dispute" -A 10Repository: sliceprotocol/slice-stellar
Length of output: 6590
🏁 Script executed:
# Check imports in the manage page
head -50 src/app/\(evidence\)/manage/page.tsxRepository: sliceprotocol/slice-stellar
Length of output: 1597
Remove any casts and normalize dispute status to prevent logic bypasses.
The any casts on lines 28 and 86 suppress type checking. Since Dispute.status is defined as number | string in src/blockchain/types.ts, string values like "0" will silently bypass all strict equality checks at lines 120, 142, and 157, leaving no action button rendered.
Suggested fix
- return disputes.filter(
- (d: any) =>
+ return disputes.filter(
+ (d: Dispute) =>
d.claimer.toLowerCase() === address.toLowerCase() ||
d.defender.toLowerCase() === address.toLowerCase(),
);- {myCases.map((dispute: any) => (
+ {myCases.map((dispute: Dispute) => (- if (dispute.status === 0) {
+ const status = Number(dispute.status);
+ if (status === 0) {
...
- else if (dispute.status === 1 || dispute.status === 2) {
+ else if (status === 1 || status === 2) {
...
- else if (dispute.status === 3 && dispute.phase === "CLOSED") {
+ else if (status === 3 && dispute.phase === "CLOSED") {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(evidence)/manage/page.tsx around lines 28 - 31, Remove the `any`
type cast from the dispute parameter in the filter function and normalize the
dispute status values to handle both string and number types consistently.
Instead of using `any`, properly type the dispute parameter and implement status
normalization (such as converting status to a consistent type like number or
string) before performing strict equality comparisons in the subsequent filter
and conditional checks to ensure values like "0" are correctly matched against 0
and prevent logic bypasses where action buttons fail to render.
| // Generate a recovery/signup link which gives us a valid token hash | ||
| // We use 'signup' type which provides a token that can be used immediately | ||
| const { data: linkData, error: linkError } = | ||
| await adminClient.auth.admin.generateLink({ | ||
| type: "magiclink", | ||
| type: 'magiclink', | ||
| email: passkey.email, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Supabase admin generateLink type options and verifyOtp type compatibility
💡 Result:
The Supabase Admin generateLink and verifyOtp methods are used for different parts of the authentication lifecycle. generateLink is an administrative function for creating links or OTPs (on a secure server), while verifyOtp is a client-side function used to complete authentication [1][2][3][4]. generateLink Type Options The admin generateLink method supports the following types [1][2][5][6]: - signup: Creates a new user; requires a password [1][5]. - magiclink: Creates a magic link for passwordless sign-in [1][5]. - invite: Generates an invitation link [1][5]. - recovery: Generates a password recovery link [1][5]. - email_change_current: Generates a link to verify the change via the current email [1][5]. - email_change_new: Generates a link to verify the change via the new email [1][5]. - phone_change: Used for phone number verification during change flows [2][5]. verifyOtp Type Compatibility The verifyOtp method handles the completion of the authentication flow [3][4]. Its type parameter must match the intent of the verification process, though the naming conventions differ from generateLink [4][7]: - Email OTPs (Numeric): Use type: 'email' [3][7]. This is the standard for verifying numeric one-time passcodes sent to an email address [7]. - SMS OTPs: Use type: 'sms' [3][4]. - Magic Links: While magic links often handle session creation automatically upon redirect, if you need to use verifyOtp for them, use type: 'magiclink' [7]. - Specific Flows: For account recovery or email changes, the type used in verifyOtp should align with the verification type expected by the specific flow (e.g., 'recovery', 'invite', or 'email_change') [4]. Important Notes: - Administrative Privilege: generateLink methods must be executed on a trusted server using your service_role (secret) key [8][9]. Never expose this key in client-side applications. - Deprecation: While some older documentation or legacy implementations may reference 'signup' or 'magiclink' types for verifyOtp, 'email' is the modern standard for general email-based OTP verification [4][7]. Always ensure your verifyOtp call matches the specific auth method that triggered the flow [4].
Citations:
- 1: https://supabase.com/docs/reference/javascript/auth-admin-generatelink
- 2: https://supabase.com/docs/reference/python/auth-admin-generatelink
- 3: https://supabase.com/docs/reference/javascript/auth-verifyotp
- 4: https://supabase.com/docs/reference/python/auth-verifyotp
- 5: https://cdn.jsdelivr.net/npm/@supabase/auth-js@2.100.1/src/GoTrueAdminApi.ts
- 6: https://supabase-auth.hexdocs.pm/Supabase.Auth.Admin.html
- 7: Docs: clarify
verifyOtptype usage for magic links vs email OTP in React quickstart supabase/supabase#41672 - 8: https://supabase.com/docs/reference/dart/auth-admin-generatelink
- 9: https://supabase.github.io/auth-js/v2/classes/GoTrueAdminApi.html
🏁 Script executed:
# First, let's read the full route file to understand the context
head -n 150 src/app/api/auth/passkey/authenticate/route.tsRepository: sliceprotocol/slice-stellar
Length of output: 3647
🏁 Script executed:
# Search for verifyOtp usage in LoginModal.tsx around the referenced line 378
rg -n "verifyOtp" --type ts --type tsx -B 3 -A 3Repository: sliceprotocol/slice-stellar
Length of output: 99
🏁 Script executed:
# Search for verifyOtp without file type restriction
rg -n "verifyOtp" -B 3 -A 3Repository: sliceprotocol/slice-stellar
Length of output: 616
Token type mismatch between generation and verification will break authentication.
The generateLink call uses type: 'magiclink' (line 82), but the response returns tokenType: 'signup' (line 112). When LoginModal.tsx calls supabase.auth.verifyOtp({ type: authResult.tokenType }), it will attempt to verify a 'magiclink' token with type: 'signup', causing verification to fail.
The comment on lines 78-79 incorrectly states "We use 'signup' type" when the code actually generates a 'magiclink' token.
Change the returned tokenType to match the generated token type:
🐛 Proposed fix: Return tokenType matching the generated 'magiclink' type
return NextResponse.json({
success: true,
userId,
email: passkey.email,
tokenHash,
- tokenType: 'signup',
+ tokenType: 'magiclink',
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/auth/passkey/authenticate/route.ts` around lines 78 - 84, The
generateLink function call is using type 'magiclink', but the response is
returning tokenType as 'signup', which creates a mismatch when the client
attempts to verify the token. Update the tokenType value in the response (around
line 112) to be 'magiclink' instead of 'signup' to match the generated token
type. Additionally, correct the comment on lines 78-79 to accurately state that
'magiclink' type is being used rather than 'signup', so the code and comments
are consistent.
Summary
Horizon.Server+Server as SorobanServerfrom/rpcsubpath), useAssignDispute.ts (usedrawDisputefrom plugin shape instead of non-existentassignDispute), BalanceCard.tsx (destructureisLoading/refetchfromuseTokenBalance; fix variable shadowing indisplayBalance), FaucetButton.tsx (userequestTokens/isRequestingfromuseFaucet), auth route (fixtokenTypevalue and comment), DisputeListView.tsx (useNumber(d.status)for robust numeric comparison), implicit-any fixes in EvidenceView / PendingExecutionsDialog / PendingPaymentsDialog / manage page / my-votes pageloading?field; onlyisLoadingremainsuseSubmitEvidencehook: type inBlockchainHooks(optional), mock implementation (toast.success+ returnstrue), proxy export inhooks.tswith nullish fallback;Disputeinterface consolidated intypes.tsand re-exported fromhooks.tstoast.info("Withdraw submission is not wired yet…")totoast.error("Withdrawal not yet available — coming soon.")stellar-registry = "0.0.5"dependency fromcontracts/slice/Cargo.toml(no import inlib.rs)// TODO: migrate queue to persistent storage once dispute volume warrants itaboveset_draft_queueinstorage.rsTest plan
pnpm tsc --noEmitpasses with zero errors (verified locally)useSubmitEvidence().submitEvidence(...)shows success toastrequestTokenscontracts/slicesucceeds withoutstellar-registry🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes