safe-global / safe-smart-account

Safe allows secure management of blockchain assets.
https://safe.global
GNU Lesser General Public License v3.0
1.84k stars 907 forks source link

Technical background for `onlyNonceZero` in `SafeToL2Migration`? #720

Closed pfedan closed 9 months ago

pfedan commented 9 months ago

I'd like to understand the background why a migration cannot be done for Safes that have a nonce other than zero. Is it really necessary?

https://github.com/safe-global/safe-contracts/blob/f03dfae65fd1d085224b00a10755c509a4eaacfe/contracts/libraries/SafeToL2Migration.sol#L74-L78

pfedan commented 9 months ago

I have found #685 and it is stated:

Only allow Safes with nonce=0, so they are unitialized and the L2 indexer is not missing some configuration changes going on between the update to L2

... but when there have not been any configuration changes, it should be allowed to exchange the singleton, IMHO.

pfedan commented 9 months ago

tagging @Uxio0

Uxio0 commented 9 months ago

... but when there have not been any configuration changes, it should be allowed to exchange the singleton, IMHO.

There's no way for the indexer to know if there have been configuration changes or not as Safes cannot be indexed without events, that's the reasoning. We are very strict in the correctness on the data that we index and give the users, and that's why we cannot fall into asumptions.

pfedan commented 9 months ago

There's no way for the indexer to know if there have been configuration changes or not as Safes cannot be indexed without events, that's the reasoning. We are very strict in the correctness on the data that we index and give the users, and that's why we cannot fall into asumptions.

Ah got it - thanks.

In my case, I hacked me through the safe tool-chain in order to fix my L2 safe with a nonce other than 0 (made one ERC20 transfer before).

Here's my journey (PSA: do not try this at home without knowing what you do)

I had to manipulate both the safe-cli and the SafeToL2Migration contract to omit the checks for nonce === 0, then deploy the contract and execute the modified update_version_to_l2 command in the safe-cli with my modified migration contract.

Uxio0 commented 9 months ago

We cannot guarantee them that your Safe will work propertly with our services/UI, but it's good as long as you understand that being an advanced user 😉

pfedan commented 9 months ago

Sent a couple wei back and forth and it seemed fine. The only thing is that the TX nonces look a bit off in the UI - but I don't care :grin:

image