Adds Contract Utils (Caveats, Delegation)#35
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Hex Validation Inconsistency
The isValidHex function is duplicated across packages/delegation-core/src/utils.ts and packages/delegation-toolkit/src/data.ts with differing implementations.
The delegation-core version uses manual regex validation and returns boolean, while the delegation-toolkit version uses viem's isHex() and returns a value is Hex type guard. This leads to inconsistent validation behavior, specifically that the delegation-toolkit version ignores the minLength constraint when exactLength is provided, unlike the delegation-core version which checks both. This duplication causes inconsistent validation and potential bugs across the codebase.
packages/delegation-core/src/utils.ts#L31-L54
packages/delegation-toolkit/src/data.ts#L10-L28
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
hanzel98
changed the title
jeffsmale90
left a comment
jeffsmale90
left a comment
There was a problem hiding this comment.
Sorry @hanzel98 I didn't have time for a full review, but some initial high level thoughts.
jeffsmale90
left a comment
jeffsmale90
left a comment
There was a problem hiding this comment.
This is a thing of beauty!
I love it!
jeffsmale90
left a comment
jeffsmale90
left a comment
There was a problem hiding this comment.
This looks really great - a very useful addition to the SDK 🎉
I added a couple of comments on the PR.
| getErc20PeriodTransferEnforcerAvailableAmount: async ( | ||
| params: ERC20PeriodTransferParams, | ||
| ): Promise<PeriodTransferResult> => { |
There was a problem hiding this comment.
It might be nice to export these actions individually too, and from @metamask/delegation-toolkit/actions
then people can be all:
const client = createPublicClient({...});
getErc20PeriodTransferEnforcerAvailableAmount(
client,
{
...
});
note: the action interface is (client, config) rather than ({ client, ...config}).
Without the overhead of the client - viem suggests that this can improve tree shaking, and recommends it for devs who are "pendantic about bundle size" 🤣 https://viem.sh/docs/clients/custom#tree-shaking
| @@ -0,0 +1,152 @@ | |||
| import { isStrictHexString } from '@metamask/utils'; | |||
There was a problem hiding this comment.
I think I probably misled you here a little - @metamask/utils isStrictHexString is correctly used in @metamask/delegation-core, but in @metamask/delegation-toolkit we should probably use viem's isHex function - their docs appear to be down rn, https://viem.sh/docs/utilities/isHex.html but there's an optional { strict: bool } parameter which defaults to true.
2 participants
📝 Description
Some caveats enforcers that distribute amounts require the user to call a function to know how many tokens the user has available. This PR adds new utilities for that function.
The delegations can be disabled and enabled at any time; this PR adds a function to validate the current state of a delegation.
🔄 What Changed?
List the specific changes made:
🚀 Why?
Explain the motivation behind these changes:
🧪 How to Test?
Describe how to test these changes:
List any breaking changes:
📋 Checklist
Check off completed items:
🔗 Related Issues
Link to related issues:
Related to # https://app.zenhub.com/workspaces/readable-permissions-67982ce51eb4360029b2c1a1/issues/gh/metamask/delegator-readable-permissions/268