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.

Sign up for GitHub

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

Jump to bottom

Add a beacon proxy contract #2411

Merged
merged 33 commits into from
Nov 26, 2020
Merged

Add a beacon proxy contract #2411

merged 33 commits into from
Nov 26, 2020

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Nov 20, 2020 •
edited
Loading

This PR adds a beacon proxy inspired by Dharma's. The code was initially developed by @f8n in https://github.com/f8n/openzeppelin-sdk/pull/6. I've changed a few things on top of that code:

  • Renamed contracts and rewrote revert reasons to conform to our conventions.
  • Edited docs and wrote what was missing.
  • Used Address.functionDelegateCall instead of inline assembly.
  • Adapted tests to run in our test suite.

@frangio
import beacon proxy files from f8n/openzeppelin-sdk
c6f7a74
@frangio
rename UpgradeabilityBeacon -> BeaconUpgradeable
dc003f8
@frangio
get contracts to compile
47d93a4
@frangio
edit tests
3ecf638
@frangio
refactor tests minimally to pass
e7ef88c
@frangio
use Address.functionDelegateCall
33c4257
@frangio
use 4 space indentation
00e3d30
@frangio
inline behavior into test file
d0a9b7c
@frangio
adapt reason strings to convention and test for them
68b56e0
@frangio
Merge branch 'master' into beacon-proxy
1124059
@frangio
widen pragma for 0.7
86d40ec
@frangio
rename clashing variable
edbad75
@frangio
add documentation or edit to conform to our conventions
cbbdcd3
@frangio
lint
f40ffc4
@frangio
fix UpgradeableProxy tests
12b4314
@frangio
fix ProxyAdmin tests
fe8037b
@frangio
fix tests under solidity-coverage (ganache)
2c0b0f4
@frangio
rename BeaconUpgradeableProxy -> BeaconProxy, Beacon -> UpgradeableBe…
3e821fe 
…acon
@frangio
fix tests under solidity-coverage (ganache)
3da41f5
@frangio frangio marked this pull request as ready for review November 20, 2020 20:18
spalladino
spalladino reviewed Nov 24, 2020
View reviewed changes
Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @frangio! I added a few comments only.

contracts/proxy/README.adoc Outdated Show resolved Hide resolved
contracts/proxy/UpgradeableBeacon.sol Outdated Show resolved Hide resolved
test/proxy/BeaconProxy.test.js Outdated Show resolved Hide resolved
test/proxy/BeaconProxy.test.js Outdated Show resolved Hide resolved
spalladino
spalladino approved these changes Nov 25, 2020
View reviewed changes
Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work with the test suite, looks much better now!

@frangio frangio merged commit 951e946 into OpenZeppelin:master Nov 26, 2020
@frangio frangio deleted the beacon-proxy branch November 26, 2020 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spalladino spalladino spalladino approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@frangio @spalladino