ApeWorX / ape-safe

Safe (Wallet) account plugin for the Ape Framework
https://www.apeworx.io/
Apache License 2.0
13 stars 8 forks source link

Arbitrum - Can't Add Safe #34

Open solarthesis opened 7 months ago

solarthesis commented 7 months ago

Environment information

$ ape --version
0.7.7

$ ape plugins list
Installed Plugins
  alchemy      0.7.1
  arbitrum     0.7.1
  etherscan    0.7.0
  foundry      0.7.1
  optimism     0.7.2
  safe         0.7.0b1
  solidity     0.7.1

What went wrong?

Running the below to add a safe

(gnosis_test_ape) thesis@ks:~/repos/gnosis_test_ape$ ape safe add 0xBFd8c1f83C5389a64166F01fea0D56d982F627Ac "arbitrum-test" --network arbitrum:mainnet:alchemy

returns an error

ERROR: (ApeAttributeError) 'GnosisSafeProxy' has no attribute 'VERSION'.

Please include information like:

How can it be fixed?

Fill this in if you have ideas on how the bug could be fixed.

linear[bot] commented 7 months ago

APE-1677 Arbitrum - Can't Add Safe

fubuloubu commented 7 months ago

Ah I think the issue here is Etherscan's "similar bytecode" issue with the API not actually showing what it's similar to despite the UI showing something for the implementation contract

It does correctly detect that it's a GnosisSafeProxy though, so perhaps we can just assume it is a Safe contract if we are trying to add it

solarthesis commented 7 months ago

ahhh alright that is pretty narky problem tbh

solarthesis commented 7 months ago

you can use the byte code trick, no? like fetch the bytecode, assert that it matches what a gnosis safe is expected to be, and just add the safe. Yeah etherscan api's not too reliable when it comes to proxies.

fubuloubu commented 7 months ago

ahhh alright that is pretty narky problem tbh

we can hack ape-etherscan if we can somehow get the data, hell it can do scraping if push comes to shove lol

fubuloubu commented 7 months ago

you can use the byte code trick, no? like fetch the bytecode, assert that it matches what a gnosis safe is expected to be, and just add the safe. Yeah etherscan api's not too reliable when it comes to proxies.

we do that for proxy detection, but one step further is what the proxy points at. I think yours and mine points at different implementation contracts

solarthesis commented 7 months ago

yeah mine is just the official safe one :thinking: i think the concern here is folks use ape-safe on a maliscious safe, right? hence these checks on bytecode? Probably id' say it's something we can live with, because there is so much we can do with different safe versions and implementations and proxies.

fubuloubu commented 7 months ago

i think the concern here is folks use ape-safe on a maliscious safe, right? hence these checks on bytecode?

It's more about not screwing up Ape, let's say you made a mistake and pointed it at some other type of contract, you'd get all sorts of errors that might be dangerous or at least annoying

Manually, you can set the contract type in ~/.ape/arbitrum/mainnet/contract_types/{address}.json to ensure the method exists and that you don't get the error above

solarthesis commented 7 months ago

i think the concern here is folks use ape-safe on a maliscious safe, right? hence these checks on bytecode?

It's more about not screwing up Ape, let's say you made a mistake and pointed it at some other type of contract, you'd get all sorts of errors that might be dangerous or at least annoying

Manually, you can set the contract type in ~/.ape/arbitrum/mainnet/contract_types/{address}.json to ensure the method exists and that you don't get the error above

probably a --force in this case would be helpful that bypasses these checks. still i think we have a design problem if a safe generated by the official gnosis app can't be added easily to ape-safe

fubuloubu commented 7 months ago

I agree, we can probably err on the side of "the user knows what they are doing" and explicitly assume that the target contract is a Safe when they add it through the normal means