tokamak-network / ton-staking-v2

8 stars 3 forks source link

H_4. approveAndCall check #9

Closed zzooppii closed 4 months ago

zzooppii commented 4 months ago

Describe the bug Rather than calling approveAndCall, wouldn't it save more on gas costs to check ton.allowance like in the _approve format and call only the swapFromTON function of the WTON Contract?

Configuration

Impact When calling approveAndCall, there is a process of encoding and decoding the address, so more gas costs are consumed.

Recommendation When calling approveAndCall, there is a process of encoding and decoding the address, so more gas costs are consumed. It seems more efficient to use swapFromTON after pre-checking with _approve.

Exploit Scenario

Demo

Zena-park commented 4 months ago

The part of code you mentioned link

require(
                ITON(ton).approveAndCall(wton, amount, abi.encode(swapProxy, swapProxy)),
                "fail to swap ton to wton"
            );

When replaced with code,

require(IWTON(wton).swapFromTON(amount),"fail to swap ton to wton"); 

An error occurred. Error: VM Exception while processing transaction: reverted with reason string 'SafeERC20: low-level call failed'

zzooppii commented 4 months ago

Does this happen even though TON approved to WTONContract in the contract?

function _approve2(uint256 _tonAmount) internal {
        uint256 allow_ = IERC20(ton).allowance(address(this), wton);
        if (allow_ < _tonAmount) IERC20(ton).approve(wton, type(uint256).max);
}

 _approve2(amount);
require(IWTON(wton).swapFromTON(amount),"fail to swap ton to wton"); 
Zena-park commented 4 months ago

Gas consumption when executing registerLayer2Candidate function

:zap: Using ton.approve && wton.safeTransferFrom uses 29839 less gas, so modify it to use ton.approve && wton.safeTransferFrom.

https://github.com/tokamak-network/ton-staking-v2/commit/d320db9c0f5bd3c5f1f7b84002611e65543fe15d

zzooppii commented 4 months ago

I thought the gas cost would differ a lot because there is an encode and decode process, but the difference in gas cost is not as much as I expected. I confirmed the change. thank you