Skip to content

Commit

Permalink
Merge pull request #2 from coinbase/conner/final-read-n-clean
Browse files Browse the repository at this point in the history 
Update naming and comments
  • Loading branch information
@ilikesymmetry
ilikesymmetry authored Oct 16, 2024
2 parents f5fd5cc + d70c6a7 commit d9b8ca4
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 80 deletions.
96 changes: 49 additions & 47 deletions src/SpendPermissionManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,31 @@ import {SignatureCheckerLib} from "solady/utils/SignatureCheckerLib.sol";

/// @title SpendPermissionManager
///
/// @notice Allow spending native and ERC20 tokens with a spend permission.
/// @notice Allow spending native and ERC-20 tokens with a spend permission.
///
/// @dev Allowance and spend values capped at uint160 ~ 1e48.
///
/// @author Coinbase (https://github.com/coinbase/smart-wallet-permissions)
/// @author Coinbase (https://github.com/coinbase/spend-permissions)
contract SpendPermissionManager is EIP712 {
/// @notice A spend permission for an external spender to spend an account's tokens.
/// @notice A spend permission for an external entity to be able to spend an account's tokens.
struct SpendPermission {
/// @dev Smart account this spend permission is valid for.
address account;
/// @dev Entity that can spend user funds.
/// @dev Entity that can spend `account`'s tokens.
address spender;
/// @dev Token address (ERC-7528 ether address or ERC-20 contract).
/// @dev Token address (ERC-7528 native token address or ERC-20 contract).
address token;
/// @dev Timestamp this spend permission is valid after (unix seconds).
uint48 start;
/// @dev Timestamp this spend permission is valid until (unix seconds).
uint48 end;
/// @dev Time duration for resetting used allowance on a recurring basis (seconds).
/// @dev Time duration for resetting used `allowance` on a recurring basis (seconds).
uint48 period;
/// @dev Maximum allowed value to spend within a recurring period.
/// @dev Maximum allowed value to spend within each `period`.
uint160 allowance;
}

/// @notice Period parameters and spend usage.
/// @notice Spend Permission usage for a certain period.
struct PeriodSpend {
/// @dev Start time of the period (unix seconds).
uint48 start;
Expand All @@ -47,7 +47,7 @@ contract SpendPermissionManager is EIP712 {
"SpendPermission(address account,address spender,address token,uint48 start,uint48 end,uint48 period,uint160 allowance)"
);

/// @notice ERC-7528 address convention for ether (https://eips.ethereum.org/EIPS/eip-7528).
/// @notice ERC-7528 address convention for native token (https://eips.ethereum.org/EIPS/eip-7528).
address public constant NATIVE_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

/// @notice Spend permission is revoked.
Expand All @@ -64,6 +64,9 @@ contract SpendPermissionManager is EIP712 {
/// @param sender Expected sender to be valid.
error InvalidSender(address sender, address expected);

/// @notice Invalid signature.
error InvalidSignature();

/// @notice Spend Permission start time is not strictly less than end time.
///
/// @param start Unix timestamp (seconds) for start of the permission.
Expand All @@ -82,7 +85,7 @@ contract SpendPermissionManager is EIP712 {
/// @notice Recurring period has not started yet.
///
/// @param currentTimestamp Current timestamp (unix seconds).
/// @param start Timestamp this spend permission is valid after (unix seconds).
/// @param start Timestamp this spend permission is valid starting at (unix seconds).
error BeforeSpendPermissionStart(uint48 currentTimestamp, uint48 start);

/// @notice Recurring period has not started yet.
Expand Down Expand Up @@ -116,11 +119,11 @@ contract SpendPermissionManager is EIP712 {
/// @param spendPermission Details of the spend permission.
event SpendPermissionRevoked(bytes32 indexed hash, address indexed account, SpendPermission spendPermission);

/// @notice Register native token spend for a spend permission period.
/// @notice Register native or ERC-20 token spend for a spend permission period.
///
/// @param hash Hash of the spend permission.
/// @param account Account that spent native token via a spend permission.
/// @param token Account that spent native token via a spend permission.
/// @param account Account that spent tokens via a spend permission.
/// @param token Address of token spent via a spend permission.
/// @param newUsage Start and end of the current period with new spend usage (struct).
event SpendPermissionUsed(
bytes32 indexed hash, address indexed account, address indexed token, PeriodSpend newUsage
Expand All @@ -136,8 +139,6 @@ contract SpendPermissionManager is EIP712 {

/// @notice Approve a spend permission via a direct call from the account.
///
/// @dev Prevent phishing approvals by rejecting simulated transactions with the approval event.
///
/// @param spendPermission Details of the spend permission.
function approve(SpendPermission calldata spendPermission) external requireSender(spendPermission.account) {
_approve(spendPermission);
Expand All @@ -152,43 +153,43 @@ contract SpendPermissionManager is EIP712 {
emit SpendPermissionRevoked(hash, spendPermission.account, spendPermission);
}

/// @notice Approve a spend permission with signature and spend tokens in one call.
///
/// @dev After initially approving a spend permission, it is more gas efficient to call `spend` for repeated use.
///
/// @param spendPermission Details of the spend permission.
/// @param signature Signed approval from the user.
/// @param recipient Address to spend tokens to.
/// @param value Amount of token attempting to spend (wei).
function spendWithSignature(
SpendPermission memory spendPermission,
bytes memory signature,
address recipient,
uint160 value
) external requireSender(spendPermission.spender) {
approveWithSignature(spendPermission, signature);
spend(spendPermission, recipient, value);
}

/// @notice Approve a spend permission via a signature from the account.
///
/// @dev Compatible with ERC-6492 signatures (https://eips.ethereum.org/EIPS/eip-6492)
/// @dev Accounts are automatically deployed if init code present in signature.
/// @dev Compatible with ERC-6492 signatures (https://eips.ethereum.org/EIPS/eip-6492).
/// @dev Accounts are automatically deployed if initCode present in signature.
///
/// @param spendPermission Details of the spend permission.
/// @param signature Signed approval from the user.
function permit(SpendPermission memory spendPermission, bytes memory signature) public {
function approveWithSignature(SpendPermission memory spendPermission, bytes memory signature) public {
// validate signature over spend permission data and optionally deploy account
if (
!SignatureCheckerLib.isValidERC6492SignatureNowAllowSideEffects(
spendPermission.account, getHash(spendPermission), signature
)
) {
revert UnauthorizedSpendPermission();
revert InvalidSignature();
}
_approve(spendPermission);
}

/// @notice Approve a spend permission and spend tokens.
///
/// @dev Approves a spend permission for the first time and spends tokens in a single transaction.
///
/// @param spendPermission Details of the spend permission.
/// @param signature Signed approval from the user.
/// @param recipient Address to spend tokens to.
/// @param value Amount of token attempting to spend (wei).
function permitAndSpend(
SpendPermission memory spendPermission,
bytes memory signature,
address recipient,
uint160 value
) public requireSender(spendPermission.spender) {
permit(spendPermission, signature);
spend(spendPermission, recipient, value);
}

/// @notice Spend tokens using a spend permission.
///
/// @param spendPermission Details of the spend permission.
Expand All @@ -199,10 +200,10 @@ contract SpendPermissionManager is EIP712 {
requireSender(spendPermission.spender)
{
_useSpendPermission(spendPermission, value);
_transferFrom(spendPermission.account, spendPermission.token, recipient, value);
_transferFrom(spendPermission.token, spendPermission.account, recipient, value);
}

/// @notice Hash a SpendPermission struct for signing in accordance with EIP-191/712.
/// @notice Hash a SpendPermission struct for signing in accordance with EIP-712.
///
/// @param spendPermission Details of the spend permission.
///
Expand All @@ -211,7 +212,7 @@ contract SpendPermissionManager is EIP712 {
return _hashTypedData(keccak256(abi.encode(MESSAGE_TYPEHASH, spendPermission)));
}

/// @notice Return if spend permission is authorized i.e. approved and not revoked.
/// @notice Return if spend permission is approved and not revoked.
///
/// @param spendPermission Details of the spend permission.
///
Expand All @@ -221,14 +222,14 @@ contract SpendPermissionManager is EIP712 {
return !_isRevoked[hash][spendPermission.account] && _isApproved[hash][spendPermission.account];
}

/// @notice Get current period usage.
/// @notice Get current period spend.
///
/// @dev Reverts if spend permission has not started or has already ended.
/// @dev Period boundaries are at fixed intervals of [start + n * period, start + (n + 1) * period - 1].
///
/// @param spendPermission Details of the spend permission.
///
/// @return currentPeriod Currently active period with spend usage (struct).
/// @return currentPeriod Currently active period with cumulative spend (struct).
function getCurrentPeriod(SpendPermission memory spendPermission) public view returns (PeriodSpend memory) {
// check current timestamp is within spend permission time range
uint48 currentTimestamp = uint48(block.timestamp);
Expand All @@ -245,8 +246,7 @@ contract SpendPermissionManager is EIP712 {
bool lastPeriodExists = lastUpdatedPeriod.spend != 0;

// last period still active if current timestamp within [start, end - 1] range.
bool lastPeriodStillActive =
currentTimestamp < uint256(lastUpdatedPeriod.start) + uint256(spendPermission.period);
bool lastPeriodStillActive = currentTimestamp < uint256(lastUpdatedPeriod.end);

if (lastPeriodExists && lastPeriodStillActive) {
return lastUpdatedPeriod;
Expand Down Expand Up @@ -326,11 +326,13 @@ contract SpendPermissionManager is EIP712 {

/// @notice Transfer assets from an account to a recipient.
///
/// @param account Address of the user account.
/// @dev Assumes ERC-20 contract will revert if transfer fails. If silently fails, spend still marked as used.
///
/// @param token Address of the token contract.
/// @param account Address of the user account.
/// @param recipient Address of the token recipient.
/// @param value Amount of tokens to transfer.
function _transferFrom(address account, address token, address recipient, uint256 value) internal {
function _transferFrom(address token, address account, address recipient, uint256 value) internal {
// transfer tokens from account to recipient
if (token == NATIVE_TOKEN) {
_execute({account: account, target: recipient, value: value, data: hex""});
Expand Down Expand Up @@ -359,7 +361,7 @@ contract SpendPermissionManager is EIP712 {
/// @return name Name string for the EIP712 domain.
/// @return version Version string for the EIP712 domain.
function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) {
name = "SpendPermissionManager";
name = "Spend Permission Manager";
version = "1";
}
}
40 changes: 22 additions & 18 deletions test/src/SpendPermissions/permit.t.sol → ...endPermissions/approveWithSignature.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import {SpendPermissionManager} from "../../../src/SpendPermissionManager.sol";
import {SpendPermissionManagerBase} from "../../base/SpendPermissionManagerBase.sol";
import {CoinbaseSmartWallet} from "smart-wallet/CoinbaseSmartWallet.sol";

contract PermitTest is SpendPermissionManagerBase {
contract ApproveWithSignatureTest is SpendPermissionManagerBase {
function setUp() public {
_initializeSpendPermissionManager();
vm.prank(owner);
account.addOwnerAddress(address(mockSpendPermissionManager));
}

function test_permit_revert_unauthorizedSpendPermission(
function test_approveWithSignature_revert_invalidSignature(
uint128 invalidPk,
address spender,
address token,
Expand All @@ -35,11 +35,11 @@ contract PermitTest is SpendPermissionManagerBase {
});

bytes memory invalidSignature = _signSpendPermission(spendPermission, invalidPk, 0);
vm.expectRevert(abi.encodeWithSelector(SpendPermissionManager.UnauthorizedSpendPermission.selector));
mockSpendPermissionManager.permit(spendPermission, invalidSignature);
vm.expectRevert(abi.encodeWithSelector(SpendPermissionManager.InvalidSignature.selector));
mockSpendPermissionManager.approveWithSignature(spendPermission, invalidSignature);
}

function test_permit_revert_invalidStartEnd(
function test_approveWithSignature_revert_invalidStartEnd(
address spender,
uint48 start,
uint48 end,
Expand All @@ -60,10 +60,12 @@ contract PermitTest is SpendPermissionManagerBase {

bytes memory signature = _signSpendPermission(spendPermission, ownerPk, 0);
vm.expectRevert(abi.encodeWithSelector(SpendPermissionManager.InvalidStartEnd.selector, start, end));
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);
}

function test_permit_revert_zeroPeriod(address spender, uint48 start, uint48 end, uint160 allowance) public {
function test_approveWithSignature_revert_zeroPeriod(address spender, uint48 start, uint48 end, uint160 allowance)
public
{
vm.assume(start < end);

SpendPermissionManager.SpendPermission memory spendPermission = SpendPermissionManager.SpendPermission({
Expand All @@ -78,10 +80,12 @@ contract PermitTest is SpendPermissionManagerBase {

bytes memory signature = _signSpendPermission(spendPermission, ownerPk, 0);
vm.expectRevert(abi.encodeWithSelector(SpendPermissionManager.ZeroPeriod.selector));
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);
}

function test_permit_revert_zeroAllowance(address spender, uint48 start, uint48 end, uint48 period) public {
function test_approveWithSignature_revert_zeroAllowance(address spender, uint48 start, uint48 end, uint48 period)
public
{
vm.assume(start < end);
vm.assume(period > 0);

Expand All @@ -97,10 +101,10 @@ contract PermitTest is SpendPermissionManagerBase {

bytes memory signature = _signSpendPermission(spendPermission, ownerPk, 0);
vm.expectRevert(abi.encodeWithSelector(SpendPermissionManager.ZeroAllowance.selector));
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);
}

function test_permit_success_isApproved(
function test_approveWithSignature_success_isApproved(
address spender,
address token,
uint48 start,
Expand All @@ -123,11 +127,11 @@ contract PermitTest is SpendPermissionManagerBase {
});

bytes memory signature = _signSpendPermission(spendPermission, ownerPk, 0);
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);
vm.assertTrue(mockSpendPermissionManager.isApproved(spendPermission));
}

function test_permit_success_emitsEvent(
function test_approveWithSignature_success_emitsEvent(
address spender,
address token,
uint48 start,
Expand Down Expand Up @@ -156,10 +160,10 @@ contract PermitTest is SpendPermissionManagerBase {
account: address(account),
spendPermission: spendPermission
});
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);
}

function test_permit_success_erc6492SignaturePreDeploy(
function test_approveWithSignature_success_erc6492SignaturePreDeploy(
uint128 ownerPk,
address spender,
address token,
Expand Down Expand Up @@ -194,14 +198,14 @@ contract PermitTest is SpendPermissionManagerBase {
vm.assertEq(counterfactualAccount.code.length, 0);

// submit the spend permission with the signature, see permit succeed
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);

// verify that the account is now deployed (has code) and that a call to isValidSignature returns true
vm.assertGt(counterfactualAccount.code.length, 0);
vm.assertTrue(mockSpendPermissionManager.isApproved(spendPermission));
}

function test_permit_success_erc6492SignatureAlreadyDeployed(
function test_approveWithSignature_success_erc6492SignatureAlreadyDeployed(
uint128 ownerPk,
address spender,
address token,
Expand Down Expand Up @@ -237,7 +241,7 @@ contract PermitTest is SpendPermissionManagerBase {
vm.assertGt(counterfactualAccount.code.length, 0);

// submit the spend permission with the signature, see permit succeed
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);
vm.assertTrue(mockSpendPermissionManager.isApproved(spendPermission));
}
}
2 changes: 1 addition & 1 deletion test/src/SpendPermissions/spend.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ contract SpendTest is SpendPermissionManagerBase {
vm.warp(start);

vm.startPrank(spender);
mockSpendPermissionManager.permit(spendPermission, signature);
mockSpendPermissionManager.approveWithSignature(spendPermission, signature);
mockSpendPermissionManager.spend(spendPermission, recipient, spend);

assertEq(address(account).balance, allowance - spend);
Expand Down
Loading

0 comments on commit d9b8ca4

Please sign in to comment.