Skip to content

Improve safety of CaveatBuilder interface #24

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

Merged
merged 10 commits into from
Jul 9, 2025

Conversation

jeffsmale90
Copy link
Collaborator

@jeffsmale90 jeffsmale90 commented Jul 7, 2025

📝 Description

Improves the safety of the caveat builder interface, to reduce the likelihood of insufficiently restricted delegations being signed.

🔄 What Changed?

List the specific changes made:

  • Updates allowEmptyCaveats to allowInsecureUnrestrictedDelegation
  • Requires allowInsecureUnrestrictedDelegation when creating and signing delegations with no caveats
  • CaveatBuilders now accept configuration parameter, rather than positional arguments
  • updates casing of calldata parameter in @metamask/delegation-core (previously included some callData parameters)

🚀 Why?

This is the first in a number of changes aimed at reducing the likelihood of insufficiently restricted delegations being created and signed.

  • allowInsecureUnrestrictedDelegation is a more meaningful flag than allowEmptyCaveats, and stands out as a red flag to developers.
  • Previously, there was no restriction on signing delegations without caveats, which could simply be passed as an empty array. Now this may only be done intentionally.
  • By providing configuration parameters to caveat builders, the likelihood of misconfiguring a delegation is reduced.

🧪 How to Test?

Describe how to test these changes:

  • Ensure that all caveat builders now require a configuration object. Ensure that all configuration objects are typed appropriately for the caveat type.
  • Attempt to create a delegation without caveats - expect an error. Pass allowInsecureUnrestrictedDelegation and expect success.
  • Attempt to sign a delegation without caveats - expect an error. Pass allowInsecureUnrestrictedDelegation and expect success.

There is good existing and new unit test coverage over this functionality.

⚠️ Breaking Changes

  • caveat builders now require configuration objects, instead of positional arguments
  • allowInsecureUnrestrictedDelegation must be passed to empty caveatBuidlers, calls to createDelegation and signDelegation

📋 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

Code snippet for each caveat builder

Allowed Calldata

caveatBuilder.addCaveat("allowedCalldata", {
  startIndex: 4,
  value: "0x1234567890abcdef"
});

Allowed Methods

caveatBuilder.addCaveat("allowedMethods", {
  selectors: [
    "0xa9059cbb", // transfer(address,uint256)
    "0x23b872dd", // transferFrom(address,address,uint256)
    "function approve(address spender, uint256 amount)"
  ]
});

Allowed Targets

caveatBuilder.addCaveat("allowedTargets", {
  targets: [
    "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
    "0xdAC17F958D2ee523a2206206994597C13D831ec7"
  ]
});

Args Equality Check

caveatBuilder.addCaveat("argsEqualityCheck", {
  args: "0x000000000000000000000000a0b86a33e6441e0fa0b1cf78a87a8a8a8a8c8f1d"
});

Block Number

caveatBuilder.addCaveat("blockNumber", {
  afterThreshold: 18500000n,
  beforeThreshold: 18600000n
});

Deployed

caveatBuilder.addCaveat("deployed", {
  contractAddress: "0x1234567890123456789012345678901234567890",
  salt: "0x0000000000000000000000000000000000000000000000000000000000000001",
  bytecode: "0x608060405234801561001057600080fd5b50600436106100365760003560e01c8063"
});

ERC1155 Balance Change

import { BalanceChangeType } from '@metamask/delegation-toolkit';

caveatBuilder.addCaveat("erc1155BalanceChange", {
  tokenAddress: "0xd07dc4262BCDbf85190C01c996b4C06a461d2430",
  recipient: "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
  tokenId: 1337n,
  balance: 5n,
  changeType: BalanceChangeType.Decrease
});

ERC20 Balance Change

import { BalanceChangeType } from '@metamask/delegation-toolkit';

caveatBuilder.addCaveat("erc20BalanceChange", {
  tokenAddress: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
  recipient: "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
  balance: 1000000000000000000n, // 1 ETH worth in wei
  changeType: BalanceChangeType.Decrease
});

ERC20 Period Transfer

caveatBuilder.addCaveat("erc20PeriodTransfer", {
  tokenAddress: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
  periodAmount: 100000000000000000000n, // 100 tokens
  periodDuration: 2592000, // 30 days in seconds
  startDate: 1672531200 // January 1, 2023
});

ERC20 Streaming

caveatBuilder.addCaveat("erc20Streaming", {
  tokenAddress: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
  initialAmount: 1000000000000000000n, // 1 token
  maxAmount: 100000000000000000000n, // 100 tokens max
  amountPerSecond: 115740740740740n, // ~10 tokens per day
  startTime: 1672531200 // January 1, 2023
});

ERC20 Transfer Amount

caveatBuilder.addCaveat("erc20TransferAmount", {
  tokenAddress: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
  maxAmount: 50000000000000000000n // 50 tokens max
});

ERC721 Balance Change

import { BalanceChangeType } from '@metamask/delegation-toolkit';

caveatBuilder.addCaveat("erc721BalanceChange", {
  tokenAddress: "0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D",
  recipient: "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
  balance: 1n,
  changeType: BalanceChangeType.Decrease
});

ERC721 Transfer

caveatBuilder.addCaveat("erc721Transfer", {
  tokenAddress: "0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D",
  tokenId: 1234n
});

Exact Calldata

caveatBuilder.addCaveat("exactCalldata", {
  calldata: "0xa9059cbb000000000000000000000000742d35cc6490c4c5237f8bb5e89d77d5b3f38f3e0000000000000000000000000000000000000000000000000de0b6b3a7640000"
});

Exact Calldata Batch

caveatBuilder.addCaveat("exactCalldataBatch", {
  executions: [
    {
      target: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
      value: 0n,
      callData: "0xa9059cbb000000000000000000000000742d35cc6490c4c5237f8bb5e89d77d5b3f38f3e0000000000000000000000000000000000000000000000000de0b6b3a7640000"
    },
    {
      target: "0xdAC17F958D2ee523a2206206994597C13D831ec7",
      value: 0n,
      callData: "0xa9059cbb000000000000000000000000742d35cc6490c4c5237f8bb5e89d77d5b3f38f3e00000000000000000000000000000000000000000000000000000000000f4240"
    }
  ]
});

Exact Execution

caveatBuilder.addCaveat("exactExecution", {
  execution: {
    target: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
    value: 1000000000000000000n, // 1 ETH
    callData: "0xa9059cbb000000000000000000000000742d35cc6490c4c5237f8bb5e89d77d5b3f38f3e0000000000000000000000000000000000000000000000000de0b6b3a7640000"
  }
});

Exact Execution Batch

caveatBuilder.addCaveat("exactExecutionBatch", {
  executions: [
    {
      target: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
      value: 0n,
      callData: "0xa9059cbb000000000000000000000000742d35cc6490c4c5237f8bb5e89d77d5b3f38f3e0000000000000000000000000000000000000000000000000de0b6b3a7640000"
    },
    {
      target: "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
      value: 500000000000000000n, // 0.5 ETH
      callData: "0x"
    }
  ]
});

ID

caveatBuilder.addCaveat("id", {
  id: 12345n
});

Limited Calls

caveatBuilder.addCaveat("limitedCalls", {
  limit: 5
});

Multi Token Period

caveatBuilder.addCaveat("multiTokenPeriod", [
  {
    tokenAddress: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
    periodAmount: 100000000000000000000n, // 100 tokens
    periodDuration: 2592000, // 30 days
    startDate: 1672531200 // January 1, 2023
  },
  {
    tokenAddress: "0xdAC17F958D2ee523a2206206994597C13D831ec7",
    periodAmount: 1000000000n, // 1000 USDT (6 decimals)
    periodDuration: 604800, // 7 days
    startDate: 1672531200 // January 1, 2023
  }
]);

Native Balance Change

import { BalanceChangeType } from '@metamask/delegation-toolkit';

caveatBuilder.addCaveat("nativeBalanceChange", {
  recipient: "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
  balance: 1000000000000000000n, // 1 ETH
  changeType: BalanceChangeType.Decrease
});

Native Token Payment

caveatBuilder.addCaveat("nativeTokenPayment", {
  recipient: "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
  amount: 2000000000000000000n // 2 ETH
});

Native Token Period Transfer

caveatBuilder.addCaveat("nativeTokenPeriodTransfer", {
  periodAmount: 5000000000000000000n, // 5 ETH per period
  periodDuration: 2592000, // 30 days in seconds
  startDate: 1672531200 // January 1, 2023
});

Native Token Streaming

caveatBuilder.addCaveat("nativeTokenStreaming", {
  initialAmount: 100000000000000000n, // 0.1 ETH
  maxAmount: 10000000000000000000n, // 10 ETH max
  amountPerSecond: 11574074074074n, // ~1 ETH per day
  startTime: 1672531200 // January 1, 2023
});

Native Token Transfer Amount

caveatBuilder.addCaveat("nativeTokenTransferAmount", {
  maxAmount: 3000000000000000000n // 3 ETH max
});

Nonce

caveatBuilder.addCaveat("nonce", {
  nonce: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
});

Ownership Transfer

caveatBuilder.addCaveat("ownershipTransfer", {
  contractAddress: "0x1234567890123456789012345678901234567890"
});

Redeemer

caveatBuilder.addCaveat("redeemer", {
  redeemers: [
    "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
    "0x8ba1f109551bD432803012645Hac136c22C985e"
  ]
});

Specific Action ERC20 Transfer Batch

caveatBuilder.addCaveat("specificActionERC20TransferBatch", {
  tokenAddress: "0xA0b86a33E6441e0fA0b1cF78A87A8a8A8a8c8f1d",
  recipient: "0x742d35Cc6490C4c5237f8Bb5e89D77d5b3F38F3e",
  amount: 25000000000000000000n, // 25 tokens
  target: "0xdAC17F958D2ee523a2206206994597C13D831ec7",
  calldata: "0xa9059cbb000000000000000000000000742d35cc6490c4c5237f8bb5e89d77d5b3f38f3e00000000000000000000000000000000000000000000000000000000000f4240"
});

Timestamp

caveatBuilder.addCaveat("timestamp", {
  afterThreshold: 1672531200, // January 1, 2023
  beforeThreshold: 1703980800  // December 31, 2023
});

Value LTE

caveatBuilder.addCaveat("valueLte", {
  maxValue: 1000000000000000000n // 1 ETH max
});

- rename allowEmptyCaveats to allowInsecureFullAuthorityDelegations
- require allowInsecureFullAuthorityDelegations when signing and creating delegations
@jeffsmale90 jeffsmale90 force-pushed the feat/allowInsecureUnrestrictedDelegation branch 2 times, most recently from d350250 to 420b110 Compare July 7, 2025 22:32
@jeffsmale90 jeffsmale90 force-pushed the feat/allowInsecureUnrestrictedDelegation branch from 420b110 to f002d56 Compare July 7, 2025 22:39
@jeffsmale90 jeffsmale90 marked this pull request as ready for review July 7, 2025 22:40
@jeffsmale90 jeffsmale90 requested a review from a team as a code owner July 7, 2025 22:40
@jeffsmale90 jeffsmale90 changed the title Improves safety of CaveatBuilder interface Improve safety of CaveatBuilder interface Jul 8, 2025
cursor[bot]

This comment was marked as outdated.

@jeffsmale90 jeffsmale90 requested a review from hanzel98 July 9, 2025 04:55
cursor[bot]

This comment was marked as outdated.

hanzel98
hanzel98 previously approved these changes Jul 9, 2025
@jeffsmale90 jeffsmale90 merged commit b43253c into main Jul 9, 2025
15 checks passed
@jeffsmale90 jeffsmale90 deleted the feat/allowInsecureUnrestrictedDelegation branch July 9, 2025 20:04
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