hats-finance / Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

0 stars 0 forks source link

Plain gas transfers will not be handled #16

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x9e96d7d06d7b1b5b9d7c91fa8943e937a5197f27d780d8ee9ffabb4143519948 Severity: low

Description:

Description

In Minter.sol, the following receive() function is implemented:

    // receive() function to handle plain gas token transfers
    receive() external payable {
        emit FundsReceived(msg.sender, msg.value);
    }

This receive functions fires an event every time plain gas tokens are transferred to the contract and logs the msg.sender and the msg.value.

However, receive() is handled differently on Oasis Sapphire Chain.

If we take a look at the Oasis Sapphire Docs, we find the following:

This means that, a direct transfer made to the Minter.sol contract will not execute the receive() function. Naturally, events are used by indexers or keepers to keep track of said events. Due to receive() not executing, the indexers/keepers will have a skewed accounting, leading to unexpected behavior.

Recommended Mitigation Steps

Consider tweaking the architecture to incorporate the quirks of Oasis Sapphire.

ilzheev commented 3 months ago

Due to receive() not executing, the indexers/keepers will have a skewed accounting, leading to unexpected behavior.

Event FundsReceived() is informational only and is not used in any accounting. Sending tokens via Oasis-native transaction will not fire receive() as described in Oasis docs – it is not an issue.

bronzepickaxe commented 3 months ago

Hi @ilzheev,

Thank you for clarifying about the usage of keepers/indexers. You are correct in the sense that this not qualify as a Medium Severity issue.

However, as per the Contest Rules, the requirement for a Low Issue is:

Given that the intended behavior of the receive() function includes firing an event and given that this will not be possible in cases, this qualifies as a Low Issue.

Thanks!

ilzheev commented 3 months ago

Event FundsReceived shows that user sent amount via EVM tx. This event is not triggered in case of Oasis-native transaction – which is intended behaviour, as described in Oasis docs:

Or, if these functions do exist, they will not be triggered.

I see no issue with this. We keep it as invalid.