bug(StdCheats): deal() hangs on reflection tokens

[maa105]

Dears

Thanks for the amazing efforts you put into this. I detected a bug in deal function when used with reflection tokens. the bug is in v1.9.2 (v1.9.1 doesnt have it). The bug is the deal function just hangs and never returns in v1.9.2 vs it throws in v1.9.1 (expected as this is a reflection token).

test to run:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;

import {Test, console} from "forge-std/Test.sol";
contract MyTest is Test {
    function testThingy() public {
        vm.createSelectFork("wss://bsc-rpc.publicnode.com"); // A BSC RPC Node. You might want to change this
        vm.writeLine("./log.txt", "start");
        deal(0xc748673057861a797275CD8A068AbB95A902e8de, address(this), 1 ether);
        vm.writeLine("./log.txt", "end");
    }
}

foundry.toml:

[profile.default]
src = "Foundry/src"
out = "out"
libs = ["Foundry/lib"]
test = "Foundry/test"
optimizer = true
optimizer_runs = 20000
viaIR = true
evm_version = "cancun"
gas_limit = "18446744073709551615"
solc = "0.8.25"
fs_permissions = [{ access = "write", path = "./"}]

I do vm.writeLine instead of console.log cause when it hangs you do not get the console.logs

I tried on multiple versions and I was able to zero in on the version where this bug happens which is v1.9.2. So for me v1.9.1 it didnt hang (it failed but didnt hang). v1.9.2 it hanged.

Related to: #409

[hesameworks]

Hey!
I'd like to look into this issue and try to reproduce it on the lastest Foundry release (v1.4.0).
Seems related to #409 - I'll run tests across multiple versions and share results soon.

[ndavd]

Hi, I was doing some research on this and #409... If I got it correctly, ERC20 deal works by assuming the balance is stored in a storage slot directly, and that when updating a balance it should correctly modify a single storage slot. That doesn't happen with reflection tokens since balanceOf(account) computes _rOwned[account] / rate, so the slot contains a scaled value of the real balance.

I wrote a separate deal implementation that works with all the mentioned tokens. I didn't open a PR since it's too specific to reflection tokens and there's no standard for those. Would appreciate some feedback first.

---

Implementation

// Set the balance for a ERC20 reflection token where balanceOf(account) = _rOwned[account] / rate
// The caller must supply the current rate (often via `reflectionFromToken(1, false)`)
// Use the alternative signature to update `totalSupply`
function dealReflectionToken(address token, address to, uint256 give, uint256 rate) internal virtual {
    dealReflectionToken(token, to, give, rate, false);
}

function dealReflectionToken(address token, address to, uint256 give, uint256 rate, bool adjust) internal virtual {
    // update total supply
    if (adjust) {
        (, bytes memory balData) = token.staticcall(abi.encodeWithSelector(0x70a08231, to));
        uint256 prevBal = abi.decode(balData, (uint256));
        (, bytes memory totSupData) = token.staticcall(abi.encodeWithSelector(0x18160ddd));
        uint256 totSup = abi.decode(totSupData, (uint256));
        if (give < prevBal) {
            totSup -= (prevBal - give);
        } else {
            totSup += (give - prevBal);
        }
        stdstore.target(token).sig(0x18160ddd).checked_write(totSup);
    }

    // update balance
    vm.record();
    (, bytes memory result) = token.staticcall(abi.encodeWithSelector(0x70a08231, to));
    (bytes32[] memory reads,) = vm.accesses(token);
    for (uint256 i = 0; i < reads.length; i++) {
        bytes32 prev = vm.load(token, reads[i]);
        vm.store(token, reads[i], bytes32(give * rate));
        (, result) = token.staticcall(abi.encodeWithSelector(0x70a08231, to));
        if (result.length >= 32 && abi.decode(result, (uint256)) == give) return;
        vm.store(token, reads[i], prev);
    }

    revert("StdCheats dealReflectionToken(address,address,uint256,uint256,bool): could not set balance.");
}

Test

function test_DealReflectionToken() public {
    vm.createSelectFork("wss://bsc-rpc.publicnode.com");

    address tAddress = 0xc748673057861a797275CD8A068AbB95A902e8de;
    IERC20 t = IERC20(tAddress);

    // reflectionFromToken(1, false) returns 1 * rate
    (, bytes memory rateData) =
        tAddress.staticcall(abi.encodeWithSignature("reflectionFromToken(uint256,bool)", 1, false));
    uint256 rate = abi.decode(rateData, (uint256));

    uint256 dealAmount = 10000e18;
    dealReflectionToken(tAddress, address(this), dealAmount, rate);
    assertEq(t.balanceOf(address(this)), dealAmount);
}

CC @gakonst

[ndavd]

Hi, @decofe @DaniPopes. I've tested #833 via the original fork test mentioned in this issue and it still hangs.

[ndavd]

@DaniPopes I've uncovered the root cause of the problem in my comment above. The current implementation works on regular ERC20s because it assumes the balance is stored in a storage slot directly, and that when updating a balance it should modify a single storage slot with that value. On reflection tokens that isn't the case. Could I get some feedback on the solution I've proposed? Thanks.