Add a variant of the Vesting wallet for updating the beneficiary #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ernestognw
reviewed
May 14, 2024
frangio
reviewed
May 14, 2024
Co-authored-by: Ernesto García <[email protected]>
frangio
approved these changes
May 15, 2024
|
I suggest using a Tenderly simulation of the multisig transaction before executing it. It should show the beneficiary storage slot as the only change. |
|
The operation will have to go through a multisig. I'll suggest this is done through defender (which includes tenderly support?) |
ernestognw
approved these changes
May 15, 2024
|
|
||
| function upgradeTo(address newImplementation) external { | ||
| require(msg.sender == _owner); | ||
| StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; |
There was a problem hiding this comment.
Without specifying, it looks weird that we're checking ownership but not emitting the event.
Suggested change
| StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; | |
| StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; | |
| // Upgraded event is not emitted given this function is only used by UUPS's rollback test. |
It's our own simulation support. If it goes through a Safe multisig it can be simulated through Tenderly as well |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces forta-network#264
The goal is to perform this on an instance that is currently using
VestingWallet(v0). Still, this solution is designed to work with all existing versions of the VestingWallet (as demonstrated by the tests)