From 9fb255d89f4756567b07b84508dfec01c7aebe1f Mon Sep 17 00:00:00 2001
From: Pedro Valido <80268365+pedrovalido@users.noreply.github.com>
Date: Mon, 7 Oct 2024 20:06:59 +0100
Subject: [PATCH] fix: factory registry

---
 contracts/helpers/FactoryRegistry.vy | 25 +++++++++-------------
 env.example                          |  2 +-
 tests/test_factory_registry.py       | 32 ++--------------------------
 3 files changed, 13 insertions(+), 46 deletions(-)

diff --git a/contracts/helpers/FactoryRegistry.vy b/contracts/helpers/FactoryRegistry.vy
index 63c7b34..1f445f3 100644
--- a/contracts/helpers/FactoryRegistry.vy
+++ b/contracts/helpers/FactoryRegistry.vy
@@ -4,18 +4,15 @@
 # @author velodrome.finance
 # @notice Sugar Factory Registry to keep track of leaf pool factories
 
-
 MAX_FACTORIES: public(constant(uint256)) = 10
 
 owner: public(address)
-poolFactories: public(DynArray[address, MAX_FACTORIES]) # camelCase to have same signature as origin registry
-pool_factory_count: public(uint256)
+pool_factories: public(DynArray[address, MAX_FACTORIES])
 pool_factory_exists: public(HashMap[address, bool])
 
 @external
 def __init__(_owner: address):
     self.owner = _owner
-    self.pool_factory_count = 0
 
 @internal
 def _only_owner():
@@ -25,13 +22,10 @@ def _only_owner():
 def approve(pool_factory: address):
     self._only_owner()
     
-    # Check if already present
     if self.pool_factory_exists[pool_factory]:
         raise "Already exists"
     
-    # Add the poolFactory to the list
-    self.poolFactories[self.pool_factory_count] = pool_factory
-    self.pool_factory_count += 1
+    self.pool_factories.append(pool_factory)
     self.pool_factory_exists[pool_factory] = True
 
 @external
@@ -42,13 +36,9 @@ def unapprove(pool_factory: address):
         raise "Not exists"
 
     for i in range(MAX_FACTORIES):
-        if self.poolFactories[i] == pool_factory:
-            # Remove the pool_factory by shifting elements
-            for j in range(0, MAX_FACTORIES):
-                if j < i: continue
-                if j >= self.pool_factory_count: break
-                self.poolFactories[j] = self.poolFactories[j + 1]
-            self.pool_factory_count -= 1
+        if self.pool_factories[i] == pool_factory:
+            self.pool_factories[i] = self.pool_factories[len(self.pool_factories) - 1]
+            self.pool_factories.pop()
             break
 
     self.pool_factory_exists[pool_factory] = False
@@ -57,3 +47,8 @@ def unapprove(pool_factory: address):
 @view
 def factoriesToPoolFactory(poolFactory: address) -> (address, address):
     return (empty(address), poolFactory)
+
+@external
+@view
+def poolFactories() -> DynArray[address, MAX_FACTORIES]:
+    return self.pool_factories
diff --git a/env.example b/env.example
index e779036..458851a 100644
--- a/env.example
+++ b/env.example
@@ -43,7 +43,7 @@ RELAY_SUGAR_ADDRESS_8453=0x8932B5FE23C07Df06533F8f09E43e7cca6a24143
 
 #MODE
 VOTER_34443=0x0000000000000000000000000000000000000000
-REGISTRY_34443=0xeFFa2e499309db6FEb3868B52AB0889b0D7825ec
+REGISTRY_34443=0x6B290762F9F9155637F0Bb6B7A5B1cEb394cceD8
 CONVERTOR_34443=0x1111111111111111111111111111111111111111
 SLIPSTREAM_HELPER_34443=0x0000000000000000000000000000000000000000
 ALM_FACTORY_34443=0x0000000000000000000000000000000000000000
diff --git a/tests/test_factory_registry.py b/tests/test_factory_registry.py
index 119166b..05a3dbc 100644
--- a/tests/test_factory_registry.py
+++ b/tests/test_factory_registry.py
@@ -5,7 +5,7 @@
 
 
 @pytest.fixture
-def factory_registry(FactoryRegistry, accounts):
+def factory_registry(FactoryRegistry):
     # Deploy the contract using the first test account as the owner
     yield FactoryRegistry.at(os.getenv('REGISTRY_34443'))
 
@@ -15,35 +15,7 @@ def test_initial_state(factory_registry):
       "0xd42C7914cF8dc24a1075E29C283C581bd1b0d3D3"
 
 
-def test_approve(factory_registry, accounts):
-    owner = factory_registry.owner()
-    pool_factory = "0x1111111111111111111111111111111111111111"
-    pool_factory_count = factory_registry.pool_factory_count()
-
-    # Approve a new pool factory
-    factory_registry.approve(pool_factory, {'from': owner})
-    assert factory_registry.pool_factory_count() == pool_factory_count + 1
-    assert factory_registry.poolFactories(0) == pool_factory
-    assert factory_registry.pool_factory_exists(pool_factory)
-
-
-def test_unapprove(factory_registry, accounts):
-    owner = factory_registry.owner()
-    pool_factory = "0x1111111111111111111111111111111111111111"
-    pool_factory_count = factory_registry.pool_factory_count()
-
-    # Approve a pool factory to set up the state for unapprove
-    factory_registry.approve(pool_factory, {'from': owner})
-    assert factory_registry.pool_factory_count() == pool_factory_count + 1
-    assert factory_registry.pool_factory_exists(pool_factory)
-
-    # Unapprove the pool factory
-    factory_registry.unapprove(pool_factory, {'from': owner})
-    assert factory_registry.pool_factory_count() == pool_factory_count
-    assert not factory_registry.pool_factory_exists(pool_factory)
-
-
 def test_factories_to_pool_factory(factory_registry):
-    pool_factory = "0x1111111111111111111111111111111111111111"
+    pool_factory: address = "0x1111111111111111111111111111111111111111"
     result = factory_registry.factoriesToPoolFactory(pool_factory)
     assert result == [ADDRESS_ZERO, pool_factory]