From e390c2a6b2ba6e2ecc8f3a72a1ea4adfabd17544 Mon Sep 17 00:00:00 2001
From: Just some guy <3859395+fubuloubu@users.noreply.github.com>
Date: Mon, 15 Mar 2021 16:06:23 -0400
Subject: [PATCH] fix: don't check governance on uninitialized vault (#245)

* fix: no need to assert governance check of uninitialized vault

NOTE: Fix missed in https://github.com/yearn/yearn-vaults/pull/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
---
 contracts/Registry.vy                     | 80 +++++++++++------------
 tests/functional/registry/test_release.py |  5 +-
 2 files changed, 39 insertions(+), 46 deletions(-)

diff --git a/contracts/Registry.vy b/contracts/Registry.vy
index 27eb7111..de2cfb22 100644
--- a/contracts/Registry.vy
+++ b/contracts/Registry.vy
@@ -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
@@ -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
@@ -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,
diff --git a/tests/functional/registry/test_release.py b/tests/functional/registry/test_release.py
index 3a4336ec..47470f49 100644
--- a/tests/functional/registry/test_release.py
+++ b/tests/functional/registry/test_release.py
@@ -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})