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

PoC for Safe simulateTransaction function #789

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
15 changes: 10 additions & 5 deletions contracts/common/StorageAccessible.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ abstract contract StorageAccessible {
* @dev Performs a delegatecall on a targetContract in the context of self.
* Internally reverts execution to avoid side effects (making it static).
*
* This method reverts with data equal to `abi.encode(bool(success), bytes(response))`.
* This method reverts with data equal to `abi.encode(bool(success), uint256(gasUsed), bytes(response))`.
* Specifically, the `returndata` after a call to this method will be:
* `success:bool || response.length:uint256 || response:bytes`.
* `success:bool || gasUsed:uint256 || response.length:uint256 || response:bytes`.
*
* @param targetContract Address of the contract containing the code to execute.
* @param calldataPayload Calldata that should be sent to the target contract (encoded method name and arguments).
Expand All @@ -43,13 +43,18 @@ abstract contract StorageAccessible {
/* solhint-disable no-inline-assembly */
/// @solidity memory-safe-assembly
assembly {
let gasBefore := gas()
let success := delegatecall(gas(), targetContract, add(calldataPayload, 0x20), mload(calldataPayload), 0, 0)
let gasAfter := gas()
let gasUsed := sub(gasBefore, gasAfter)
Copy link
Member

@mmv08 mmv08 Jul 8, 2024

Choose a reason for hiding this comment

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

This can be returned by the targetContract. You can check the SimulateTxAccessor contract for an example. You can basically combine its approach with your original contract for detailed simulations

Copy link
Author

Choose a reason for hiding this comment

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

this is redundant since SimulateTxAccessor.simulate returndata includes the safeTxGas:

(uint256 estimate, bool success, bytes memory returnData)


// Load free memory location
let ptr := mload(0x40)
mstore(ptr, success)
mstore(add(ptr, 0x20), returndatasize())
returndatacopy(add(ptr, 0x40), 0, returndatasize())
revert(ptr, add(returndatasize(), 0x40))
mstore(add(ptr, 0x20), gasUsed)
mstore(add(ptr, 0x40), returndatasize())
returndatacopy(add(ptr, 0x60), 0, returndatasize())
revert(ptr, add(returndatasize(), 0x60))
}
/* solhint-enable no-inline-assembly */
}
Expand Down
50 changes: 47 additions & 3 deletions src/utils/execution.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Signer, BigNumberish, BaseContract, ethers } from "ethers";
import { AddressZero } from "@ethersproject/constants";
import { Safe } from "../../typechain-types";
import { Safe, SimulateTxAccessor } from "../../typechain-types";

export const EIP_DOMAIN = {
EIP712Domain: [
Expand Down Expand Up @@ -148,8 +148,8 @@ export const buildSignatureBytes = (signatures: SafeSignature[]): string => {
let dynamicBytes = "";
for (const sig of signatures) {
if (sig.dynamic) {
/*
A contract signature has a static part of 65 bytes and the dynamic part that needs to be appended
/*
A contract signature has a static part of 65 bytes and the dynamic part that needs to be appended
at the end of signature bytes.
The signature format is
Signature type == 0
Expand Down Expand Up @@ -200,6 +200,50 @@ export const executeTx = async (safe: Safe, safeTx: SafeTransaction, signatures:
);
};

export const simulateTx = async (
safe: Safe,
simulateTxAccessor: SimulateTxAccessor,
safeTx: SafeTransaction,
overrides?: any,
): Promise<{
success: boolean;
gasUsed: bigint;
returndata: string;
}> => {
try {
const simulateTxAccessorAddress = await simulateTxAccessor.getAddress();
const simulateSafeTxEncodedPayload = simulateTxAccessor.interface.encodeFunctionData("simulate", [
safeTx.to,
safeTx.value,
safeTx.data,
safeTx.operation,
]);
await safe.simulateAndRevert.staticCall(simulateTxAccessorAddress, simulateSafeTxEncodedPayload, overrides || {}); // must revert
} catch (error: any) {
// Extract the revert data
const revertData = error.data;
// Check if the data is present and has the expected format
if (revertData && revertData.length >= 98) {
// Decode the success flag (first 32 bytes)
const success = ethers.toBeHex(revertData.slice(0, 32)) === "0x1";
// Decode the gasUsed (next 32 bytes)
const gasUsed = ethers.toBigInt("0x" + revertData.slice(32, 64));
// Decode the response length (next 32 bytes)
const responseLength = Number(ethers.toBigInt("0x" + revertData.slice(64, 96)));
// Decode the response data (remaining bytes)
const returndata = revertData.slice(96, 96 + responseLength * 2);
return {
success,
gasUsed,
returndata, // Assuming the response contains the txHash
};
} else {
console.error("Unexpected revert data format", revertData);
}
}
throw new Error("simulateAndRevert must revert");
};

export const buildContractCall = async (
contract: BaseContract,
method: string,
Expand Down
60 changes: 46 additions & 14 deletions test/core/Safe.Execution.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from "chai";
import hre, { deployments, ethers } from "hardhat";
import { deployContract, getSafeWithOwners } from "../utils/setup";
import { deployContract, getSafeWithOwners, getSimulateTxAccessor } from "../utils/setup";
import {
safeApproveHash,
buildSignatureBytes,
Expand All @@ -9,6 +9,7 @@ import {
executeTx,
calculateSafeTransactionHash,
buildContractCall,
simulateTx,
} from "../../src/utils/execution";

import { chainId } from "../utils/encoding";
Expand Down Expand Up @@ -41,12 +42,16 @@ describe("Safe", () => {
}
}`;
const reverter = await deployContract(user1, reverterSource);

const accessor = await getSimulateTxAccessor();

return {
safe: await getSafeWithOwners([user1.address]),
reverter,
storageSetter,
nativeTokenReceiver,
signers,
accessor,
};
});

Expand Down Expand Up @@ -181,15 +186,21 @@ describe("Safe", () => {
});

it("should revert on unknown operation", async () => {
const { safe, signers } = await setupTests();
const { safe, signers, accessor } = await setupTests();
const [user1] = signers;
const safeAddress = await safe.getAddress();

const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce(), operation: 2 });
const simulationResult = await simulateTx(safe, accessor, tx);

// expect to be reverted with no string
expect(simulationResult.success).to.be.eq(false);
expect(simulationResult.returndata).to.be.eq("");
await expect(executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)])).to.be.reverted;
});

it("should emit payment in success event", async () => {
const { safe, signers } = await setupTests();
const { safe, signers, accessor } = await setupTests();
const [user1, user2] = signers;
const safeAddress = await safe.getAddress();
const tx = buildSafeTransaction({
Expand All @@ -206,6 +217,10 @@ describe("Safe", () => {
expect(await hre.ethers.provider.getBalance(safeAddress)).to.be.eq(ethers.parseEther("1"));

let executedTx: any;

const simulationResult = await simulateTx(safe, accessor, tx);
expect(simulationResult.success).to.be.eq(true);

await expect(
executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]).then((tx) => {
executedTx = tx;
Expand All @@ -230,7 +245,7 @@ describe("Safe", () => {
});

it("should emit payment in failure event", async () => {
const { safe, storageSetter, signers } = await setupTests();
const { safe, storageSetter, signers, accessor } = await setupTests();
const [user1, user2] = signers;
const safeAddress = await safe.getAddress();
const storageSetterAddress = await storageSetter.getAddress();
Expand All @@ -249,6 +264,9 @@ describe("Safe", () => {
const userBalance = await hre.ethers.provider.getBalance(user2.address);
await expect(await hre.ethers.provider.getBalance(safeAddress)).to.eq(ethers.parseEther("1"));

const simulationResult = await simulateTx(safe, accessor, tx);

expect(simulationResult.success).to.be.eq(false);
let executedTx: any;
await expect(
executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]).then((tx) => {
Expand All @@ -271,16 +289,16 @@ describe("Safe", () => {
});

it("should be possible to manually increase gas", async () => {
const { safe, signers } = await setupTests();
const { safe, signers, accessor } = await setupTests();
const [user1] = signers;
const safeAddress = await safe.getAddress();
const gasUserSource = `
contract GasUser {

uint256[] public data;

constructor() payable {}

function nested(uint256 level, uint256 count) external {
if (level == 0) {
for (uint256 i = 0; i < count; i++) {
Expand All @@ -290,7 +308,7 @@ describe("Safe", () => {
}
this.nested(level - 1, count);
}

function useGas(uint256 count) public {
this.nested(6, count);
this.nested(8, count);
Expand All @@ -301,18 +319,30 @@ describe("Safe", () => {
const data = gasUser.interface.encodeFunctionData("useGas", [80]);
const safeTxGas = 10000;
const tx = buildSafeTransaction({ to, data, safeTxGas, nonce: await safe.nonce() });

let simulationResult = await simulateTx(safe, accessor, tx, { gasLimit: 170000 });
expect(simulationResult.success).to.be.eq(false);
await expect(
executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)], { gasLimit: 170000 }),
"Safe transaction should fail with low gasLimit",
).to.emit(safe, "ExecutionFailure");

await expect(
executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)], { gasLimit: 6000000 }),
"Safe transaction should succeed with high gasLimit",
).to.emit(safe, "ExecutionSuccess");
simulationResult = await simulateTx(safe, accessor, tx, { gasLimit: 6000000 });
expect(simulationResult.success).to.be.eq(true);
const executedTxPromise = executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)], { gasLimit: 6000000 });
const executedTxResponse = await executedTxPromise;
const receipt = await hre.ethers.provider.getTransactionReceipt(executedTxResponse.hash);
// everything else(i.e. other than the safe tx gas which is being estimated by simulateTransaction)
// includes intrinsic/base gas cost and signature validation, etc
const maxGasForEverythingElse = 50_000n; // it's reasonable that this will take the most
expect(receipt?.gasUsed).to.be.gt(simulationResult.gasUsed);
expect(receipt?.gasUsed).to.be.lt(simulationResult.gasUsed + maxGasForEverythingElse);
await expect(executedTxPromise, "Safe transaction should succeed with high gasLimit").to.emit(safe, "ExecutionSuccess");

// This should only work if the gasPrice is 0
tx.gasPrice = 1;
simulationResult = await simulateTx(safe, accessor, tx, { gasLimit: 6000000 });
expect(simulationResult.success).to.be.eq(false);
await user1.sendTransaction({ to: safeAddress, value: ethers.parseEther("1") });
await expect(
executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)], { gasLimit: 6000000 }),
Expand All @@ -321,7 +351,7 @@ describe("Safe", () => {
});

it("should forward all the gas to the native token refund receiver", async () => {
const { safe, nativeTokenReceiver, signers } = await setupTests();
const { safe, nativeTokenReceiver, signers, accessor } = await setupTests();
const [user1] = signers;
const safeAddress = await safe.getAddress();
const nativeTokenReceiverAddress = await nativeTokenReceiver.getAddress();
Expand All @@ -338,6 +368,8 @@ describe("Safe", () => {
await user1.sendTransaction({ to: safeAddress, value: ethers.parseEther("1") });
await expect(await hre.ethers.provider.getBalance(safeAddress)).to.eq(ethers.parseEther("1"));

const simulationResult = await simulateTx(safe, accessor, tx);
expect(simulationResult.success).to.be.eq(true);
const executedTx = await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)], { gasLimit: 500000 });
const receipt = await hre.ethers.provider.getTransactionReceipt(executedTx.hash);
const receiptLogs = receipt?.logs ?? [];
Expand Down
Loading