Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(forge test): --isolate does not work as expected with setUp() #9564

Closed
2 tasks done
sakulstra opened this issue Dec 16, 2024 · 3 comments · Fixed by #9940
Closed
2 tasks done

bug(forge test): --isolate does not work as expected with setUp() #9564

sakulstra opened this issue Dec 16, 2024 · 3 comments · Fixed by #9940
Labels
Cmd-forge-test Command: forge test T-bug Type: bug

Comments

@sakulstra
Copy link
Contributor

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (59f354c 2024-12-11T00:29:25.224313000Z)

What command(s) is the bug in?

forge test --isolate

Operating System

macOS (Apple Silicon)

Describe the bug

Hello, i'm trying to improve the gas tests on the aave-dao repository atm and am facing an issue with isolation.
As far as i can understand the isolate flag should isolate top level calls from the test setup, when i test gas usage of the getUserAccountData method, it seems like it is in fact getting affected by the test setup.

In case one the setUp did not yet touch the aave oracle, so fetching the price via AaveOracle::getAssetPrice(USDX: [0xffD4505B3452Dc22f8473616d50503bA9E1710Ac]) costs [7865] gas.

In the second case the setUp did borrow some assets, which in turn will fetch oracle prices for validation. Due to storage hotness the cost dropped to [1365].

Case 1:

    ├─ [38625] InitializableImmutableAdminUpgradeabilityProxy::fallback(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   ├─ [37998] PoolInstance::getUserAccountData(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [delegatecall]
    │   │   ├─ [474] PoolAddressesProvider::getPriceOracle() [staticcall]
    │   │   │   └─ ← [Return] AaveOracle: [0xD02933ADB0575485D0c88aFC00815b9a862D92D9]
    │   │   ├─ [32578] PoolLogic::26ec273f(000000000000000000000000000000000000000000000000000000000000003400000000000000000000000000000000000000000000000000000000000000360000000000000000000000000000000000000000000000000000000000000037000000000000000000000000000000000000000000000000000000000000002200000000000000000000000000000000000000000000000000000000000000030000000000000000000000006ca6d1e2d5347bfab1d91e883f1915560e09129d000000000000000000000000d02933adb0575485d0c88afc00815b9a862d92d90000000000000000000000000000000000000000000000000000000000000000) [delegatecall]
    │   │   │   ├─ [7865] AaveOracle::getAssetPrice(USDX: [0xffD4505B3452Dc22f8473616d50503bA9E1710Ac]) [staticcall]
    │   │   │   │   ├─ [2279] MockAggregator::latestAnswer() [staticcall]
    │   │   │   │   │   └─ ← [Return] 100000000 [1e8]
    │   │   │   │   └─ ← [Return] 100000000 [1e8]
    │   │   │   ├─ [1268] InitializableImmutableAdminUpgradeabilityProxy::fallback(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   │   │   │   ├─ [665] ATokenInstance::scaledBalanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [delegatecall]
    │   │   │   │   │   └─ ← [Return] 999999999996955860 [9.999e17]
    │   │   │   │   └─ ← [Return] 999999999996955860 [9.999e17]
    │   │   │   ├─ [7865] AaveOracle::getAssetPrice(WETH: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
    │   │   │   │   ├─ [2279] MockAggregator::latestAnswer() [staticcall]
    │   │   │   │   │   └─ ← [Return] 180000000000 [1.8e11]
    │   │   │   │   └─ ← [Return] 180000000000 [1.8e11]
    │   │   │   ├─ [1268] InitializableImmutableAdminUpgradeabilityProxy::fallback(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   │   │   │   ├─ [665] ATokenInstance::scaledBalanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [delegatecall]
    │   │   │   │   │   └─ ← [Return] 999999999695585997 [9.999e17]
    │   │   │   │   └─ ← [Return] 999999999695585997 [9.999e17]
    │   │   │   └─ ← [Return] 0x0000000000000000000000000000000000000000000000056bc75e574be60800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000478eae10805042d000000000000000000000000000000000000000000000000000000000000002198000000000000000000000000000000000000000000000000000000000000203affffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
    │   │   └─ ← [Return] 100000000180000000000 [1e20], 0, 82500000148500000000 [8.25e19], 8600, 8250, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]
    │   └─ ← [Return] 100000000180000000000 [1e20], 0, 82500000148500000000 [8.25e19], 8600, 8250, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]
    ├─ [0] VM::snapshotGasLastCall("Pool.Getters", "getUserAccountData: supplies: 2, borrows: 0")
    │   └─ ← [Return] 38625 [3.862e4]

Case 2:

─ [28791] InitializableImmutableAdminUpgradeabilityProxy::fallback(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   ├─ [28164] PoolInstance::getUserAccountData(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [delegatecall]
    │   │   ├─ [474] PoolAddressesProvider::getPriceOracle() [staticcall]
    │   │   │   └─ ← [Return] AaveOracle: [0xD02933ADB0575485D0c88aFC00815b9a862D92D9]
    │   │   ├─ [22744] PoolLogic::26ec273f(000000000000000000000000000000000000000000000000000000000000003400000000000000000000000000000000000000000000000000000000000000360000000000000000000000000000000000000000000000000000000000000037000000000000000000000000000000000000000000000000000000000000002600000000000000000000000000000000000000000000000000000000000000030000000000000000000000006ca6d1e2d5347bfab1d91e883f1915560e09129d000000000000000000000000d02933adb0575485d0c88afc00815b9a862d92d90000000000000000000000000000000000000000000000000000000000000000) [delegatecall]
    │   │   │   ├─ [1365] AaveOracle::getAssetPrice(USDX: [0xffD4505B3452Dc22f8473616d50503bA9E1710Ac]) [staticcall]
    │   │   │   │   ├─ [279] MockAggregator::latestAnswer() [staticcall]
    │   │   │   │   │   └─ ← [Return] 100000000 [1e8]
    │   │   │   │   └─ ← [Return] 100000000 [1e8]
    │   │   │   ├─ [1268] InitializableImmutableAdminUpgradeabilityProxy::fallback(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   │   │   │   ├─ [665] ATokenInstance::scaledBalanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [delegatecall]
    │   │   │   │   │   └─ ← [Return] 999999999996955860 [9.999e17]
    │   │   │   │   └─ ← [Return] 999999999996955860 [9.999e17]
    │   │   │   ├─ [1365] AaveOracle::getAssetPrice(WBTC: [0x3C8Ca53ee5661D29d3d3C0732689a4b86947EAF0]) [staticcall]
    │   │   │   │   ├─ [279] MockAggregator::latestAnswer() [staticcall]
    │   │   │   │   │   └─ ← [Return] 2700000000000 [2.7e12]
    │   │   │   │   └─ ← [Return] 2700000000000 [2.7e12]
    │   │   │   ├─ [1290] InitializableImmutableAdminUpgradeabilityProxy::fallback(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   │   │   │   ├─ [687] VariableDebtTokenInstance::scaledBalanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [delegatecall]
    │   │   │   │   │   └─ ← [Return] 100000 [1e5]
    │   │   │   │   └─ ← [Return] 100000 [1e5]
    │   │   │   ├─ [1365] AaveOracle::getAssetPrice(WETH: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
    │   │   │   │   ├─ [279] MockAggregator::latestAnswer() [staticcall]
    │   │   │   │   │   └─ ← [Return] 180000000000 [1.8e11]
    │   │   │   │   └─ ← [Return] 180000000000 [1.8e11]
    │   │   │   ├─ [1268] InitializableImmutableAdminUpgradeabilityProxy::fallback(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   │   │   │   ├─ [665] ATokenInstance::scaledBalanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [delegatecall]
    │   │   │   │   │   └─ ← [Return] 999999999695585997 [9.999e17]
    │   │   │   │   └─ ← [Return] 999999999695585997 [9.999e17]
    │   │   │   └─ ← [Return] 0x0000000000000000000000000000000000000000000000056bc75e574be6080000000000000000000000000000000000000000000000000000000000a0eebb0000000000000000000000000000000000000000000000000478eae107641572000000000000000000000000000000000000000000000000000000000000002198000000000000000000000000000000000000000000000000000000000000203a000000000000000000000000000000000000000066eb3bffe19a0eec9394bda1
    │   │   └─ ← [Return] 100000000180000000000 [1e20], 2700000000 [2.7e9], 82500000145800000000 [8.25e19], 8600, 8250, 31851851909185185185185185185 [3.185e28]
    │   └─ ← [Return] 100000000180000000000 [1e20], 2700000000 [2.7e9], 82500000145800000000 [8.25e19], 8600, 8250, 31851851909185185185185185185 [3.185e28]
    ├─ [0] VM::snapshotGasLastCall("Pool.Getters", "getUserAccountData: supplies: 2, borrows: 1")
    │   └─ ← [Return] 28791 [2.879e4]
@sakulstra sakulstra added T-bug Type: bug T-needs-triage Type: this issue needs to be labelled labels Dec 16, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Dec 16, 2024
@zerosnacks
Copy link
Member

cc @klkvr

@zerosnacks zerosnacks added Cmd-forge-test Command: forge test and removed T-needs-triage Type: this issue needs to be labelled labels Jan 3, 2025
@zerosnacks zerosnacks changed the title --isolate does not work properly bug(forge test): --isolate does not work as expected with setUp() Jan 3, 2025
@sakulstra
Copy link
Contributor Author

@zerosnacks , pretty sure it's not related to setUp

Both cases have the exact same setUp, the difference is that in one the borrow action seems to make the oracle hot for gas measuring.

function test_getUserAccountData_twoSupplies() external {
    _supplyOnReserve(user, 1 ether, tokenList.usdx);
    _supplyOnReserve(user, 1 ether, tokenList.weth);

    contracts.poolProxy.getUserAccountData(user);
    vm.snapshotGasLastCall('Pool.Getters', 'getUserAccountData: supplies: 2, borrows: 0');
  }

  function test_getUserAccountData_twoSupplies_oneBorrows() external {
    _supplyOnReserve(user, 1 ether, tokenList.usdx);
    _supplyOnReserve(user, 1 ether, tokenList.weth);

    _supplyOnReserve(address(1), 0.001e8, tokenList.wbtc);
    vm.prank(user);
    contracts.poolProxy.borrow(tokenList.wbtc, 0.001e8, 2, 0, user); <- this alters the gas consumption of

    contracts.poolProxy.getUserAccountData(user); <- this
    vm.snapshotGasLastCall('Pool.Getters', 'getUserAccountData: supplies: 2, borrows: 1');
  }

@grandizzy
Copy link
Collaborator

@sakulstra mind to check if same when using before test setups? https://book.getfoundry.sh/forge/writing-tests#before-test-setups This should ensure before test setups are different txes than the test one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmd-forge-test Command: forge test T-bug Type: bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants