Skip to content

Commit ae4cda5

Browse files
kronosapiensclaude
andcommitted
Add ApprovalPolicy support to controller toWasmPolicies
The keychain's toWasmPolicies creates ApprovalPolicy for approve methods with spender/amount fields, but the controller SDK always created CallPolicy. This caused merkle root mismatches for SessionConnector, NodeProvider, and TelegramProvider users who define approve policies with spending limits. Changes: - Add ApprovalPolicy handling to match keychain implementation - Update SessionContracts type to include Approval methods - Add comprehensive tests for ApprovalPolicy handling Fixes #2370 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 4489a92 commit ae4cda5

File tree

3 files changed

+229
-7
lines changed

3 files changed

+229
-7
lines changed

packages/controller/src/__tests__/toWasmPolicies.test.ts

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,4 +177,203 @@ describe("toWasmPolicies", () => {
177177
expect(result).toEqual([]);
178178
});
179179
});
180+
181+
describe("ApprovalPolicy handling", () => {
182+
test("creates ApprovalPolicy for approve methods with spender and amount", () => {
183+
const policies: ParsedSessionPolicies = {
184+
verified: false,
185+
contracts: {
186+
"0xTOKEN": {
187+
methods: [
188+
{
189+
entrypoint: "approve",
190+
spender: "0xSPENDER",
191+
amount: "1000000000000000000",
192+
authorized: true,
193+
},
194+
],
195+
},
196+
},
197+
};
198+
199+
const result = toWasmPolicies(policies);
200+
201+
expect(result).toHaveLength(1);
202+
expect(result[0]).toEqual({
203+
target: "0xTOKEN",
204+
spender: "0xSPENDER",
205+
amount: "1000000000000000000",
206+
});
207+
// Should NOT have method or authorized fields
208+
expect(result[0]).not.toHaveProperty("method");
209+
expect(result[0]).not.toHaveProperty("authorized");
210+
});
211+
212+
test("converts numeric amount to string in ApprovalPolicy", () => {
213+
const policies: ParsedSessionPolicies = {
214+
verified: false,
215+
contracts: {
216+
"0xTOKEN": {
217+
methods: [
218+
{
219+
entrypoint: "approve",
220+
spender: "0xSPENDER",
221+
amount: 1000000000000000000,
222+
authorized: true,
223+
},
224+
],
225+
},
226+
},
227+
};
228+
229+
const result = toWasmPolicies(policies);
230+
231+
expect(result[0]).toHaveProperty("amount", "1000000000000000000");
232+
});
233+
234+
test("falls back to CallPolicy for approve without spender", () => {
235+
const warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
236+
237+
const policies: ParsedSessionPolicies = {
238+
verified: false,
239+
contracts: {
240+
"0xTOKEN": {
241+
methods: [
242+
{
243+
entrypoint: "approve",
244+
authorized: true,
245+
},
246+
],
247+
},
248+
},
249+
};
250+
251+
const result = toWasmPolicies(policies);
252+
253+
expect(result).toHaveLength(1);
254+
expect(result[0]).toHaveProperty("method");
255+
expect(result[0]).toHaveProperty("authorized", true);
256+
expect(result[0]).not.toHaveProperty("spender");
257+
expect(warnSpy).toHaveBeenCalledWith(
258+
expect.stringContaining("[DEPRECATED]"),
259+
);
260+
261+
warnSpy.mockRestore();
262+
});
263+
264+
test("falls back to CallPolicy for approve without amount", () => {
265+
const warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
266+
267+
const policies: ParsedSessionPolicies = {
268+
verified: false,
269+
contracts: {
270+
"0xTOKEN": {
271+
methods: [
272+
{
273+
entrypoint: "approve",
274+
spender: "0xSPENDER",
275+
authorized: true,
276+
},
277+
],
278+
},
279+
},
280+
};
281+
282+
const result = toWasmPolicies(policies);
283+
284+
expect(result).toHaveLength(1);
285+
expect(result[0]).toHaveProperty("method");
286+
expect(result[0]).not.toHaveProperty("spender");
287+
expect(warnSpy).toHaveBeenCalled();
288+
289+
warnSpy.mockRestore();
290+
});
291+
292+
test("creates CallPolicy for non-approve methods", () => {
293+
const policies: ParsedSessionPolicies = {
294+
verified: false,
295+
contracts: {
296+
"0xCONTRACT": {
297+
methods: [
298+
{
299+
entrypoint: "transfer",
300+
authorized: true,
301+
},
302+
],
303+
},
304+
},
305+
};
306+
307+
const result = toWasmPolicies(policies);
308+
309+
expect(result).toHaveLength(1);
310+
expect(result[0]).toHaveProperty("target", "0xCONTRACT");
311+
expect(result[0]).toHaveProperty("method");
312+
expect(result[0]).toHaveProperty("authorized", true);
313+
});
314+
315+
test("handles mixed approve and non-approve methods", () => {
316+
const policies: ParsedSessionPolicies = {
317+
verified: false,
318+
contracts: {
319+
"0xTOKEN": {
320+
methods: [
321+
{
322+
entrypoint: "approve",
323+
spender: "0xSPENDER",
324+
amount: "1000",
325+
authorized: true,
326+
},
327+
{
328+
entrypoint: "transfer",
329+
authorized: true,
330+
},
331+
],
332+
},
333+
},
334+
};
335+
336+
const result = toWasmPolicies(policies);
337+
338+
expect(result).toHaveLength(2);
339+
340+
// First should be approve (sorted alphabetically)
341+
const approvePolicy = result[0];
342+
expect(approvePolicy).toHaveProperty("spender", "0xSPENDER");
343+
expect(approvePolicy).toHaveProperty("amount", "1000");
344+
345+
// Second should be transfer
346+
const transferPolicy = result[1];
347+
expect(transferPolicy).toHaveProperty("method");
348+
expect(transferPolicy).toHaveProperty("authorized", true);
349+
});
350+
351+
test("sorts approve policies correctly among other methods", () => {
352+
const policies: ParsedSessionPolicies = {
353+
verified: false,
354+
contracts: {
355+
"0xTOKEN": {
356+
methods: [
357+
{ entrypoint: "transfer", authorized: true },
358+
{
359+
entrypoint: "approve",
360+
spender: "0xSPENDER",
361+
amount: "1000",
362+
authorized: true,
363+
},
364+
{ entrypoint: "balance_of", authorized: true },
365+
],
366+
},
367+
},
368+
};
369+
370+
const result = toWasmPolicies(policies);
371+
372+
expect(result).toHaveLength(3);
373+
// Sorted order: approve, balance_of, transfer
374+
expect(result[0]).toHaveProperty("spender"); // approve -> ApprovalPolicy
375+
expect(result[1]).toHaveProperty("method"); // balance_of -> CallPolicy
376+
expect(result[2]).toHaveProperty("method"); // transfer -> CallPolicy
377+
});
378+
});
180379
});

packages/controller/src/policies.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
Approval,
23
ContractPolicy,
34
Method,
45
SessionPolicies,
@@ -14,7 +15,7 @@ export type ParsedSessionPolicies = {
1415
export type SessionContracts = Record<
1516
string,
1617
Omit<ContractPolicy, "methods"> & {
17-
methods: (Method & { authorized?: boolean })[];
18+
methods: ((Method | Approval) & { authorized?: boolean })[];
1819
}
1920
>;
2021

packages/controller/src/utils.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Policy } from "@cartridge/controller-wasm/controller";
1+
import { Policy, ApprovalPolicy } from "@cartridge/controller-wasm/controller";
22
import { Policies, SessionPolicies } from "@cartridge/presets";
33
import { ChainId } from "@starknet-io/types-js";
44
import {
@@ -105,11 +105,33 @@ export function toWasmPolicies(policies: ParsedSessionPolicies): Policy[] {
105105
toArray(methods)
106106
.slice()
107107
.sort((a, b) => a.entrypoint.localeCompare(b.entrypoint))
108-
.map((m) => ({
109-
target,
110-
method: hash.getSelectorFromName(m.entrypoint),
111-
authorized: m.authorized,
112-
})),
108+
.map((m): Policy => {
109+
// Check if this is an approve entrypoint with spender and amount
110+
if (m.entrypoint === "approve") {
111+
if ("spender" in m && "amount" in m && m.spender && m.amount) {
112+
const approvalPolicy: ApprovalPolicy = {
113+
target,
114+
spender: m.spender,
115+
amount: String(m.amount),
116+
};
117+
return approvalPolicy;
118+
}
119+
120+
// Fall back to CallPolicy with deprecation warning
121+
console.warn(
122+
`[DEPRECATED] Approve method without spender and amount fields will be rejected in future versions. ` +
123+
`Please update your preset or policies to include both 'spender' and 'amount' fields for approve calls on contract ${target}. ` +
124+
`Example: { entrypoint: "approve", spender: "0x...", amount: "0x..." }`,
125+
);
126+
}
127+
128+
// For non-approve methods and legacy approve, create a regular CallPolicy
129+
return {
130+
target,
131+
method: hash.getSelectorFromName(m.entrypoint),
132+
authorized: m.authorized,
133+
};
134+
}),
113135
),
114136
...(policies.messages ?? [])
115137
.map((p) => {

0 commit comments

Comments
 (0)