Skip to content

Commit

Permalink
Improve stake & lock UX (#16)
Browse files Browse the repository at this point in the history
* Improve stake & lock UX

* Skip calling .permit() if the data might be bad.

* forge fmt

* update the if statement wrapping permit call

* flip the if statement

* >= for permit.deadline check

---------

Co-authored-by: bxmmm1 <bxmmm1@users.noreply.github.com>
  • Loading branch information
bxmmm1 and bxmmm1 committed Jul 31, 2024
1 parent ed7744d commit 2d26e21
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 49 deletions.
4 changes: 2 additions & 2 deletions mainnet-contracts/script/DeployPufferL2Depositor.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ contract DeployPufferL2Depositor is Script {

vm.startBroadcast();

depositor = new PufferL2Depositor(address(accessManager), weth);

address pufLockerImpl = address(new PufLocker());
pufLocker = PufLocker(
address(new ERC1967Proxy(pufLockerImpl, abi.encodeCall(PufLocker.initialize, (address(accessManager)))))
);

depositor = new PufferL2Depositor(address(accessManager), weth, pufLocker);

bytes[] memory calldatas = new bytes[](4);

bytes4[] memory publicSelectors = new bytes4[](3);
Expand Down
15 changes: 10 additions & 5 deletions mainnet-contracts/src/PufLocker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract PufLocker is IPufLocker, AccessManagedUpgradeable, UUPSUpgradeable, Puf
* @inheritdoc IPufLocker
* @dev Restricted in this context is like `whenNotPaused` modifier from Pausable.sol
*/
function deposit(address token, uint128 lockPeriod, Permit calldata permitData)
function deposit(address token, address recipient, uint128 lockPeriod, Permit calldata permitData)
external
isAllowedToken(token)
restricted
Expand All @@ -55,8 +55,13 @@ contract PufLocker is IPufLocker, AccessManagedUpgradeable, UUPSUpgradeable, Puf
if (lockPeriod < $.minLockPeriod || lockPeriod > $.maxLockPeriod) {
revert InvalidLockPeriod();
}
// if the first 32 bytes of the signature is non-zero
if (permitData.r != 0) {

// The users that use a smart wallet and do not use the Permit and they do the .approve and then .deposit.
// They might get confused when they open Etherscan, and see:
// "Although one or more Error Occurred [execution reverted] Contract Execution Completed"

// To avoid that, we don't want to call the permit function if it is not necessary.
if (permitData.deadline >= block.timestamp) {
// https://docs.openzeppelin.com/contracts/5.x/api/token/erc20#security_considerations
try ERC20Permit(token).permit({
owner: msg.sender,
Expand All @@ -73,9 +78,9 @@ contract PufLocker is IPufLocker, AccessManagedUpgradeable, UUPSUpgradeable, Puf

uint128 releaseTime = uint128(block.timestamp) + lockPeriod;

$.deposits[msg.sender][token].push(Deposit(uint128(permitData.amount), releaseTime));
$.deposits[recipient][token].push(Deposit(uint128(permitData.amount), releaseTime));

emit Deposited(msg.sender, token, uint128(permitData.amount), releaseTime);
emit Deposited(recipient, token, uint128(permitData.amount), releaseTime);
}

/**
Expand Down
62 changes: 48 additions & 14 deletions mainnet-contracts/src/PufferL2Depositor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/ERC2
import { Permit } from "./structs/Permit.sol";
import { IWETH } from "./interface/IWETH.sol";
import { IPufferL2Depositor } from "./interface/IPufferL2Depositor.sol";
import { IPufLocker } from "./interface/IPufLocker.sol";

/**
* @title Puffer L2 Depositor contract
Expand All @@ -25,11 +26,14 @@ contract PufferL2Depositor is IPufferL2Depositor, AccessManaged {

address public immutable WETH;

IPufLocker public immutable PUFFER_LOCKER;

mapping(address token => address pufToken) public tokens;
mapping(address migrator => bool isAllowed) public isAllowedMigrator;

constructor(address accessManager, address weth) AccessManaged(accessManager) {
constructor(address accessManager, address weth, IPufLocker locker) AccessManaged(accessManager) {
WETH = weth;
PUFFER_LOCKER = locker;
_addNewToken(weth);
}

Expand All @@ -44,13 +48,19 @@ contract PufferL2Depositor is IPufferL2Depositor, AccessManaged {
* @inheritdoc IPufferL2Depositor
* @dev Restricted in this context is like `whenNotPaused` modifier from Pausable.sol
*/
function deposit(address token, address account, Permit calldata permitData, uint256 referralCode)
external
onlySupportedTokens(token)
restricted
{
// if the first 32 bytes of the signature is non-zero
if (permitData.r != 0) {
function deposit(
address token,
address account,
Permit calldata permitData,
uint256 referralCode,
uint128 lockPeriod
) external onlySupportedTokens(token) restricted {
// The users that use a smart wallet and do not use the Permit and they do the .approve and then .deposit.
// They might get confused when they open Etherscan, and see:
// "Although one or more Error Occurred [execution reverted] Contract Execution Completed"

// To avoid that, we don't want to call the permit function if it is not necessary.
if (permitData.deadline >= block.timestamp) {
// https://docs.openzeppelin.com/contracts/5.x/api/token/erc20#security_considerations
try ERC20Permit(token).permit({
owner: msg.sender,
Expand All @@ -66,6 +76,7 @@ contract PufferL2Depositor is IPufferL2Depositor, AccessManaged {
IERC20(token).safeTransferFrom(msg.sender, address(this), permitData.amount);

_deposit({
lockPeriod: lockPeriod,
token: token,
depositor: msg.sender,
account: account,
Expand All @@ -78,10 +89,17 @@ contract PufferL2Depositor is IPufferL2Depositor, AccessManaged {
* @inheritdoc IPufferL2Depositor
* @dev Restricted in this context is like `whenNotPaused` modifier from Pausable.sol
*/
function depositETH(address account, uint256 referralCode) external payable restricted {
function depositETH(address account, uint256 referralCode, uint128 lockPeriod) external payable restricted {
IWETH(WETH).deposit{ value: msg.value }();

_deposit({ token: WETH, depositor: msg.sender, account: account, amount: msg.value, referralCode: referralCode });
_deposit({
token: WETH,
depositor: msg.sender,
account: account,
amount: msg.value,
referralCode: referralCode,
lockPeriod: lockPeriod
});
}

/**
Expand Down Expand Up @@ -120,14 +138,30 @@ contract PufferL2Depositor is IPufferL2Depositor, AccessManaged {
*/
function revertIfPaused() external restricted { }

function _deposit(address token, address depositor, address account, uint256 amount, uint256 referralCode)
internal
{
function _deposit(
address token,
address depositor,
address account,
uint256 amount,
uint256 referralCode,
uint128 lockPeriod
) internal {
PufToken pufToken = PufToken(tokens[token]);

IERC20(token).safeIncreaseAllowance(address(pufToken), amount);

pufToken.deposit(depositor, account, amount);
// If the lockPeriod is greater than 0 we wrap and then deposit the wrapped tokens to the locker contract
if (lockPeriod > 0) {
pufToken.deposit(depositor, address(this), amount);
IERC20(address(pufToken)).safeIncreaseAllowance(address(PUFFER_LOCKER), amount);
Permit memory permitData;
permitData.amount = amount;
// Tokens are being deposited to the locker contract for the account
PUFFER_LOCKER.deposit(address(pufToken), account, lockPeriod, permitData);
} else {
// The account will receive the ERC20 tokens
pufToken.deposit(depositor, account, amount);
}

emit DepositedToken(token, msg.sender, account, amount, referralCode);
}
Expand Down
3 changes: 2 additions & 1 deletion mainnet-contracts/src/interface/IPufLocker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ interface IPufLocker {
/**
* @notice Deposit tokens into the locker
* @param token The address of the token to deposit
* @param token The address of the recipient
* @param lockPeriod The lock period for the deposit
* @param permitData The permit data for the deposit
*/
function deposit(address token, uint128 lockPeriod, Permit calldata permitData) external;
function deposit(address token, address recipient, uint128 lockPeriod, Permit calldata permitData) external;

/**
* @notice Withdraws specified deposits for a given token and transfers the funds to the recipient
Expand Down
10 changes: 8 additions & 2 deletions mainnet-contracts/src/interface/IPufferL2Depositor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,20 @@ interface IPufferL2Depositor {
*
* @dev Restricted in this context is like `whenNotPaused` modifier from Pausable.sol
*/
function deposit(address token, address account, Permit calldata permitData, uint256 referralCode) external;
function deposit(
address token,
address account,
Permit calldata permitData,
uint256 referralCode,
uint128 lockPeriod
) external;

/**
* @notice Deposits naative ETH by wrapping it into WETH and then depositing to corresponding token contract
*
* @dev Restricted in this context is like `whenNotPaused` modifier from Pausable.sol
*/
function depositETH(address account, uint256 referralCode) external payable;
function depositETH(address account, uint256 referralCode, uint128 lockPeriod) external payable;

/**
* @notice Called by the Token contracts to check if the system is paused
Expand Down
25 changes: 11 additions & 14 deletions mainnet-contracts/test/unit/PufLocker.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import { IPufLocker } from "../../src/interface/IPufLocker.sol";
import { AccessManager } from "@openzeppelin/contracts/access/manager/AccessManager.sol";
import { Multicall } from "@openzeppelin/contracts/utils/Multicall.sol";
import { ROLE_ID_OPERATIONS_MULTISIG, PUBLIC_ROLE } from "../../script/Roles.sol";
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import { IAccessManaged } from "@openzeppelin/contracts/access/manager/IAccessManaged.sol";
import { ERC20Mock } from "../mocks/ERC20Mock.sol";
Expand Down Expand Up @@ -75,7 +72,7 @@ contract PufLockerTest is UnitTestHelper {
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());

pufLocker.deposit(address(mockToken), 61, permit); // Lock for 1 minute
pufLocker.deposit(address(mockToken), address(this), 61, permit); // Lock for 1 minute
}

function testRevert_SetAllowedToken_Unauthorized() public {
Expand Down Expand Up @@ -105,7 +102,7 @@ contract PufLockerTest is UnitTestHelper {
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());

pufLocker.deposit(address(mockToken), 3600, permit); // Lock for 1 hour
pufLocker.deposit(address(mockToken), bob, 3600, permit); // Lock for 1 hour
(PufLocker.Deposit[] memory deposits) = pufLocker.getDeposits(bob, address(mockToken), 0, 1);
assertEq(deposits.length, 1, "Should have 1 deposit");
assertEq(deposits[0].amount, 10e18, "Deposit amount should be 100 tokens");
Expand All @@ -118,7 +115,7 @@ contract PufLockerTest is UnitTestHelper {
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());
vm.expectRevert(abi.encodeWithSelector(InvalidAmount.selector));
pufLocker.deposit(address(mockToken), 3600, permit); // Lock for 1 hour with 0 amount
pufLocker.deposit(address(mockToken), bob, 3600, permit); // Lock for 1 hour with 0 amount
vm.stopPrank();
}

Expand All @@ -128,7 +125,7 @@ contract PufLockerTest is UnitTestHelper {
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());
vm.expectRevert(abi.encodeWithSelector(IPufLocker.InvalidLockPeriod.selector));
pufLocker.deposit(address(mockToken), 30, permit); // Lock for 30 seconds, which is less than minLockPeriod
pufLocker.deposit(address(mockToken), bob, 30, permit); // Lock for 30 seconds, which is less than minLockPeriod
vm.stopPrank();
}

Expand All @@ -137,7 +134,7 @@ contract PufLockerTest is UnitTestHelper {
uint256 amount = 10e18;
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());
pufLocker.deposit(address(mockToken), 60, permit); // Lock for 1 minute
pufLocker.deposit(address(mockToken), bob, 60, permit); // Lock for 1 minute
vm.warp(block.timestamp + 61); // Fast forward time by 61 seconds

uint256[] memory indexes = new uint256[](1);
Expand All @@ -155,7 +152,7 @@ contract PufLockerTest is UnitTestHelper {
uint256 amount = 10e18;
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());
pufLocker.deposit(address(mockToken), 3600, permit); // Lock for 1 hour
pufLocker.deposit(address(mockToken), bob, 3600, permit); // Lock for 1 hour
uint256[] memory indexes = new uint256[](1);
indexes[0] = 0;

Expand All @@ -169,7 +166,7 @@ contract PufLockerTest is UnitTestHelper {
uint256 amount = 10e18;
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());
pufLocker.deposit(address(mockToken), 60, permit); // Lock for 1 minute
pufLocker.deposit(address(mockToken), bob, 60, permit); // Lock for 1 minute
vm.warp(block.timestamp + 61); // Fast forward time by 61 seconds

uint256[] memory indexes = new uint256[](2);
Expand All @@ -186,7 +183,7 @@ contract PufLockerTest is UnitTestHelper {
uint256 amount = 10e18;
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());
pufLocker.deposit(address(mockToken), 60, permit); // Lock for 1 minute
pufLocker.deposit(address(mockToken), bob, 60, permit); // Lock for 1 minute
vm.warp(block.timestamp + 61); // Fast forward time by 61 seconds

uint256[] memory indexes = new uint256[](1);
Expand All @@ -206,20 +203,20 @@ contract PufLockerTest is UnitTestHelper {
uint256 amount = 2e18;
Permit memory permit =
_signPermit(_testTemps("bob", address(pufLocker), amount, block.timestamp), mockToken.DOMAIN_SEPARATOR());
pufLocker.deposit(address(mockToken), 3601, permit);
pufLocker.deposit(address(mockToken), bob, 3601, permit);

uint256 amount2 = 4e18;
Permit memory permit2 =
_signPermit(_testTemps("bob", address(pufLocker), amount2, block.timestamp), mockToken.DOMAIN_SEPARATOR());
mockToken.approve(address(pufLocker), amount2);
pufLocker.deposit(address(mockToken), 3620, permit2);
pufLocker.deposit(address(mockToken), bob, 3620, permit2);

uint256 amount3 = 5e18;
Permit memory permit3 = _signPermit(
_testTemps("bob", address(pufLocker), amount3, block.timestamp + 2), mockToken.DOMAIN_SEPARATOR()
);
mockToken.approve(address(pufLocker), amount3);
pufLocker.deposit(address(mockToken), 3630, permit3);
pufLocker.deposit(address(mockToken), bob, 3630, permit3);

// Get the first 2 deposits
PufLocker.Deposit[] memory depositsPage1 = pufLocker.getDeposits(bob, address(mockToken), 0, 2);
Expand Down
Loading

0 comments on commit 2d26e21

Please sign in to comment.