From f0ef08dae28dd464fad51b259d40b0cbd36cb597 Mon Sep 17 00:00:00 2001 From: x48 Date: Tue, 9 Mar 2021 16:17:18 -0600 Subject: [PATCH 1/4] feat(registry): add ability to track endorsed vault tokens --- contracts/Registry.vy | 18 +++++ tests/functional/registry/test_deployment.py | 78 ++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/contracts/Registry.vy b/contracts/Registry.vy index e31551df..845fa56a 100644 --- a/contracts/Registry.vy +++ b/contracts/Registry.vy @@ -23,6 +23,11 @@ releases: public(HashMap[uint256, address]) nextDeployment: public(HashMap[address, uint256]) vaults: public(HashMap[address, HashMap[uint256, address]]) +# Maintain a list of endorsed vault tokens +tokensList: public(address[65536]) +tokensMap: public(HashMap[address, bool]) +tokensCount: public(uint256) + # 2-phase commit governance: public(address) pendingGovernance: public(address) @@ -128,6 +133,16 @@ def _registerRelease(vault: address): log NewRelease(release_id, vault, Vault(vault).apiVersion()) +@internal +def _registerToken(token: address): + tokenExists: bool = self.tokensMap[token] + if tokenExists: + return + self.tokensList[self.tokensCount] = token + self.tokensMap[token] = True + self.tokensCount = self.tokensCount + 1 + + @internal def _registerDeployment(token: address, vault: address): # Check if there is an existing deployment for this token at the particular api version @@ -144,6 +159,9 @@ def _registerDeployment(token: address, vault: address): self.vaults[token][deployment_id] = vault self.nextDeployment[token] = deployment_id + 1 + # Register tokens for endorsed vaults + self._registerToken(token) + # Log the deployment for external listeners (e.g. Graph) log NewVault(token, deployment_id, vault, Vault(vault).apiVersion()) diff --git a/tests/functional/registry/test_deployment.py b/tests/functional/registry/test_deployment.py index d011b048..468f7dd2 100644 --- a/tests/functional/registry/test_deployment.py +++ b/tests/functional/registry/test_deployment.py @@ -1,4 +1,82 @@ import brownie +from brownie import ZERO_ADDRESS + + +def test_endorsed_vault_token_tracking( + gov, guardian, rewards, registry, Vault, create_token, create_vault, rando +): + # Create a token and vault + token_1 = create_token() + vault_1 = create_vault(token_1, version="1.0.0") + assert registry.nextRelease() == 0 # Make sure no releases have been deployed + + # Token tracking state variables should start off uninitialized + assert registry.tokensList(0) == ZERO_ADDRESS + assert registry.tokensMap(token_1) == False + assert registry.tokensCount() == 0 + + # Endorsing a vault registers a vault token + registry.newRelease(vault_1, {"from": gov}) + registry.endorseVault(vault_1, {"from": gov}) + assert registry.nextRelease() == 1 # Make sure the release was deployed + assert registry.latestVault(token_1) == vault_1 + assert registry.tokensList(0) == token_1 + assert registry.tokensList(1) == ZERO_ADDRESS + assert registry.tokensMap(token_1) == True + assert registry.tokensCount() == 1 + + # Create a new release using the same token + vault_2 = create_vault(token_1, version="2.0.0") + registry.newRelease(vault_2, {"from": gov}) + assert registry.latestVault(token_1) == vault_1 + assert registry.nextRelease() == 2 # Make sure the release was deployed + + # Endorsing a vault with the same token bumps the "latestVault" associated with the token + registry.endorseVault(vault_2, {"from": gov}) + assert registry.latestVault(token_1) == vault_2 + assert registry.latestRelease() == vault_2.apiVersion() == "2.0.0" + + # Tokens can only be registered one time (no duplicates) + assert registry.tokensList(0) == token_1 + assert registry.tokensList(1) == ZERO_ADDRESS + assert registry.tokensCount() == 1 + + # Create a new endorsed vault with a new token + token_2 = create_token() + registry.newVault(token_2, guardian, rewards, "", "", {"from": gov}) + + # New endorsed vaults should register tokens + assert registry.tokensList(0) == token_1 + assert registry.tokensList(1) == token_2 + assert registry.tokensList(2) == ZERO_ADDRESS + assert registry.tokensMap(token_1) == True + assert registry.tokensMap(token_2) == True + assert registry.tokensCount() == 2 + + # Create a new experimental vault with a new token + token_3 = create_token() + vault_3 = registry.newExperimentalVault( + token_3, gov, guardian, rewards, "", "", {"from": gov} + ).return_value + + # New experimental (unendorsed) vaults should not register tokens + assert registry.tokensList(0) == token_1 + assert registry.tokensList(1) == token_2 + assert registry.tokensList(2) == ZERO_ADDRESS + assert registry.tokensMap(token_1) == True + assert registry.tokensMap(token_2) == True + assert registry.tokensCount() == 2 + + # Endorsing a vault should register a token + registry.endorseVault(vault_3) + assert registry.tokensList(0) == token_1 + assert registry.tokensList(1) == token_2 + assert registry.tokensList(2) == token_3 + assert registry.tokensList(3) == ZERO_ADDRESS + assert registry.tokensMap(token_1) == True + assert registry.tokensMap(token_2) == True + assert registry.tokensMap(token_3) == True + assert registry.tokensCount() == 3 def test_deployment_management( From 7a259e955e19ed66f225793223c2e9f988f95f5e Mon Sep 17 00:00:00 2001 From: x48 Date: Wed, 10 Mar 2021 09:29:49 -0600 Subject: [PATCH 2/4] refactor(registry): make token tracking more gas efficient --- contracts/Registry.vy | 22 ++++--- tests/functional/registry/test_deployment.py | 60 ++++++++++---------- 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/contracts/Registry.vy b/contracts/Registry.vy index 845fa56a..0fc80831 100644 --- a/contracts/Registry.vy +++ b/contracts/Registry.vy @@ -23,10 +23,14 @@ releases: public(HashMap[uint256, address]) nextDeployment: public(HashMap[address, uint256]) vaults: public(HashMap[address, HashMap[uint256, address]]) -# Maintain a list of endorsed vault tokens -tokensList: public(address[65536]) -tokensMap: public(HashMap[address, bool]) -tokensCount: public(uint256) +# Index of token added => token address +tokens: public(HashMap[uint256, address]) + +# len(tokens) +numTokens: public(uint256) + +# Inclusion check for token +isRegistered: public(HashMap[address, bool]) # 2-phase commit governance: public(address) @@ -135,12 +139,12 @@ def _registerRelease(vault: address): @internal def _registerToken(token: address): - tokenExists: bool = self.tokensMap[token] - if tokenExists: + tokenIsRegistered: bool = self.isRegistered[token] + if tokenIsRegistered: return - self.tokensList[self.tokensCount] = token - self.tokensMap[token] = True - self.tokensCount = self.tokensCount + 1 + self.tokens[self.numTokens] = token + self.isRegistered[token] = True + self.numTokens = self.numTokens + 1 @internal diff --git a/tests/functional/registry/test_deployment.py b/tests/functional/registry/test_deployment.py index 468f7dd2..75795094 100644 --- a/tests/functional/registry/test_deployment.py +++ b/tests/functional/registry/test_deployment.py @@ -11,19 +11,19 @@ def test_endorsed_vault_token_tracking( assert registry.nextRelease() == 0 # Make sure no releases have been deployed # Token tracking state variables should start off uninitialized - assert registry.tokensList(0) == ZERO_ADDRESS - assert registry.tokensMap(token_1) == False - assert registry.tokensCount() == 0 + assert registry.tokens(0) == ZERO_ADDRESS + assert registry.isRegistered(token_1) == False + assert registry.numTokens() == 0 # Endorsing a vault registers a vault token registry.newRelease(vault_1, {"from": gov}) registry.endorseVault(vault_1, {"from": gov}) assert registry.nextRelease() == 1 # Make sure the release was deployed assert registry.latestVault(token_1) == vault_1 - assert registry.tokensList(0) == token_1 - assert registry.tokensList(1) == ZERO_ADDRESS - assert registry.tokensMap(token_1) == True - assert registry.tokensCount() == 1 + assert registry.tokens(0) == token_1 + assert registry.tokens(1) == ZERO_ADDRESS + assert registry.isRegistered(token_1) == True + assert registry.numTokens() == 1 # Create a new release using the same token vault_2 = create_vault(token_1, version="2.0.0") @@ -37,21 +37,21 @@ def test_endorsed_vault_token_tracking( assert registry.latestRelease() == vault_2.apiVersion() == "2.0.0" # Tokens can only be registered one time (no duplicates) - assert registry.tokensList(0) == token_1 - assert registry.tokensList(1) == ZERO_ADDRESS - assert registry.tokensCount() == 1 + assert registry.tokens(0) == token_1 + assert registry.tokens(1) == ZERO_ADDRESS + assert registry.numTokens() == 1 # Create a new endorsed vault with a new token token_2 = create_token() registry.newVault(token_2, guardian, rewards, "", "", {"from": gov}) # New endorsed vaults should register tokens - assert registry.tokensList(0) == token_1 - assert registry.tokensList(1) == token_2 - assert registry.tokensList(2) == ZERO_ADDRESS - assert registry.tokensMap(token_1) == True - assert registry.tokensMap(token_2) == True - assert registry.tokensCount() == 2 + assert registry.tokens(0) == token_1 + assert registry.tokens(1) == token_2 + assert registry.tokens(2) == ZERO_ADDRESS + assert registry.isRegistered(token_1) == True + assert registry.isRegistered(token_2) == True + assert registry.numTokens() == 2 # Create a new experimental vault with a new token token_3 = create_token() @@ -60,23 +60,23 @@ def test_endorsed_vault_token_tracking( ).return_value # New experimental (unendorsed) vaults should not register tokens - assert registry.tokensList(0) == token_1 - assert registry.tokensList(1) == token_2 - assert registry.tokensList(2) == ZERO_ADDRESS - assert registry.tokensMap(token_1) == True - assert registry.tokensMap(token_2) == True - assert registry.tokensCount() == 2 + assert registry.tokens(0) == token_1 + assert registry.tokens(1) == token_2 + assert registry.tokens(2) == ZERO_ADDRESS + assert registry.isRegistered(token_1) == True + assert registry.isRegistered(token_2) == True + assert registry.numTokens() == 2 # Endorsing a vault should register a token registry.endorseVault(vault_3) - assert registry.tokensList(0) == token_1 - assert registry.tokensList(1) == token_2 - assert registry.tokensList(2) == token_3 - assert registry.tokensList(3) == ZERO_ADDRESS - assert registry.tokensMap(token_1) == True - assert registry.tokensMap(token_2) == True - assert registry.tokensMap(token_3) == True - assert registry.tokensCount() == 3 + assert registry.tokens(0) == token_1 + assert registry.tokens(1) == token_2 + assert registry.tokens(2) == token_3 + assert registry.tokens(3) == ZERO_ADDRESS + assert registry.isRegistered(token_1) == True + assert registry.isRegistered(token_2) == True + assert registry.isRegistered(token_3) == True + assert registry.numTokens() == 3 def test_deployment_management( From a890b624cdd9b892dee940690ab8ec43c00c1701 Mon Sep 17 00:00:00 2001 From: x48 Date: Wed, 10 Mar 2021 20:08:06 -0600 Subject: [PATCH 3/4] refactor(registry): implement registry suggestions and refactor test --- contracts/Registry.vy | 15 +- tests/functional/registry/test_deployment.py | 137 +++++++------------ 2 files changed, 53 insertions(+), 99 deletions(-) diff --git a/contracts/Registry.vy b/contracts/Registry.vy index 0fc80831..2b32759b 100644 --- a/contracts/Registry.vy +++ b/contracts/Registry.vy @@ -137,16 +137,6 @@ def _registerRelease(vault: address): log NewRelease(release_id, vault, Vault(vault).apiVersion()) -@internal -def _registerToken(token: address): - tokenIsRegistered: bool = self.isRegistered[token] - if tokenIsRegistered: - return - self.tokens[self.numTokens] = token - self.isRegistered[token] = True - self.numTokens = self.numTokens + 1 - - @internal def _registerDeployment(token: address, vault: address): # Check if there is an existing deployment for this token at the particular api version @@ -164,7 +154,10 @@ def _registerDeployment(token: address, vault: address): self.nextDeployment[token] = deployment_id + 1 # Register tokens for endorsed vaults - self._registerToken(token) + if not self.isRegistered[token]: + self.isRegistered[token] = True + self.tokens[self.numTokens] = token + self.numTokens += 1 # Log the deployment for external listeners (e.g. Graph) log NewVault(token, deployment_id, vault, Vault(vault).apiVersion()) diff --git a/tests/functional/registry/test_deployment.py b/tests/functional/registry/test_deployment.py index 75795094..880ac1b3 100644 --- a/tests/functional/registry/test_deployment.py +++ b/tests/functional/registry/test_deployment.py @@ -2,129 +2,80 @@ from brownie import ZERO_ADDRESS -def test_endorsed_vault_token_tracking( +def test_deployment_management( gov, guardian, rewards, registry, Vault, create_token, create_vault, rando ): - # Create a token and vault - token_1 = create_token() - vault_1 = create_vault(token_1, version="1.0.0") - assert registry.nextRelease() == 0 # Make sure no releases have been deployed + v1_token = create_token() + + # No deployments yet for token + with brownie.reverts(): + registry.latestVault(v1_token) # Token tracking state variables should start off uninitialized assert registry.tokens(0) == ZERO_ADDRESS - assert registry.isRegistered(token_1) == False + assert not registry.isRegistered(v1_token) assert registry.numTokens() == 0 - # Endorsing a vault registers a vault token - registry.newRelease(vault_1, {"from": gov}) - registry.endorseVault(vault_1, {"from": gov}) - assert registry.nextRelease() == 1 # Make sure the release was deployed - assert registry.latestVault(token_1) == vault_1 - assert registry.tokens(0) == token_1 - assert registry.tokens(1) == ZERO_ADDRESS - assert registry.isRegistered(token_1) == True - assert registry.numTokens() == 1 - - # Create a new release using the same token - vault_2 = create_vault(token_1, version="2.0.0") - registry.newRelease(vault_2, {"from": gov}) - assert registry.latestVault(token_1) == vault_1 - assert registry.nextRelease() == 2 # Make sure the release was deployed - - # Endorsing a vault with the same token bumps the "latestVault" associated with the token - registry.endorseVault(vault_2, {"from": gov}) - assert registry.latestVault(token_1) == vault_2 - assert registry.latestRelease() == vault_2.apiVersion() == "2.0.0" - - # Tokens can only be registered one time (no duplicates) - assert registry.tokens(0) == token_1 - assert registry.tokens(1) == ZERO_ADDRESS - assert registry.numTokens() == 1 - - # Create a new endorsed vault with a new token - token_2 = create_token() - registry.newVault(token_2, guardian, rewards, "", "", {"from": gov}) - - # New endorsed vaults should register tokens - assert registry.tokens(0) == token_1 - assert registry.tokens(1) == token_2 - assert registry.tokens(2) == ZERO_ADDRESS - assert registry.isRegistered(token_1) == True - assert registry.isRegistered(token_2) == True - assert registry.numTokens() == 2 - - # Create a new experimental vault with a new token - token_3 = create_token() - vault_3 = registry.newExperimentalVault( - token_3, gov, guardian, rewards, "", "", {"from": gov} - ).return_value - - # New experimental (unendorsed) vaults should not register tokens - assert registry.tokens(0) == token_1 - assert registry.tokens(1) == token_2 - assert registry.tokens(2) == ZERO_ADDRESS - assert registry.isRegistered(token_1) == True - assert registry.isRegistered(token_2) == True - assert registry.numTokens() == 2 - - # Endorsing a vault should register a token - registry.endorseVault(vault_3) - assert registry.tokens(0) == token_1 - assert registry.tokens(1) == token_2 - assert registry.tokens(2) == token_3 - assert registry.tokens(3) == ZERO_ADDRESS - assert registry.isRegistered(token_1) == True - assert registry.isRegistered(token_2) == True - assert registry.isRegistered(token_3) == True - assert registry.numTokens() == 3 - - -def test_deployment_management( - gov, guardian, rewards, registry, Vault, create_token, create_vault, rando -): - token = create_token() - - # No deployments yet for token - with brownie.reverts(): - registry.latestVault(token) + # New release does not add new token + v1_vault = create_vault(v1_token, version="1.0.0") + registry.newRelease(v1_vault, {"from": gov}) + assert registry.tokens(0) == ZERO_ADDRESS + assert not registry.isRegistered(v1_token) + assert registry.numTokens() == 0 # Creating the first deployment makes `latestVault()` work - v1_vault = create_vault(token, version="1.0.0") - registry.newRelease(v1_vault, {"from": gov}) registry.endorseVault(v1_vault, {"from": gov}) - assert registry.latestVault(token) == v1_vault + assert registry.latestVault(v1_token) == v1_vault assert registry.latestRelease() == v1_vault.apiVersion() == "1.0.0" + # Endorsing a vault with a new token registers a new token + assert registry.tokens(0) == v1_token + assert registry.isRegistered(v1_token) + assert registry.numTokens() == 1 + # Can't deploy the same vault api version twice, proxy or not with brownie.reverts(): - registry.newVault(token, guardian, rewards, "", "", {"from": gov}) + registry.newVault(v1_token, guardian, rewards, "", "", {"from": gov}) # New release overrides previous release v2_vault = create_vault(version="2.0.0") # Uses different token registry.newRelease(v2_vault, {"from": gov}) - assert registry.latestVault(token) == v1_vault + assert registry.latestVault(v1_token) == v1_vault assert registry.latestRelease() == v2_vault.apiVersion() == "2.0.0" # You can deploy proxy Vaults, linked to the latest release + assert registry.numTokens() == 1 proxy_vault = Vault.at( - registry.newVault(token, guardian, rewards, "", "", {"from": gov}).return_value + registry.newVault( + v1_token, guardian, rewards, "", "", {"from": gov} + ).return_value ) assert proxy_vault.apiVersion() == v2_vault.apiVersion() == "2.0.0" assert proxy_vault.rewards() == rewards assert proxy_vault.guardian() == guardian - assert registry.latestVault(token) == proxy_vault + assert registry.latestVault(v1_token) == proxy_vault + + # Tokens can only be registered one time (no duplicates) + assert registry.numTokens() == 1 # You can deploy proxy Vaults, linked to a previous release - token = create_token() + v2_token = create_token() proxy_vault = Vault.at( registry.newVault( - token, guardian, rewards, "", "", 1, {"from": gov} + v2_token, guardian, rewards, "", "", 1, {"from": gov} ).return_value ) assert proxy_vault.apiVersion() == v1_vault.apiVersion() == "1.0.0" assert proxy_vault.rewards() == rewards assert proxy_vault.guardian() == guardian - assert registry.latestVault(token) == proxy_vault + assert registry.latestVault(v2_token) == proxy_vault + + # Adding a new endorsed vault with `newVault()` registers a new token + assert registry.tokens(0) == v1_token + assert registry.tokens(1) == v2_token + assert registry.isRegistered(v1_token) + assert registry.isRegistered(v2_token) + assert registry.numTokens() == 2 # Not just anyone can create a new endorsed Vault, only governance can! with brownie.reverts(): @@ -159,10 +110,20 @@ def test_experimental_deployments( experimental_vault.setGovernance(gov, {"from": rando}) experimental_vault.acceptGovernance({"from": gov}) + # New experimental (unendorsed) vaults should not register tokens + assert registry.tokens(0) == ZERO_ADDRESS + assert not registry.isRegistered(token) + assert registry.numTokens() == 0 + # You can only endorse a vault if it creates an new deployment registry.endorseVault(experimental_vault, {"from": gov}) assert registry.latestVault(token) == experimental_vault + # Endorsing experimental vaults should register a token + assert registry.tokens(0) == token + assert registry.isRegistered(token) + assert registry.numTokens() == 1 + # You can't endorse a vault if it would overwrite a current deployment experimental_vault = Vault.at( registry.newExperimentalVault( From cb0ae74c966655e561bd3782201edfb2496ebc61 Mon Sep 17 00:00:00 2001 From: Just some guy <3859395+fubuloubu@users.noreply.github.com> Date: Thu, 11 Mar 2021 12:14:10 -0500 Subject: [PATCH 4/4] refactor: minor whitespace shift --- contracts/Registry.vy | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/Registry.vy b/contracts/Registry.vy index 2b32759b..e6dcaa12 100644 --- a/contracts/Registry.vy +++ b/contracts/Registry.vy @@ -25,10 +25,8 @@ vaults: public(HashMap[address, HashMap[uint256, address]]) # Index of token added => token address tokens: public(HashMap[uint256, address]) - # len(tokens) numTokens: public(uint256) - # Inclusion check for token isRegistered: public(HashMap[address, bool])