From 5e89d24116f7e1ee2c89e8117b0d6a5188ace7c8 Mon Sep 17 00:00:00 2001 From: Ben DiFrancesco Date: Wed, 7 Feb 2024 10:47:12 -0500 Subject: [PATCH] Implement, test, and document permit-based stake & stakeMore methods --- src/UniStaker.sol | 54 +++++++ src/interfaces/IERC20Delegates.sol | 9 ++ test/UniStaker.t.sol | 246 ++++++++++++++++++++++++++++- test/mocks/MockERC20Votes.sol | 25 ++- 4 files changed, 327 insertions(+), 7 deletions(-) diff --git a/src/UniStaker.sol b/src/UniStaker.sol index 3c5f45a..b0868fe 100644 --- a/src/UniStaker.sol +++ b/src/UniStaker.sol @@ -275,6 +275,33 @@ contract UniStaker is INotifiableRewardReceiver, ReentrancyGuard, Multicall, EIP _depositId = _stake(msg.sender, _amount, _delegatee, _beneficiary); } + /// @notice Method to stake tokens to a new deposit. The caller must approve the staking + /// contract to spend at least the would-be staked amount of the token via a signature which is + /// is also provided, and is passed to the token contract's permit method before the staking + /// operation occurs. + /// @param _amount Quantity of the staking token to stake. + /// @param _delegatee Address to assign the governance voting weight of the staked tokens. + /// @param _beneficiary Address that will accrue rewards for this stake. + /// @param _deadline The timestamp at which the permit signature should expire. + /// @param _v ECDSA signature component: Parity of the `y` coordinate of point `R` + /// @param _r ECDSA signature component: x-coordinate of `R` + /// @param _s ECDSA signature component: `s` value of the signature + /// @return _depositId Unique identifier for this deposit. + /// @dev Neither the delegatee nor the beneficiary may be the zero address. The deposit will be + /// owned by the message sender. + function permitAndStake( + uint256 _amount, + address _delegatee, + address _beneficiary, + uint256 _deadline, + uint8 _v, + bytes32 _r, + bytes32 _s + ) external nonReentrant returns (DepositIdentifier _depositId) { + STAKE_TOKEN.permit(msg.sender, address(this), _amount, _deadline, _v, _r, _s); + _depositId = _stake(msg.sender, _amount, _delegatee, _beneficiary); + } + /// @notice Stake tokens to a new deposit on behalf of a user, using a signature to validate the /// user's intent. The caller must pre-approve the staking contract to spend at least the /// would-be staked amount of the token. @@ -318,6 +345,33 @@ contract UniStaker is INotifiableRewardReceiver, ReentrancyGuard, Multicall, EIP _stakeMore(deposit, _depositId, _amount); } + /// @notice Add more staking tokens to an existing deposit. A staker should call this method when + /// they have an existing deposit, and wish to stake more while retaining the same delegatee and + /// beneficiary. The caller must approve the staking contract to spend at least the would-be + /// staked amount of the token via a signature which is is also provided, and is passed to the + /// token contract's permit method before the staking operation occurs. + /// @param _depositId Unique identifier of the deposit to which stake will be added. + /// @param _amount Quantity of stake to be added. + /// @param _deadline The timestamp at which the permit signature should expire. + /// @param _v ECDSA signature component: Parity of the `y` coordinate of point `R` + /// @param _r ECDSA signature component: x-coordinate of `R` + /// @param _s ECDSA signature component: `s` value of the signature + /// @dev The message sender must be the owner of the deposit. + function permitAndStakeMore( + DepositIdentifier _depositId, + uint256 _amount, + uint256 _deadline, + uint8 _v, + bytes32 _r, + bytes32 _s + ) external nonReentrant { + Deposit storage deposit = deposits[_depositId]; + _revertIfNotDepositOwner(deposit, msg.sender); + + STAKE_TOKEN.permit(msg.sender, address(this), _amount, _deadline, _v, _r, _s); + _stakeMore(deposit, _depositId, _amount); + } + /// @notice Add more staking tokens to an existing deposit on behalf of a user, using a signature /// to validate the user's intent. A staker should call this method when they have an existing /// deposit, and wish to stake more while retaining the same delegatee and beneficiary. diff --git a/src/interfaces/IERC20Delegates.sol b/src/interfaces/IERC20Delegates.sol index 2c6e469..46353e3 100644 --- a/src/interfaces/IERC20Delegates.sol +++ b/src/interfaces/IERC20Delegates.sol @@ -15,6 +15,15 @@ interface IERC20Delegates { function totalSupply() external view returns (uint256); function transfer(address dst, uint256 rawAmount) external returns (bool); function transferFrom(address src, address dst, uint256 rawAmount) external returns (bool); + function permit( + address owner, + address spender, + uint256 rawAmount, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) external; // ERC20Votes delegation methods function delegate(address delegatee) external; diff --git a/test/UniStaker.t.sol b/test/UniStaker.t.sol index 16e666b..51d79e9 100644 --- a/test/UniStaker.t.sol +++ b/test/UniStaker.t.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.23; import {Vm, Test, stdStorage, StdStorage, console2} from "forge-std/Test.sol"; import {UniStaker, DelegationSurrogate, IERC20, IERC20Delegates} from "src/UniStaker.sol"; import {UniStakerHarness} from "test/harnesses/UniStakerHarness.sol"; -import {ERC20VotesMock} from "test/mocks/MockERC20Votes.sol"; +import {ERC20VotesMock, ERC20Permit} from "test/mocks/MockERC20Votes.sol"; import {ERC20Fake} from "test/fakes/ERC20Fake.sol"; contract UniStakerTest is Test { @@ -21,6 +21,9 @@ contract UniStakerTest is Test { ) ); + bytes32 constant PERMIT_TYPEHASH = + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + event RewardNotifierSet(address indexed account, bool isEnabled); event AdminSet(address indexed oldAdmin, address indexed newAdmin); @@ -706,6 +709,93 @@ contract Stake is UniStakerTest { } } +contract PermitAndStake is UniStakerTest { + using stdStorage for StdStorage; + + function testFuzz_PerformsTheApprovalByCallingPermitThenPerformsStake( + uint256 _depositorPrivateKey, + uint256 _depositAmount, + address _delegatee, + address _beneficiary, + uint256 _deadline, + uint256 _currentNonce + ) public { + vm.assume(_delegatee != address(0) && _beneficiary != address(0)); + _deadline = bound(_deadline, block.timestamp + 1, type(uint256).max); + _depositorPrivateKey = bound(_depositorPrivateKey, 1, 100e18); + address _depositor = vm.addr(_depositorPrivateKey); + _depositAmount = _boundMintAmount(_depositAmount); + _mintGovToken(_depositor, _depositAmount); + + stdstore.target(address(govToken)).sig("nonces(address)").with_key(_depositor).checked_write( + _currentNonce + ); + + bytes32 _message = keccak256( + abi.encode( + PERMIT_TYPEHASH, + _depositor, + address(uniStaker), + _depositAmount, + govToken.nonces(_depositor), + _deadline + ) + ); + + bytes32 _messageHash = + keccak256(abi.encodePacked("\x19\x01", govToken.DOMAIN_SEPARATOR(), _message)); + (uint8 _v, bytes32 _r, bytes32 _s) = vm.sign(_depositorPrivateKey, _messageHash); + + vm.prank(_depositor); + UniStaker.DepositIdentifier _depositId = + uniStaker.permitAndStake(_depositAmount, _delegatee, _beneficiary, _deadline, _v, _r, _s); + UniStaker.Deposit memory _deposit = _fetchDeposit(_depositId); + + assertEq(_deposit.balance, _depositAmount); + assertEq(_deposit.owner, _depositor); + assertEq(_deposit.delegatee, _delegatee); + assertEq(_deposit.beneficiary, _beneficiary); + } + + function testFuzz_RevertIf_ThePermitSignatureIsInvalid( + address _notDepositor, + uint256 _depositorPrivateKey, + uint256 _depositAmount, + address _delegatee, + address _beneficiary, + uint256 _deadline + ) public { + vm.assume(_delegatee != address(0) && _beneficiary != address(0)); + _deadline = bound(_deadline, block.timestamp + 1, type(uint256).max); + _depositorPrivateKey = bound(_depositorPrivateKey, 1, 100e18); + address _depositor = vm.addr(_depositorPrivateKey); + vm.assume(_notDepositor != _depositor); + _depositAmount = _boundMintAmount(_depositAmount); + _mintGovToken(_depositor, _depositAmount); + + bytes32 _message = keccak256( + abi.encode( + PERMIT_TYPEHASH, + _notDepositor, + address(uniStaker), + _depositAmount, + govToken.nonces(_depositor), + _deadline + ) + ); + + bytes32 _messageHash = + keccak256(abi.encodePacked("\x19\x01", govToken.DOMAIN_SEPARATOR(), _message)); + (uint8 _v, bytes32 _r, bytes32 _s) = vm.sign(_depositorPrivateKey, _messageHash); + + vm.prank(_notDepositor); + vm.expectRevert( + abi.encodeWithSelector(ERC20Permit.ERC2612InvalidSigner.selector, _depositor, _notDepositor) + ); + uniStaker.permitAndStake(_depositAmount, _delegatee, _beneficiary, _deadline, _v, _r, _s); + } +} + contract StakeOnBehalf is UniStakerTest { using stdStorage for StdStorage; @@ -1052,6 +1142,160 @@ contract StakeMore is UniStakerTest { } } +contract PermitAndStakeMore is UniStakerTest { + using stdStorage for StdStorage; + + function testFuzz_PerformsTheApprovalByCallingPermitThenPerformsStakeMore( + uint256 _depositorPrivateKey, + uint256 _initialDepositAmount, + uint256 _stakeMoreAmount, + address _delegatee, + address _beneficiary, + uint256 _currentNonce, + uint256 _deadline + ) public { + vm.assume(_delegatee != address(0) && _beneficiary != address(0)); + _depositorPrivateKey = bound(_depositorPrivateKey, 1, 100e18); + address _depositor = vm.addr(_depositorPrivateKey); + _deadline = bound(_deadline, block.timestamp + 1, type(uint256).max); + + UniStaker.DepositIdentifier _depositId; + (_initialDepositAmount, _depositId) = + _boundMintAndStake(_depositor, _initialDepositAmount, _delegatee, _beneficiary); + + _stakeMoreAmount = _boundToRealisticStake(_stakeMoreAmount); + _mintGovToken(_depositor, _stakeMoreAmount); + + stdstore.target(address(govToken)).sig("nonces(address)").with_key(_depositor).checked_write( + _currentNonce + ); + + // Separate scope to avoid stack to deep errors + { + bytes32 _message = keccak256( + abi.encode( + PERMIT_TYPEHASH, + _depositor, + address(uniStaker), + _stakeMoreAmount, + govToken.nonces(_depositor), + _deadline + ) + ); + + bytes32 _messageHash = + keccak256(abi.encodePacked("\x19\x01", govToken.DOMAIN_SEPARATOR(), _message)); + (uint8 _v, bytes32 _r, bytes32 _s) = vm.sign(_depositorPrivateKey, _messageHash); + + vm.prank(_depositor); + uniStaker.permitAndStakeMore(_depositId, _stakeMoreAmount, _deadline, _v, _r, _s); + } + + UniStaker.Deposit memory _deposit = _fetchDeposit(_depositId); + + assertEq(_deposit.balance, _initialDepositAmount + _stakeMoreAmount); + assertEq(_deposit.owner, _depositor); + assertEq(_deposit.delegatee, _delegatee); + assertEq(_deposit.beneficiary, _beneficiary); + } + + function testFuzz_RevertIf_CallerIsNotTheDepositOwner( + address _depositor, + uint256 _notDepositorPrivateKey, + uint256 _initialDepositAmount, + uint256 _stakeMoreAmount, + address _delegatee, + address _beneficiary, + uint256 _deadline + ) public { + vm.assume(_delegatee != address(0) && _beneficiary != address(0)); + _notDepositorPrivateKey = bound(_notDepositorPrivateKey, 1, 100e18); + address _notDepositor = vm.addr(_notDepositorPrivateKey); + vm.assume(_depositor != _notDepositor); + _deadline = bound(_deadline, block.timestamp + 1, type(uint256).max); + + UniStaker.DepositIdentifier _depositId; + (_initialDepositAmount, _depositId) = + _boundMintAndStake(_depositor, _initialDepositAmount, _delegatee, _beneficiary); + + _stakeMoreAmount = _boundToRealisticStake(_stakeMoreAmount); + _mintGovToken(_depositor, _stakeMoreAmount); + + // Separate scope to avoid stack to deep errors + { + bytes32 _message = keccak256( + abi.encode( + PERMIT_TYPEHASH, + _notDepositor, + address(uniStaker), + _stakeMoreAmount, + govToken.nonces(_depositor), + _deadline + ) + ); + + bytes32 _messageHash = + keccak256(abi.encodePacked("\x19\x01", govToken.DOMAIN_SEPARATOR(), _message)); + (uint8 _v, bytes32 _r, bytes32 _s) = vm.sign(_notDepositorPrivateKey, _messageHash); + + vm.prank(_notDepositor); + vm.expectRevert( + abi.encodeWithSelector( + UniStaker.UniStaker__Unauthorized.selector, bytes32("not owner"), _notDepositor + ) + ); + uniStaker.permitAndStakeMore(_depositId, _stakeMoreAmount, _deadline, _v, _r, _s); + } + } + + function testFuzz_RevertIf_ThePermitSignatureIsInvalid( + uint256 _initialDepositAmount, + address _delegatee, + address _beneficiary + ) public { + vm.assume(_delegatee != address(0) && _beneficiary != address(0)); + + // We can't fuzz the these values because we need to pre-compute the invalid + // recovered signer so we can expect it in the revert error message thrown + (address _depositor, uint256 _depositorPrivateKey) = makeAddrAndKey("depositor"); + uint256 _stakeMoreAmount = 1578e18; + uint256 _deadline = 1e18 days; + uint256 _wrongNonce = 1; + // If any of the values defined above are changed, the expected recovered address must also + // be recalculated and updated. + address _expectedRecoveredSigner = address(0xF03C6C880C40b5698e466C136C460ea71A0C5E33); + + UniStaker.DepositIdentifier _depositId; + (_initialDepositAmount, _depositId) = + _boundMintAndStake(_depositor, _initialDepositAmount, _delegatee, _beneficiary); + _mintGovToken(_depositor, _stakeMoreAmount); + + bytes32 _message = keccak256( + abi.encode( + PERMIT_TYPEHASH, + _depositor, + address(uniStaker), + _stakeMoreAmount, + _wrongNonce, // intentionally incorrect nonce, which should be 0 + _deadline + ) + ); + + bytes32 _messageHash = + keccak256(abi.encodePacked("\x19\x01", govToken.DOMAIN_SEPARATOR(), _message)); + + (uint8 _v, bytes32 _r, bytes32 _s) = vm.sign(_depositorPrivateKey, _messageHash); + + vm.prank(_depositor); + vm.expectRevert( + abi.encodeWithSelector( + ERC20Permit.ERC2612InvalidSigner.selector, _expectedRecoveredSigner, _depositor + ) + ); + uniStaker.permitAndStakeMore(_depositId, _stakeMoreAmount, _deadline, _v, _r, _s); + } +} + contract StakeMoreOnBehalf is UniStakerTest { using stdStorage for StdStorage; diff --git a/test/mocks/MockERC20Votes.sol b/test/mocks/MockERC20Votes.sol index f6fd45c..3e63324 100644 --- a/test/mocks/MockERC20Votes.sol +++ b/test/mocks/MockERC20Votes.sol @@ -1,17 +1,18 @@ // SPDX-License-Identifier: AGPL-3.0-only pragma solidity 0.8.23; -import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol"; import {IERC20Delegates} from "src/interfaces/IERC20Delegates.sol"; +import {ERC20, ERC20Permit} from "openzeppelin/token/ERC20/extensions/ERC20Permit.sol"; -/// @dev An ERC20 token that allows for public minting and mocks the delegation methods used in -/// ERC20Votes governance tokens. It does not included check pointing functionality. This contract -/// is intended only for use as a stand in for contracts that interface with ERC20Votes tokens. -contract ERC20VotesMock is IERC20Delegates, ERC20 { +/// @dev An ERC20Permit token that allows for public minting and mocks the delegation methods used +/// in ERC20Votes governance tokens. It does not included check pointing functionality. This +/// contract is intended only for use as a stand in for contracts that interface with ERC20Votes +// tokens. +contract ERC20VotesMock is IERC20Delegates, ERC20Permit { /// @dev Track delegations for mocked delegation methods mapping(address account => address delegate) private delegations; - constructor() ERC20("Governance Token", "GOV") {} + constructor() ERC20("Governance Token", "GOV") ERC20Permit("Governance Token") {} /// @dev Public mint function useful for testing function mint(address _account, uint256 _value) public { @@ -88,4 +89,16 @@ contract ERC20VotesMock is IERC20Delegates, ERC20 { { return ERC20.transferFrom(src, dst, rawAmount); } + + function permit( + address owner, + address spender, + uint256 rawAmount, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public override(IERC20Delegates, ERC20Permit) { + return ERC20Permit.permit(owner, spender, rawAmount, deadline, v, r, s); + } }