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

Technical background for onlyNonceZero in SafeToL2Migration? #720

Closed
pfedan opened this issue Dec 22, 2023 · 6 comments
Closed

Technical background for onlyNonceZero in SafeToL2Migration? #720

pfedan opened this issue Dec 22, 2023 · 6 comments

Comments

@pfedan
Copy link

pfedan commented Dec 22, 2023

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 pfedan changed the title Technical background for onlyNonceZero? Technical background for onlyNonceZero in SafeToL2Migration? Dec 22, 2023
@pfedan
Copy link
Author

pfedan commented Dec 22, 2023

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
Copy link
Author

pfedan commented Dec 22, 2023

tagging @Uxio0

@Uxio0
Copy link
Member

Uxio0 commented Dec 22, 2023

... 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
Copy link
Author

pfedan commented Dec 22, 2023

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.

@pfedan pfedan closed this as completed Dec 22, 2023
@Uxio0
Copy link
Member

Uxio0 commented Dec 22, 2023

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
Copy link
Author

pfedan commented Dec 22, 2023

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 😁

image

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

2 participants