Skip to content
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

Unnecessary future deadline value passed to swap functions #2

Open
andreiashu opened this issue Oct 23, 2021 · 0 comments
Open

Unnecessary future deadline value passed to swap functions #2

andreiashu opened this issue Oct 23, 2021 · 0 comments

Comments

@andreiashu
Copy link
Member

andreiashu commented Oct 23, 2021

Description

Treasury.buybackRnbw() uses Uniswap V2 to convert underlying tokens into USDC:

rnbwBought = IUniswapV2Router02(router).swapExactTokensForTokens(
usdcBalance,
0,
path,
address(this),
block.timestamp + 60
)[0];

The deadline argument passed to swapExactTokensForTokens function is 60 blocks into the future. The deadline parameter is useful for frontend and other off-chain software to ensure there's a deadline after which a swap transaction will revert.

In this case, passing just block.timestamp is enough to ensure correct behavior:

Uniswap's swapExactTokensForTokens definition:

    function swapExactTokensForTokens(
        uint amountIn,
        uint amountOutMin,
        address[] calldata path,
        address to,
        uint deadline
    ) external override ensure(deadline) returns (uint[] memory amounts) {

Uniswap ensure modifier:

    modifier ensure(uint deadline) {
        require(deadline >= block.timestamp, 'UniswapV2Router: EXPIRED');
        _;
    }

Recommendation

Instead of block.timestamp + 60 just pass block.timestamp as the deadline argument to swapExactTokensForTokens call.

Notes

A similar change can be made to the originSwap call:

uint256 targetAmount =
ICurve(curveAddress).originSwap(
_underlying,
USDC,
_underlyingAmount,
0,
block.timestamp + 60
);

The issue here though is that you still need to add +1 to the block.timestamp because of the way the deadline modifier in Curve.sol is defined. Because of this, we leave it to the Xave Finance team the decision change the call to originSwap since there are no (gas) benefits, although it might provide more clarity to the reader:

    modifier deadline(uint256 _deadline) {
        require(block.timestamp < _deadline, "Curve/tx-deadline-passed");
        _;
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant