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

Closes #201: Allow storage to be read on-chain #269

Merged
merged 11 commits into from
Mar 5, 2021
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
1 change: 1 addition & 0 deletions .husky/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
_
21 changes: 21 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

# Redirect output to stderr.
exec 1>&2

# prevent it.only or describe.only commited
if [ "$allowonlytests" != "true" ] &&
test $(git diff --cached | grep -E "\b(it|describe).only\(" | wc -l) != 0
then
cat <<\EOF
Error: Attempt to add it.only or describe.only - which may disable all other tests

If you know what you are doing you can disable this check using:

git config hooks.allowonlytests true
EOF
exit 1
fi

exit 0
4 changes: 3 additions & 1 deletion contracts/GnosisSafe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import "./base/FallbackManager.sol";
import "./common/MasterCopy.sol";
import "./common/SignatureDecoder.sol";
import "./common/SecuredTokenTransfer.sol";
import "./common/StorageAccessible.sol";
import "./interfaces/ISignatureValidator.sol";
import "./external/GnosisSafeMath.sol";

Expand All @@ -13,7 +14,7 @@ import "./external/GnosisSafeMath.sol";
/// @author Richard Meissner - <[email protected]>
/// @author Ricardo Guilherme Schmidt - (Status Research & Development GmbH) - Gas Token Payment
contract GnosisSafe
is MasterCopy, ModuleManager, OwnerManager, SignatureDecoder, SecuredTokenTransfer, ISignatureValidatorConstants, FallbackManager {
is MasterCopy, ModuleManager, OwnerManager, SignatureDecoder, SecuredTokenTransfer, ISignatureValidatorConstants, FallbackManager, StorageAccessible {

using GnosisSafeMath for uint256;

Expand Down Expand Up @@ -265,6 +266,7 @@ contract GnosisSafe
/// @param data Data payload of Safe transaction.
/// @param operation Operation type of Safe transaction.
/// @return Estimate without refunds and overhead fees (base transaction and payload data gas costs).
/// @notice Deprecated in favor of common/StorageAccessible.sol and will be removed in next version.
function requiredTxGas(address to, uint256 value, bytes calldata data, Enum.Operation operation)
external
returns (uint256)
Expand Down
44 changes: 44 additions & 0 deletions contracts/accessors/SimulateTxAccessor.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
pragma solidity >=0.5.0 <0.6.0;

import "../base/Executor.sol";

/// @title Simulate Transaction Accessor - can be used with StorageAccessible to simulate Safe transactions
/// @author Richard Meissner - <[email protected]>
contract SimulateTxAccessor is Executor {

bytes32 constant private GUARD_VALUE = keccak256("simulate_tx_accessor.guard.bytes32");
bytes32 guard;

constructor() public {
guard = GUARD_VALUE;
}

modifier onlyDelegateCall() {
require(guard != GUARD_VALUE, "SimulateTxAccessor should only be called via delegatecall");
_;
}

function simulate(address to, uint256 value, bytes calldata data, Enum.Operation operation)
external
onlyDelegateCall()
returns(uint256 estimate, bool success, bytes memory returnData)
{
uint256 startGas = gasleft();
success = execute(to, value, data, operation, gasleft());
estimate = startGas - gasleft();
// solium-disable-next-line security/no-inline-assembly
assembly {
// Load free memory location
let ptr := mload(0x40)
// We allocate memory for the return data by setting the free memory location to
// current free memory location + data size + 32 bytes for data size value
mstore(0x40, add(ptr, add(returndatasize(), 0x20)))
// Store the size
mstore(ptr, returndatasize())
// Store the data
returndatacopy(add(ptr, 0x20), 0, returndatasize())
// Point the return data to the correct memory location
returnData := ptr
}
}
}
1 change: 0 additions & 1 deletion contracts/common/SignatureDecoder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ pragma solidity >=0.5.0 <0.6.0;


/// @title SignatureDecoder - Decodes signatures that a encoded as bytes
/// @author Ricardo Guilherme Schmidt (Status Research & Development GmbH)
/// @author Richard Meissner - <[email protected]>
contract SignatureDecoder {

Expand Down
86 changes: 86 additions & 0 deletions contracts/common/StorageAccessible.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
pragma solidity >=0.5.0 <0.6.0;

/// @title StorageAccessible - generic base contract that allows callers to access all internal storage.
/// @notice Adjusted version of https://github.com/gnosis/util-contracts/blob/3db1e531cb243a48ea91c60a800d537c1000612a/contracts/StorageAccessible.sol
contract StorageAccessible {
bytes4 internal constant SIMULATE_DELEGATECALL_INTERNAL_SELECTOR = bytes4(
keccak256("simulateDelegatecallInternal(address,bytes)")
);

/**
* @dev Reads `length` bytes of storage in the currents contract
* @param offset - the offset in the current contract's storage in words to start reading from
* @param length - the number of words (32 bytes) of data to read
* @return the bytes that were read.
*/
function getStorageAt(uint256 offset, uint256 length)
public
view
returns (bytes memory)
{
bytes memory result = new bytes(length * 32);
for (uint256 index = 0; index < length; index++) {
assembly {
let word := sload(add(offset, index))
mstore(add(add(result, 0x20), mul(index, 0x20)), word)
}
}
return result;
}

/**
* @dev Performs a delegetecall on a targetContract in the context of self.
* Internally reverts execution to avoid side effects (making it static). Catches revert and returns encoded result as 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).
*/
function simulateDelegatecall(
address targetContract,
bytes memory calldataPayload
) public returns (bytes memory) {
bytes memory innerCall = abi.encodeWithSelector(
SIMULATE_DELEGATECALL_INTERNAL_SELECTOR,
targetContract,
calldataPayload
);
(, bytes memory response) = address(this).call(innerCall);
bool innerSuccess = response[response.length - 1] == 0x01;
setLength(response, response.length - 1);
if (innerSuccess) {
return response;
} else {
revertWith(response);
}
}

/**
* @dev Performs a delegetecall on a targetContract in the context of self.
* Internally reverts execution to avoid side effects (making it static). Returns encoded result as revert message
* concatenated with the success flag of the inner call as a last byte.
* @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).
*/
function simulateDelegatecallInternal(
Uxio0 marked this conversation as resolved.
Show resolved Hide resolved
address targetContract,
bytes memory calldataPayload
) public returns (bytes memory) {
(bool success, bytes memory response) = targetContract.delegatecall(
calldataPayload
);
revertWith(abi.encodePacked(response, success));
}

function revertWith(bytes memory response) internal pure {
Uxio0 marked this conversation as resolved.
Show resolved Hide resolved
// solium-disable-next-line security/no-inline-assembly
assembly {
revert(add(response, 0x20), mload(response))
}
}

function setLength(bytes memory buffer, uint256 length) internal pure {
// solium-disable-next-line security/no-inline-assembly
assembly {
mstore(buffer, length)
}
}
}
14 changes: 14 additions & 0 deletions contracts/interfaces/ViewStorageAccessible.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
pragma solidity >=0.5.0 <0.6.0;

/// @title ViewStorageAccessible - Interface on top of StorageAccessible base class to allow simulations from view functions
/// @notice Adjusted version of https://github.com/gnosis/util-contracts/blob/3db1e531cb243a48ea91c60a800d537c1000612a/contracts/StorageAccessible.sol
interface ViewStorageAccessible {
/**
* @dev Same as `simulateDelegatecall` on StorageAccessible. Marked as view so that it can be called from external contracts
* that want to run simulations from within view functions. Will revert if the invoked simulation attempts to change state.
*/
function simulateDelegatecall(
address targetContract,
bytes calldata calldataPayload
) external view returns (bytes memory);
}
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"lint:sol": "solhint 'contracts/**/*.sol'",
"lint:ts": "eslint --max-warnings 0 .",
"fmt:sol": "prettier 'contracts/**/*.sol' -w",
"prepack": "yarn build"
"prepack": "yarn build",
"prepare": "husky install"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -58,6 +59,7 @@
"ethereum-waffle": "^3.2.0",
"hardhat": "^2.0.8",
"hardhat-deploy": "^0.7.0-beta.38",
"husky": "^5.1.3",
"prettier": "^2.1.2",
"prettier-plugin-solidity": "^1.0.0-alpha.60",
"solhint": "^3.3.2",
Expand Down
20 changes: 20 additions & 0 deletions src/deploy/deploy_accessors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { DeployFunction } from "hardhat-deploy/types";
import { HardhatRuntimeEnvironment } from "hardhat/types";

const deploy: DeployFunction = async function (
hre: HardhatRuntimeEnvironment,
) {
const { deployments, getNamedAccounts } = hre;
const { deployer } = await getNamedAccounts();
const { deploy } = deployments;

await deploy("SimulateTxAccessor", {
from: deployer,
args: [],
log: true,
deterministicDeployment: true,
});
};

deploy.tags = ['accessors']
export default deploy;
103 changes: 103 additions & 0 deletions test/accessors/SimulateTxAccessor.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { expect } from "chai";
import hre, { deployments, waffle } from "hardhat";
import "@nomiclabs/hardhat-ethers";
import { deployContract, getSimulateTxAccessor, getSafeWithOwners } from "../utils/setup";
import { buildContractCall } from "../utils/execution";
import { parseEther } from "ethers/lib/utils";

describe("SimulateTxAccessor", async () => {

const [user1, user2] = waffle.provider.getWallets();

const setupTests = deployments.createFixture(async ({ deployments }) => {
await deployments.fixture();
const accessor = await getSimulateTxAccessor()
const source = `
contract Test {
function sendAndReturnBalance(address payable target, uint256 amount) public returns (uint256) {
(bool success,) = target.call{ value: amount }("");
require(success, "Transfer failed");
return target.balance;
}
}`
const interactor = await deployContract(user1, source);
return {
safe: await getSafeWithOwners([user1.address]),
accessor,
interactor
}
})

describe("estimate", async () => {

it('should enforce delegatecall', async () => {
const { accessor } = await setupTests()
const source = `
contract Test {
function killme() public {
selfdestruct(payable(msg.sender));
}
}`
const killLib = await deployContract(user1, source);
const tx = buildContractCall(killLib, "killme", [], 0)

let code = await hre.ethers.provider.getCode(accessor.address)
await expect(
accessor.simulate(tx.to, tx.value, tx.data, tx.operation)
).to.be.revertedWith("SimulateTxAccessor should only be called via delegatecall")

expect(await hre.ethers.provider.getCode(accessor.address)).to.be.eq(code)
})

it('simulate call', async () => {
const { safe, accessor } = await setupTests()
const tx = buildContractCall(safe, "getOwners", [], 0)
const simulationData = accessor.interface.encodeFunctionData("simulate", [tx.to, tx.value, tx.data, tx.operation])
const acccessibleData = await safe.callStatic.simulateDelegatecall(accessor.address, simulationData)
const simulation = accessor.interface.decodeFunctionResult("simulate", acccessibleData)
expect(
safe.interface.decodeFunctionResult("getOwners", simulation.returnData)[0]
).to.be.deep.eq([user1.address])
expect(
simulation.success
).to.be.true
expect(
simulation.estimate.toNumber()
).to.be.lte(10000)
})

it('simulate delegatecall', async () => {
const { safe, accessor, interactor } = await setupTests()
await user1.sendTransaction({to: safe.address, value: parseEther("1")})
const userBalance = await hre.ethers.provider.getBalance(user2.address)
const tx = buildContractCall(interactor, "sendAndReturnBalance", [user2.address, parseEther("1")], 0, true)
const simulationData = accessor.interface.encodeFunctionData("simulate", [tx.to, tx.value, tx.data, tx.operation])
const acccessibleData = await safe.callStatic.simulateDelegatecall(accessor.address, simulationData)
const simulation = accessor.interface.decodeFunctionResult("simulate", acccessibleData)
expect(
interactor.interface.decodeFunctionResult("sendAndReturnBalance", simulation.returnData)[0]
).to.be.deep.eq(userBalance.add(parseEther("1")))
expect(
simulation.success
).to.be.true
expect(
simulation.estimate.toNumber()
).to.be.lte(15000)
})

it('simulate revert', async () => {
const { safe, accessor, interactor } = await setupTests()
const tx = buildContractCall(interactor, "sendAndReturnBalance", [user2.address, parseEther("1")], 0, true)
const simulationData = accessor.interface.encodeFunctionData("simulate", [tx.to, tx.value, tx.data, tx.operation])
const acccessibleData = await safe.callStatic.simulateDelegatecall(accessor.address, simulationData)
const simulation = accessor.interface.decodeFunctionResult("simulate", acccessibleData)
expect(simulation.returnData).to.be.deep.eq("0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000f5472616e73666572206661696c65640000000000000000000000000000000000")
expect(
simulation.success
).to.be.false
expect(
simulation.estimate.toNumber()
).to.be.lte(20000)
})
})
})
2 changes: 1 addition & 1 deletion test/core/GnosisSafe.Estimation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("GnosisSafe", async () => {
}
})

describe.only("requiredTxGas", async () => {
describe("requiredTxGas", async () => {

it('should revert without reason if tx fails', async () => {
const { safe, reverter } = await setupTests()
Expand Down
Loading