Skip to content

Commit

Permalink
Merge pull request #141 from rhinestonewtf/feature/expect-revert-reason
Browse files Browse the repository at this point in the history
Feature/expect revert reason
  • Loading branch information
kopy-kat authored Aug 23, 2024
2 parents d2ed91a + 3d62a11 commit a945dac
Show file tree
Hide file tree
Showing 9 changed files with 270 additions and 55 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@rhinestone/modulekit",
"version": "0.4.10",
"version": "0.4.11",
"description": "A development kit for building and testing smart account modules.",
"license": "GPL-3.0",
"author": {
Expand Down
12 changes: 9 additions & 3 deletions src/test/ModuleKitHelpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ library ModuleKitHelpers {
address module
)
internal
view
returns (bool)
{
return HelperBase(instance.accountHelper).isModuleInstalled(instance, moduleTypeId, module);
Expand All @@ -183,7 +182,6 @@ library ModuleKitHelpers {
bytes memory data
)
internal
view
returns (bool)
{
return HelperBase(instance.accountHelper).isModuleInstalled(
Expand Down Expand Up @@ -312,7 +310,15 @@ library ModuleKitHelpers {
//////////////////////////////////////////////////////////////////////////*/

function expect4337Revert(AccountInstance memory) internal {
writeExpectRevert(1);
writeExpectRevert("");
}

function expect4337Revert(AccountInstance memory, bytes4 selector) internal {
writeExpectRevert(abi.encodePacked(selector));
}

function expect4337Revert(AccountInstance memory, bytes memory message) internal {
writeExpectRevert(message);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/test/helpers/HelperBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,8 @@ abstract contract HelperBase {
address module
)
public
view
virtual
deployAccountForAction(instance)
returns (bool)
{
return isModuleInstalled(instance, moduleTypeId, module, "");
Expand All @@ -284,8 +284,8 @@ abstract contract HelperBase {
bytes memory additionalContext
)
public
view
virtual
deployAccountForAction(instance)
returns (bool)
{
return IERC7579Account(instance.account).isModuleInstalled(
Expand Down
2 changes: 1 addition & 1 deletion src/test/helpers/KernelHelpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,9 @@ contract KernelHelpers is HelperBase {
bytes memory data
)
public
view
virtual
override
deployAccountForAction(instance)
returns (bool)
{
if (moduleTypeId == MODULE_TYPE_HOOK) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/helpers/SafeHelpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ contract SafeHelpers is HelperBase {
bytes memory data
)
public
view
virtual
override
deployAccountForAction(instance)
returns (bool)
{
if (moduleTypeId == MODULE_TYPE_HOOK) {
Expand Down
37 changes: 31 additions & 6 deletions src/test/utils/ERC4337Helpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import { GasParser } from "./gas/GasParser.sol";
import {
getSimulateUserOp,
getExpectRevert,
writeExpectRevert,
getExpectRevertMessage,
clearExpectRevert,
getGasIdentifier,
writeGasIdentifier,
writeInstalledModule,
Expand All @@ -31,6 +32,8 @@ library ERC4337Helpers {
error UserOperationReverted(
bytes32 userOpHash, address sender, uint256 nonce, bytes revertReason
);
error InvalidRevertMessage(bytes4 expected, bytes4 reason);
error InvalidRevertMessageBytes(bytes expected, bytes reason);

function exec4337(PackedUserOperation[] memory userOps, IEntryPoint onEntryPoint) internal {
uint256 isExpectRevert = getExpectRevert();
Expand All @@ -49,10 +52,12 @@ library ERC4337Helpers {
// Execute userOps
address payable beneficiary = payable(address(0x69));
bytes memory userOpCalldata = abi.encodeCall(IEntryPoint.handleOps, (userOps, beneficiary));
(bool success,) = address(onEntryPoint).call(userOpCalldata);
(bool success, bytes memory returnData) = address(onEntryPoint).call(userOpCalldata);

if (isExpectRevert == 0) {
require(success, "UserOperation execution failed");
} else if (isExpectRevert == 2 && !success) {
checkRevertMessage(returnData);
}

// Parse logs and determine if a revert happened
Expand All @@ -68,14 +73,17 @@ library ERC4337Helpers {
abi.decode(logs[i].data, (uint256, bool, uint256, uint256));
totalUserOpGas = actualGasUsed;
if (!userOpSuccess) {
bytes32 userOpHash = logs[i].topics[1];
if (isExpectRevert == 0) {
bytes32 userOpHash = logs[i].topics[1];
bytes memory revertReason = getUserOpRevertReason(logs, userOpHash);
revert UserOperationReverted(
userOpHash, address(bytes20(logs[i].topics[2])), nonce, revertReason
);
} else {
writeExpectRevert(0);
if (isExpectRevert == 2) {
checkRevertMessage(getUserOpRevertReason(logs, userOpHash));
}
clearExpectRevert();
}
}
}
Expand Down Expand Up @@ -115,7 +123,7 @@ library ERC4337Helpers {
require(!success, "UserOperation execution did not fail as expected");
}
}
writeExpectRevert(0);
clearExpectRevert();

// Calculate gas for userOp
string memory gasIdentifier = getGasIdentifier();
Expand All @@ -139,7 +147,7 @@ library ERC4337Helpers {

function getUserOpRevertReason(
VmSafe.Log[] memory logs,
bytes32 /* userOpHash */
bytes32 userOpHash
)
internal
pure
Expand All @@ -150,12 +158,29 @@ library ERC4337Helpers {
if (
logs[i].topics[0]
== 0x1c4fada7374c0a9ee8841fc38afe82932dc0f8e69012e927f061a8bae611a201
&& logs[i].topics[1] == userOpHash
) {
(, revertReason) = abi.decode(logs[i].data, (uint256, bytes));
}
}
}

function checkRevertMessage(bytes memory actualReason) internal view {
bytes memory revertMessage = getExpectRevertMessage();

if (revertMessage.length == 4) {
bytes4 expected = bytes4(revertMessage);
bytes4 actual = bytes4(actualReason);
if (expected != actual) {
revert InvalidRevertMessage(expected, actual);
}
} else {
if (revertMessage.length != actualReason.length) {
revert InvalidRevertMessageBytes(revertMessage, actualReason);
}
}
}

function calculateGas(
PackedUserOperation[] memory userOps,
IEntryPoint onEntryPoint,
Expand Down
33 changes: 31 additions & 2 deletions src/test/utils/Storage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,18 @@ pragma solidity ^0.8.23;
EXPECT REVERT
//////////////////////////////////////////////////////////////*/

function writeExpectRevert(uint256 value) {
bytes32 slot = keccak256("ModuleKit.ExpectSlot");
function writeExpectRevert(bytes memory message) {
uint256 value = 1;
bytes32 slot = keccak256("ModuleKit.ExpectMessageSlot");

if (message.length > 0) {
value = 2;
assembly {
sstore(slot, message)
}
}

slot = keccak256("ModuleKit.ExpectSlot");
assembly {
sstore(slot, value)
}
Expand All @@ -19,6 +29,25 @@ function getExpectRevert() view returns (uint256 value) {
}
}

function getExpectRevertMessage() view returns (bytes memory data) {
bytes32 slot = keccak256("ModuleKit.ExpectMessageSlot");
assembly {
data := sload(slot)
}
}

function clearExpectRevert() {
bytes32 slot = keccak256("ModuleKit.ExpectSlot");
assembly {
sstore(slot, 0)
}

slot = keccak256("ModuleKit.ExpectMessageSlot");
assembly {
sstore(slot, 0)
}
}

/*//////////////////////////////////////////////////////////////
GAS IDENTIFIER
//////////////////////////////////////////////////////////////*/
Expand Down
Loading

0 comments on commit a945dac

Please sign in to comment.