From 4796383c05231d4e21225405c28db57e1feb3dc0 Mon Sep 17 00:00:00 2001 From: Santiago Gonzalez Date: Mon, 22 Apr 2024 19:00:14 -0500 Subject: [PATCH] misc fixes on member registry + tests --- contracts/NetworkRegistry.sol | 2 +- contracts/registry/MemberRegistry.sol | 6 +++-- .../NetworkRegistry.behavior.ts | 6 ++--- .../NetworkRegistry.dao.behavior.ts | 6 ++--- test/networkRegistry/NetworkRegistry.ts | 20 +++++++------- test/utils/networkRegistry.ts | 26 ++++++++++++------- 6 files changed, 38 insertions(+), 28 deletions(-) diff --git a/contracts/NetworkRegistry.sol b/contracts/NetworkRegistry.sol index 7400ba0..c0350f3 100644 --- a/contracts/NetworkRegistry.sol +++ b/contracts/NetworkRegistry.sol @@ -55,7 +55,7 @@ error NetworkRegistry__UnAuthorizedCalldata(); * It should also be able to use member activity to distribute funds escrowed on a 0xSplit contract. * Features and important things to consider: * - There are syncing methods for adding/updating members, update registry activity & split funds across networks - * using on a time-weighted formula. + * based on a time-weighted formula. * - Funds are escrowed in a 0xSplit contract so in order to split funds the NetworkRegistry must be set * as the controller. * - A NetworkRegistry contract can be setup either as the main registry (updater == address(0)) or as a replica. diff --git a/contracts/registry/MemberRegistry.sol b/contracts/registry/MemberRegistry.sol index f1eebd2..21bc72b 100644 --- a/contracts/registry/MemberRegistry.sol +++ b/contracts/registry/MemberRegistry.sol @@ -127,6 +127,8 @@ abstract contract MemberRegistry is Initializable, IMemberRegistry { _setNewMember(_members[i], _activityMultipliers[i], _startDates[i]); } members.totalActiveMembers += batchSize; + // make sure registry is ahead of a member most recent start date + lastActivityUpdate = uint32(block.timestamp); } /** @@ -173,10 +175,10 @@ abstract contract MemberRegistry is Initializable, IMemberRegistry { function _removeMember(address _memberAddress) internal virtual { uint256 memberId = _getMemberId(_memberAddress); if (memberId == 0) revert MemberRegistry__NotRegistered(_memberAddress); + DataTypes.Member storage member = _getMemberById(memberId); uint256 maxId = totalMembers(); + if (member.activityMultiplier > 0) --members.totalActiveMembers; if (memberId != maxId) { - DataTypes.Member storage member = _getMemberById(memberId); - if (member.activityMultiplier > 0) --members.totalActiveMembers; DataTypes.Member memory swapMember = _getMemberById(maxId); // swap index members.index[swapMember.account] = memberId; diff --git a/test/networkRegistry/NetworkRegistry.behavior.ts b/test/networkRegistry/NetworkRegistry.behavior.ts index 86bc7f7..561ca56 100644 --- a/test/networkRegistry/NetworkRegistry.behavior.ts +++ b/test/networkRegistry/NetworkRegistry.behavior.ts @@ -6,7 +6,7 @@ import { ethers, getUnnamedAccounts } from "hardhat"; import { PERCENTAGE_SCALE } from "../../constants"; import { SampleSplit, readSampleSplit } from "../../src/utils"; import { ConnextMock, NetworkRegistry, PGContribCalculator, SplitMain, TestERC20 } from "../../types"; -import { deploySplit, hashSplit, summonRegistryProxy } from "../utils"; +import { deploySplit, hashSplit, summonNetworkRegistryProxy } from "../utils"; import { NetworkRegistryProps, User, acceptNetworkSplitControl, registryFixture } from "./NetworkRegistry.fixture"; describe("NetworkRegistry E2E tests", function () { @@ -81,7 +81,7 @@ describe("NetworkRegistry E2E tests", function () { await l1DepositTx.wait(); // Summon Main Registry - const l1RegistryAddress = await summonRegistryProxy( + const l1RegistryAddress = await summonNetworkRegistryProxy( l1CalculatorLibrary.address, { connext: connext.address, @@ -114,7 +114,7 @@ describe("NetworkRegistry E2E tests", function () { await l2DepositTx.wait(); // Summon a Replica Registry - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: connext.address, diff --git a/test/networkRegistry/NetworkRegistry.dao.behavior.ts b/test/networkRegistry/NetworkRegistry.dao.behavior.ts index a09e676..4d9fadc 100644 --- a/test/networkRegistry/NetworkRegistry.dao.behavior.ts +++ b/test/networkRegistry/NetworkRegistry.dao.behavior.ts @@ -10,7 +10,7 @@ import { ethers, getUnnamedAccounts, network } from "hardhat"; import { PERCENTAGE_SCALE } from "../../constants"; import { SampleSplit, readSampleSplit } from "../../src/utils"; import { ConnextMock, GnosisSafe, NetworkRegistry, PGContribCalculator, SplitMain, TestERC20 } from "../../types"; -import { deploySplit, hashSplit, summonRegistryProxy } from "../utils"; +import { deploySplit, hashSplit, summonNetworkRegistryProxy } from "../utils"; // TODO: this should be fixed in the baal-contracts repo import { defaultDAOSettings, submitAndProcessProposal } from "../utils"; import { NetworkRegistryProps, User, registryFixture } from "./NetworkRegistry.fixture"; @@ -121,7 +121,7 @@ describe("NetworkRegistry + DAO E2E tests", function () { await l1DepositTx.wait(); // Summon Main Registry - const l1RegistryAddress = await summonRegistryProxy( + const l1RegistryAddress = await summonNetworkRegistryProxy( l1CalculatorLibrary.address, { connext: connext.address, @@ -173,7 +173,7 @@ describe("NetworkRegistry + DAO E2E tests", function () { await l2DepositTx.wait(); // Summon a Replica Registry - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: connext.address, diff --git a/test/networkRegistry/NetworkRegistry.ts b/test/networkRegistry/NetworkRegistry.ts index 6012a55..8865f71 100644 --- a/test/networkRegistry/NetworkRegistry.ts +++ b/test/networkRegistry/NetworkRegistry.ts @@ -19,7 +19,7 @@ import { SplitMain, } from "../../types"; import { Member } from "../types"; -import { deploySplit, generateMemberBatch, hashSplit, summonRegistryProxy } from "../utils"; +import { deploySplit, generateMemberBatch, hashSplit, summonNetworkRegistryProxy } from "../utils"; import { NetworkRegistryProps, User, acceptNetworkSplitControl, registryFixture } from "./NetworkRegistry.fixture"; describe("NetworkRegistry", function () { @@ -73,7 +73,7 @@ describe("NetworkRegistry", function () { ); // Summon Main Registry - const l1RegistryAddress = await summonRegistryProxy( + const l1RegistryAddress = await summonNetworkRegistryProxy( l1CalculatorLibrary.address, { connext: connext.address, @@ -102,7 +102,7 @@ describe("NetworkRegistry", function () { ); // Summon a Replica Registry - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: connext.address, @@ -420,7 +420,7 @@ describe("NetworkRegistry", function () { it("Should be able to set new updater settings", async () => { const signer = await ethers.getSigner(users.owner.address); // Summon a Replica Registry - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: connext.address, @@ -588,7 +588,7 @@ describe("NetworkRegistry", function () { splitConfig.distributorFee, users.owner.address, ); - const l1RegistryAddress = await summonRegistryProxy( + const l1RegistryAddress = await summonNetworkRegistryProxy( l1CalculatorLibrary.address, { connext: connext.address, @@ -620,7 +620,7 @@ describe("NetworkRegistry", function () { splitConfig.distributorFee, users.owner.address, ); - const l1RegistryAddress = await summonRegistryProxy( + const l1RegistryAddress = await summonNetworkRegistryProxy( l1CalculatorLibrary.address, { connext: connext.address, @@ -1582,7 +1582,7 @@ describe("NetworkRegistry", function () { ), ).to.be.revertedWithCustomError(l1NetworkRegistry, "NetworkRegistry__ConnextOnly"); const connextCaller = await ethers.getSigner(users.applicant.address); - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: users.applicant.address, // fake connext caller @@ -1636,7 +1636,7 @@ describe("NetworkRegistry", function () { const parentChainId = 1; const signer = await ethers.getSigner(users.owner.address); - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: connext.address, @@ -2584,7 +2584,7 @@ describe("NetworkRegistry", function () { const totalValue = relayerFees.reduce((a: BigNumber, b: BigNumber) => a.add(b), BigNumber.from(0)); const signer = await ethers.getSigner(users.owner.address); - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: connext.address, @@ -3063,7 +3063,7 @@ describe("NetworkRegistry", function () { it("Should not be able upgrade replica contract by direct call", async () => { const signer = await ethers.getSigner(users.owner.address); - const l2RegistryAddress = await summonRegistryProxy( + const l2RegistryAddress = await summonNetworkRegistryProxy( l2Registry.calculatorLibrary.address, { connext: connext.address, diff --git a/test/utils/networkRegistry.ts b/test/utils/networkRegistry.ts index ee38c90..d68a58d 100644 --- a/test/utils/networkRegistry.ts +++ b/test/utils/networkRegistry.ts @@ -27,21 +27,16 @@ import { Member, NetworkRegistryArgs } from "../types"; // return registryAddress; // }; -export const summonRegistryProxy = async ( +const summonRegistryProxy = async ( calculatorLibraryAddress: string, - registryArgs: NetworkRegistryArgs, + initializationParams: string, registryName: string = "NetworkRegistry", + registryContract: string = "NetworkRegistry", ) => { - const { connext, updaterDomainId, updaterAddress, splitMain, split, owner } = registryArgs; - const initializationParams = ethers.utils.defaultAbiCoder.encode( - ["address", "uint32", "address", "address", "address", "address"], - [connext, updaterDomainId, updaterAddress, splitMain, split, owner], - ); - const { deployer } = await getNamedAccounts(); const registryDeployed = await deployments.deploy(registryName, { - contract: "NetworkRegistry", + contract: registryContract, from: deployer, args: [], libraries: { @@ -60,6 +55,19 @@ export const summonRegistryProxy = async ( return registryDeployed.address; }; +export const summonNetworkRegistryProxy = async ( + calculatorLibraryAddress: string, + registryArgs: NetworkRegistryArgs, + registryName: string = "NetworkRegistry", +) => { + const { connext, updaterDomainId, updaterAddress, splitMain, split, owner } = registryArgs; + const initializationParams = ethers.utils.defaultAbiCoder.encode( + ["address", "uint32", "address", "address", "address", "address"], + [connext, updaterDomainId, updaterAddress, splitMain, split, owner], + ); + + return await summonRegistryProxy(calculatorLibraryAddress, initializationParams, registryName, "NetworkRegistry"); +}; export const generateMemberBatch = async (totalMembers: number): Promise> => { const accounts = await getUnnamedAccounts(); const members = accounts.slice(0, totalMembers);