Skip to content

feat(session): spending limit step for register session #2303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Larkooo wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(session): spending limit step for register session #2303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18c191b
feat(session): spending limit step for register session
Larkooo Dec 23, 2025
bfe4b75
tool versions
Larkooo Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .tool-versions
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
scarb 2.9.4
starknet-foundry 0.38.3
nodejs v20.11.1
nodejs 20.19.5
84 changes: 67 additions & 17 deletions packages/keychain/src/components/connect/RegisterSession.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
type ContractType,
type ParsedSessionPolicies,
useCreateSession,
hasApprovalPolicies,
} from "@/hooks/session";
import { Button, LayoutContent, SliderIcon } from "@cartridge/ui";
import { useCallback, useEffect, useMemo, useState } from "react";
Expand All @@ -18,6 +19,7 @@ import {
TransactionExecutionStatus,
TransactionFinalityStatus,
} from "starknet";
import { SpendingLimitPage } from "./SpendingLimitPage";

const requiredPolicies: Array<ContractType> = ["VRF"];

Expand Down Expand Up @@ -52,9 +54,28 @@ const RegisterSessionLayout = ({
const [transactions, setTransactions] = useState<Call[] | undefined>(
undefined,
);
const [isConnecting, setIsConnecting] = useState(false);
const [error, setError] = useState<Error | undefined>();

const { duration, isEditable, onToggleEditable } = useCreateSession();

const hasTokenApprovals = useMemo(
() => hasApprovalPolicies(policies),
[policies],
);

const defaultStep = useMemo<"summary" | "spending-limit">(() => {
return policies?.verified && hasTokenApprovals
? "spending-limit"
: "summary";
}, [policies?.verified, hasTokenApprovals]);

const [step, setStep] = useState<"summary" | "spending-limit">(defaultStep);

useEffect(() => {
setStep(defaultStep);
}, [defaultStep]);

const expiresAt = useMemo(() => {
return duration + now();
}, [duration]);
Expand Down Expand Up @@ -83,31 +104,60 @@ const RegisterSessionLayout = ({
return;
}

const { transaction_hash } = await controller.registerSession(
origin,
expiresAt,
policies,
publicKey,
maxFee,
);

await controller.provider.waitForTransaction(transaction_hash, {
retryInterval: 1000,
successStates: [
TransactionExecutionStatus.SUCCEEDED,
TransactionFinalityStatus.ACCEPTED_ON_L2,
],
});

onConnect(transaction_hash, expiresAt);
try {
setError(undefined);
setIsConnecting(true);

const { transaction_hash } = await controller.registerSession(
origin,
expiresAt,
policies,
publicKey,
maxFee,
);

await controller.provider.waitForTransaction(transaction_hash, {
retryInterval: 1000,
successStates: [
TransactionExecutionStatus.SUCCEEDED,
TransactionFinalityStatus.ACCEPTED_ON_L2,
],
});

onConnect(transaction_hash, expiresAt);
} catch (e) {
setError(e as Error);
} finally {
setIsConnecting(false);
}
Copy link

@cursor cursor bot Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration error swallowing prevents display in ExecutionContainer

The onRegisterSession function catches errors but does not re-throw them, causing the ExecutionContainer to assume the operation succeeded. Consequently, errors occurring during registration are swallowed and not displayed to the user on the summary page, as the local error state is only consumed by the non-rendered SpendingLimitPage.

Additional Locations (1)

Fix in Cursor Fix in Web

},
[controller, expiresAt, policies, publicKey, onConnect, origin],
);

// Handler for the spending limit page to proceed to registration
const handleSpendingLimitConfirm = useCallback(() => {
if (hasTokenApprovals && step === "spending-limit") {
setStep("summary");
}
}, [hasTokenApprovals, step]);
Copy link

@cursor cursor bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirm and Back buttons perform identical actions

Medium Severity

The handleSpendingLimitConfirm callback passed to SpendingLimitPage as onConnect only calls setStep("summary"), which is exactly the same action as the onBack callback. Both the "Confirm" and "Back" buttons result in identical navigation to the summary step. In contrast, CreateSession.tsx correctly calls handlePrimaryAction() for onConnect, which triggers the actual session creation. The isConnecting and error props passed to SpendingLimitPage will also never reflect meaningful state since registration only occurs after leaving this page.

🔬 Verification Test

Why verification test was not possible: This is a React component logic bug that requires UI interaction to demonstrate. The bug can be verified by code inspection: both onBack={() => setStep("summary")} (line 155) and handleSpendingLimitConfirm (which calls setStep("summary") at lines 138-142) result in the same state transition. Comparing with CreateSession.tsx lines 195-197 shows onConnect should call the actual action handler (handlePrimaryAction), not just navigate to another step.

Additional Locations (1)

Fix in Cursor Fix in Web


Copy link

@cursor cursor bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SpendingLimitPage unnecessarily blocked by transactions loading check

Low Severity

The if (!transactions) check at line 144 returns "Loading" before the SpendingLimitPage check at line 148. However, SpendingLimitPage doesn't use transactions at all—it only needs policies from context. This causes users to see an unnecessary loading state while transactions are being prepared, when SpendingLimitPage could display immediately. The SpendingLimitPage conditional should be checked before the transactions loading check to avoid this unnecessary delay. CreateSession.tsx handles this correctly by not blocking on transactions.

🔬 Verification Test

Why verification test was not possible: This is a React component render order issue that manifests as a UX delay. The bug can be verified by code inspection: SpendingLimitPage (line 151) only receives policies, isConnecting, error, onBack, and onConnect as props—none of which depend on transactions. Yet the if (!transactions) return <div>Loading</div> check (line 144) executes first, blocking SpendingLimitPage from rendering until transactions load. Comparing with CreateSession.tsx lines 184-199 shows the correct pattern where SpendingLimitPage is rendered without waiting for transactions.

Additional Locations (1)

Fix in Cursor Fix in Web

if (!transactions) {
return <div>Loading</div>;
}

// Show spending limit page for verified sessions with token approvals
if (hasTokenApprovals && step === "spending-limit") {
return (
<SpendingLimitPage
policies={policies}
isConnecting={isConnecting}
error={error}
onBack={() => setStep("summary")}
onConnect={handleSpendingLimitConfirm}
/>
);
}

return (
<ExecutionContainer
title="Register Session"
Expand Down
Loading