-
Notifications
You must be signed in to change notification settings - Fork 86
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
2436 deployment scripts for lightclientarbitrum upgrade scripts lightclient #2449
2436 deployment scripts for lightclientarbitrum upgrade scripts lightclient #2449
Conversation
… the updateStateHistory from Lightclient during the initialization step as precompiles cannot be called during a forge script deployment
…rum-upgrade-scripts-lightclient
…mpiles cannot be called during a forge script deploy
(LC.LightClientState memory state, LC.StakeTableState memory stakeState) = | ||
abi.decode(result, (LC.LightClientState, LC.StakeTableState)); | ||
|
||
proxyAddress = Upgrades.deployUUPSProxy( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this a proxy address? why cant we just deploy an implementation contract and then upgrade our existing proxy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opted for a deploy instead of an upgrade whilst dealing with the precompile issue. once this quick review is fine, i can continue with the upgrade steps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created a ticket for the upgrade script as the safe transaction service is not supported in arbitrum sepolia. this service is needed for proposing transactions to the safe multisig wallet, created another ticket #2457
@@ -9,6 +9,11 @@ interface ArbSys { | |||
} | |||
|
|||
contract LightClientArbitrum is LightClient { | |||
/// @custom:oz-upgrades-unsafe-allow constructor | |||
constructor() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to do this? it should inherit the constructor from LightClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's right, this code lingered here because i originally planned to override the initialize function and wanted to be explicit. will remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…rum-upgrade-scripts-lightclient
Closes #2436
This PR:
initialize()
method onLightClient.sol
, it no longer updates the state history upon initialization as precompiles can't be called when deploying to networksThis PR does not:
Key places to review: