Add 1wei tolerance for bad token detection

[fleupold]

Description

This is a pretty hacky PR, but given that we didn't manage to get rid of bad token detection in the last quarter and it's not clear when we will find the ressources for it, I feel the impact this change can have may be worth its ugliness.

There are a lot of tokens (e.g. stETH, eUSD, oETH, etc.) whose transfer logic is based on "shares" (cf. eUSD transfer implementation). The logic to convert shares into token amounts requires a division which can lead to off by one error:

Relevant solidity code

function balanceOf(address _account) public view returns (uint256) {
        return getMintedEUSDByShares(_sharesOf(_account));
    }

function _transfer(
        address _sender,
        address _recipient,
        uint256 _amount
    ) internal {
        uint256 _sharesToTransfer = getSharesByMintedEUSD(_amount);
        _transferShares(_sender, _recipient, _sharesToTransfer);
        emit Transfer(_sender, _recipient, _amount);
        emit TransferShares(_sender, _recipient, _sharesToTransfer);
    }

function getMintedEUSDByShares(
        uint256 _sharesAmount
    ) public view returns (uint256) {
        uint256 totalSharesAmount = _getTotalShares();
        if (totalShares == 0) {
            return 0;
        } else {
            return
                _sharesAmount.mul(_getTotalMintedEUSD()).div(totalSharesAmount);
        }
    }

function getSharesByMintedEUSD(
        uint256 _EUSDAmount
    ) public view returns (uint256) {
        uint256 totalMintedEUSD = _getTotalMintedEUSD();
        if (totalMintedEUSD == 0) {
            return 0;
        } else {
            return _EUSDAmount.mul(_getTotalShares()).div(totalMintedEUSD);
        }
    }

Consider the following example:

• totalMintedEUSD: 100
• total shares: 50
• balanceOf(A) = 4 shares = 8 eUSD

Transferring 3 eUSD would lead to transferring 1 share (3 * 50 / 100). After the transfer the user therefore has 5 shares = 10 eUSD (5 * 100 / 50) which is not in line with our expectation (8 + 3 = 11). This 1 wei discrepency is causing the bad token detector to fire.

Changes

• Pass the balance check if the observed balance is not more than 1 wei less than expected (this also allows higher balances)

How to test

Not sure if this warrants an e2e test in and of itself (I assume it will be quite involved to deploy the token as in the example)? For now I would rely on the existing unit tests. In particular

cargo test -p shared mainnet_tokens -- --nocapture --ignored

now shows 0x0aacfbec6a24756c20d41914f2caba817c0d8521 as supported 💪

[MartinquaXD]

This is equivalent

Suggested change

	if balance_after_in
	< computed_balance_after_in
	.checked_sub(U256::one())
	.unwrap_or_default()
	{
	if balance_after_in < computed_balance_after_in.saturating_sub(U256::one()) {

[sunce86]

Lg. Not sure how easy would be to write a unit test to confirm behaviour.

[MartinquaXD]

The first idea I had was to create a forked e2e test that tries to trade the problematic token. But TBH I wouldn't worry about it given that we execute these code paths A LOT and the unit test confirms the token is okay now.

[fleupold]

Lg. Not sure how easy would be to write a unit test to confirm behaviour.

There is an integration test, which has a token that was bad before and is now supported.

[squadgazzz]

Looks good!