Add createERC20TokenPeriodTransferTerms for @metamask/delegation-core#15
Merged
Conversation
createERC20TokenPeriodTransferTerms for @metamask/delegation-core
779e02e to
9ab2e58
Compare
9ab2e58 to
7e2f727
Compare
There was a problem hiding this comment.
Pull Request Overview
Adds a new terms builder for periodic ERC20 token transfers in @metamask/delegation-core and updates related builders to use prefixed addresses.
- Introduce
createERC20TokenPeriodTransferTermsin the core package. - Update
erc20PeriodTransferBuilderin the toolkit to call the new core function. - Simplify ERC20 streaming builder to include the full
0x-prefixed address. - Export and document the new terms builder in the core README.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/delegation-toolkit/src/caveatBuilder/erc20PeriodTransferBuilder.ts | Replace manual term concatenation and validation with core’s createERC20TokenPeriodTransferTerms. |
| packages/delegation-core/src/index.ts | Export createERC20TokenPeriodTransferTerms. |
| packages/delegation-core/src/caveats/index.ts | Add export for the new caveat terms builder. |
| packages/delegation-core/src/caveats/erc20TokenPeriodTransfer.ts | Implement new terms builder with validation and encoding logic. |
| packages/delegation-core/src/caveats/erc20Streaming.ts | Rename tokenAddressHex to prefixedTokenAddressHex and include full 0x prefix. |
| packages/delegation-core/README.md | Document the new createERC20TokenPeriodTransferTerms API. |
Comments suppressed due to low confidence (2)
packages/delegation-core/README.md:199
- The documentation states the terms are 128 bytes (32 bytes per parameter), but the implementation uses a 20-byte address plus three 32-byte values (116 bytes total). Please update the description to match the actual encoding or pad the address to 32 bytes.
**Returns:** `Hex | Uint8Array` - 128-byte encoded terms (32 bytes per parameter)
packages/delegation-core/src/caveats/erc20Streaming.ts:74
- [nitpick] Consider renaming
prefixedTokenAddressHexto a more concise or descriptive name such astokenAddressWithPrefix, or revert totokenAddressHexif the prefixing behavior is consistent across builders.
let prefixedTokenAddressHex: string;
hanzel98
approved these changes
Jul 1, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
Adds terms builder to @metamask/delegation-core for the ERC20 Token Periodic Transfer caveat builder.
🔄 What Changed?
List the specific changes made:
createERC20TokenPeriodTransferTermsfunction to @metamask/delegation-coreerc20PeriodTransferBuilderto consume this new terms buildercreateERC20StreamingTerms, by using the prefixed token address, instead of stripping the prefix, and re-adding later.🚀 Why?
This caveat is now used within the readable permissions gator snaps, so building the terms must be performed within @metamask/delegation-core, so as to not require @metamask/delegation-toolkit dependency within the snaps.
🧪 How to Test?
Existing unit tests for erc20PeriodTransferBuilder, and e2e integration tests cover this functionality, as well as new unit tests covering the new
createERC20TokenPeriodTransferTermsfunction.List any breaking changes:
📋 Checklist
Check off completed items:
🔗 Related Issues
Link to related issues:
Closes #
Related to #
📚 Additional Notes
Any additional information, concerns, or context: