Limited Functionality Due to Default Reentrancy Lock in ink! Pair Contract.

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0x8ab813928c2f33a6c444369efda328c6b30d66e814b93504197b0d89437057c8 Severity: medium

Description:

Description

The ink! implementation has a default reentrancy lock, resulting in the automatic locking of all functions within the pair contract. In the Solidity version of UniswapV2, reentrancy is selectively applied to certain functions (mint, swap, skim, sync, burn), while other functions remain callable. However, in the Rust implementation, all functions are locked by default.

Impact

The reentrancy lock imposes limitations on the functionality of the pair contract, leading to temporary inaccessibility and a potential Denial of Service (DOS) scenario.

Impact on Trading Strategies: Traders and liquidity providers relying on specific functionalities may face challenges executing their strategies. For instance, certain trading strategies involving multiple transactions in quick succession could be hindered.
Reduced Efficiency: Locking all functions by default may reduce the contract's efficiency. Functions that don't involve state changes, other than the specified ones (mint, swap, skim, sync, burn), may not need to be locked. The reentrancy lock might be overly restrictive.

Affected Functions

Functions that change the state and might be impacted include:

Getter functions that may be affected:

Proof of Concept

In the swap function, there is a callback to the to address, causing all subsequent calls to the pair contract to be locked.

This renders some functionalities temporarily unusable.

if let Some(data) = data {
    // Call the callback.
    let mut swap_callee: contract_ref!(SwapCallee) = to.into();
    swap_callee.swap_call(self.env().caller(), amount_0_out, amount_1_out, data);
}

Scenario

For readonly functions, Bob wants to get a flash loan from the contract. In the callback, he might need the price or balance of one of the tokens, but the transaction will revert. Bob is forced to read all states before the callback (which is not the case in the Solidity implementation).

For other functions, if Bob wants to get one of the tokens in the contract and then use that token with the pair token for a farming strategy or other strategies, the transaction reverts. Users aren't able to use their LP token in the callback.

Revised Code File (Optional)

Consider allowing reentrancy in the contract and selectively lock only specific functions (mint, swap, skim, sync, burn) to address the issue.

hats-finance / AlephZeroAMM-0x0d88a9ece90994ecb3ba704730819d71c139f60f