-
-
Notifications
You must be signed in to change notification settings - Fork 16
Improve declarative delegation interface #63
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.
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
Changes from all commits
22597f3
677075e
11441de
3a8ab8b
967410b
63c03c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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; | ||
|
||
/** | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same simplification as above - There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same simplification as above - There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
Or make it that if both parameters are defined and error is thrown to protect the user from unforeseen behavior. Same with other scopes. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
||
/** | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same simplification as above - I wonder if there's value in deduplicating this logic. Maybe a concern for a separate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
}); | ||
} | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// 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; | ||
|
||
/** | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same simplification as above - There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} |
There was a problem hiding this comment.
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 toallowedCalldata?.length > 0
because this will never be true ifallowedCalldata
is undefined.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.