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

Web UI shows warning with 1.4.1 wallet created in CLI #3647

Closed
JOY opened this issue May 2, 2024 · 19 comments
Closed

Web UI shows warning with 1.4.1 wallet created in CLI #3647

JOY opened this issue May 2, 2024 · 19 comments

Comments

@JOY
Copy link

JOY commented May 2, 2024

Bug description

Web UI shows warning with 1.4.1 wallet created in CLI: This Safe Account was created with an unsupported base contract. The web interface might not work correctly. We recommend using the command line interface instead.

Environment

  • Browser: Chrome
  • Wallet: MetaMask
  • Chain: Binance Smart Chain Mainnet

Steps to reproduce

  1. Create a 1.4.1 Safe via the CLI
  2. Open it in the UI
  3. It shows warning in the web UI: This Safe Account was created with an unsupported base contract. The web interface might not work correctly. We recommend using the command line interface instead.

Expected result

It should shows no warning since web UI was confirmed to support 1.4.1

Obtained result

It shows warning in the web UI: This Safe Account was created with an unsupported base contract. The web interface might not work correctly. We recommend using the command line interface instead.

Screenshots

image

@JOY JOY added the bug label May 2, 2024
@katspaugh
Copy link
Member

@JOY
Copy link
Author

JOY commented May 4, 2024

Did you use this base contract? https://github.com/safe-global/safe-deployments/blob/main/src/assets/v1.4.1/safe_l2.json#L10

I use this one: https://github.com/safe-global/safe-deployments/blob/main/src/assets/v1.4.1/safe.json
The link above when I load the wallet in Binance Smart Chain. So it should be Layer 1, right?

@katspaugh
Copy link
Member

I don't know the reason for this but the UI historically considers all chains other than mainnet an "L2" even if they are technically L1.

@nlordell @mmv08 do you guys know if that's correct in the 1.4.1 context?

@nlordell
Copy link
Contributor

nlordell commented May 4, 2024

I think this refers to which flavour of Safe contract is expected.

For Ethereum mainnet, the Safe contract is used, and for other networks, the SafeL2 contract is used. The difference between the two are additional events with a lot of data that get emitted in the SafeL2 version. These events allow the Safe transaction service to index Safe account transactions using only events and standard Eth RPC methods. However, this has a non negligible gas overhead which is why this is not used on Ethereum mainnet, which instead relies on more complicated and non-standard tracing techniques to index Safe transactions. These indexing techniques are not generally available on all networks which is why the transaction service can't just rely on tracing everywhere.

Hope this helps explain a bit.

@katspaugh
Copy link
Member

katspaugh commented May 4, 2024

Thanks for the detailed response @nlordell! 🙏

I’ll close this as won’t-fix then, since the UI is correct to warn about the L1 mastercopy being used for BNB.

@katspaugh katspaugh closed this as not planned Won't fix, can't repro, duplicate, stale May 4, 2024
@JOY
Copy link
Author

JOY commented May 4, 2024

Thanks for the detailed response @nlordell! 🙏

I’ll close this as won’t-fix then, since the UI is correct to warn about the L1 mastercopy being used for BNB.

Hi,

I tried with SafeL2 and it works for BNB and even ETH. However, the UI shows the warning and even unsupported contract for Arbitrum (using SafeL2).
Wallet address: 0x7c74e813daEE4c9b475cFFC6352FD79616f81AA1

image

image

@katspaugh
Copy link
Member

Thanks, we’ll check.

@katspaugh katspaugh reopened this May 4, 2024
@huckym
Copy link

huckym commented May 28, 2024

I can confirm the same issue when using 1.4.1+L2 w/ 4337 modules. web-ui loads the safe on sepolia, polygon and gnosis, fine. But complains about unsupported base contract for optimism and arbitrum. The mobile wallet app on ios doesn't seem to load any safes w/ 1.4.1

I am using permissionless.js to create the safes.

@nlordell
Copy link
Contributor

nlordell commented May 29, 2024

Hmm, I wonder if in this case it is an issue where the Safe v1.4.1 addresses aren't correctly configured in either the CGW or in the web interface for those chains?

@katspaugh
Copy link
Member

katspaugh commented May 29, 2024

Looks like it was fixed, https://app.safe.global/settings/setup?safe=arb1:0x7c74e813daEE4c9b475cFFC6352FD79616f81AA1 is shown as 1.4.1+L2 ✅ and there's no warning.

Edit: I was looking at the wrong Safe

@huckym
Copy link

huckym commented May 29, 2024

Looks like it was fixed, https://app.safe.global/settings/setup?safe=arb:0x7c74e813daEE4c9b475cFFC6352FD79616f81AA1 is shown as 1.4.1+L2 ✅ and there's no warning.

I'm surprised it is working for you. I just tried the safe you pointed to on both safari and chrome browsers on my macbook and both pop-up the warning

@katspaugh
Copy link
Member

My bad, I initially pasted the wrong link. I do see the warning in your Safe.

@huckym
Copy link

huckym commented May 29, 2024

Hmm, I wonder if in this case it is an issue where the Safe v1.4.1 addresses aren't correctly configured in either the CGW or in the web interface for those chains?

I am not sure. One other thought was if this asset was being read by the wallet to determine supported contracts on different chain. 0.3.0 only lists contracts for sepolia while 0.2.0 lists the entire bunch.
https://github.com/safe-global/safe-modules-deployments/tree/main/src/assets/safe-4337-module/v0.3.0

However, that wouldn't explain why 1.4.1+L2 w/ 4337 is showing up fine for polygon.
Even for polygon safe, there is a note about the fallback handler stating the 4337 module is an "unofficial fallback handler" but other than that, it seems to be working fine

@katspaugh
Copy link
Member

It seems to be indeed something on CGW, it returns

"implementation": {
"value": "0x29fcB43b46531BcA003ddC8FCB67FFE91900C762",
"name": "SafeL2 1.4.1",
"logoUri": "https://safe-transaction-assets.safe.global/contracts/logos/0x29fcB43b46531BcA003ddC8FCB67FFE91900C762.png"
},
"implementationVersionState": "UNKNOWN",

See https://safe-client.safe.global/v1/chains/42161/safes/0x7c74e813daEE4c9b475cFFC6352FD79616f81AA1

@iamacook @hectorgomezv can you take this ticket to your tracker?

@huckym
Copy link

huckym commented May 29, 2024

Here's my config for the safe;

{
        entryPoint: ENTRYPOINT_ADDRESS_V07,
        safeVersion: "1.4.1",
        saltNonce: 0n,
        addModuleLibAddress: "0x2dd68b007B46fBe91B9A7c3EDa5A7a1063cB5b47",
        safe4337ModuleAddress: "0x75cf11467937ce3F2f357CE24ffc3DBF8fD5c226",
        safeProxyFactoryAddress: "0x4e1DCf7AD4e460CfD30791CCC4F9c8a4f820ec67",
        safeSingletonAddress: "0x29fcB43b46531BcA003ddC8FCB67FFE91900C762",
        multiSendAddress: "0x38869bf66a61cF6bDB996A6aE40D5853Fd43B526",
        multiSendCallOnlyAddress: "0x9641d764fc13c8B624c04430C7356C1C7C8102e2",
        // setupTransactions: [paymasterApproval],
      }

@iamacook
Copy link
Member

It seems to be indeed something on CGW, it returns

"implementation": {
"value": "0x29fcB43b46531BcA003ddC8FCB67FFE91900C762",
"name": "SafeL2 1.4.1",
"logoUri": "https://safe-transaction-assets.safe.global/contracts/logos/0x29fcB43b46531BcA003ddC8FCB67FFE91900C762.png"
},
"implementationVersionState": "UNKNOWN",

See https://safe-client.safe.global/v1/chains/42161/safes/0x7c74e813daEE4c9b475cFFC6352FD79616f81AA1

@iamacook @hectorgomezv can you take this ticket to your tracker?

The Transaction Service does not have 0x29fcB43b46531BcA003ddC8FCB67FFE91900C762 listed as a supported singleton on Arbitrum meaning we return UNKNOWN.

If it is an official singleton (and I assume it is as it's listed on mainnet and the Safe was created via the CLI), it needs to be added to the Transaction Service to resolve this issue. cc @safe-global/core-api

@Uxio0
Copy link
Member

Uxio0 commented May 30, 2024

@katspaugh
Copy link
Member

katspaugh commented May 30, 2024

@JOY please feel free to open an issue in the CLI as per the link above. Thanks everyone.

@katspaugh katspaugh closed this as not planned Won't fix, can't repro, duplicate, stale May 30, 2024
@katspaugh
Copy link
Member

@JOY nvm, Uxio went ahead and added the deployment.

I see that https://app.safe.global/settings/setup?safe=arb1:0x7c74e813daEE4c9b475cFFC6352FD79616f81AA1 is now operational. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants