Skip to content

Commit d4eff4a

Browse files
grypezclaude
grypez
and
claude
committed
fix(evm-wallet): address code review — guards, baggage validation, tests
- Remove HOME_SECTION_LIMIT demo counter from homeSection exo (rate-limiting belongs on-chain via limitedCalls caveat, not in the vat) - Add superstruct validation (DelegationGrantStruct) on baggage restore in delegator-vat and redeemer-vat instead of unsafe `as` casts - Tighten delegation-twin guard/type mismatch: M.bigint() guard now matches `amount: bigint` param type; remove redundant BigInt coercions - Add home-coordinator.test.ts covering configureBundler URL/chainId validation and revokeGrant error paths - Add peer-wallet integration test: delegation relay (away → home) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4d3f87e commit d4eff4a

8 files changed

Lines changed: 310 additions & 44 deletions

File tree

‎packages/evm-wallet-experiment/src/lib/delegation-twin.ts‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,14 @@ export function makeDelegationTwin(
5353
const exo = makeDiscoverableExo(
5454
`DelegationTwin:transferNative:${idPrefix}`,
5555
{
56-
async transferNative(
57-
recipient: Address,
58-
amount: string | number | bigint,
59-
): Promise<Hex> {
60-
const amt = BigInt(amount);
61-
62-
if (maxAmount !== undefined && amt > maxAmount) {
63-
throw new Error(`Amount ${amt} exceeds limit ${maxAmount}`);
56+
async transferNative(recipient: Address, amount: bigint): Promise<Hex> {
57+
if (maxAmount !== undefined && amount > maxAmount) {
58+
throw new Error(`Amount ${amount} exceeds limit ${maxAmount}`);
6459
}
6560

6661
const execution: Execution = {
6762
target: recipient,
68-
value: `0x${amt.toString(16)}`,
63+
value: `0x${amount.toString(16)}`,
6964
callData: '0x' as Hex,
7065
};
7166

@@ -108,30 +103,28 @@ export function makeDelegationTwin(
108103
async transferFungible(
109104
tokenAddress: Address,
110105
recipient: Address,
111-
amount: string | number | bigint,
106+
amount: bigint,
112107
): Promise<Hex> {
113-
const amt = BigInt(amount);
114-
115-
if (amt > max - spent) {
108+
if (amount > max - spent) {
116109
throw new Error(
117-
`Insufficient budget: requested ${amt}, remaining ${max - spent}`,
110+
`Insufficient budget: requested ${amount}, remaining ${max - spent}`,
118111
);
119112
}
120113

121114
// Reserve before the await so concurrent calls see the updated budget.
122-
spent += amt;
115+
spent += amount;
123116

124117
const execution: Execution = {
125118
target: tokenAddress,
126119
value: '0x0' as Hex,
127-
callData: encodeTransfer(recipient, amt),
120+
callData: encodeTransfer(recipient, amount),
128121
};
129122

130123
try {
131124
return await redeemFn(execution);
132125
} catch (error) {
133126
// Roll back on redeemFn failure.
134-
spent -= amt;
127+
spent -= amount;
135128
throw error;
136129
}
137130
},

‎packages/evm-wallet-experiment/src/types.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,32 @@ export type TransferFungibleGrant = {
349349
delegation: Delegation;
350350
};
351351

352+
const BigintStruct = define<bigint>(
353+
'bigint',
354+
(value) => typeof value === 'bigint',
355+
);
356+
357+
export const TransferNativeGrantStruct = object({
358+
method: literal('transferNative'),
359+
to: optional(AddressStruct),
360+
maxAmount: optional(BigintStruct),
361+
totalLimit: optional(BigintStruct),
362+
delegation: DelegationStruct,
363+
});
364+
365+
export const TransferFungibleGrantStruct = object({
366+
method: literal('transferFungible'),
367+
token: AddressStruct,
368+
to: optional(AddressStruct),
369+
maxAmount: optional(BigintStruct),
370+
delegation: DelegationStruct,
371+
});
372+
373+
export const DelegationGrantStruct = union([
374+
TransferNativeGrantStruct,
375+
TransferFungibleGrantStruct,
376+
]);
377+
352378
/** Discriminated union of all supported semantic delegation grant types. */
353379
export type DelegationGrant = TransferNativeGrant | TransferFungibleGrant;
354380

‎packages/evm-wallet-experiment/src/vats/away-coordinator.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,13 +1781,15 @@ export function buildRootObject(
17811781
/**
17821782
* Transfer native ETH.
17831783
* Tries each delegation twin in order; falls back to calling home.
1784+
* Errors from matched twins propagate — they are not swallowed and do
1785+
* not fall through to the home section.
17841786
*
17851787
* @param to - Recipient address.
17861788
* @param amount - Amount in wei.
17871789
* @returns The transaction hash.
17881790
*/
17891791
async transferNative(to: Address, amount: bigint): Promise<Hex> {
1790-
const amt = BigInt(amount as unknown as string | number | bigint);
1792+
const amt = amount;
17911793
const matching = delegationSections.filter(
17921794
(sec) => sec.method === 'transferNative',
17931795
);
@@ -1826,7 +1828,7 @@ export function buildRootObject(
18261828
to: Address,
18271829
amount: bigint,
18281830
): Promise<Hex> {
1829-
const amt = BigInt(amount as unknown as string | number | bigint);
1831+
const amt = amount;
18301832
const tokenLower = token.toLowerCase() as Address;
18311833
const matching = delegationSections.filter(
18321834
(sec) => sec.method === 'transferFungible' && sec.token === tokenLower,

‎packages/evm-wallet-experiment/src/vats/delegator-vat.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { makeDefaultExo } from '@metamask/kernel-utils/exo';
22
import type { Baggage } from '@metamask/ocap-kernel';
3+
import { create } from '@metamask/superstruct';
34

45
import {
56
ENFORCER_CONTRACT_KEY_MAP,
@@ -25,6 +26,7 @@ import type {
2526
TransferFungibleGrant,
2627
TransferNativeGrant,
2728
} from '../types.ts';
29+
import { DelegationGrantStruct } from '../types.ts';
2830

2931
const harden = globalThis.harden ?? (<T>(value: T): T => value);
3032

@@ -53,8 +55,8 @@ export function buildRootObject(
5355
): object {
5456
const grants: Map<string, DelegationGrant> = baggage.has('grants')
5557
? new Map(
56-
Object.entries(
57-
baggage.get('grants') as Record<string, DelegationGrant>,
58+
Object.entries(baggage.get('grants') as Record<string, unknown>).map(
59+
([id, raw]) => [id, create(raw, DelegationGrantStruct)],
5860
),
5961
)
6062
: new Map();

‎packages/evm-wallet-experiment/src/vats/home-coordinator.test.ts‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
import type { Baggage } from '@metamask/ocap-kernel';
2+
import { describe, expect, it, vi } from 'vitest';
3+
4+
import type { Address, Delegation, DelegationGrant, Hex } from '../types.ts';
5+
import { buildRootObject } from './home-coordinator.ts';
6+
import { makeMockBaggage } from '../../test/helpers.ts';
7+
8+
vi.mock('@endo/eventual-send', () => ({
9+
E: (target: Record<string, (...args: unknown[]) => unknown>) =>
10+
new Proxy(target, {
11+
get(obj, prop: string) {
12+
return async (...args: unknown[]) =>
13+
Promise.resolve(obj[prop]?.(...args));
14+
},
15+
}),
16+
}));
17+
vi.mock('@metamask/kernel-utils/exo', () => ({
18+
makeDefaultExo: (_name: string, methods: Record<string, unknown>) => methods,
19+
}));
20+
vi.mock('@metamask/kernel-utils/discoverable', () => ({
21+
makeDiscoverableExo: (_name: string, methods: Record<string, unknown>) =>
22+
methods,
23+
}));
24+
vi.mock('../lib/sdk.ts', () => ({
25+
setSdkLogger: vi.fn(),
26+
registerEnvironment: vi.fn(),
27+
resolveEnvironment: vi.fn().mockReturnValue({ chainId: 11155111 }),
28+
getDelegationManagerAddress: vi
29+
.fn()
30+
.mockReturnValue('0xdb9B1e94B5b69Df7e401DDbedE43491141047dB3' as Address),
31+
buildSdkRedeemCallData: vi.fn().mockReturnValue('0x' as Hex),
32+
buildSdkDisableCallData: vi.fn().mockReturnValue('0x' as Hex),
33+
buildBatchExecuteCallData: vi.fn().mockReturnValue('0x' as Hex),
34+
computeSmartAccountAddress: vi.fn(),
35+
isEip7702Delegated: vi.fn().mockResolvedValue(false),
36+
prepareUserOpTypedData: vi.fn(),
37+
}));
38+
39+
// ---------------------------------------------------------------------------
40+
// Helpers
41+
// ---------------------------------------------------------------------------
42+
43+
type HomeCoordinator = {
44+
bootstrap(
45+
vats: Record<string, unknown>,
46+
services: Record<string, unknown>,
47+
): Promise<void>;
48+
configureBundler(config: {
49+
bundlerUrl: string;
50+
chainId: number;
51+
usePaymaster?: boolean;
52+
}): Promise<void>;
53+
revokeGrant(id: string): Promise<Hex>;
54+
};
55+
56+
const SIGNED_DELEGATION: Delegation = {
57+
id: '0xaaaa000000000000000000000000000000000000000000000000000000000000',
58+
delegator: '0x1111111111111111111111111111111111111111' as Address,
59+
delegate: '0x2222222222222222222222222222222222222222' as Address,
60+
authority:
61+
'0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' as Hex,
62+
caveats: [],
63+
salt: '0x01' as Hex,
64+
chainId: 11155111,
65+
status: 'signed',
66+
};
67+
68+
const REVOKED_DELEGATION: Delegation = {
69+
...SIGNED_DELEGATION,
70+
id: '0xbbbb000000000000000000000000000000000000000000000000000000000000',
71+
status: 'revoked',
72+
};
73+
74+
const NATIVE_GRANT: DelegationGrant = {
75+
method: 'transferNative',
76+
delegation: SIGNED_DELEGATION,
77+
};
78+
79+
const REVOKED_GRANT: DelegationGrant = {
80+
method: 'transferNative',
81+
delegation: REVOKED_DELEGATION,
82+
};
83+
84+
async function makeCoordinator(
85+
delegatorVat?: Record<string, unknown>,
86+
): Promise<HomeCoordinator> {
87+
const baggage = makeMockBaggage();
88+
const coordinator = buildRootObject(
89+
{},
90+
undefined,
91+
baggage as unknown as Baggage,
92+
) as unknown as HomeCoordinator;
93+
94+
if (delegatorVat) {
95+
await coordinator.bootstrap({ delegator: delegatorVat }, {});
96+
}
97+
98+
return coordinator;
99+
}
100+
101+
// ---------------------------------------------------------------------------
102+
// Tests
103+
// ---------------------------------------------------------------------------
104+
105+
describe('home-coordinator', () => {
106+
describe('configureBundler — URL validation', () => {
107+
it.each([
108+
'file:///etc/passwd',
109+
'ftp://host/path',
110+
'ws://host/path',
111+
'',
112+
'not-a-url',
113+
])('rejects non-HTTP(S) URL: %s', async (bundlerUrl) => {
114+
const coordinator = await makeCoordinator();
115+
await expect(
116+
coordinator.configureBundler({ bundlerUrl, chainId: 1 }),
117+
).rejects.toThrow('Invalid bundler URL');
118+
});
119+
120+
it.each([0, -1, 1.5, NaN])(
121+
'rejects invalid chain ID: %s',
122+
async (chainId) => {
123+
const coordinator = await makeCoordinator();
124+
await expect(
125+
coordinator.configureBundler({
126+
bundlerUrl: 'https://api.pimlico.io/v2/sepolia/rpc?apikey=x',
127+
chainId,
128+
}),
129+
).rejects.toThrow('Invalid chain ID');
130+
},
131+
);
132+
});
133+
134+
describe('revokeGrant', () => {
135+
it('throws when grant is not found', async () => {
136+
const mockDelegator = { listGrants: vi.fn().mockResolvedValue([]) };
137+
const coordinator = await makeCoordinator(mockDelegator);
138+
139+
await expect(
140+
coordinator.revokeGrant(SIGNED_DELEGATION.id),
141+
).rejects.toThrow('not found');
142+
});
143+
144+
it('throws when grant is already revoked', async () => {
145+
const mockDelegator = {
146+
listGrants: vi.fn().mockResolvedValue([REVOKED_GRANT]),
147+
};
148+
const coordinator = await makeCoordinator(mockDelegator);
149+
150+
await expect(
151+
coordinator.revokeGrant(REVOKED_DELEGATION.id),
152+
).rejects.toThrow('already revoked');
153+
});
154+
155+
it('throws when delegatorVat is not available', async () => {
156+
const coordinator = await makeCoordinator();
157+
158+
await expect(
159+
coordinator.revokeGrant(SIGNED_DELEGATION.id),
160+
).rejects.toThrow('Delegator vat not available');
161+
});
162+
163+
it('calls delegatorVat.listGrants to look up the grant', async () => {
164+
const mockDelegator = {
165+
listGrants: vi.fn().mockResolvedValue([NATIVE_GRANT]),
166+
removeGrant: vi.fn().mockResolvedValue(undefined),
167+
};
168+
const coordinator = await makeCoordinator(mockDelegator);
169+
170+
// revokeGrant will fail past the lookup (no bundler/provider configured)
171+
// but listGrants must have been called once.
172+
await coordinator
173+
.revokeGrant(SIGNED_DELEGATION.id)
174+
.catch((_err) => undefined);
175+
expect(mockDelegator.listGrants).toHaveBeenCalledOnce();
176+
});
177+
});
178+
});

‎packages/evm-wallet-experiment/src/vats/home-coordinator.ts‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,21 +1089,10 @@ export function buildRootObject(
10891089
// homeSection exo — built once, after all internal functions are defined
10901090
// ---------------------------------------------------------------------------
10911091

1092-
// Demo limit: each method throws after 2 uses
1093-
let transferNativeUses = 0;
1094-
let transferFungibleUses = 0;
1095-
const HOME_SECTION_LIMIT = 2;
1096-
10971092
const homeSection = makeDiscoverableExo(
10981093
'HomeWallet',
10991094
{
11001095
async transferNative(to: Address, amount: bigint): Promise<Hex> {
1101-
if (transferNativeUses >= HOME_SECTION_LIMIT) {
1102-
throw new Error(
1103-
`Home transferNative limit (${HOME_SECTION_LIMIT}) exhausted`,
1104-
);
1105-
}
1106-
transferNativeUses += 1;
11071096
const from = await resolveOwnerAddress();
11081097
const amountHex: Hex = `0x${amount.toString(16)}`;
11091098
if (!providerVat) {
@@ -1138,20 +1127,11 @@ export function buildRootObject(
11381127
to: Address,
11391128
amount: bigint,
11401129
): Promise<Hex> {
1141-
if (transferFungibleUses >= HOME_SECTION_LIMIT) {
1142-
throw new Error(
1143-
`Home transferFungible limit (${HOME_SECTION_LIMIT}) exhausted`,
1144-
);
1145-
}
1146-
transferFungibleUses += 1;
11471130
const from = await resolveOwnerAddress();
11481131
if (!providerVat) {
11491132
throw new Error('Provider not configured');
11501133
}
1151-
const callData = encodeTransfer(
1152-
to,
1153-
BigInt(amount as unknown as string | number | bigint),
1154-
);
1134+
const callData = encodeTransfer(to, amount);
11551135
const chainId = await resolveChainId();
11561136
const nonce = await E(providerVat).getNonce(from);
11571137
const fees = await E(providerVat).getGasFees();

‎packages/evm-wallet-experiment/src/vats/redeemer-vat.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { makeDefaultExo } from '@metamask/kernel-utils/exo';
22
import type { Baggage } from '@metamask/ocap-kernel';
3+
import { create } from '@metamask/superstruct';
34

45
import type { DelegationGrant } from '../types.ts';
6+
import { DelegationGrantStruct } from '../types.ts';
57

68
const harden = globalThis.harden ?? (<T>(value: T): T => value);
79

@@ -21,8 +23,8 @@ export function buildRootObject(
2123
// Restore from baggage on resuscitation
2224
const grants: Map<string, DelegationGrant> = baggage.has('grants')
2325
? new Map(
24-
Object.entries(
25-
baggage.get('grants') as Record<string, DelegationGrant>,
26+
Object.entries(baggage.get('grants') as Record<string, unknown>).map(
27+
([id, raw]) => [id, create(raw, DelegationGrantStruct)],
2628
),
2729
)
2830
: new Map();

0 commit comments

Comments
 (0)