Skip to content

fix: use transfer amount enforcers for fixed allowances#282

Draft
mj-kiwi wants to merge 2 commits into
mainfrom
fix/fixed-allowance-use-transfer-amount-enforcers
Draft

fix: use transfer amount enforcers for fixed allowances#282
mj-kiwi wants to merge 2 commits into
mainfrom
fix/fixed-allowance-use-transfer-amount-enforcers

Conversation

@mj-kiwi

@mj-kiwi mj-kiwi commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

📝 Description

Provide a brief description of the changes in this PR

🔄 What Changed?

List the specific changes made:

🚀 Why?

Explain the motivation behind these changes:

🧪 How to Test?

Describe how to test these changes:

  • Manual testing steps:
  • Automated tests added/updated
  • All existing tests pass

⚠️ Breaking Changes

List any breaking changes:

  • No breaking changes
  • Breaking changes (describe below):

📋 Checklist

Check off completed items:

  • Code follows the project's coding standards
  • Self-review completed
  • Documentation updated (if needed)
  • Tests added/updated
  • Changelog updated (if needed)
  • All CI checks pass

🔗 Related Issues

Link to related issues:
Closes #
Related to #

📚 Additional Notes

Any additional information, concerns, or context:

Comment on lines 92 to 94
if (getByteLength(terms) !== EXPECTED_TERMS_BYTELENGTH) {
throw new Error('Invalid erc20-token-allowance terms: expected 116 bytes');
throw new Error('Invalid erc20-token-allowance terms: expected 52 bytes');
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should use the decodeERC20TransferAmountTerms function from @metamask/delegation-core instead of bespoke decoding (we had bespoke decoding because the duration is invalid - UINT256_MAX, and it decodes to number type), which includes explicit validation of the terms length

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants