Skip to content

Commit c317122

Browse files
committed
fix: avoid unnecessary Safe proxy calls
1 parent 0f1a967 commit c317122

3 files changed

Lines changed: 76 additions & 13 deletions

File tree

src/ape_ethereum/ecosystem.py

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -559,22 +559,33 @@ def str_to_slot(text):
559559
except ApeException:
560560
pass
561561

562-
# safe >=1.1.0 provides `masterCopy()`, which is also stored in slot 0
563-
# call it and check that target matches
564-
try:
565-
singleton = ContractCall(MASTER_COPY_ABI, address)(skip_trace=True)
566-
slot_0 = self.provider.get_storage(address, 0)
567-
target = self.conversion_manager.convert(slot_0[-20:], AddressType)
568-
# NOTE: `target` is set in initialized proxies
569-
if target != ZERO_ADDRESS and target == singleton:
570-
return ProxyInfo(type=ProxyType.GnosisSafe, target=target, abi=MASTER_COPY_ABI)
571-
572-
except ApeException:
573-
pass
562+
# Safe >1.0.0 provides `masterCopy()`, which is also stored in slot 0.
563+
# Check the bytecode marker first to avoid an extra call for most non-Safe contracts.
564+
master_copy_selector = self.get_method_selector(MASTER_COPY_ABI).hex()
565+
safe_master_copy_markers = (
566+
# Safe v1.1.0 through v1.4.1 compares calldata against a padded PUSH32.
567+
f"7f{master_copy_selector}{'00' * 28}",
568+
# Optimized builds may reconstruct the same padded selector with PUSH4 + SHL.
569+
"63530ca43760e11b14",
570+
# Safe v1.5.0+ compares the first 4 calldata bytes against a PUSH4.
571+
f"63{master_copy_selector}",
572+
)
573+
if any(marker in code for marker in safe_master_copy_markers):
574+
try:
575+
slot_0 = self.provider.get_storage(address, 0)
576+
target = self.conversion_manager.convert(slot_0[-20:], AddressType)
577+
# NOTE: `target` is set in initialized proxies
578+
if target != ZERO_ADDRESS and target == ContractCall(MASTER_COPY_ABI, address)(
579+
skip_trace=True
580+
):
581+
return ProxyInfo(type=ProxyType.GnosisSafe, target=target, abi=MASTER_COPY_ABI)
582+
583+
except ApeException:
584+
pass
574585

575586
# eip-897 delegate proxy, read `proxyType()` and `implementation()`
576587
# perf: only make a call when a proxyType() selector is mentioned in the code
577-
eip897_pattern = b"\x63" + keccak(text="proxyType()")[:4]
588+
eip897_pattern = b"\x63" + self.get_method_selector(PROXY_TYPE_ABI)
578589
if eip897_pattern.hex() in code:
579590
try:
580591
proxy_type = ContractCall(PROXY_TYPE_ABI, address)(skip_trace=True)
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// SPDX-License-Identifier: LGPL-3.0-only
2+
3+
pragma solidity >=0.7.0 <0.9.0;
4+
5+
6+
// Copied from https://github.com/safe-global/safe-smart-account/blob/v1.5.0/contracts/proxies/SafeProxy.sol
7+
8+
contract SafeProxyV150 {
9+
// Singleton always needs to be first declared variable, to ensure that it is at the same location in the contracts to which calls are delegated.
10+
address internal singleton;
11+
12+
/**
13+
* @notice Constructor function sets address of singleton contract.
14+
* @param _singleton Singleton address.
15+
*/
16+
constructor(address _singleton) {
17+
require(_singleton != address(0), "Invalid singleton address provided");
18+
singleton = _singleton;
19+
}
20+
21+
/// @dev Fallback function forwards all transactions and returns all received return data.
22+
fallback() external payable {
23+
/* solhint-disable no-inline-assembly */
24+
assembly {
25+
let _singleton := sload(0)
26+
// 0xa619486e == uint32(bytes4(keccak256("masterCopy()"))).
27+
if eq(shr(224, calldataload(0)), 0xa619486e) {
28+
mstore(0x6c, shl(96, _singleton))
29+
return(0x60, 0x20)
30+
}
31+
calldatacopy(0, 0, calldatasize())
32+
let success := delegatecall(gas(), _singleton, 0, calldatasize(), 0, 0)
33+
returndatacopy(0, 0, returndatasize())
34+
if iszero(success) {
35+
revert(0, returndatasize())
36+
}
37+
return(0, returndatasize())
38+
}
39+
/* solhint-enable no-inline-assembly */
40+
}
41+
}

tests/functional/geth/test_proxy.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,17 @@ def test_gnosis_safe(project, geth_contract, owner, ethereum, chain):
6262
assert isinstance(proxy_instance_ref_2.myNumber(), int)
6363

6464

65+
@geth_process_test
66+
def test_gnosis_safe_v150(project, geth_contract, owner, ethereum):
67+
target = geth_contract.address
68+
proxy_instance = owner.deploy(project.SafeProxyV150, target)
69+
70+
actual = ethereum.get_proxy_info(proxy_instance.address)
71+
assert actual is not None
72+
assert actual.type == ProxyType.GnosisSafe
73+
assert actual.target == target
74+
75+
6576
@geth_process_test
6677
def test_openzeppelin(project, geth_contract, owner, ethereum, sender):
6778
constructor_contract = owner.deploy(project.SolFallbackAndReceive)

0 commit comments

Comments
 (0)