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

[WIP] Proxy Direct #111

Closed
wants to merge 1 commit into from
Closed
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
7 changes: 7 additions & 0 deletions foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,10 @@ solc = "0.8.19"

# See more config options https://github.com/foundry-rs/foundry/tree/master/config
evm_version = "paris"

[profile.proxy]
src = "src-proxy"
libs = []
optimizer = true
optimizer_runs = 1
bytecode_hash = "none"
43 changes: 43 additions & 0 deletions src-proxy/ProxyDirect.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// SPDX-License-Identifier: BSD-3-Clause
pragma solidity 0.8.19;

contract ProxyDirect {
/// @notice Address of the EOA signer or the EIP-1271 contract that verifies signed operations for this wallet
address public immutable signer;

/// @notice Address of the executor contract, if any, empowered to direct-execute unsigned operations for this wallet
address public immutable executor;
Comment on lines +6 to +9
Copy link
Contributor

@cwang25 cwang25 Dec 4, 2023

Choose a reason for hiding this comment

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

Correct me if I'm wrong.
When proxy delegate call, the immutable reads will reads the value from implementations immutable instead of proxy's ? (I think I ran into that issues before when integrating proxy into test suites)
So I assume this route, will also require changes to implementation contract to have special getter?

or in here we won't have this kind of issue 🤔 ? (something special in assembly?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the implementation contract will read the value from the proxy using a call. The proxy has built-in, public getters for these immutables.


/// @notice Address of the quark wallet implementation code
address immutable walletImplementation;

/**
* @notice Construct a new QuarkWallet
* @param signer_ The address that is allowed to sign QuarkOperations for this wallet
* @param executor_ The address that is allowed to directly execute Quark scripts for this wallet
*/
constructor(address implementation_, address signer_, address executor_) {
signer = signer_;
executor = executor_;
walletImplementation = implementation_;
}

/**
* @notice Proxy calls into the underlying wallet implementation
*/
fallback(bytes calldata /* data */) external payable returns (bytes memory) {
address walletImplementation_ = walletImplementation;
assembly {
let calldataLen := calldatasize()
calldatacopy(0, 0, calldataLen)
let succ := delegatecall(gas(), walletImplementation_, 0x00, calldataLen, 0x00, 0x00)
let retSz := returndatasize()
returndatacopy(0, 0, retSz)
if succ {
return(0, retSz)
}

revert(0, retSz)
Comment on lines +36 to +40
Copy link
Contributor

@cwang25 cwang25 Dec 4, 2023

Choose a reason for hiding this comment

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

Should we check the return data is 0?
Seems OZ's approach check the return data.
like: https://github.com/compound-finance/quark/pull/108/files

 switch result
            // delegatecall returns 0 on error.
            case 0 { revert(0, returndatasize()) }
            default { return(0, returndatasize()) }

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these two code blocks are equivalent right? If succ == 0, revert

Copy link
Contributor

@cwang25 cwang25 Dec 4, 2023

Choose a reason for hiding this comment

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

oh yeah that's right.
just saw oz explicitly mentioning checking zero, but works the same as bool.

}
}
}
96 changes: 96 additions & 0 deletions src-proxy/ProxyDirectOutput.yul
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Note: outputs to 241 bytes

object "ProxyDirect_48" {
code {
{
/// @src 0:66:1588 "contract ProxyDirect {..."
let _1 := memoryguard(0xe0)
if callvalue() { revert(0, 0) }
let programSize := datasize("ProxyDirect_48")
let argSize := sub(codesize(), programSize)
let newFreePtr := add(_1, and(add(argSize, 31), not(31)))
if or(gt(newFreePtr, sub(shl(64, 1), 1)), lt(newFreePtr, _1))
{
mstore(/** @src -1:-1:-1 */ 0, /** @src 0:66:1588 "contract ProxyDirect {..." */ shl(224, 0x4e487b71))
mstore(4, 0x41)
revert(/** @src -1:-1:-1 */ 0, /** @src 0:66:1588 "contract ProxyDirect {..." */ 0x24)
}
mstore(64, newFreePtr)
codecopy(_1, programSize, argSize)
if slt(sub(add(_1, argSize), _1), 96)
{
revert(/** @src -1:-1:-1 */ 0, 0)
}
/// @src 0:66:1588 "contract ProxyDirect {..."
let value0 := abi_decode_address_fromMemory(_1)
let value1 := abi_decode_address_fromMemory(add(_1, 32))
let value2 := abi_decode_address_fromMemory(add(_1, 64))
/// @src 0:851:867 "signer = signer_"
mstore(128, value1)
/// @src 0:877:897 "executor = executor_"
mstore(160, value2)
/// @src 0:907:945 "walletImplementation = implementation_"
mstore(192, value0)
/// @src 0:66:1588 "contract ProxyDirect {..."
let _2 := mload(64)
let _3 := datasize("ProxyDirect_48_deployed")
codecopy(_2, dataoffset("ProxyDirect_48_deployed"), _3)
setimmutable(_2, "4", mload(/** @src 0:851:867 "signer = signer_" */ 128))
/// @src 0:66:1588 "contract ProxyDirect {..."
setimmutable(_2, "7", mload(/** @src 0:877:897 "executor = executor_" */ 160))
/// @src 0:66:1588 "contract ProxyDirect {..."
setimmutable(_2, "10", mload(/** @src 0:907:945 "walletImplementation = implementation_" */ 192))
/// @src 0:66:1588 "contract ProxyDirect {..."
return(_2, _3)
}
function abi_decode_address_fromMemory(offset) -> value
{
value := mload(offset)
if iszero(eq(value, and(value, sub(shl(160, 1), 1)))) { revert(0, 0) }
}
}
/// @use-src 0:"src-proxy/ProxyDirect.sol"
object "ProxyDirect_48_deployed" {
code {
{
/// @src 0:66:1588 "contract ProxyDirect {..."
mstore(64, 128)
if iszero(lt(calldatasize(), 4))
{
let _1 := 0
switch shr(224, calldataload(_1))
case 0x238ac933 {
if callvalue() { revert(_1, _1) }
if slt(add(calldatasize(), not(3)), _1) { revert(_1, _1) }
mstore(128, and(/** @src 0:208:239 "address public immutable signer" */ loadimmutable("4"), /** @src 0:66:1588 "contract ProxyDirect {..." */ sub(shl(160, 1), 1)))
return(128, 32)
}
case 0xc34c08e5 {
if callvalue() { revert(_1, _1) }
if slt(add(calldatasize(), not(3)), _1) { revert(_1, _1) }
let memPos := mload(64)
mstore(memPos, and(/** @src 0:368:401 "address public immutable executor" */ loadimmutable("7"), /** @src 0:66:1588 "contract ProxyDirect {..." */ sub(shl(160, 1), 1)))
return(memPos, 32)
}
}
/// @ast-id 47 @src 0:1043:1586 "fallback(bytes calldata /* data *\/) external payable returns (bytes memory) {..."
/** @ast-id 47 */ /** @ast-id 47 */ pop(/** @ast-id 47 */ /** @ast-id 47 */ fun())
}
/// @ast-id 47
function fun() -> var_mpos
{
/// @src 0:1105:1117 "bytes memory"
var_mpos := /** @src 0:66:1588 "contract ProxyDirect {..." */ 96
/// @src 0:1191:1580 "assembly {..."
let _1 := 0
calldatacopy(_1, _1, calldatasize())
let usr$succ := delegatecall(gas(), /** @src 0:1161:1181 "walletImplementation" */ loadimmutable("10"), /** @src 0:1191:1580 "assembly {..." */ _1, calldatasize(), _1, _1)
let usr$retSz := returndatasize()
returndatacopy(_1, _1, usr$retSz)
if usr$succ { return(_1, usr$retSz) }
revert(_1, usr$retSz)
}
}
data ".metadata" hex""
}
}