Skip to content

bedrock: Make portal upgradeable#2825

Merged
tynes merged 12 commits intodevelopfrom
m/upgradeable-portal
Jun 27, 2022
Merged

bedrock: Make portal upgradeable#2825
tynes merged 12 commits intodevelopfrom
m/upgradeable-portal

Conversation

@maurelian
Copy link
Copy Markdown
Contributor

@maurelian maurelian commented Jun 20, 2022

Description

Adds upgradability to the OptimismPortal contract, and makes its parent contract ResourceMetering amenable to upgradeability.

Additional context

  1. Used OZ's Initializable code and pattern.
  2. ResourceMetering doesn't inherit from Initializable itself, but the contract has been made abstract, and the constructor was converted to an internal function called __ResourceMetering_init. This makes it easier to use in 3rd party contracts which are not upgradeable.
  3. Because the Portal's constructor calls initialize(). The contract's implementation still works without being behind a proxy. This is nice because it means we don't need to add a proxy to unit tests, but integration tests should be updated to deploy behind a proxy.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jun 20, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 4007f2c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/contracts-bedrock Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mergify mergify bot added the conflict label Jun 20, 2022
@maurelian maurelian force-pushed the m/upgradeable-portal branch from 21942e4 to a5008f8 Compare June 20, 2022 19:06
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 20, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@mergify mergify bot removed the conflict label Jun 20, 2022
@maurelian maurelian force-pushed the m/upgradeable-portal branch from a5008f8 to 86a4257 Compare June 20, 2022 19:07
@maurelian maurelian mentioned this pull request Jun 20, 2022
Merged
tynes
tynes reviewed Jun 21, 2022
View reviewed changes
@maurelian maurelian marked this pull request as draft June 22, 2022 16:51
@maurelian
Copy link
Copy Markdown
Contributor Author

maurelian commented Jun 22, 2022

Made into a draft. Will combine all upgradability changes into a single PR.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 22, 2022

Hey @maurelian! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Jun 22, 2022
@maurelian maurelian force-pushed the m/upgradeable-portal branch from 86a4257 to bd9ed34 Compare June 23, 2022 02:51
@mergify mergify bot removed the conflict label Jun 23, 2022
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 23, 2022

Hey @maurelian! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Jun 23, 2022
tynes
tynes reviewed Jun 23, 2022
View reviewed changes
@maurelian maurelian force-pushed the m/upgradeable-portal branch from bd9ed34 to 11f0f8c Compare June 23, 2022 13:16
@mergify mergify bot removed the conflict label Jun 23, 2022
@maurelian maurelian marked this pull request as ready for review June 23, 2022 13:45
tynes
tynes reviewed Jun 23, 2022
View reviewed changes
tynes
tynes reviewed Jun 23, 2022
View reviewed changes
@tynes
Copy link
Copy Markdown
Contributor

tynes commented Jun 23, 2022

Could also use an update to the hh deploy script to call updateToAndCall where initialize() is called

@tynes
Copy link
Copy Markdown
Contributor

tynes commented Jun 23, 2022

Perhaps a rebase will trigger CI

@maurelian maurelian force-pushed the m/upgradeable-portal branch from 11f0f8c to 3d6518d Compare June 23, 2022 20:34
@maurelian
Copy link
Copy Markdown
Contributor Author

maurelian commented Jun 23, 2022

Could also use an update to the hh deploy script to call updateToAndCall where initialize() is called

Already had that locally, just pushed :)

tynes
tynes reviewed Jun 23, 2022
View reviewed changes
@maurelian maurelian force-pushed the m/upgradeable-portal branch from 3d6518d to b5f22e6 Compare June 23, 2022 23:09
@maurelian maurelian force-pushed the m/upgradeable-portal branch from c98ee00 to e9c39c2 Compare June 24, 2022 01:26
@mattsse mattsse mentioned this pull request Jun 24, 2022
Closed
2 tasks
Nicca42
Nicca42 reviewed Jun 24, 2022
View reviewed changes
Nicca42
Nicca42 suggested changes Jun 24, 2022
View reviewed changes
@maurelian maurelian force-pushed the m/upgradeable-portal branch from 42c0aef to c08af18 Compare June 24, 2022 19:17
@maurelian maurelian requested a review from Nicca42 June 24, 2022 19:18
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 27, 2022

Hey @maurelian! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Jun 27, 2022
@tynes
Copy link
Copy Markdown
Contributor

tynes commented Jun 27, 2022

This is almost ready to be merged, looks like it just has a merge conflict now

@maurelian maurelian force-pushed the m/upgradeable-portal branch from c08af18 to 1b28918 Compare June 27, 2022 17:44
@mergify mergify bot removed the conflict label Jun 27, 2022
maurelian added 12 commits June 27, 2022 14:34
@maurelian
bedrock: Make portal upgradeable
ea40c66
@maurelian
bedrock: rebuild bindings and snapshot
56f9784
@maurelian
bedrock: update portal storage layout
00107ef
@maurelian
bedrock: Test oracle behind proxy
bc7894c 
--wip--
@maurelian
bedrock: Test portal behind a proxy
8556bc8
@maurelian
bedrock: Set l2Sender in initialize
0a9485c
@maurelian
bedrock: workarounds for forge test overflow bug
e91294b
@maurelian
bedrock: reorder inheritance in portal
9a2a277 
This shifts the order of inheritance so that the initializable
values are in the first slot of the portal, which is consistent with
all other contracts we have that inherit from initializable
@maurelian
Revert "bedrock: workarounds for forge test overflow bug"
c574f27 
Forge seems to be working again!?

This reverts commit bb8cdf2.
@maurelian
bedrock: Make ResourceMetering non-abstract
9d6bbaa
@maurelian
bedrock: Add test for storage access post upgrade
5d927b4
@maurelian
chore: update gas snapshot and bindings
4007f2c
@maurelian maurelian force-pushed the m/upgradeable-portal branch from 1b28918 to 4007f2c Compare June 27, 2022 18:34
@tynes tynes merged commit ef29d8a into develop Jun 27, 2022
@tynes tynes deleted the m/upgradeable-portal branch June 27, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tynes tynes tynes approved these changes

@Nicca42 Nicca42 Awaiting requested review from Nicca42

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maurelian @tynes @Nicca42