Skip to content

Commit

Permalink
fix: don't check governance on uninitialized vault (#245)
Browse files Browse the repository at this point in the history
* fix: no need to assert governance check of uninitialized vault

NOTE: Fix missed in #193

* refactor: merge `_registerRelease` as it's not used more than once

* refactor: move `_registerVault` to right before it's used first

* test: update test that should now work with this change
  • Loading branch information
fubuloubu authored Mar 15, 2021
1 parent d4902df commit e390c2a
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 46 deletions.
80 changes: 37 additions & 43 deletions contracts/Registry.vy
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,22 @@ def latestVault(token: address) -> address:
return self.vaults[token][self.numVaults[token] - 1] # dev: no vault for token


@internal
def _registerRelease(vault: address):
@external
def newRelease(vault: address):
"""
@notice
Add a previously deployed Vault as the template contract for the latest release,
to be used by further "forwarder-style" delegatecall proxy contracts that can be
deployed from the registry throw other methods (to save gas).
@dev
Throws if caller isn't `self.governance`.
Throws if `vault`'s governance isn't `self.governance`.
Throws if the api version is the same as the previous release.
Emits a `NewVault` event.
@param vault The vault that will be used as the template contract for the next release.
"""
assert msg.sender == self.governance # dev: unauthorized

# Check if the release is different from the current one
# NOTE: This doesn't check for strict semver-style linearly increasing release versions
release_id: uint256 = self.numReleases # Next id in series
Expand All @@ -135,6 +149,26 @@ def _registerRelease(vault: address):
log NewRelease(release_id, vault, Vault(vault).apiVersion())


@internal
def _newProxyVault(
token: address,
governance: address,
rewards: address,
guardian: address,
name: String[64],
symbol: String[32],
releaseTarget: uint256,
) -> address:
release: address = self.releases[releaseTarget]
assert release != ZERO_ADDRESS # dev: unknown release
vault: address = create_forwarder_to(release)

# NOTE: Must initialize the Vault atomically with deploying it
Vault(vault).initialize(token, governance, rewards, name, symbol, guardian)

return vault


@internal
def _registerVault(token: address, vault: address):
# Check if there is an existing deployment for this token at the particular api version
Expand All @@ -156,51 +190,11 @@ def _registerVault(token: address, vault: address):
self.isRegistered[token] = True
self.tokens[self.numTokens] = token
self.numTokens += 1

# Log the deployment for external listeners (e.g. Graph)
log NewVault(token, vault_id, vault, Vault(vault).apiVersion())


@external
def newRelease(vault: address):
"""
@notice
Add a previously deployed Vault as the template contract for the latest release,
to be used by further "forwarder-style" delegatecall proxy contracts that can be
deployed from the registry throw other methods (to save gas).
@dev
Throws if caller isn't `self.governance`.
Throws if `vault`'s governance isn't `self.governance`.
Throws if the api version is the same as the previous release.
Emits a `NewVault` event.
@param vault The vault that will be used as the template contract for the next release.
"""
assert msg.sender == self.governance # dev: unauthorized
assert Vault(vault).governance() == msg.sender # dev: not governed

self._registerRelease(vault)


@internal
def _newProxyVault(
token: address,
governance: address,
rewards: address,
guardian: address,
name: String[64],
symbol: String[32],
releaseTarget: uint256,
) -> address:
release: address = self.releases[releaseTarget]
assert release != ZERO_ADDRESS # dev: unknown release
vault: address = create_forwarder_to(release)

# NOTE: Must initialize the Vault atomically with deploying it
Vault(vault).initialize(token, governance, rewards, name, symbol, guardian)

return vault


@external
def newVault(
token: address,
Expand Down
5 changes: 2 additions & 3 deletions tests/functional/registry/test_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ def test_release_management(gov, registry, create_vault, rando):
with brownie.reverts():
registry.endorseVault(v1_vault)

# Check that newRelease raises if vault governance is a rando
# Check that newRelease works even if vault governance is not gov
bad_vault = create_vault(governance=rando)
with brownie.reverts():
registry.newRelease(bad_vault, {"from": gov})
registry.newRelease(bad_vault, {"from": gov})

0 comments on commit e390c2a

Please sign in to comment.