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.

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

Improvement add transaction pre hooks #19

Merged
merged 5 commits into from
Aug 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions contracts/Safe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ contract Safe is
address gasToken,
address payable refundReceiver,
bytes memory signatures
) external payable override returns (bool success) {
onBeforeExecTransaction(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, signatures);
) public payable virtual override returns (bool success) {
onBeforeExecTransaction(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, signatures, success);
bytes32 txHash;
// Use scope here to limit variable lifetime and prevent `stack too deep` errors
{
Expand Down Expand Up @@ -467,6 +467,8 @@ contract Safe is
* @param value Ether value of module transaction.
* @param data Data payload of module transaction.
* @param operation Operation type of module transaction.
* @param success Boolean flag indicating if the call succeeded.

*/
function onBeforeExecTransaction(
address to,
Expand All @@ -478,6 +480,8 @@ contract Safe is
uint256 gasPrice,
address gasToken,
address payable refundReceiver,
bytes memory signatures
bytes memory signatures,
bool success

) internal virtual {}
}
14 changes: 5 additions & 9 deletions contracts/SafeL2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ contract SafeL2 is Safe {
* @inheritdoc Safe
*/
function onBeforeExecTransaction(

address to,
uint256 value,
bytes calldata data,
Expand All @@ -47,7 +46,9 @@ contract SafeL2 is Safe {
uint256 gasPrice,
address gasToken,
address payable refundReceiver,
bytes memory signatures
bytes memory signatures,
bool /*success*/

) internal override {
bytes memory additionalInfo;
{
Expand All @@ -72,13 +73,8 @@ contract SafeL2 is Safe {
* @inheritdoc ModuleManager

*/
function onBeforeExecTransactionFromModule(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation,
bytes memory /* context */
) internal override {
function onBeforeExecTransactionFromModule(address to, uint256 value, bytes memory data, Enum.Operation operation) internal override {

emit SafeModuleTransaction(msg.sender, to, value, data, operation);

}
Expand Down
51 changes: 7 additions & 44 deletions contracts/base/ModuleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,9 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
uint256 value,
bytes memory data,
Enum.Operation operation
) external override returns (bool success) {
success = _execTransactionFromModule(to, value, data, operation, "");
}

function _execTransactionFromModule(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation,
bytes calldata context
) internal returns (bool success) {
(address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation, context);
) public override returns (bool success) {
onBeforeExecTransactionFromModule(to, value, data, operation);
(address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation);
success = execute(to, value, data, operation, type(uint256).max);
postModuleExecution(guard, guardHash, success);

Expand All @@ -197,31 +188,10 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
uint256 value,
bytes memory data,
Enum.Operation operation
) external override returns (bool success, bytes memory returnData) {
(success, returnData) = _execTransactionFromModuleReturnData(to, value, data, operation, "");
}
) public override returns (bool success, bytes memory returnData) {
onBeforeExecTransactionFromModule(to, value, data, operation);
(address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation);

/**
* @inheritdoc IModuleManager
*/
function execTransactionFromModuleReturnData(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation,
bytes calldata context
) external override returns (bool success, bytes memory returnData) {
(success, returnData) = _execTransactionFromModuleReturnData(to, value, data, operation, context);
}

function _execTransactionFromModuleReturnData(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation,
bytes calldata context
) internal returns (bool success, bytes memory returnData) {
(address guard, bytes32 guardHash) = preModuleExecution(to, value, data, operation, context);
success = execute(to, value, data, operation, type(uint256).max);
/* solhint-disable no-inline-assembly */
/// @solidity memory-safe-assembly
Expand Down Expand Up @@ -331,17 +301,10 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {

/**
* @notice A hook that gets called before execution of {execTransactionFromModule*} methods.

* @param to Destination address of module transaction.
* @param value Ether value of module transaction.
* @param data Data payload of module transaction.
* @param operation Operation type of module transaction.
*/
function onBeforeExecTransactionFromModule(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation,
bytes calldata context
) internal virtual {}
function onBeforeExecTransactionFromModule(address to, uint256 value, bytes memory data, Enum.Operation operation) internal virtual {}
}
10 changes: 7 additions & 3 deletions test/core/Safe.Execution.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,13 @@ describe("Safe", () => {
await user1.sendTransaction({ to: safeAddress, value: ethers.parseEther("1") });
const userBalance = await hre.ethers.provider.getBalance(user2.address);
expect(await hre.ethers.provider.getBalance(safeAddress)).to.be.eq(ethers.parseEther("1"));

const executedTx = await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]);
await expect(executedTx).to.emit(safe, "ExecutionSuccess");
let executedTx: any;
await expect(
executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]).then((tx) => {
executedTx = tx;
return tx;
}),
).to.emit(safe, "ExecutionSuccess");

const receipt = await hre.ethers.provider.getTransactionReceipt(executedTx!.hash);
const receiptLogs = receipt?.logs ?? [];
Expand Down
Loading