XDEFI contest - 0xsanson's results

Handle

0xsanson

Vulnerability details

In the relock function:

// Throw convenient error if trying to re-lock more than was unlocked. `amountUnlocked_ - lockAmount_` would have reverted below anyway.
require(lockAmount_ <= amountUnlocked_, "INSUFFICIENT_AMOUNT_UNLOCKED");

// Handle the lock position creation and get the tokenId of the locked position.
newTokenId_ = _lock(lockAmount_, duration_, destination_);

uint256 withdrawAmount = amountUnlocked_ - lockAmount_;

The last line can use unchecked math since the underflow was already considered before. It saves a bit of gas.

Handle

0xsanson

Vulnerability details

Impact

Throughout the code the safe functions safeTransfer and safeTransferFrom are used when dealing with XDEFI. Isn't this token a standard ERC20? I believe the normal ERC20 transfer functions can be used. The advantage is gaining some 100s gas otherwise spent in unneeded logic.

Proof of Concept

grep safeT *.sol

Recommended Mitigation Steps

Consider removing the SafeERC20 library.

Handle

0xsanson

Vulnerability details

The state variable MAX_TOTAL_XDEFI_SUPPLY can be declared constant to save gas so it won't be read from storage.

uint88 internal /*constant*/ MAX_TOTAL_XDEFI_SUPPLY = uint88(240_000_000_000_000_000_000_000_000);

Handle

0xsanson

Vulnerability details

The current implementation uses _locked = 0 for the unlocked state and _locked = 1 for the locked one. It's better to have two non-zero values from an optimization point of view; it can surprisingly save thousands of gas in some test cases.

Consider rewriting the modifier to:

modifier noReenter() {
    require(_locked == 1, "LOCKED");
    _locked = uint256(2);
    _;
    _locked = uint256(1);
}

(also add a _locked = 1 in the constructor).

Tools Used

hardhat gas reporter