Skip to content

feat: add example spending limit hook #102

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

Closed
wants to merge 38 commits into from
Closed

Conversation

zhongeric
Copy link
Collaborator

No description provided.

@zhongeric zhongeric changed the base branch from main to develop-set-can-execute-hook April 7, 2025 19:21
@zhongeric zhongeric changed the base branch from develop-set-can-execute-hook to main April 8, 2025 01:27
/// @dev An enumerable set of the periods.
EnumerableSetLib.Uint8Set periods;
/// @dev Mapping of `uint8(period)` to `TokenPeriodSpendStorage`.
mapping(uint256 => TokenPeriodSpendStorage) spends;
Copy link
Member

Choose a reason for hiding this comment

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

why is this BY period? How would that work if you have two spend periods for the same token that overlap? You'll have to check both periods states and combine the spends somehow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you have two periods for the same token they all must be true:

  • Minute, 10 AND hour, 100

You would loop through all of the periods, increment the spend for each and check none exceed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You add the same amount to each tracked period spend

/// @dev Set the spend limit for a key hash.
/// Uses msg.sender to compute the accountKeyHash.
function setSpendLimit(bytes32 keyHash, address token, SpendPeriod period, uint256 limit) external {
AccountKeyHash accountKeyHash = keyHash.wrap(msg.sender);
Copy link
Member

Choose a reason for hiding this comment

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

maybe use a different word? wrap is keyword for a custom type

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok! I did hashSender

// Collect the ERC20 balances before the batch execution.
for (uint256 i; i < erc20s.length(); ++i) {
address token = erc20s.getAddress(i);
balancesBefore.set(i, SafeTransferLib.balanceOf(token, msg.sender));
Copy link
Member

Choose a reason for hiding this comment

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

this could have duplicates no?

// to guard, as anyone else can directly submit the calldata and the signature.
uint32 fnSel = uint32(bytes4(LibBytes.loadCalldata(data, 0x00)));
// `transfer(address,uint256)`.
if (fnSel == 0xa9059cbb) {
Copy link
Member

Choose a reason for hiding this comment

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

why are we not doing a balance check?

// where the actual token transfers might not match the calldata amounts.
// There is no strict definition on what constitutes spending,
// and we want to be as conservative as possible.
Math.max(
Copy link
Member

Choose a reason for hiding this comment

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

this is just odd to me... what if you like flash loaned something? it would still count it as a spend here even though your balance stayed under some limit this would count it incorrectly no?

@snreynolds
Copy link
Member

Closing as we may explore a simpler spending limit example :)

@snreynolds snreynolds closed this Jun 2, 2025
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