add support for prank(sender, origin) and startPrank(sender, origin) cheatcodes #336

karmacoma-eth · 2024-08-02T00:22:34Z

No description provided.

(still need to fix the callers)

karmacoma-eth · 2024-08-02T00:26:13Z

tests/regression/test/Prank.t.sol

+    function check_prank_ConstructorCreate2(address user, bytes32 salt) public {
+        vm.prank(user);
+        ConstructorRecorder recorder = new ConstructorRecorder{salt:salt}();
+        assert(recorder.caller() == user);
+    }


look at us, we do support CREATE2 now 🙌

karmacoma-eth · 2024-08-02T00:42:59Z

not sure what's going on with CI, some jobs flagging with:

 From https://github.com/Vectorized/multicaller
 * branch            b4a0dd037f1d770b2e9ae0b80bbd989707df43d0 -> FETCH_HEAD
fatal: Unable to find current revision in submodule path '../lib/multicaller/lib/forge-std'
fatal: Failed to recurse into submodule path '../lib/multicaller'
Your project has missing dependencies that could not be installed.
Error: 
failed to resolve file: "/home/runner/work/halmos/halmos/tests/regression/../lib/openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol": No such file or directory (os error 2); check configured remappings
	--> /home/runner/work/halmos/halmos/tests/regression/test/Proxy.t.sol
	openzeppelin/proxy/ERC1967/ERC1967Proxy.sol

karmacoma-eth · 2024-08-02T00:48:26Z

not sure what's going on with CI, some jobs flagging with:

 From https://github.com/Vectorized/multicaller
 * branch            b4a0dd037f1d770b2e9ae0b80bbd989707df43d0 -> FETCH_HEAD
fatal: Unable to find current revision in submodule path '../lib/multicaller/lib/forge-std'
fatal: Failed to recurse into submodule path '../lib/multicaller'
Your project has missing dependencies that could not be installed.
Error: 
failed to resolve file: "/home/runner/work/halmos/halmos/tests/regression/../lib/openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol": No such file or directory (os error 2); check configured remappings
	--> /home/runner/work/halmos/halmos/tests/regression/test/Proxy.t.sol
	openzeppelin/proxy/ERC1967/ERC1967Proxy.sol

fixed itself with enough re-running 🤷‍♂️

(it removes the extra dependency on forge-std)

The foundry step is failing with weird errors: Submodule 'tests/lib/forge-std' (https://github.com/foundry-rs/forge-std) registered for path '../lib/forge-std' Submodule 'tests/lib/halmos-cheatcodes' (https://github.com/a16z/halmos-cheatcodes) registered for path '../lib/halmos-cheatcodes' Submodule 'tests/lib/openzeppelin-contracts' (https://github.com/OpenZeppelin/openzeppelin-contracts) registered for path '../lib/openzeppelin-contracts' Submodule 'tests/lib/solady' (https://github.com/Vectorized/solady) registered for path '../lib/solady' Submodule 'tests/lib/solmate' (https://github.com/transmissions11/solmate) registered for path '../lib/solmate' fatal: not a git repository: /home/runner/work/halmos/halmos/tests/lib/forge-std/../../../.git/modules/tests/lib/forge-std Failed to clone 'tests/lib/forge-std'. Retry scheduled fatal: destination path '/home/runner/work/halmos/halmos/tests/lib/halmos-cheatcodes' already exists and is not an empty directory. fatal: clone of 'https://github.com/a16z/halmos-cheatcodes' into submodule path '/home/runner/work/halmos/halmos/tests/lib/halmos-cheatcodes' failed Failed to clone 'tests/lib/halmos-cheatcodes'. Retry scheduled

Suspecting race conditions here: ``` Build failed: ['forge', 'build', '--ast', '--root', 'tests/regression', '--extra-output', 'storageLayout', 'metadata'] ----------------------------- Captured stderr call ----------------------------- Failed to install solc 0.8.26: Text file busy (os error 26) Error: Text file busy (os error 26) ```

This reverts commit 8bb0d6f.

karmacoma-eth · 2024-08-02T18:11:41Z

.github/workflows/test.yml

@@ -53,4 +53,4 @@ jobs:
        run: python -m pip install -e .

      - name: Run pytest
-        run: pytest -n 4 -v -k "not long and not ffi" --ignore=tests/lib --halmos-options="-st ${{ matrix.parallel }} --storage-layout ${{ matrix.storage-layout }} --solver-timeout-assertion 0 ${{ inputs.halmos-options }}" ${{ inputs.pytest-options }}
+        run: pytest -n 1 -v -k "not long and not ffi" --ignore=tests/lib --halmos-options="--debug -st ${{ matrix.parallel }} --storage-layout ${{ matrix.storage-layout }} --solver-timeout-assertion 0 ${{ inputs.halmos-options }}" ${{ inputs.pytest-options }}


pytest -n 1 is what fixed CI, it seems that the issue was running multiple forge commands in parallel would result in race conditions trying to clone submodules or install solc

src/halmos/sevm.py

daejunpark · 2024-08-02T22:55:04Z

src/halmos/sevm.py

@@ -1610,6 +1619,7 @@ def callback(new_ex: Exec, stack, step_id):
                new_ex.jumpis = deepcopy(ex.jumpis)
                new_ex.symbolic = ex.symbolic
                new_ex.prank = deepcopy(ex.prank)
+                new_ex.origin = ex.origin


nice to restore it here, but don't forget it in other places that restore vm states:

in the callback for create():

halmos/src/halmos/sevm.py

Lines 1938 to 1944 in 09b6438

# restore vm state

new_ex.pgm = ex.pgm

new_ex.pc = ex.pc

new_ex.st = deepcopy(ex.st)

new_ex.jumpis = deepcopy(ex.jumpis)

new_ex.symbolic = ex.symbolic

new_ex.prank = deepcopy(ex.prank)

at the end of deploy_test():

halmos/src/halmos/__main__.py

Lines 461 to 466 in 09b6438

# reset vm state

ex.pc = 0

ex.st = State()

ex.context.output = CallOutput()

ex.jumpis = {}

ex.prank = Prank()

probably we need more tests to cover these cases:

checking tx.origin restored after contract creation

checking tx.origin restored after test contract constructor when there is startPrank() but no closing stopPrank() in the constructor

tests added in 1ea690c

karmacoma-eth · 2024-08-03T01:52:20Z

tests/regression/test/Prank.t.sol

+        assert(target.origin() == origin);
+
+        // but the inner call is not pranked
+        checkNotPranked(inner, address(target));


this also fails in foundry 🤨 is there an issue or is my mental model just wrong?

update: my mental model was wrong, and so the implementation was wrong too. I checked that all the tests in PrankTest now behave the same in both foundry and halmos (df2c7a3)

daejunpark

I love the new method of maintaining the caller and origin using the call context and having them seamlessly interact with the prank context. Also appreciate the comprehensive tests! Thanks for the effort!

daejunpark · 2024-08-13T21:31:53Z

Note: issues #342 and #343 are orthogonal to this PR, so please feel free to merge this as is, and address them later.

daejunpark · 2024-08-13T21:47:40Z

src/halmos/cheatcodes.py

-        return True
+    def startPrank(self, sender: Address, origin: Address | None = None) -> bool:
+        result = self.prank(sender, origin)
+        self.keep = result if result else self.keep


This is accurate but a bit convoluted. For better code readability, I'd suggest having prank() take an additional argument (defaulting to False if not provided) that is assigned to self.keep. Then, have startPrank() call prank(..., keep=True).

yeah you're right, in 17eba86

karmacoma-eth added 5 commits July 31, 2024 08:57

WIP: beginning of startPrank(address sender, address origin)

f1e803b

add tests/test_prank.py

022e2df

WIP: rework Prank class

c1d5f9e

(still need to fix the callers)

add an origin field to Exec and wire pranks to it

15ffcdb

finish wiring up new prank cheatcodes

73f4478

karmacoma-eth commented Aug 2, 2024

View reviewed changes

karmacoma-eth requested a review from daejunpark August 2, 2024 00:26

Merge branch 'main' into start-prank-origin

d62c1eb

karmacoma-eth added 10 commits August 2, 2024 08:56

simplify prank Target: recordCaller() -> reset()

1c3373b

update tests/lib/multicaller to v1.3.2

9939ca5

(it removes the extra dependency on forge-std)

remove shallow from .gitmodules

3ec4901

delete multicaller

324f77d

replace multicaller submodule with a snapshot of the file we need

b6c1e48

update tests/lib/openzeppelin-contracts@v5.0.2

c051be4

test.yml: add --debug to halmos options

707b9e1

Revert "test.yml: recursively checkout submodules"

de64d95

This reverts commit 8bb0d6f.

karmacoma-eth commented Aug 2, 2024

View reviewed changes

Merge branch 'main' into start-prank-origin

09b6438

daejunpark reviewed Aug 2, 2024

View reviewed changes

karmacoma-eth added 2 commits August 2, 2024 18:43

add Prank test with nested contexts

1689032

add more Prank tests with nested contexts

09b8066

karmacoma-eth commented Aug 3, 2024

View reviewed changes

karmacoma-eth marked this pull request as draft August 6, 2024 16:36

karmacoma-eth added 3 commits August 9, 2024 15:12

wire Prank inside CallContext rather than Exec

df2c7a3

add a startPrank in constructor test

1ea690c

add test_prank_in_context

c1c64e6

karmacoma-eth marked this pull request as ready for review August 9, 2024 23:34

daejunpark mentioned this pull request Aug 13, 2024

Messages for unknown calls #342

Open

daejunpark approved these changes Aug 13, 2024

View reviewed changes

daejunpark reviewed Aug 13, 2024

View reviewed changes

karmacoma-eth added 2 commits August 13, 2024 15:21

less convoluted code in prank/startPrank

17eba86

Merge branch 'main' into start-prank-origin

24f1493

karmacoma-eth enabled auto-merge (squash) August 13, 2024 22:23

karmacoma-eth merged commit f029418 into main Aug 13, 2024
32 checks passed

karmacoma-eth deleted the start-prank-origin branch August 13, 2024 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for prank(sender, origin) and startPrank(sender, origin) cheatcodes #336

add support for prank(sender, origin) and startPrank(sender, origin) cheatcodes #336

karmacoma-eth commented Aug 2, 2024

karmacoma-eth Aug 2, 2024

karmacoma-eth commented Aug 2, 2024

karmacoma-eth commented Aug 2, 2024

karmacoma-eth Aug 2, 2024

daejunpark Aug 2, 2024

daejunpark Aug 2, 2024

karmacoma-eth Aug 9, 2024

karmacoma-eth Aug 3, 2024

karmacoma-eth Aug 9, 2024

daejunpark left a comment

daejunpark commented Aug 13, 2024

daejunpark Aug 13, 2024 •

edited

Loading

karmacoma-eth Aug 13, 2024

	# restore vm state
	new_ex.pgm = ex.pgm
	new_ex.pc = ex.pc
	new_ex.st = deepcopy(ex.st)
	new_ex.jumpis = deepcopy(ex.jumpis)
	new_ex.symbolic = ex.symbolic
	new_ex.prank = deepcopy(ex.prank)

	# reset vm state
	ex.pc = 0
	ex.st = State()
	ex.context.output = CallOutput()
	ex.jumpis = {}
	ex.prank = Prank()

add support for prank(sender, origin) and startPrank(sender, origin) cheatcodes #336

add support for prank(sender, origin) and startPrank(sender, origin) cheatcodes #336

Conversation

karmacoma-eth commented Aug 2, 2024

Choose a reason for hiding this comment

karmacoma-eth commented Aug 2, 2024

karmacoma-eth commented Aug 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daejunpark left a comment

Choose a reason for hiding this comment

daejunpark commented Aug 13, 2024

daejunpark Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daejunpark Aug 13, 2024 •

edited

Loading