Merge registry with factory + move transferOwnership to registry
#81
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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
PaulRBerg
force-pushed
the
refactor/registry
branch
2 times, most recently
from
05cca67 to
c2fe949
Compare
build: switch to "forge-std" fork docs: update NatSpec comments feat: add "transientProxyOwner" in registry refactor: load owner from registry in proxy constructor refactor: rename "deployer" to "operator" in "DeployProxy" event refactor: update scripts to match latest contract API refactor: transfer ownership via registry test: call "setUp" in proxy tests test: delete "deployProxy" helper function test: fuzz addresses in proxy "transferOwnership" test: fuzz origin, operator, and owner test: name all fuzz tests as "testFuzz_*" test: rename "deployer" to "origin" in "computeProxyAddrss" test: test already existing proxy in all deploy functions test: test registry "transferOwnership" test: update tests to match latest contract API test: use named args for "changePrank"
PaulRBerg
force-pushed
the
refactor/registry
branch
from
c2fe949 to
fff7a2c
Compare
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.
Description
As discussed in #79, and alluded to in #80, this PR merges the registry with the factory, and moves the
transferOwnershipfunction to the registry so that the proxies mapping can get updated when ownership is transferred.This PR addresses the concern raised by @razgraf regarding the soundness of the
currentProxiesmapping. Now, the proxy returned bygetProxy(the successor ofgetCurrentProxy) is definitely a proxy owned byowner.But this PR solves the problem in a non-disruptive way, i.e. it will still be possible to transfer ownership (via the registry).
Deployment Cost
The deployment cost has lowered a bit (by 12k gas).
In
v4.0.0-beta.1, the cost was ~528,529, whereas with the version in this PR, it costs ~516,709.