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

feat(forge): change startPrank to overwrite existing prank instead of erroring #4826

Merged
merged 16 commits into from
May 11, 2023
Merged

feat(forge): change startPrank to overwrite existing prank instead of erroring #4826

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2f89779
init start change prank
4meta5 Apr 26, 2023
a19209a
testChangePrank
4meta5 Apr 26, 2023
97c99f1
revert startChangePrank and change startPrank to overwrite existing p…
4meta5 Apr 27, 2023
4efb00b
add tests for prank0 after prank1 and prank1 after prank0
4meta5 Apr 28, 2023
4b476c1
fmt
4meta5 Apr 28, 2023
15ac502
add error if prank is not used at least once before overwritten as pe…
4meta5 Apr 28, 2023
853731b
fmt
4meta5 Apr 28, 2023
f68dff3
Merge branch 'master' into feat-start-change-prank
mattsse Apr 29, 2023
4aad6f3
unit test for startPrank0 - and startPrank1 -
4meta5 Apr 29, 2023
56fe709
fix
4meta5 Apr 29, 2023
a29f337
remove clones by only updating prank after first time applied
4meta5 Apr 29, 2023
2a89075
fmt
4meta5 Apr 29, 2023
c8d8506
more readable names
4meta5 Apr 29, 2023
70a8e12
Merge branch 'master' into feat-start-change-prank
Evalir May 10, 2023
bb779a5
chore: fix/add tests, use ensure util
Evalir May 10, 2023
a48965a
chore: add missing edge case test
Evalir May 10, 2023
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
28 changes: 24 additions & 4 deletions testdata/cheats/Prank.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,28 @@ contract PrankTest is DSTest {
);
}

function testStartPrank0AfterPrank1(address sender, address origin) public {
// Perform the prank
address oldOrigin = tx.origin;
Victim victim = new Victim();
cheats.startPrank(sender, origin);
victim.assertCallerAndOrigin(
sender, "msg.sender was not set during prank", origin, "tx.origin was not set during prank"
);

// Overwrite the prank
cheats.startPrank(sender);
victim.assertCallerAndOrigin(
sender, "msg.sender was not set during prank", oldOrigin, "tx.origin invariant failed"
);

cheats.stopPrank();
// Ensure we cleaned up correctly after stopping the prank
victim.assertCallerAndOrigin(
address(this), "msg.sender was not cleaned up", oldOrigin, "tx.origin invariant failed"
);
}
Comment on lines +173 to +193
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm: this should be the edge case you're talking about @mds1 right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep that's it, thanks!


function testStartPrank1AfterStartPrank0(address sender, address origin) public {
// Perform the prank
address oldOrigin = tx.origin;
Expand All @@ -186,9 +208,7 @@ contract PrankTest is DSTest {

// Overwrite the prank
cheats.startPrank(sender, origin);
victim.assertCallerAndOrigin(
sender, "msg.sender was not set during prank", origin, "tx.origin was not set"
);
victim.assertCallerAndOrigin(sender, "msg.sender was not set during prank", origin, "tx.origin was not set");

// Ensure prank is still up as startPrank covers multiple calls
victim.assertCallerAndOrigin(
Expand All @@ -210,7 +230,7 @@ contract PrankTest is DSTest {
// try to overwrite the prank. This should fail.
cheats.startPrank(address(this), origin);
}

function testFailOverwriteUnusedPrankAfterSuccessfulPrank(address sender, address origin) public {
// Set the prank, but not use it
address oldOrigin = tx.origin;
Expand Down