-
-
Notifications
You must be signed in to change notification settings - Fork 6
Add @metamask/delegation-core package #9
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
Conversation
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
74d6801
to
6d01af9
Compare
0a9c099
to
86cbf28
Compare
- add as dependency to @metamask/delegation-toolkit
…Builders. - exactCalldata - nativeTokenPeriodTransfer - nativeTokenStreaming - timestamp - valueLte - erc20TokenStreaming
…k/delegation-toolkit CaveatBuilders.
86cbf28
to
60a476f
Compare
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.
Pull Request Overview
This PR adds a new package (@metamask/delegation-core) that provides low‐level delegation functionality to support encoding/decoding of caveat terms and delegations while minimizing external dependencies. Key changes include:
- Introducing several caveat term builders (e.g. for value limits, timestamps, token streaming, and exact calldata) that replace manual encoding logic.
- Adding comprehensive unit tests for the new functionality and integrating the new core functions into existing toolkit utilities.
- Updating package configuration and build settings for the new package.
Reviewed Changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/delegator-e2e/package.json | Updated vitest dependency version for e2e testing. |
packages/delegation-toolkit/** | Updated caveat builders to delegate functionality to delegation-core. |
packages/delegation-core/** | New package implementation including caveat builders, encoding/decoding functions, types, and unit tests. |
export const toHexString = ({ | ||
value, | ||
size, | ||
}: { | ||
value: bigint | number; | ||
size: number; | ||
}): string => { | ||
return value.toString(16).padStart(size * 2, '0'); |
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.
[nitpick] Consider documenting that toHexString intentionally omits the '0x' prefix so that callers know to add it later – alternatively, including the prefix directly might simplify usage and improve consistency with other utilities.
export const toHexString = ({ | |
value, | |
size, | |
}: { | |
value: bigint | number; | |
size: number; | |
}): string => { | |
return value.toString(16).padStart(size * 2, '0'); | |
/** | |
* Converts a number or bigint to a hexadecimal string with a `0x` prefix. | |
* Pads the result to the specified size (in bytes) with leading zeros. | |
* | |
* @param value - The number or bigint to convert. | |
* @param size - The size (in bytes) to pad the result to. | |
* @returns The hexadecimal string with a `0x` prefix. | |
*/ | |
export const toPrefixedHexString = ({ | |
value, | |
size, | |
}: { | |
value: bigint | number; | |
size: number; | |
}): string => { | |
return '0x' + value.toString(16).padStart(size * 2, '0'); |
Copilot uses AI. Check for mistakes.
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.
Good call - I think adding a comment to the function makes sense. This exists because @metamask/utils only provides functionality to convert to prefixed Hex strings, while we need unprefixed.
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.
Looks good! Just a suggestion regarding how we expose caveats:
The package is currently defined as:
Core utilities for creating and managing delegation terms and caveats in the MetaMask Delegation Framework.
However, the package only exposes some caveats, while the core caveatBuilder
remains in the @metamask/delegation-toolkit
package.
My suggestion is to move the caveatBuilder
into this new @metamask/delegation-core
package as well.
If the goal is to provide a single core package that exposes delegation and caveat functionality to consumers, having caveat creation split between @metamask/delegation-core
and @metamask/delegation-toolkit
could be confusing for users.
The intention is to implement all caveats eventually, but for now, only those that are needed for readable permssions. I don't intend for external developers to have to import delegation-core directly. @metamask/delegation-toolkit provides the (slightly) higher-level abstraction, and depends on delegation-core. This is intended to be used internally in snaps and extension. Developers who want the (slightly) higher level abstractions, and more friendly API, can use @metamask/delegation-toolkit. Implementing caveatbuilder abstraction within delegation-core brings a lot with it, including DelegatorEnvironment etc. I want to keep delegation-core as minimal and low level as possible. |
ecf867d
to
f4f5906
Compare
Got it, I'll go ahead and approve this so we can push this forward. |
📝 Description
Creates a new package @metamask/delegation-core that provides low-level delegation functionality, with minimal dependencies.
🔄 What Changed?
Hex
orUint8Array
🚀 Why?
This adds a lower level utils library that can be used across the extension and snaps without including a dependency on Viem, or the full Delegation Toolkit. This is designed to do a lot of the lower level encoding required in the Delegation Toolkit, to avoid duplication of logic.
🧪 How to Test?
There is comprehensive unit test coverage for the functionality added to @metamask/delegation-core, and the impacted functionality in @metamask/delegation-toolkit.
📋 Checklist
Check off completed items:
Additional comments
I've kept each change in the PR clearly isolated, so it's probably best to review by changeset.