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.

Sign up for GitHub

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

fix(forge): loadAllocs cheatcode doesn't persist when called in setUp #6297

Merged
merged 1 commit into from
Nov 13, 2023
Merged

fix(forge): loadAllocs cheatcode doesn't persist when called in setUp #6297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/evm/core/src/backend/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,9 @@ impl DatabaseExt for Backend {
// Set the account's nonce and balance.
state_acc.info.nonce = acc.nonce.unwrap_or_default();
state_acc.info.balance = acc.balance.to_alloy();

// Touch the account to ensure the loaded information persists if called in `setUp`.
journaled_state.touch(addr);
}

Ok(())
Expand Down
39 changes: 38 additions & 1 deletion testdata/cheats/loadAllocs.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,43 @@ import "./Vm.sol";

contract LoadAllocsTest is DSTest {
Vm constant vm = Vm(HEVM_ADDRESS);
string allocsPath;
address constant ALLOCD = address(0x420);
address constant ALLOCD_B = address(0x421);

uint256 snapshotId;
string allocsPath;

function setUp() public {
allocsPath = string.concat(vm.projectRoot(), "/fixtures/Json/test_allocs.json");

// Snapshot the state; We'll restore it in each test that loads allocs inline.
snapshotId = vm.snapshot();

// Load the allocs file.
vm.loadAllocs(allocsPath);
}

/// @dev Checks that the `loadAllocs` cheatcode persists account info if called in `setUp`
function testLoadAllocsStaticSetup() public {
// Balance should be `0xabcd`
assertEq(ALLOCD.balance, 0xabcd);

// Code should be a simple store / return, returning `0x42`
(bool success, bytes memory rd) = ALLOCD.staticcall("");
assertTrue(success);
uint256 ret = abi.decode(rd, (uint256));
assertEq(ret, 0x42);

// Storage should have been set in slot 0x1, equal to `0xbeef`
assertEq(uint256(vm.load(ALLOCD, bytes32(uint256(0x10 << 248)))), 0xbeef);
}

/// @dev Checks that the `loadAllocs` cheatcode persists account info if called inline
function testLoadAllocsStatic() public {
// Restore the state snapshot prior to the allocs file being loaded.
vm.revertTo(snapshotId);

Comment on lines +42 to +44
Copy link
Collaborator

@mds1 mds1 Nov 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Evalir do we have confidence/good tests for snapshot/revertTo cheats? We don't verify that state was correctly reset after this call and I know there's been some bugs on those cheats in the past

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did check it manually, but didn't commit them due to the tests in Snapshots.t.sol. Glad to add an explicit test for this in loadAllocs.t.sol, but figured we didn't want to cross-wires 😄

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I agree the tests for proper revertTo behavior are better suited elsewhere, so I'm ok leaving it as-is

// Load the allocs file
vm.loadAllocs(allocsPath);

// Balance should be `0xabcd`
Expand All @@ -30,7 +58,11 @@ contract LoadAllocsTest is DSTest {
assertEq(uint256(vm.load(ALLOCD, bytes32(uint256(0x10 << 248)))), 0xbeef);
}

/// @dev Checks that the `loadAllocs` cheatcode overrides existing account information (if present)
function testLoadAllocsOverride() public {
// Restore the state snapshot prior to the allocs file being loaded.
vm.revertTo(snapshotId);

// Populate the alloc'd account's code.
vm.etch(ALLOCD, hex"FF");
assertEq(ALLOCD.code, hex"FF");
Expand All @@ -52,7 +84,12 @@ contract LoadAllocsTest is DSTest {
assertEq(ALLOCD.balance, 0xabcd);
}

/// @dev Checks that the `loadAllocs` cheatcode does not override existing account information if there is no data
/// within the allocs/genesis file for the account field (i.e., partial overrides)
function testLoadAllocsPartialOverride() public {
// Restore the state snapshot prior to the allocs file being loaded.
vm.revertTo(snapshotId);

// Populate the alloc'd account's code.
vm.etch(ALLOCD_B, hex"FF");
assertEq(ALLOCD_B.code, hex"FF");
Expand Down