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

Improve Escrow.t.sol hook input and revert tests #430

Merged
merged 3 commits into from
Sep 18, 2024
Merged

Improve Escrow.t.sol hook input and revert tests #430

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4aa7646
test: use hookShouldRevert pattern in `Escrow.t.sol`
BenSparksCode Sep 16, 2024
ffba055
test: improve DummyDAppControl, Escrow tests
BenSparksCode Sep 18, 2024
1c476d9
chore: remove TODO
BenSparksCode Sep 18, 2024
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
61 changes: 38 additions & 23 deletions test/Escrow.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ contract EscrowTest is BaseTest {
.withAllowAllocateValueFailure(true) // Allow the value allocation to fail
.build()
);
executeHookCase(false, block.timestamp * 2, noError);

(UserOperation memory userOp,,) = executeHookCase(block.timestamp * 2, noError);
bytes memory expectedInput = abi.encode(userOp);
assertEq(expectedInput, dAppControl.preOpsInputData(), "preOpsInputData should match expectedInput");
}

// Ensure metacall reverts with the proper error when the preOps hook reverts.
Expand All @@ -154,7 +157,8 @@ contract EscrowTest is BaseTest {
.withReuseUserOp(true) // Allow metacall to revert
.build()
);
executeHookCase(true, 0, AtlasErrors.PreOpsFail.selector);
dAppControl.setPreOpsShouldRevert(true);
executeHookCase(0, AtlasErrors.PreOpsFail.selector);
}

// Ensure the user operation executes successfully. To ensure the operation's returned data is as expected, we
Expand All @@ -168,7 +172,9 @@ contract EscrowTest is BaseTest {
.withAllowAllocateValueFailure(true) // Allow the value allocation to fail
.build()
);
executeHookCase(false, block.timestamp * 3, noError);
executeHookCase(block.timestamp * 3, noError);
bytes memory expectedInput = abi.encode(block.timestamp * 3);
assertEq(expectedInput, dAppControl.userOpInputData(), "userOpInputData should match expectedInput");
}

// Ensure metacall reverts with the proper error when the user operation reverts.
Expand All @@ -178,7 +184,8 @@ contract EscrowTest is BaseTest {
.withReuseUserOp(true) // Allow metacall to revert
.build()
);
executeHookCase(true, 0, AtlasErrors.UserOpFail.selector);
dAppControl.setUserOpShouldRevert(true);
executeHookCase(0, AtlasErrors.UserOpFail.selector);
}

// Ensure metacall reverts with the proper error when the allocateValue hook reverts.
Expand All @@ -193,8 +200,7 @@ contract EscrowTest is BaseTest {
);

dAppControl.setAllocateValueShouldRevert(true);

executeHookCase(false, 1, AtlasErrors.AllocateValueFail.selector);
executeHookCase(1, AtlasErrors.AllocateValueFail.selector);
}

// Ensure the postOps hook is successfully called. No return data is expected from the postOps hook, so we do not
Expand All @@ -205,7 +211,9 @@ contract EscrowTest is BaseTest {
.withRequirePostOps(true) // Execute the postOps hook
.build()
);
executeHookCase(false, 0, noError);
executeHookCase(0, noError);
bytes memory expectedInput = abi.encode(true, new bytes(0));
assertEq(expectedInput, dAppControl.postOpsInputData(), "postOpsInputData should match expectedInput");
}

// Ensure metacall reverts with the proper error when the postOps hook reverts.
Expand All @@ -219,7 +227,8 @@ contract EscrowTest is BaseTest {
.withAllowAllocateValueFailure(true) // Allow the value allocation to fail
.build()
);
executeHookCase(false, 1, AtlasErrors.PostOpsFail.selector);
dAppControl.setPostOpsShouldRevert(true);
executeHookCase(1, AtlasErrors.PostOpsFail.selector);
}

// Ensure the allocateValue hook is successfully called. No return data is expected from the allocateValue hook, so
Expand All @@ -231,35 +240,37 @@ contract EscrowTest is BaseTest {
.withTrackUserReturnData(true) // Track the user operation's return data
.build()
);
uint256 userOpArg = 321;

vm.prank(userEOA);
address executionEnvironment = atlas.createExecutionEnvironment(userEOA, address(dAppControl));
executeHookCase(userOpArg, noError);

vm.expectEmit(false, false, false, true, executionEnvironment);
emit MEVPaymentSuccess(address(0), defaultBidAmount);
this.executeHookCase(false, 0, noError);
bytes memory expectedInput = abi.encode(address(0), defaultBidAmount, abi.encode(userOpArg));
assertEq(expectedInput, dAppControl.allocateValueInputData(), "allocateValueInputData should match expectedInput");
}

function executeHookCase(bool hookShouldRevert, uint256 expectedHookReturnValue, bytes4 expectedError) public {
function executeHookCase(uint256 expectedHookReturnValue, bytes4 expectedError) public returns(
UserOperation memory userOp,
SolverOperation[] memory solverOps,
DAppOperation memory dappOp
) {
bool revertExpected = expectedError != noError;

UserOperation memory userOp = validUserOperation(address(dAppControl))
userOp = validUserOperation(address(dAppControl))
.withData(
abi.encodeWithSelector(
dAppControl.userOperationCall.selector,
hookShouldRevert,
expectedHookReturnValue
)
)
.signAndBuild(address(atlasVerification), userPK);

SolverOperation[] memory solverOps = new SolverOperation[](1);
solverOps = new SolverOperation[](1);
solverOps[0] = validSolverOperation(userOp)
.withBidAmount(defaultBidAmount)
.withData(abi.encode(expectedHookReturnValue))
.signAndBuild(address(atlasVerification), solverOnePK);

DAppOperation memory dappOp = validDAppOperation(userOp, solverOps).build();
dappOp = validDAppOperation(userOp, solverOps).build();

if (revertExpected) {
vm.expectRevert(expectedError);
Expand Down Expand Up @@ -385,7 +396,7 @@ contract EscrowTest is BaseTest {
);

UserOperation memory userOp = validUserOperation(address(dAppControl))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, false, 1))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, 1))
.signAndBuild(address(atlasVerification), userPK);

SolverOperation[] memory solverOps = new SolverOperation[](1);
Expand All @@ -394,6 +405,8 @@ contract EscrowTest is BaseTest {
.signAndBuild(address(atlasVerification), solverOnePK);

uint256 result = (1 << uint256(SolverOutcome.PreSolverFailed));
dAppControl.setPreSolverShouldRevert(true);

executeSolverOperationCase(userOp, solverOps, false, false, result, true);
}

Expand All @@ -409,7 +422,7 @@ contract EscrowTest is BaseTest {
);

UserOperation memory userOp = validUserOperation(address(dAppControl))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, false, 1))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, 1))
.signAndBuild(address(atlasVerification), userPK);

SolverOperation[] memory solverOps = new SolverOperation[](1);
Expand All @@ -418,6 +431,8 @@ contract EscrowTest is BaseTest {
.signAndBuild(address(atlasVerification), solverOnePK);

uint256 result = (1 << uint256(SolverOutcome.PostSolverFailed));
dAppControl.setPostSolverShouldRevert(true);

executeSolverOperationCase(userOp, solverOps, true, false, result, true);
}

Expand Down Expand Up @@ -508,7 +523,7 @@ contract EscrowTest is BaseTest {
);

userOp = validUserOperation(address(dAppControl))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, false, expectedDataValue))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, expectedDataValue))
.signAndBuild(address(atlasVerification), userPK);

solverOps[0] = validSolverOperation(userOp)
Expand Down Expand Up @@ -539,7 +554,7 @@ contract EscrowTest is BaseTest {
);

userOp = validUserOperation(address(dAppControl))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, false, dataValue))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, dataValue))
.signAndBuild(address(atlasVerification), userPK);

solverOps[0] = validSolverOperation(userOp)
Expand All @@ -565,7 +580,7 @@ contract EscrowTest is BaseTest {
defaultAtlasWithCallConfig(callConfig);

userOp = validUserOperation(address(dAppControl))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, false, 0))
.withData(abi.encodeWithSelector(dAppControl.userOperationCall.selector, 0))
.signAndBuild(address(atlasVerification), userPK);

solverOps = new SolverOperation[](1);
Expand Down
79 changes: 52 additions & 27 deletions test/base/DummyDAppControl.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ contract DummyDAppControl is DAppControl {
bool public allocateValueShouldRevert;
bool public postOpsShouldRevert;

// TODO add storage vars to store input data for each hook, then test hooks were passed the correct data
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this is done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oof yes it is. Will remove

bytes public preOpsInputData;
bytes public userOpInputData;
bytes public preSolverInputData;
bytes public postSolverInputData;
bytes public allocateValueInputData;
bytes public postOpsInputData;

event MEVPaymentSuccess(address bidToken, uint256 bidAmount);

constructor(
Expand All @@ -38,38 +46,37 @@ contract DummyDAppControl is DAppControl {
function _checkUserOperation(UserOperation memory) internal pure virtual override { }

function _preOpsCall(UserOperation calldata userOp) internal virtual override returns (bytes memory) {
if (userOp.data.length == 0) {
return new bytes(0);
}
bool shouldRevert = DummyDAppControl(CONTROL).preOpsShouldRevert();
require(!shouldRevert, "_preOpsCall revert requested");

DummyDAppControl(CONTROL).setInputData(abi.encode(userOp), 0);
console.logBytes(abi.encode(userOp));

if (userOp.data.length == 0) return new bytes(0);

(bool success, bytes memory data) = address(userOp.dapp).call(userOp.data);
require(success, "_preOpsCall reverted");
(, bytes memory data) = address(userOp.dapp).call(userOp.data);
return data;
}

function _postOpsCall(bool, bytes calldata data) internal pure virtual override {
if (data.length == 0) return;

(bool shouldRevert) = abi.decode(data, (bool));
function _postOpsCall(bool solved, bytes calldata data) internal virtual override {
bool shouldRevert = DummyDAppControl(CONTROL).postOpsShouldRevert();
require(!shouldRevert, "_postOpsCall revert requested");
}

function _preSolverCall(SolverOperation calldata, bytes calldata returnData) internal view virtual override {
if (returnData.length == 0) {
return;
}
DummyDAppControl(CONTROL).setInputData(abi.encode(solved, data), 5);
}

(bool shouldRevert) = abi.decode(returnData, (bool));
function _preSolverCall(SolverOperation calldata solverOp, bytes calldata returnData) internal virtual override {
bool shouldRevert = DummyDAppControl(CONTROL).preSolverShouldRevert();
require(!shouldRevert, "_preSolverCall revert requested");
}

function _postSolverCall(SolverOperation calldata, bytes calldata returnData) internal pure virtual override {
if (returnData.length == 0) {
return;
}
DummyDAppControl(CONTROL).setInputData(abi.encode(solverOp, returnData), 2);
}

(bool shouldRevert) = abi.decode(returnData, (bool));
function _postSolverCall(SolverOperation calldata solverOp, bytes calldata returnData) internal virtual override {
bool shouldRevert = DummyDAppControl(CONTROL).postSolverShouldRevert();
require(!shouldRevert, "_postSolverCall revert requested");

DummyDAppControl(CONTROL).setInputData(abi.encode(solverOp, returnData), 3);
}

function _allocateValueCall(
Expand All @@ -81,13 +88,12 @@ contract DummyDAppControl is DAppControl {
virtual
override
{
if (data.length == 0) {
return;
}

bool shouldRevert = DummyDAppControl(CONTROL).allocateValueShouldRevert();
require(!shouldRevert, "_allocateValueCall revert requested");
emit MEVPaymentSuccess(bidToken, winningAmount);

DummyDAppControl(CONTROL).setInputData(abi.encode(bidToken, winningAmount, data), 4);

// emit MEVPaymentSuccess(bidToken, winningAmount);
}

function getBidValue(SolverOperation calldata solverOp) public view virtual override returns (uint256) {
Expand All @@ -100,8 +106,12 @@ contract DummyDAppControl is DAppControl {
// Custom functions
// ****************************************

function userOperationCall(bool shouldRevert, uint256 returnValue) public pure returns (uint256) {
function userOperationCall(uint256 returnValue) public returns (uint256) {
bool shouldRevert = DummyDAppControl(CONTROL).userOpShouldRevert();
require(!shouldRevert, "userOperationCall revert requested");

DummyDAppControl(CONTROL).setInputData(abi.encode(returnValue), 1);

return returnValue;
}

Expand Down Expand Up @@ -130,4 +140,19 @@ contract DummyDAppControl is DAppControl {
function setPostOpsShouldRevert(bool _postOpsShouldRevert) public {
postOpsShouldRevert = _postOpsShouldRevert;
}

// Called by the EE to save input data for testing after the metacall ends
function setInputData(
bytes memory inputData,
uint256 hook // 0: preOps, 1: userOp, 2: preSolver, 3: postSolver, 4: allocateValue, 5: postOps
)
public
{
if (hook == 0) preOpsInputData = inputData;
if (hook == 1) userOpInputData = inputData;
if (hook == 2) preSolverInputData = inputData;
if (hook == 3) postSolverInputData = inputData;
if (hook == 4) allocateValueInputData = inputData;
if (hook == 5) postOpsInputData = inputData;
}
}
Loading