Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 20 additions & 18 deletions packages/delegation-toolkit/src/caveatBuilder/resolveCaveats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export type Caveats = CaveatBuilder | (Caveat | CoreCaveatConfiguration)[];
* @param config - The configuration for the caveat builder.
* @param config.environment - The environment to be used for the caveat builder.
* @param config.scope - The scope to be used for the caveat builder.
* @param config.caveats - The caveats to be resolved, which can be either a CaveatBuilder or an array of Caveat or CaveatConfiguration.
* @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.
* @returns The resolved array of caveats.
*/
export const resolveCaveats = ({
Expand All @@ -20,27 +20,29 @@ export const resolveCaveats = ({
}: {
environment: DeleGatorEnvironment;
scope: ScopeConfig;
caveats: Caveats;
caveats?: Caveats;
}) => {
const scopeCaveatBuilder = createCaveatBuilderFromScope(environment, scope);

if ('build' in caveats && typeof caveats.build === 'function') {
(caveats as CaveatBuilder).build().forEach((caveat) => {
scopeCaveatBuilder.addCaveat(caveat);
});
} else if (Array.isArray(caveats)) {
caveats.forEach((caveat) => {
try {
if ('type' in caveat) {
const { type, ...config } = caveat;
scopeCaveatBuilder.addCaveat(type, config);
} else {
scopeCaveatBuilder.addCaveat(caveat);
if (caveats) {
if ('build' in caveats && typeof caveats.build === 'function') {
(caveats as CaveatBuilder).build().forEach((caveat) => {
scopeCaveatBuilder.addCaveat(caveat);
});
} else if (Array.isArray(caveats)) {
caveats.forEach((caveat) => {
try {
if ('type' in caveat) {
const { type, ...config } = caveat;
scopeCaveatBuilder.addCaveat(type, config);
} else {
scopeCaveatBuilder.addCaveat(caveat);
}
} catch (error) {
throw new Error(`Invalid caveat: ${(error as Error).message}`);
}
} catch (error) {
throw new Error(`Invalid caveat: ${(error as Error).message}`);
}
});
});
}
}

return scopeCaveatBuilder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const isFunctionCallConfig = (
* @param config - Configuration object containing allowed targets, methods, and optionally calldata.
* @returns A configured caveat builder with the specified caveats.
* @throws Error if any of the required parameters are invalid.
* @throws Error if both allowedCalldata and exactCalldata are provided simultaneously.
*/
export function createFunctionCallCaveatBuilder(
environment: DeleGatorEnvironment,
Expand All @@ -42,16 +43,21 @@ export function createFunctionCallCaveatBuilder(
throw new Error('Invalid Function Call configuration');
}

if (allowedCalldata && allowedCalldata.length > 0 && exactCalldata) {
throw new Error(
'Cannot specify both allowedCalldata and exactCalldata. Please use only one calldata restriction type.',
);
}

const caveatBuilder = createCaveatBuilder(environment)
.addCaveat('allowedTargets', { targets })
.addCaveat('allowedMethods', { selectors });

if (allowedCalldata) {
if (allowedCalldata && allowedCalldata.length > 0) {
Comment on lines +46 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: these two allowedCalldata && allowedCalldata.length > 0 checks could be simplified to allowedCalldata?.length > 0 because this will never be true if allowedCalldata is undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I change it I get this issue

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah my bad - because the operator is not an equality operator, this doesn't work (can't do > with undefined).

You could coalesce and keep the optional chaining, or you could change to an equality operator. Or you could leave it as it is.

if (allowedCalldata?.length !== 0) { ... } // actually, you'd probably want to switch the two cases and make this an `===` comparison 
if (allowedCalldata?.length ?? 0 > 0) { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I tried to change it using those cool ideas but I got lint errors.

allowedCalldata.forEach((calldataConfig) => {
caveatBuilder.addCaveat('allowedCalldata', calldataConfig);
});
}
if (exactCalldata) {
} else if (exactCalldata) {
caveatBuilder.addCaveat('exactCalldata', exactCalldata);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import type { DeleGatorEnvironment } from '../../types';
import type { AllowedCalldataBuilderConfig } from '../allowedCalldataBuilder';
import { createCaveatBuilder } from '../coreCaveatBuilder';
import type { CoreCaveatBuilder } from '../coreCaveatBuilder';
import type { ExactCalldataBuilderConfig } from '../exactCalldataBuilder';
import type {
nativeTokenPeriodTransfer,
NativeTokenPeriodTransferBuilderConfig,
} from '../nativeTokenPeriodTransferBuilder';

export type NativeTokenPeriodicScopeConfig = {
type: typeof nativeTokenPeriodTransfer;
allowedCalldata?: AllowedCalldataBuilderConfig[];
exactCalldata?: ExactCalldataBuilderConfig;
} & NativeTokenPeriodTransferBuilderConfig;

/**
Expand All @@ -17,19 +21,49 @@ export type NativeTokenPeriodicScopeConfig = {
* @param config - Configuration object containing native token periodic transfer parameters.
* @returns A configured caveat builder with native token period transfer and exact calldata caveats.
* @throws Error if any of the native token periodic transfer parameters are invalid.
* @throws Error if both allowedCalldata and exactCalldata are provided simultaneously.
* @throws Error if the environment is not properly configured.
*/
export function createNativeTokenPeriodicCaveatBuilder(
environment: DeleGatorEnvironment,
config: NativeTokenPeriodicScopeConfig,
): CoreCaveatBuilder {
return createCaveatBuilder(environment)
.addCaveat('exactCalldata', {
const {
periodAmount,
periodDuration,
startDate,
allowedCalldata,
exactCalldata,
} = config;

if (allowedCalldata && allowedCalldata.length > 0 && exactCalldata) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: same simplification as above - allowedCalldata?.length > 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get an issue if I change it

throw new Error(
'Cannot specify both allowedCalldata and exactCalldata. Please use only one calldata restriction type.',
);
}

const caveatBuilder = createCaveatBuilder(environment);

// Add calldata restrictions
if (allowedCalldata && allowedCalldata.length > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: same simplification as above - allowedCalldata?.length > 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get an issue if I change it

allowedCalldata.forEach((calldataConfig) => {
caveatBuilder.addCaveat('allowedCalldata', calldataConfig);
});
} else if (exactCalldata) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So here the user can specify either allowedCalldata or exactCalldata. But the function accepts both - this could confuse the developer. I would set it to either:

calldataRestriction?: AllowedCalldataBuilderConfig[] |  ExactCalldataBuilderConfig

Or make it that if both parameters are defined and error is thrown to protect the user from unforeseen behavior.

Same with other scopes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure implemented the protection when both are passed

caveatBuilder.addCaveat('exactCalldata', exactCalldata);
} else {
// Default behavior: only allow empty calldata
caveatBuilder.addCaveat('exactCalldata', {
calldata: '0x',
})
.addCaveat('nativeTokenPeriodTransfer', {
periodAmount: config.periodAmount,
periodDuration: config.periodDuration,
startDate: config.startDate,
});
}

// Add native token period transfer restriction
caveatBuilder.addCaveat('nativeTokenPeriodTransfer', {
periodAmount,
periodDuration,
startDate,
});

return caveatBuilder;
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import type { DeleGatorEnvironment } from '../../types';
import type { AllowedCalldataBuilderConfig } from '../allowedCalldataBuilder';
import { createCaveatBuilder } from '../coreCaveatBuilder';
import type { CoreCaveatBuilder } from '../coreCaveatBuilder';
import type { ExactCalldataBuilderConfig } from '../exactCalldataBuilder';
import type {
nativeTokenStreaming,
NativeTokenStreamingBuilderConfig,
} from '../nativeTokenStreamingBuilder';

export type NativeTokenStreamingScopeConfig = {
type: typeof nativeTokenStreaming;
allowedCalldata?: AllowedCalldataBuilderConfig[];
exactCalldata?: ExactCalldataBuilderConfig;
} & NativeTokenStreamingBuilderConfig;

/**
Expand All @@ -17,20 +21,51 @@ export type NativeTokenStreamingScopeConfig = {
* @param config - Configuration object containing native token streaming parameters.
* @returns A configured caveat builder with native token streaming and exact calldata caveats.
* @throws Error if any of the native token streaming parameters are invalid.
* @throws Error if both allowedCalldata and exactCalldata are provided simultaneously.
* @throws Error if the environment is not properly configured.
*/
export function createNativeTokenStreamingCaveatBuilder(
environment: DeleGatorEnvironment,
config: NativeTokenStreamingScopeConfig,
): CoreCaveatBuilder {
return createCaveatBuilder(environment)
.addCaveat('exactCalldata', {
const {
initialAmount,
maxAmount,
amountPerSecond,
startTime,
allowedCalldata,
exactCalldata,
} = config;

if (allowedCalldata && allowedCalldata.length > 0 && exactCalldata) {
throw new Error(
'Cannot specify both allowedCalldata and exactCalldata. Please use only one calldata restriction type.',
);
}

const caveatBuilder = createCaveatBuilder(environment);

// Add calldata restrictions
if (allowedCalldata && allowedCalldata.length > 0) {
Comment on lines +40 to +49
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: same simplification as above - allowedCalldata?.length > 0

I wonder if there's value in deduplicating this logic. Maybe a concern for a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get an issue if I change it

allowedCalldata.forEach((calldataConfig) => {
caveatBuilder.addCaveat('allowedCalldata', calldataConfig);
});
} else if (exactCalldata) {
caveatBuilder.addCaveat('exactCalldata', exactCalldata);
} else {
// Default behavior: only allow empty calldata
caveatBuilder.addCaveat('exactCalldata', {
calldata: '0x',
})
.addCaveat('nativeTokenStreaming', {
initialAmount: config.initialAmount,
maxAmount: config.maxAmount,
amountPerSecond: config.amountPerSecond,
startTime: config.startTime,
});
}

// Add native token streaming restriction
caveatBuilder.addCaveat('nativeTokenStreaming', {
initialAmount,
maxAmount,
amountPerSecond,
startTime,
});

return caveatBuilder;
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import type { DeleGatorEnvironment } from '../../types';
import type { AllowedCalldataBuilderConfig } from '../allowedCalldataBuilder';
import { createCaveatBuilder } from '../coreCaveatBuilder';
import type { CoreCaveatBuilder } from '../coreCaveatBuilder';
import type { ExactCalldataBuilderConfig } from '../exactCalldataBuilder';
import type {
nativeTokenTransferAmount,
NativeTokenTransferAmountBuilderConfig,
} from '../nativeTokenTransferAmountBuilder';

export type NativeTokenTransferScopeConfig = {
type: typeof nativeTokenTransferAmount;
allowedCalldata?: AllowedCalldataBuilderConfig[];
exactCalldata?: ExactCalldataBuilderConfig;
} & NativeTokenTransferAmountBuilderConfig;

/**
Expand All @@ -17,17 +21,41 @@ export type NativeTokenTransferScopeConfig = {
* @param config - Configuration object containing native token transfer parameters.
* @returns A configured caveat builder with native token transfer amount and exact calldata caveats.
* @throws Error if any of the native token transfer parameters are invalid.
* @throws Error if both allowedCalldata and exactCalldata are provided simultaneously.
* @throws Error if the environment is not properly configured.
*/
export function createNativeTokenTransferCaveatBuilder(
environment: DeleGatorEnvironment,
config: NativeTokenTransferScopeConfig,
): CoreCaveatBuilder {
return createCaveatBuilder(environment)
.addCaveat('exactCalldata', {
const { maxAmount, allowedCalldata, exactCalldata } = config;

if (allowedCalldata && allowedCalldata.length > 0 && exactCalldata) {
throw new Error(
'Cannot specify both allowedCalldata and exactCalldata. Please use only one calldata restriction type.',
);
}

const caveatBuilder = createCaveatBuilder(environment);

// Add calldata restrictions
if (allowedCalldata && allowedCalldata.length > 0) {
Comment on lines +33 to +42
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: same simplification as above - allowedCalldata?.length > 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get an issue if I change it

allowedCalldata.forEach((calldataConfig) => {
caveatBuilder.addCaveat('allowedCalldata', calldataConfig);
});
} else if (exactCalldata) {
caveatBuilder.addCaveat('exactCalldata', exactCalldata);
} else {
// Default behavior: only allow empty calldata
caveatBuilder.addCaveat('exactCalldata', {
calldata: '0x',
})
.addCaveat('nativeTokenTransferAmount', {
maxAmount: config.maxAmount,
});
}

// Add native token transfer amount restriction
caveatBuilder.addCaveat('nativeTokenTransferAmount', {
maxAmount,
});

return caveatBuilder;
}
3 changes: 1 addition & 2 deletions packages/delegation-toolkit/src/delegation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,9 @@ type BaseCreateDelegationOptions = {
environment: DeleGatorEnvironment;
scope: ScopeConfig;
from: Hex;
caveats: Caveats;
caveats?: Caveats;
parentDelegation?: Delegation | Hex;
salt?: Hex;
allowInsecureUnrestrictedDelegation?: boolean;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ describe('DelegationManager - Delegation Management', () => {
targets: [alice.address],
selectors: ['0x00000000'],
},
caveats: [],
});

const encodedData = DelegationManager.encode.disableDelegation({
Expand All @@ -117,7 +116,6 @@ describe('DelegationManager - Delegation Management', () => {
targets: [alice.address],
selectors: ['0x00000000'],
},
caveats: [],
});

const encodedData = DelegationManager.encode.enableDelegation({
Expand All @@ -143,7 +141,6 @@ describe('DelegationManager - Delegation Management', () => {
targets: [alice.address],
selectors: ['0x00000000'],
},
caveats: [],
});

const execution = createExecution({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe('createNativeTokenPeriodicCaveatBuilder', () => {
const environment = {
caveatEnforcers: {
ExactCalldataEnforcer: randomAddress(),
AllowedCalldataEnforcer: randomAddress(),
NativeTokenPeriodTransferEnforcer: randomAddress(),
},
} as unknown as DeleGatorEnvironment;
Expand Down Expand Up @@ -46,4 +47,38 @@ describe('createNativeTokenPeriodicCaveatBuilder', () => {
},
]);
});

it('creates a native token periodic transfer CaveatBuilder with empty allowedCalldata array (should fall back to default)', () => {
const config: NativeTokenPeriodicScopeConfig = {
type: 'nativeTokenPeriodTransfer',
periodAmount: 1000n,
periodDuration: 1000,
startDate: Math.floor(Date.now() / 1000),
allowedCalldata: [], // Empty array should trigger fallback to default exactCalldata
};

const caveatBuilder = createNativeTokenPeriodicCaveatBuilder(
environment,
config,
);

const caveats = caveatBuilder.build();

expect(caveats).to.deep.equal([
{
enforcer: environment.caveatEnforcers.ExactCalldataEnforcer,
args: '0x',
terms: '0x',
},
{
enforcer: environment.caveatEnforcers.NativeTokenPeriodTransferEnforcer,
args: '0x',
terms: concat([
toHex(config.periodAmount, { size: 32 }),
toHex(config.periodDuration, { size: 32 }),
toHex(config.startDate, { size: 32 }),
]),
},
]);
});
});
Loading
Loading