Skip to content

Commit 3fa9795

Browse files
jeffsmale90jeffsmale90
committed
Tighten up requirements for createDelegation and createOpenDelegation params:
- parentDelegation may not be ROOT_AUTHORITY - resolveCaveats now accepts isScopeOptional rather than hasInheritance
1 parent 2b5c60f commit 3fa9795

4 files changed

Lines changed: 111 additions & 40 deletions

File tree

‎packages/smart-accounts-kit/src/caveatBuilder/resolveCaveats.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,22 @@ export type Caveats = CaveatBuilder | (Caveat | CoreCaveatConfiguration)[];
1515
* @param config.environment - The environment to be used for the caveat builder.
1616
* @param config.scope - The scope to be used for the caveat builder. Optional - when not provided and redelegating, scope is inherited from the parent delegation.
1717
* @param config.caveats - The caveats to be resolved, which can be either a CaveatBuilder or an array of Caveat or CaveatConfiguration. Optional - if not provided, only scope caveats will be used.
18-
* @param config.hasInheritance - Whether the delegation has inheritance.
18+
* @param config.isScopeOptional - Whether the scope is optional.
1919
* @returns The resolved array of caveats.
2020
*/
2121
export const resolveCaveats = ({
2222
environment,
2323
scope,
2424
caveats,
25-
hasInheritance,
25+
isScopeOptional,
2626
}: {
2727
environment: SmartAccountsEnvironment;
2828
scope?: ScopeConfig;
2929
caveats?: Caveats;
30-
hasInheritance: boolean;
30+
isScopeOptional: boolean;
3131
}) => {
32-
if (!scope && !hasInheritance) {
33-
throw new Error('Scope is required when the delegation has no inheritance');
32+
if (!scope && !isScopeOptional) {
33+
throw new Error('Scope is required');
3434
}
3535

3636
// Create base caveat builder from scope if provided, otherwise use core caveat builder

‎packages/smart-accounts-kit/src/delegation.ts‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -275,20 +275,35 @@ const extractLeafDelegation = (
275275
* @param options - The options for resolving the parent delegation.
276276
* @param options.parentDelegation - The parent delegation or its hash.
277277
* @param options.parentPermissionContext - The permission context containing the parent delegation.
278+
* @param options.scope - The scope to be used for the delegation.
278279
* @returns The resolved parent delegation, or undefined if neither is provided.
279280
* @internal
280281
*/
281282
const resolveParentDelegation = ({
282283
parentDelegation,
283284
parentPermissionContext,
285+
scope,
284286
}: Pick<
285287
BaseCreateDelegationOptions,
286-
'parentDelegation' | 'parentPermissionContext'
288+
'parentDelegation' | 'parentPermissionContext' | 'scope'
287289
>): Delegation | Hex | undefined => {
288290
if (parentPermissionContext) {
289291
return extractLeafDelegation(parentPermissionContext);
290292
}
291293

294+
if (
295+
typeof parentDelegation === 'string' &&
296+
parentDelegation.toLowerCase() === ROOT_AUTHORITY.toLowerCase()
297+
) {
298+
throw new Error(`Invalid parent delegation - cannot be ${ROOT_AUTHORITY}`);
299+
}
300+
301+
if (!parentDelegation && !scope) {
302+
throw new Error(
303+
'Invalid arguments - must specify either parentDelegation, parentPermissionContext, or scope',
304+
);
305+
}
306+
292307
return parentDelegation;
293308
};
294309

@@ -345,17 +360,17 @@ export const createDelegation = (
345360
): Delegation => {
346361
const parentDelegation = resolveParentDelegation(options);
347362

363+
const hasInheritance = parentDelegation !== undefined;
364+
348365
const caveats = resolveCaveats({
349366
environment: options.environment,
350367
scope: options.scope,
351368
caveats: options.caveats,
352-
hasInheritance: Boolean(parentDelegation),
369+
isScopeOptional: hasInheritance,
353370
});
354371

355372
trackSmartAccountsKitFunctionCall('createDelegation', {
356-
hasParentDelegation:
357-
options.parentDelegation !== undefined ||
358-
options.parentPermissionContext !== undefined,
373+
hasInheritance,
359374
scope: options.scope?.type ?? null,
360375
caveatNames: getCaveatNames({
361376
caveats,
@@ -384,17 +399,17 @@ export const createOpenDelegation = (
384399
): Delegation => {
385400
const parentDelegation = resolveParentDelegation(options);
386401

402+
const hasInheritance = parentDelegation !== undefined;
403+
387404
const caveats = resolveCaveats({
388405
environment: options.environment,
389406
scope: options.scope,
390407
caveats: options.caveats,
391-
hasInheritance: Boolean(parentDelegation),
408+
isScopeOptional: hasInheritance,
392409
});
393410

394411
trackSmartAccountsKitFunctionCall('createOpenDelegation', {
395-
hasParentDelegation:
396-
options.parentDelegation !== undefined ||
397-
options.parentPermissionContext !== undefined,
412+
hasInheritance,
398413
scope: options.scope?.type ?? null,
399414
caveatNames: getCaveatNames({
400415
caveats,

‎packages/smart-accounts-kit/test/caveatBuilder/resolveCaveats.test.ts‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('resolveCaveats', () => {
4848
environment,
4949
scope: erc20Scope,
5050
caveats: caveatBuilder,
51-
hasInheritance: false,
51+
isScopeOptional: false,
5252
});
5353

5454
// 4 caveats: 2 from the scope, 2 from the builder
@@ -66,7 +66,7 @@ describe('resolveCaveats', () => {
6666
environment,
6767
scope: erc20Scope,
6868
caveats: caveatBuilder,
69-
hasInheritance: false,
69+
isScopeOptional: false,
7070
});
7171

7272
expect(result).to.be.an('array');
@@ -85,7 +85,7 @@ describe('resolveCaveats', () => {
8585
environment,
8686
scope: erc20Scope,
8787
caveats: caveatBuilder as any,
88-
hasInheritance: false,
88+
isScopeOptional: false,
8989
});
9090

9191
expect(result).to.be.an('array');
@@ -101,7 +101,7 @@ describe('resolveCaveats', () => {
101101
environment,
102102
scope: erc20Scope,
103103
caveats,
104-
hasInheritance: false,
104+
isScopeOptional: false,
105105
});
106106

107107
expect(result).to.be.an('array');
@@ -129,7 +129,7 @@ describe('resolveCaveats', () => {
129129
environment,
130130
scope: erc20Scope,
131131
caveats: caveatConfigs,
132-
hasInheritance: false,
132+
isScopeOptional: false,
133133
});
134134

135135
expect(result).to.be.an('array');
@@ -140,7 +140,7 @@ describe('resolveCaveats', () => {
140140
environment,
141141
scope: erc20Scope,
142142
caveats: [],
143-
hasInheritance: false,
143+
isScopeOptional: false,
144144
});
145145
expect(result.length).to.be.greaterThan(scopeOnlyResult.length);
146146
});
@@ -164,7 +164,7 @@ describe('resolveCaveats', () => {
164164
environment,
165165
scope: erc20Scope,
166166
caveats: mixedCaveats,
167-
hasInheritance: false,
167+
isScopeOptional: false,
168168
});
169169

170170
expect(result).to.be.an('array');
@@ -180,7 +180,7 @@ describe('resolveCaveats', () => {
180180
environment,
181181
scope: erc20Scope,
182182
caveats: [],
183-
hasInheritance: false,
183+
isScopeOptional: false,
184184
});
185185

186186
expect(result).to.be.an('array');
@@ -205,7 +205,7 @@ describe('resolveCaveats', () => {
205205
environment,
206206
scope: erc721Scope,
207207
caveats: [caveatConfig],
208-
hasInheritance: false,
208+
isScopeOptional: false,
209209
});
210210

211211
expect(result).to.be.an('array');
@@ -217,14 +217,14 @@ describe('resolveCaveats', () => {
217217
environment,
218218
scope: erc20Scope,
219219
caveats: [mockCaveat1],
220-
hasInheritance: false,
220+
isScopeOptional: false,
221221
});
222222

223223
const resultWithoutCaveats = resolveCaveats({
224224
environment,
225225
scope: erc20Scope,
226226
caveats: [],
227-
hasInheritance: false,
227+
isScopeOptional: false,
228228
});
229229

230230
expect(resultWithCaveats.length).to.be.greaterThan(
@@ -247,37 +247,37 @@ describe('resolveCaveats', () => {
247247
environment,
248248
scope: erc20Scope,
249249
caveats: [invalidType as any],
250-
hasInheritance: false,
250+
isScopeOptional: false,
251251
});
252252
}).to.throw('Invalid caveat');
253253
});
254254
});
255255

256-
describe('hasInheritance', () => {
256+
describe('isScopeOptional', () => {
257257
it('should throw if scope is not provided and the delegation has no inheritance', () => {
258258
expect(() =>
259259
resolveCaveats({
260260
environment,
261261
caveats: [mockCaveat1],
262-
hasInheritance: false,
262+
isScopeOptional: false,
263263
}),
264-
).to.throw('Scope is required when the delegation has no inheritance');
264+
).to.throw('Scope is required');
265265
});
266266

267-
it('should throw if neither scope nor caveats are provided and the delegation has no inheritance', () => {
267+
it('should throw if neither scope nor caveats are provided and scope is optional', () => {
268268
expect(() =>
269269
resolveCaveats({
270270
environment,
271-
hasInheritance: false,
271+
isScopeOptional: false,
272272
}),
273-
).to.throw('Scope is required when the delegation has no inheritance');
273+
).to.throw('Scope is required');
274274
});
275275

276-
it('should resolve caveats without a scope when the delegation has inheritance', () => {
276+
it('should resolve caveats without a scope when scope is optional', () => {
277277
const result = resolveCaveats({
278278
environment,
279279
caveats: [mockCaveat1, mockCaveat2],
280-
hasInheritance: true,
280+
isScopeOptional: true,
281281
});
282282

283283
// No scope caveats are added when the scope is inherited from the parent
@@ -286,21 +286,21 @@ describe('resolveCaveats', () => {
286286
expect(result).to.deep.include(mockCaveat2);
287287
});
288288

289-
it('should return an empty array when no scope, no caveats and the delegation has inheritance', () => {
289+
it('should return an empty array when no scope, no caveats and scope is optional', () => {
290290
const result = resolveCaveats({
291291
environment,
292-
hasInheritance: true,
292+
isScopeOptional: true,
293293
});
294294

295295
expect(result).to.deep.equal([]);
296296
});
297297

298-
it('should still apply scope caveats when both scope and inheritance are provided', () => {
298+
it('should still apply scope caveats when both scope and scope is optional', () => {
299299
const result = resolveCaveats({
300300
environment,
301301
scope: erc20Scope,
302302
caveats: [mockCaveat1],
303-
hasInheritance: true,
303+
isScopeOptional: true,
304304
});
305305

306306
// Scope caveats are still produced when an explicit scope is provided

‎packages/smart-accounts-kit/test/delegation.test.ts‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,20 @@ describe('createDelegation', () => {
225225
});
226226
});
227227

228+
it('throws if parent delegation is root authority with differing case', () => {
229+
const mixedCaseRootAuthority: Hex = `0x${ROOT_AUTHORITY.slice(2).toUpperCase()}`;
230+
231+
expect(() =>
232+
createDelegation({
233+
environment: smartAccountEnvironment,
234+
scope: erc20Scope,
235+
to: mockDelegate,
236+
from: mockDelegator,
237+
parentDelegation: mixedCaseRootAuthority,
238+
}),
239+
).toThrow(`Invalid parent delegation - cannot be ${ROOT_AUTHORITY}`);
240+
});
241+
228242
it('should create a delegation with caveats', () => {
229243
const caveats: Caveat[] = [
230244
{
@@ -360,7 +374,22 @@ describe('createDelegation', () => {
360374
to: mockDelegate,
361375
from: mockDelegator,
362376
} as any);
363-
}).toThrow('Scope is required when the delegation has no inheritance');
377+
}).toThrow(
378+
'Invalid arguments - must specify either parentDelegation, parentPermissionContext, or scope',
379+
);
380+
});
381+
382+
it('throws if scope is explicitly undefined and no inheritance is provided', () => {
383+
expect(() => {
384+
createDelegation({
385+
environment: smartAccountEnvironment,
386+
to: mockDelegate,
387+
from: mockDelegator,
388+
scope: undefined,
389+
} as any);
390+
}).toThrow(
391+
'Invalid arguments - must specify either parentDelegation, parentPermissionContext, or scope',
392+
);
364393
});
365394

366395
describe('parentPermissionContext support', () => {
@@ -558,6 +587,19 @@ describe('createOpenDelegation', () => {
558587
});
559588
});
560589

590+
it('throws if parent delegation is root authority with differing case', () => {
591+
const mixedCaseRootAuthority: Hex = `0x${ROOT_AUTHORITY.slice(2).toUpperCase()}`;
592+
593+
expect(() =>
594+
createOpenDelegation({
595+
environment: smartAccountEnvironment,
596+
scope: erc20Scope,
597+
from: mockDelegator,
598+
parentDelegation: mixedCaseRootAuthority,
599+
}),
600+
).toThrow(`Invalid parent delegation - cannot be ${ROOT_AUTHORITY}`);
601+
});
602+
561603
it('should create an open delegation with caveats', () => {
562604
const caveats: Caveat[] = [
563605
{
@@ -669,7 +711,21 @@ describe('createOpenDelegation', () => {
669711
environment: smartAccountEnvironment,
670712
from: mockDelegator,
671713
} as any);
672-
}).toThrow('Scope is required when the delegation has no inheritance');
714+
}).toThrow(
715+
'Invalid arguments - must specify either parentDelegation, parentPermissionContext, or scope',
716+
);
717+
});
718+
719+
it('throws if scope is explicitly undefined and no inheritance is provided', () => {
720+
expect(() => {
721+
createOpenDelegation({
722+
environment: smartAccountEnvironment,
723+
from: mockDelegator,
724+
scope: undefined,
725+
} as any);
726+
}).toThrow(
727+
'Invalid arguments - must specify either parentDelegation, parentPermissionContext, or scope',
728+
);
673729
});
674730

675731
describe('parentPermissionContext support', () => {

0 commit comments

Comments
 (0)