diff --git a/test/AdminRecipientRegistry.t.sol b/test/AdminRecipientRegistry.t.sol index fbcc9f3..e73cc32 100644 --- a/test/AdminRecipientRegistry.t.sol +++ b/test/AdminRecipientRegistry.t.sol @@ -19,12 +19,14 @@ contract AdminRecipientRegistryTest is TestWrapper { registry.initialize(ADMIN); } - function test_Initialize() public view { + function test_WhenInitializing() external view { + // it should set admin and zero recipients assertEq(registry.owner(), ADMIN); assertEq(registry.getRecipientCount(), 0); } - function test_QueueAndUpdateRecipient() public { + function test_WhenQueuingASingleAddition() external { + // it should queue recipient and process correctly vm.prank(ADMIN); vm.expectEmit(true, false, false, false); emit IRecipientRegistry.RecipientQueued(RECIPIENT_1, true); @@ -54,7 +56,8 @@ contract AdminRecipientRegistryTest is TestWrapper { assertEq(recipients[0], RECIPIENT_1); } - function test_QueueMultipleRecipients() public { + function test_WhenQueuingMultipleAdditions() external { + // it should queue and process all recipients address[] memory toAdd = new address[](3); toAdd[0] = RECIPIENT_1; toAdd[1] = RECIPIENT_2; @@ -75,7 +78,8 @@ contract AdminRecipientRegistryTest is TestWrapper { assertTrue(registry.isRecipient(RECIPIENT_3)); } - function test_QueueAndRemoveRecipient() public { + function test_WhenQueuingARemoval() external { + // it should remove recipient after processing vm.startPrank(ADMIN); registry.queueRecipientAddition(RECIPIENT_1); registry.queueRecipientAddition(RECIPIENT_2); @@ -103,7 +107,8 @@ contract AdminRecipientRegistryTest is TestWrapper { assertEq(registry.getRecipientCount(), 1); } - function test_QueueMultipleRemoval() public { + function test_WhenQueuingMultipleRemovals() external { + // it should remove selected recipients vm.startPrank(ADMIN); // Add recipients @@ -131,13 +136,15 @@ contract AdminRecipientRegistryTest is TestWrapper { assertEq(registry.getRecipientCount(), 2); } - function test_RevertOnInvalidRecipient() public { + function test_RevertWhen_QueuingAnInvalidRecipient() external { + // it should revert vm.prank(ADMIN); vm.expectRevert(); registry.queueRecipientAddition(address(0)); } - function test_RevertOnDuplicateRecipient() public { + function test_RevertWhen_QueuingADuplicateRecipient() external { + // it should revert vm.startPrank(ADMIN); registry.queueRecipientAddition(RECIPIENT_1); registry.processQueue(); @@ -147,19 +154,22 @@ contract AdminRecipientRegistryTest is TestWrapper { vm.stopPrank(); } - function test_RevertOnRemovingNonExistent() public { + function test_RevertWhen_RemovingANonExistentRecipient() external { + // it should revert vm.prank(ADMIN); vm.expectRevert(); registry.queueRecipientRemoval(RECIPIENT_1); } - function test_OnlyAdminCanQueue() public { + function test_RevertWhen_ANonAdminQueuesAddition() external { + // it should revert vm.prank(address(0xdead)); vm.expectRevert(); registry.queueRecipientAddition(RECIPIENT_1); } - function test_OnlyAdminCanQueueRemoval() public { + function test_RevertWhen_ANonAdminQueuesRemoval() external { + // it should revert vm.prank(ADMIN); registry.queueRecipientAddition(RECIPIENT_1); registry.processQueue(); @@ -169,7 +179,8 @@ contract AdminRecipientRegistryTest is TestWrapper { registry.queueRecipientRemoval(RECIPIENT_1); } - function test_TransferAdmin() public { + function test_WhenTransferringAdmin() external { + // it should allow new admin and revoke old address newAdmin = address(0xBEEF); vm.prank(ADMIN); @@ -189,7 +200,8 @@ contract AdminRecipientRegistryTest is TestWrapper { registry.queueRecipientAddition(RECIPIENT_2); } - function test_LargeScaleOperations() public { + function test_WhenPerformingLargeScaleOperations() external { + // it should handle 100 additions and 50 removals vm.startPrank(ADMIN); // Queue many recipients diff --git a/test/AdminRecipientRegistry.tree b/test/AdminRecipientRegistry.tree new file mode 100644 index 0000000..272e36f --- /dev/null +++ b/test/AdminRecipientRegistry.tree @@ -0,0 +1,25 @@ +AdminRecipientRegistryTest +├── when initializing +│ └── it should set admin and zero recipients +├── when queuing a single addition +│ └── it should queue recipient and process correctly +├── when queuing multiple additions +│ └── it should queue and process all recipients +├── when queuing a removal +│ └── it should remove recipient after processing +├── when queuing multiple removals +│ └── it should remove selected recipients +├── when queuing an invalid recipient +│ └── it should revert +├── when queuing a duplicate recipient +│ └── it should revert +├── when removing a non-existent recipient +│ └── it should revert +├── when a non-admin queues addition +│ └── it should revert +├── when a non-admin queues removal +│ └── it should revert +├── when transferring admin +│ └── it should allow new admin and revoke old +└── when performing large scale operations + └── it should handle 100 additions and 50 removals \ No newline at end of file diff --git a/test/BreadKitTest.t.sol b/test/BreadKitTest.t.sol index 264be74..8709043 100644 --- a/test/BreadKitTest.t.sol +++ b/test/BreadKitTest.t.sol @@ -43,7 +43,9 @@ contract BreadKitTest is TestWrapper { token.mint{value: 1}(0x0000000000000000000000000000000000000009); } - function test_mint() public { + // ============ when minting tokens ============ + + function test_WhenMintingTokens_ShouldUpdateSupplyBalanceAndYieldBacking() public { uint256 supplyBefore = token.totalSupply(); uint256 balBefore = token.balanceOf(address(this)); uint256 contractBalBefore = IERC20(SX_DAI).balanceOf(address(token)); @@ -78,7 +80,9 @@ contract BreadKitTest is TestWrapper { assertEq(token.balanceOf(address(this)), 1.5 ether); } - function test_finalizeNewYieldClaimer_revertsBeforeTimelock() public { + // ============ when finalizing new yield claimer ============ + + function test_RevertWhen_FinalizingNewYieldClaimer_GivenTimelockHasNotElapsed() public { address newClaimer = address(0xCAFE); token.prepareNewYieldClaimer(newClaimer); @@ -90,7 +94,7 @@ contract BreadKitTest is TestWrapper { token.finalizeNewYieldClaimer(); } - function test_finalizeNewYieldClaimer_succeedsAfterTimelock() public { + function test_WhenFinalizingNewYieldClaimer_GivenTimelockHasElapsed_ShouldUpdateYieldClaimer() public { address newClaimer = address(0xCAFE); token.prepareNewYieldClaimer(newClaimer); @@ -103,7 +107,7 @@ contract BreadKitTest is TestWrapper { assertEq(AbstractToken(address(token)).yieldClaimer(), newClaimer); } - function test_finalizeNewYieldClaimer_clearsPendingState() public { + function test_WhenFinalizingNewYieldClaimer_ShouldClearPendingStateAfterFinalization() public { address newClaimer = address(0xCAFE); token.prepareNewYieldClaimer(newClaimer); diff --git a/test/BreadKitTest.tree b/test/BreadKitTest.tree new file mode 100644 index 0000000..808b6f9 --- /dev/null +++ b/test/BreadKitTest.tree @@ -0,0 +1,9 @@ +BreadKitTest +├── when minting tokens +│ └── it should update supply balance and yield backing +└── when finalizing new yield claimer + ├── given timelock has not elapsed + │ └── it should revert with TimelockNotElapsed + ├── given timelock has elapsed + │ └── it should update yield claimer + └── it should clear pending state after finalization \ No newline at end of file diff --git a/test/CycleModule.t.sol b/test/CycleModule.t.sol index 1f802a9..190a03f 100644 --- a/test/CycleModule.t.sol +++ b/test/CycleModule.t.sol @@ -25,19 +25,36 @@ contract CycleModuleTest is Test { cycleModule.setDistributionManager(distManager); } - function testInitialState() public view { + // ============ when initializing ============ + + function test_WhenInitializing_ItShouldSetInitialStateCorrectly() public view { assertEq(cycleModule.getCurrentCycle(), 1); assertEq(cycleModule.cycleLength(), CYCLE_LENGTH); assertEq(cycleModule.lastCycleStartBlock(), START_BLOCK); assertEq(cycleModule.owner(), owner); } - function testCannotReinitialize() public { + function test_RevertWhen_Initializing_Reinitialize() public { vm.expectRevert(Initializable.InvalidInitialization.selector); cycleModule.initialize(200, owner); } - function testNotInitializedFunctions() public { + function test_RevertWhen_Initializing_ZeroAddressOwner() public { + CycleModule impl = new CycleModule(); + CycleModule newModule = CycleModule(address(new ERC1967Proxy(address(impl), ""))); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableInvalidOwner.selector, address(0))); + newModule.initialize(100, address(0)); + } + + function test_RevertWhen_Initializing_ImplementationInitialization() public { + CycleModule impl = new CycleModule(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(100, owner); + } + + // ============ when not initialized ============ + + function test_RevertWhen_NotInitialized_AllStateQueries() public { // Deploy a proxy without initialization data to get an uninitialized module CycleModule impl = new CycleModule(); CycleModule uninitializedModule = CycleModule(address(new ERC1967Proxy(address(impl), ""))); @@ -61,15 +78,23 @@ contract CycleModuleTest is Test { uninitializedModule.updateCycleLength(200); } - function testCycleCompletion() public { + // ============ when checking cycle completion before end ============ + + function test_WhenCheckingCycleCompletionBeforeEnd_ItShouldReturnFalse() public view { assertFalse(cycleModule.isCycleComplete()); + } + + // ============ when checking cycle completion at end ============ + function test_WhenCheckingCycleCompletionAtEnd_ItShouldReturnTrue() public { // Move to end of cycle vm.roll(START_BLOCK + CYCLE_LENGTH); assertTrue(cycleModule.isCycleComplete()); } - function testStartNewCycle() public { + // ============ when starting a new cycle as distribution manager ============ + + function test_WhenStartingNewCycleAsDistributionManager_ItShouldAdvanceToNextCycle() public { // Move to end of cycle vm.roll(START_BLOCK + CYCLE_LENGTH); @@ -82,7 +107,9 @@ contract CycleModuleTest is Test { assertFalse(cycleModule.isCycleComplete()); } - function testCannotStartNewCycleEarly() public { + // ============ when starting a new cycle before completion ============ + + function test_RevertWhen_StartingNewCycleBeforeCompletion_InvalidCycleTransition() public { // Try to start new cycle before current one is complete vm.roll(START_BLOCK + CYCLE_LENGTH - 1); @@ -91,7 +118,9 @@ contract CycleModuleTest is Test { cycleModule.startNewCycle(); } - function testNonDistributionManagerCannotStartCycle() public { + // ============ when starting a new cycle as non distribution manager ============ + + function test_RevertWhen_StartingNewCycleAsNonDistributionManager_OnlyDistributionManager() public { vm.roll(START_BLOCK + CYCLE_LENGTH); vm.prank(user); @@ -99,95 +128,100 @@ contract CycleModuleTest is Test { cycleModule.startNewCycle(); } - function testOwnerCannotStartCycle() public { - vm.roll(START_BLOCK + CYCLE_LENGTH); - - vm.expectRevert(AbstractCycleModule.OnlyDistributionManager.selector); - cycleModule.startNewCycle(); - } + // ============ when starting a new cycle without distribution manager set ============ - function testOwnership() public { - assertEq(cycleModule.owner(), owner); + function test_RevertWhen_StartingNewCycleWithoutDistributionManagerSet_DistributionManagerNotSet() public { + // Deploy a fresh cycle module without setting distribution manager + CycleModule impl = new CycleModule(); + bytes memory initData = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, CYCLE_LENGTH, owner); + CycleModule freshModule = CycleModule(address(new ERC1967Proxy(address(impl), initData))); - cycleModule.transferOwnership(user); - assertEq(cycleModule.owner(), user); + vm.roll(START_BLOCK + CYCLE_LENGTH); + vm.expectRevert(AbstractCycleModule.DistributionManagerNotSet.selector); + freshModule.startNewCycle(); } - function testGetBlocksUntilNextCycle() public view { + // ============ when querying blocks until next cycle ============ + + function test_WhenQueryingBlocksUntilNextCycle_ItShouldReturnFullCycleLengthAtStart() public view { assertEq(cycleModule.getBlocksUntilNextCycle(), CYCLE_LENGTH); } - function testGetBlocksUntilNextCyclePartway() public { + function test_WhenQueryingBlocksUntilNextCycle_ItShouldReturnRemainingBlocksPartway() public { vm.roll(START_BLOCK + 25); assertEq(cycleModule.getBlocksUntilNextCycle(), 75); } - function testGetBlocksUntilNextCycleComplete() public { + function test_WhenQueryingBlocksUntilNextCycle_ItShouldReturnZeroWhenComplete() public { vm.roll(START_BLOCK + CYCLE_LENGTH); assertEq(cycleModule.getBlocksUntilNextCycle(), 0); } - function testGetCycleProgress() public view { + // ============ when querying cycle progress ============ + + function test_WhenQueryingCycleProgress_ItShouldReturnZeroAtStart() public view { assertEq(cycleModule.getCycleProgress(), 0); } - function testGetCycleProgressPartway() public { + function test_WhenQueryingCycleProgress_ItShouldReturn50Halfway() public { vm.roll(START_BLOCK + 50); assertEq(cycleModule.getCycleProgress(), 50); } - function testGetCycleProgressComplete() public { + function test_WhenQueryingCycleProgress_ItShouldReturn100WhenComplete() public { vm.roll(START_BLOCK + CYCLE_LENGTH); assertEq(cycleModule.getCycleProgress(), 100); } - function testUpdateCycleLength() public { + // ============ when updating cycle length as owner ============ + + function test_WhenUpdatingCycleLengthAsOwner_ItShouldUpdateTheCycleLength() public { uint256 newLength = 200; cycleModule.updateCycleLength(newLength); assertEq(cycleModule.cycleLength(), newLength); } - function testCannotUpdateCycleLengthToZero() public { + // ============ when updating cycle length to zero ============ + + function test_RevertWhen_UpdatingCycleLengthToZero_InvalidCycleLength() public { vm.expectRevert(AbstractCycleModule.InvalidCycleLength.selector); cycleModule.updateCycleLength(0); } - function testNonOwnerCannotUpdateCycleLength() public { + // ============ when updating cycle length as non owner ============ + + function test_RevertWhen_UpdatingCycleLengthAsNonOwner() public { vm.prank(user); vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, user)); cycleModule.updateCycleLength(200); } - function testAnyoneCanInitializeOnce() public { - CycleModule impl = new CycleModule(); - CycleModule newModule = CycleModule(address(new ERC1967Proxy(address(impl), ""))); + // ============ when setting distribution manager as owner ============ - vm.prank(user); - newModule.initialize(100, user); + function test_WhenSettingDistributionManagerAsOwner_ItShouldUpdateDistributionManager() public { + address newManager = address(0x99); + cycleModule.setDistributionManager(newManager); + assertEq(cycleModule.distributionManager(), newManager); + } - // User is now the owner - assertEq(newModule.owner(), user); - assertEq(newModule.cycleLength(), 100); + // ============ when setting zero distribution manager ============ - // Cannot reinitialize - vm.expectRevert(Initializable.InvalidInitialization.selector); - newModule.initialize(200, user); + function test_RevertWhen_SettingZeroDistributionManager_ZeroAddress() public { + vm.expectRevert(AbstractCycleModule.ZeroAddress.selector); + cycleModule.setDistributionManager(address(0)); } - function testCannotInitializeWithZeroAddress() public { - CycleModule impl = new CycleModule(); - CycleModule newModule = CycleModule(address(new ERC1967Proxy(address(impl), ""))); - vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableInvalidOwner.selector, address(0))); - newModule.initialize(100, address(0)); - } + // ============ when setting distribution manager as non owner ============ - function testImplementationCannotBeInitialized() public { - CycleModule impl = new CycleModule(); - vm.expectRevert(Initializable.InvalidInitialization.selector); - impl.initialize(100, owner); + function test_RevertWhen_SettingDistributionManagerAsNonOwner() public { + vm.prank(user); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, user)); + cycleModule.setDistributionManager(address(0x99)); } - function testMultipleCycles() public { + // ============ when running multiple cycles ============ + + function test_WhenRunningMultipleCycles_ItShouldCorrectlyTrackCycleNumberAndStartBlock() public { vm.startPrank(distManager); // Complete first cycle @@ -204,31 +238,30 @@ contract CycleModuleTest is Test { vm.stopPrank(); } - function testSetDistributionManager() public { - address newManager = address(0x99); - cycleModule.setDistributionManager(newManager); - assertEq(cycleModule.distributionManager(), newManager); - } + // ============ when managing ownership ============ - function testNonOwnerCannotSetDistributionManager() public { - vm.prank(user); - vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, user)); - cycleModule.setDistributionManager(address(0x99)); - } + function test_WhenManagingOwnership_ItShouldTransferOwnershipCorrectly() public { + assertEq(cycleModule.owner(), owner); - function testCannotSetZeroDistributionManager() public { - vm.expectRevert(AbstractCycleModule.ZeroAddress.selector); - cycleModule.setDistributionManager(address(0)); + cycleModule.transferOwnership(user); + assertEq(cycleModule.owner(), user); } - function testStartNewCycleRevertsWhenDistributionManagerNotSet() public { - // Deploy a fresh cycle module without setting distribution manager + // ============ when anyone initializes ============ + + function test_WhenAnyoneInitializes_ItShouldAllowFirstCallerToBecomeOwner() public { CycleModule impl = new CycleModule(); - bytes memory initData = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, CYCLE_LENGTH, owner); - CycleModule freshModule = CycleModule(address(new ERC1967Proxy(address(impl), initData))); + CycleModule newModule = CycleModule(address(new ERC1967Proxy(address(impl), ""))); - vm.roll(START_BLOCK + CYCLE_LENGTH); - vm.expectRevert(AbstractCycleModule.DistributionManagerNotSet.selector); - freshModule.startNewCycle(); + vm.prank(user); + newModule.initialize(100, user); + + // User is now the owner + assertEq(newModule.owner(), user); + assertEq(newModule.cycleLength(), 100); + + // Cannot reinitialize + vm.expectRevert(Initializable.InvalidInitialization.selector); + newModule.initialize(200, user); } } diff --git a/test/CycleModule.tree b/test/CycleModule.tree new file mode 100644 index 0000000..0f9ba58 --- /dev/null +++ b/test/CycleModule.tree @@ -0,0 +1,46 @@ +CycleModuleTest +├── when initializing +│ ├── it should set initial state correctly +│ ├── it should revert on reinitialize +│ ├── it should revert with zero address owner +│ └── it should prevent implementation initialization +├── when not initialized +│ └── it should revert all state queries +├── when checking cycle completion before end +│ └── it should return false +├── when checking cycle completion at end +│ └── it should return true +├── when starting a new cycle as distribution manager +│ └── it should advance to next cycle +├── when starting a new cycle before completion +│ └── it should revert with InvalidCycleTransition +├── when starting a new cycle as non distribution manager +│ └── it should revert with OnlyDistributionManager +├── when starting a new cycle without distribution manager set +│ └── it should revert with DistributionManagerNotSet +├── when querying blocks until next cycle +│ ├── it should return full cycle length at start +│ ├── it should return remaining blocks partway +│ └── it should return zero when complete +├── when querying cycle progress +│ ├── it should return zero at start +│ ├── it should return 50 halfway +│ └── it should return 100 when complete +├── when updating cycle length as owner +│ └── it should update the cycle length +├── when updating cycle length to zero +│ └── it should revert with InvalidCycleLength +├── when updating cycle length as non owner +│ └── it should revert +├── when setting distribution manager as owner +│ └── it should update distribution manager +├── when setting zero distribution manager +│ └── it should revert with ZeroAddress +├── when setting distribution manager as non owner +│ └── it should revert +├── when running multiple cycles +│ └── it should correctly track cycle number and start block +├── when managing ownership +│ └── it should transfer ownership correctly +└── when anyone initializes + └── it should allow first caller to become owner \ No newline at end of file diff --git a/test/DistributionCycleIntegration.t.sol b/test/DistributionCycleIntegration.t.sol index 5c3b61a..97048eb 100644 --- a/test/DistributionCycleIntegration.t.sol +++ b/test/DistributionCycleIntegration.t.sol @@ -61,7 +61,9 @@ contract DistributionCycleIntegrationTest is Test { cycleModule.setDistributionManager(address(baseManager)); } - function testClaimAndDistributeAdvancesCycle() public { + // ============ when calling claimAndDistribute ============ + + function test_WhenCallingClaimAndDistribute_ShouldAdvanceTheCycleAtomically() public { // Move to end of cycle vm.roll(START_BLOCK + CYCLE_LENGTH); assertEq(cycleModule.getCurrentCycle(), 1); @@ -117,7 +119,9 @@ contract DistributionCycleIntegrationTest is Test { assertFalse(cycleModule.isCycleComplete()); } - function testIsDistributionReadyReturnsFalseWhenNotWired() public { + // ============ when manager is not wired as distribution manager ============ + + function test_WhenManagerIsNotWiredAsDistributionManager_ShouldReturnFalseForIsDistributionReady() public { // Deploy a fresh manager NOT wired as distribution manager on the cycle module BaseDistributionManager managerImpl = new BaseDistributionManager(); diff --git a/test/DistributionCycleIntegration.tree b/test/DistributionCycleIntegration.tree new file mode 100644 index 0000000..fcd7ed0 --- /dev/null +++ b/test/DistributionCycleIntegration.tree @@ -0,0 +1,5 @@ +DistributionCycleIntegrationTest +├── when calling claimAndDistribute +│ └── it should advance the cycle atomically +└── when manager is not wired as distribution manager + └── it should return false for isDistributionReady \ No newline at end of file diff --git a/test/FactoryModuleDeployment.t.sol b/test/FactoryModuleDeployment.t.sol index 04943e5..0d80e53 100644 --- a/test/FactoryModuleDeployment.t.sol +++ b/test/FactoryModuleDeployment.t.sol @@ -67,9 +67,9 @@ contract FactoryModuleDeploymentTest is Test { return address(new UpgradeableBeacon(impl, owner)); } - // ============ CycleModule via Factory ============ + // ============ when creating a cycle module ============ - function test_createCycleModule() public { + function test_WhenCreatingACycleModule_ShouldDeployAndInitializeCorrectly() public { bytes memory payload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 1000, owner); address module = factory.create(cycleModuleBeacon, payload, keccak256("cycle-salt")); @@ -79,9 +79,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(CycleModule(module).owner(), owner); } - // ============ AdminRecipientRegistry via Factory ============ + // ============ when creating an admin recipient registry ============ - function test_createAdminRecipientRegistry() public { + function test_WhenCreatingAnAdminRecipientRegistry_ShouldDeployAndInitializeCorrectly() public { bytes memory payload = abi.encodeWithSelector(AdminRecipientRegistry.initialize.selector, owner); address module = factory.create(adminRegistryBeacon, payload, keccak256("admin-registry-salt")); @@ -90,9 +90,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(registry.owner(), owner); } - // ============ RecipientRegistry via Factory ============ + // ============ when creating a recipient registry ============ - function test_createRecipientRegistry() public { + function test_WhenCreatingARecipientRegistry_ShouldDeployAndInitializeCorrectly() public { bytes memory payload = abi.encodeWithSelector(RecipientRegistry.initialize.selector, owner); address module = factory.create(registryBeacon, payload, keccak256("registry-salt")); @@ -101,9 +101,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(registry.owner(), owner); } - // ============ VotingRecipientRegistry via Factory ============ + // ============ when creating a voting recipient registry ============ - function test_createVotingRecipientRegistry() public { + function test_WhenCreatingAVotingRecipientRegistry_ShouldDeployAndInitializeCorrectly() public { address[] memory initialRecipients = new address[](2); initialRecipients[0] = address(0x111); initialRecipients[1] = address(0x222); @@ -119,9 +119,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(registry.owner(), owner); } - // ============ EqualDistributionStrategy via Factory ============ + // ============ when creating an equal distribution strategy ============ - function test_createEqualDistributionStrategy() public { + function test_WhenCreatingAnEqualDistributionStrategy_ShouldDeployAndInitializeCorrectly() public { // Deploy a registry first for the strategy to use bytes memory registryPayload = abi.encodeWithSelector(AdminRecipientRegistry.initialize.selector, owner); address registry = factory.create(adminRegistryBeacon, registryPayload, keccak256("strat-registry-salt")); @@ -148,9 +148,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(strategy.distributionManager(), mockDistManager); } - // ============ VotingDistributionStrategy via Factory ============ + // ============ when creating a voting distribution strategy ============ - function test_createVotingDistributionStrategy() public { + function test_WhenCreatingAVotingDistributionStrategy_ShouldDeployAndInitializeCorrectly() public { bytes memory registryPayload = abi.encodeWithSelector(AdminRecipientRegistry.initialize.selector, owner); address registry = factory.create(adminRegistryBeacon, registryPayload, keccak256("vstrat-registry-salt")); @@ -187,9 +187,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(strategy.distributionManager(), mockDistManager); } - // ============ BasisPointsVotingModule via Factory ============ + // ============ when creating a basis points voting module ============ - function test_createBasisPointsVotingModule() public { + function test_WhenCreatingABasisPointsVotingModule_ShouldDeployAndInitializeCorrectly() public { // Deploy dependencies first bytes memory cyclePayload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 1000, owner); address cycleAddr = factory.create(cycleModuleBeacon, cyclePayload, keccak256("vm-cycle-salt")); @@ -224,9 +224,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(votingModule.owner(), owner); } - // ============ BaseDistributionManager via Factory ============ + // ============ when creating a base distribution manager ============ - function test_createBaseDistributionManager() public { + function test_WhenCreatingABaseDistributionManager_ShouldDeployAndInitializeCorrectly() public { // Deploy dependencies bytes memory cyclePayload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 1000, owner); address cycleAddr = factory.create(cycleModuleBeacon, cyclePayload, keccak256("bdm-cycle-salt")); @@ -259,9 +259,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(address(manager.distributionStrategy()), mockStrategy); } - // ============ MultiStrategyDistributionManager via Factory ============ + // ============ when creating a multi strategy distribution manager ============ - function test_createMultiStrategyDistributionManager() public { + function test_WhenCreatingAMultiStrategyDistributionManager_ShouldDeployAndInitializeCorrectly() public { bytes memory cyclePayload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 1000, owner); address cycleAddr = factory.create(cycleModuleBeacon, cyclePayload, keccak256("msdm-cycle-salt")); @@ -298,9 +298,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(manager.getStrategyCount(), 2); } - // ============ Deterministic Address Computation ============ + // ============ when computing addresses ============ - function test_computeAddress() public view { + function test_WhenComputingAddresses_ShouldReturnDeterministicAddress() public view { bytes memory payload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 1000, owner); bytes32 salt = keccak256("compute-test"); @@ -308,7 +308,7 @@ contract FactoryModuleDeploymentTest is Test { assertTrue(predicted != address(0)); } - function test_computeAddressMatchesCreate() public { + function test_WhenComputingAddresses_ShouldMatchActualDeploymentAddress() public { bytes memory payload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 500, owner); bytes32 salt = keccak256("match-test"); @@ -317,9 +317,9 @@ contract FactoryModuleDeploymentTest is Test { assertEq(predicted, actual); } - // ============ Access Control ============ + // ============ when using non-allowlisted beacon ============ - function test_createRevertsForNonAllowlistedBeacon() public { + function test_RevertWhen_UsingNonAllowlistedBeacon() public { address fakeBeacon = address(0x999); bytes memory payload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 1000, owner); @@ -327,9 +327,9 @@ contract FactoryModuleDeploymentTest is Test { factory.create(fakeBeacon, payload, keccak256("bad-salt")); } - // ============ Full System Assembly via Factory ============ + // ============ when assembling full system ============ - function test_assembleFullSystemViaFactory() public { + function test_WhenAssemblingFullSystem_ShouldDeployAndWireAllModules() public { // 1. Deploy CycleModule bytes memory cyclePayload = abi.encodeWithSelector(AbstractCycleModule.initialize.selector, 1000, owner); address cycleAddr = factory.create(cycleModuleBeacon, cyclePayload, keccak256("sys-cycle")); diff --git a/test/FactoryModuleDeployment.tree b/test/FactoryModuleDeployment.tree new file mode 100644 index 0000000..106fc7a --- /dev/null +++ b/test/FactoryModuleDeployment.tree @@ -0,0 +1,26 @@ +FactoryModuleDeploymentTest +├── when creating a cycle module +│ └── it should deploy and initialize correctly +├── when creating an admin recipient registry +│ └── it should deploy and initialize correctly +├── when creating a recipient registry +│ └── it should deploy and initialize correctly +├── when creating a voting recipient registry +│ └── it should deploy and initialize correctly +├── when creating an equal distribution strategy +│ └── it should deploy and initialize correctly +├── when creating a voting distribution strategy +│ └── it should deploy and initialize correctly +├── when creating a basis points voting module +│ └── it should deploy and initialize correctly +├── when creating a base distribution manager +│ └── it should deploy and initialize correctly +├── when creating a multi strategy distribution manager +│ └── it should deploy and initialize correctly +├── when computing addresses +│ ├── it should return deterministic address +│ └── it should match actual deployment address +├── when using non-allowlisted beacon +│ └── it should revert +└── when assembling full system + └── it should deploy and wire all modules \ No newline at end of file diff --git a/test/MockDistributionManager.t.sol b/test/MockDistributionManager.t.sol index c91b5e7..7bd777d 100644 --- a/test/MockDistributionManager.t.sol +++ b/test/MockDistributionManager.t.sol @@ -13,19 +13,25 @@ contract MockDistributionManagerTest is Test { manager = new MockDistributionManagerSimple(); } - function test_InitialState() public view { + // ============ when checking initial state ============ + + function test_WhenCheckingInitialState_ShouldHaveCorrectDefaults() public view { assertEq(manager.BLOCKS_PER_CYCLE(), 200, "Blocks per cycle should be 200"); assertEq(manager.getLastDistributionBlock(), block.number, "Last distribution should be deployment block"); assertFalse(manager.isDistributionReady(), "Should not be ready immediately after deployment"); } - function test_IsDistributionReady_After200Blocks() public { + // ============ when checking distribution readiness ============ + + function test_WhenCheckingDistributionReadiness_ShouldReturnFalseBefore200Blocks() public { // Fast forward 199 blocks vm.roll(block.number + 199); assertFalse(manager.isDistributionReady(), "Should not be ready at 199 blocks"); + } + function test_WhenCheckingDistributionReadiness_ShouldReturnTrueAt200Blocks() public { // Fast forward to exactly 200 blocks - vm.roll(block.number + 1); + vm.roll(block.number + 200); assertTrue(manager.isDistributionReady(), "Should be ready at 200 blocks"); // Fast forward more @@ -33,24 +39,34 @@ contract MockDistributionManagerTest is Test { assertTrue(manager.isDistributionReady(), "Should still be ready after 200 blocks"); } - function test_BlocksUntilDistribution() public { + // ============ when querying blocks until distribution ============ + + function test_WhenQueryingBlocksUntilDistribution_ShouldReturn200AtStart() public view { assertEq(manager.blocksUntilDistribution(), 200, "Should be 200 blocks until distribution"); + } + function test_WhenQueryingBlocksUntilDistribution_ShouldReturn100AtHalfway() public { vm.roll(block.number + 100); assertEq(manager.blocksUntilDistribution(), 100, "Should be 100 blocks until distribution"); + } - vm.roll(block.number + 100); + function test_WhenQueryingBlocksUntilDistribution_ShouldReturn0WhenReady() public { + vm.roll(block.number + 200); assertEq(manager.blocksUntilDistribution(), 0, "Should be 0 blocks until distribution"); vm.roll(block.number + 50); assertEq(manager.blocksUntilDistribution(), 0, "Should still be 0 when overdue"); } - function test_ClaimAndDistribute() public { + // ============ when executing claimAndDistribute ============ + + function test_RevertWhen_ExecutingClaimAndDistribute_WhenNotReady() public { // Try to execute too early vm.expectRevert("Not ready"); manager.claimAndDistribute(); + } + function test_WhenExecutingClaimAndDistribute_ShouldExecuteAndResetTimer() public { // Fast forward 200 blocks vm.roll(block.number + 200); @@ -67,7 +83,9 @@ contract MockDistributionManagerTest is Test { assertEq(manager.blocksUntilDistribution(), 200, "Should be 200 blocks until next distribution"); } - function test_MultipleDistributions() public { + // ============ when running multiple distributions ============ + + function test_WhenRunningMultipleDistributions_ShouldTrackCycleProgression() public { uint256 startBlock = block.number; // First distribution diff --git a/test/MockDistributionManager.tree b/test/MockDistributionManager.tree new file mode 100644 index 0000000..d79dd20 --- /dev/null +++ b/test/MockDistributionManager.tree @@ -0,0 +1,15 @@ +MockDistributionManagerTest +├── when checking initial state +│ └── it should have correct defaults +├── when checking distribution readiness +│ ├── it should return false before 200 blocks +│ └── it should return true at 200 blocks +├── when querying blocks until distribution +│ ├── it should return 200 at start +│ ├── it should return 100 at halfway +│ └── it should return 0 when ready +├── when executing claimAndDistribute +│ ├── it should revert when not ready +│ └── it should execute and reset timer +└── when running multiple distributions + └── it should track cycle progression \ No newline at end of file diff --git a/test/RecipientRegistry.t.sol b/test/RecipientRegistry.t.sol index 43d8f28..a2ec0bc 100644 --- a/test/RecipientRegistry.t.sol +++ b/test/RecipientRegistry.t.sol @@ -18,12 +18,16 @@ contract RecipientRegistryTest is TestWrapper { registry.initialize(address(this)); } - function test_Initialize() public view { + function test_WhenInitializing() external view { + // it should set owner and zero recipients assertEq(registry.owner(), address(this)); assertEq(registry.getRecipientCount(), 0); } - function test_QueueRecipientAddition() public { + // ── when queuing a recipient for addition ── + + function test_GivenTheRecipientIsValid() external { + // it should add to queue and emit RecipientQueued vm.expectEmit(true, true, false, true); emit IRecipientRegistry.RecipientQueued(RECIPIENT_1, true); @@ -35,7 +39,8 @@ contract RecipientRegistryTest is TestWrapper { assertTrue(registry.isQueuedForAddition(RECIPIENT_1)); } - function test_QueueMultipleAdditions() public { + function test_GivenTheRecipientIsValid_ShouldSupportMultipleAdditions() external { + // it should support multiple additions registry.queueRecipientAddition(RECIPIENT_1); registry.queueRecipientAddition(RECIPIENT_2); registry.queueRecipientAddition(RECIPIENT_3); @@ -47,7 +52,70 @@ contract RecipientRegistryTest is TestWrapper { assertEq(queued[2], RECIPIENT_3); } - function test_ProcessQueueAdditions() public { + function test_RevertGiven_TheRecipientIsAddressZero() external { + // it should revert + vm.expectRevert(); + registry.queueRecipientAddition(address(0)); + } + + function test_RevertGiven_TheRecipientIsAlreadyActive() external { + // it should revert + registry.queueRecipientAddition(RECIPIENT_1); + registry.processQueue(); + + vm.expectRevert(); + registry.queueRecipientAddition(RECIPIENT_1); + } + + function test_GivenTheRecipientIsAlreadyQueued() external { + // it should revert with RecipientAlreadyQueued + registry.queueRecipientAddition(RECIPIENT_1); + + vm.expectRevert(IRecipientRegistry.RecipientAlreadyQueued.selector); + registry.queueRecipientAddition(RECIPIENT_1); + } + + // ── when queuing a recipient for removal ── + + function test_GivenTheRecipientIsActive() external { + // it should add to removal queue and emit RecipientQueued + // First add a recipient + registry.queueRecipientAddition(RECIPIENT_1); + registry.processQueue(); + + // Queue removal + vm.expectEmit(true, true, false, true); + emit IRecipientRegistry.RecipientQueued(RECIPIENT_1, false); + + registry.queueRecipientRemoval(RECIPIENT_1); + + address[] memory queued = registry.getQueuedRemovals(); + assertEq(queued.length, 1); + assertEq(queued[0], RECIPIENT_1); + assertTrue(registry.isQueuedForRemoval(RECIPIENT_1)); + } + + function test_RevertGiven_TheRecipientIsNotActive() external { + // it should revert + vm.expectRevert(); + registry.queueRecipientRemoval(RECIPIENT_1); + } + + function test_GivenTheRecipientIsAlreadyQueuedForRemoval() external { + // it should revert with RecipientAlreadyQueued + registry.queueRecipientAddition(RECIPIENT_1); + registry.processQueue(); + + registry.queueRecipientRemoval(RECIPIENT_1); + + vm.expectRevert(IRecipientRegistry.RecipientAlreadyQueued.selector); + registry.queueRecipientRemoval(RECIPIENT_1); + } + + // ── when processing the queue ── + + function test_GivenTheQueueHasAdditions() external { + // it should add recipients and emit events registry.queueRecipientAddition(RECIPIENT_1); registry.queueRecipientAddition(RECIPIENT_2); @@ -76,24 +144,8 @@ contract RecipientRegistryTest is TestWrapper { assertEq(registry.getQueuedAdditions().length, 0); } - function test_QueueRecipientRemoval() public { - // First add a recipient - registry.queueRecipientAddition(RECIPIENT_1); - registry.processQueue(); - - // Queue removal - vm.expectEmit(true, true, false, true); - emit IRecipientRegistry.RecipientQueued(RECIPIENT_1, false); - - registry.queueRecipientRemoval(RECIPIENT_1); - - address[] memory queued = registry.getQueuedRemovals(); - assertEq(queued.length, 1); - assertEq(queued[0], RECIPIENT_1); - assertTrue(registry.isQueuedForRemoval(RECIPIENT_1)); - } - - function test_ProcessQueueRemovals() public { + function test_GivenTheQueueHasRemovals() external { + // it should remove recipients and emit events // Add recipients registry.queueRecipientAddition(RECIPIENT_1); registry.queueRecipientAddition(RECIPIENT_2); @@ -130,7 +182,8 @@ contract RecipientRegistryTest is TestWrapper { assertEq(registry.getQueuedRemovals().length, 0); } - function test_ProcessQueueMixed() public { + function test_GivenTheQueueHasMixedOperations() external { + // it should process additions and removals together // Add initial recipients registry.queueRecipientAddition(RECIPIENT_1); registry.queueRecipientAddition(RECIPIENT_2); @@ -164,7 +217,16 @@ contract RecipientRegistryTest is TestWrapper { assertTrue(registry.isRecipient(RECIPIENT_4)); } - function test_ClearAdditionQueue() public { + function test_GivenTheQueueIsEmpty() external { + // it should not revert + registry.processQueue(); + assertEq(registry.getRecipientCount(), 0); + } + + // ── when clearing queues ── + + function test_WhenClearingTheAdditionQueue() external { + // it should remove all queued additions registry.queueRecipientAddition(RECIPIENT_1); registry.queueRecipientAddition(RECIPIENT_2); @@ -175,7 +237,28 @@ contract RecipientRegistryTest is TestWrapper { assertEq(registry.getQueuedAdditions().length, 0); } - function test_ReQueueRecipientAfterRemoval() public { + function test_WhenClearingTheRemovalQueue() external { + // it should remove all queued removals + // Add recipients first + registry.queueRecipientAddition(RECIPIENT_1); + registry.queueRecipientAddition(RECIPIENT_2); + registry.processQueue(); + + // Queue removals + registry.queueRecipientRemoval(RECIPIENT_1); + registry.queueRecipientRemoval(RECIPIENT_2); + + assertEq(registry.getQueuedRemovals().length, 2); + + registry.clearRemovalQueue(); + + assertEq(registry.getQueuedRemovals().length, 0); + } + + // ── when re-queuing ── + + function test_WhenRe_queuingAfterRemoval() external { + // it should allow re-adding a previously removed recipient registry.queueRecipientAddition(RECIPIENT_1); registry.processQueue(); @@ -197,7 +280,8 @@ contract RecipientRegistryTest is TestWrapper { assertEq(registry.getRecipientCount(), 1); } - function test_ReQueueRecipientAfterClearingAdditionQueue() public { + function test_WhenRe_queuingAfterClearing() external { + // it should allow re-adding after clearing addition queue registry.queueRecipientAddition(RECIPIENT_1); registry.clearAdditionQueue(); @@ -215,24 +299,32 @@ contract RecipientRegistryTest is TestWrapper { assertEq(registry.getRecipientCount(), 1); } - function test_ClearRemovalQueue() public { - // Add recipients first + // ── when checking queue independence ── + + function test_WhenCheckingQueueIndependence() external { + // it should keep addition and removal queues independent registry.queueRecipientAddition(RECIPIENT_1); - registry.queueRecipientAddition(RECIPIENT_2); registry.processQueue(); - // Queue removals + registry.queueRecipientAddition(RECIPIENT_2); registry.queueRecipientRemoval(RECIPIENT_1); - registry.queueRecipientRemoval(RECIPIENT_2); - assertEq(registry.getQueuedRemovals().length, 2); + assertTrue(registry.isQueuedForAddition(RECIPIENT_2)); + assertTrue(registry.isQueuedForRemoval(RECIPIENT_1)); + assertFalse(registry.isQueuedForAddition(RECIPIENT_1)); + assertFalse(registry.isQueuedForRemoval(RECIPIENT_2)); - registry.clearRemovalQueue(); + registry.processQueue(); - assertEq(registry.getQueuedRemovals().length, 0); + assertFalse(registry.isRecipient(RECIPIENT_1)); + assertTrue(registry.isRecipient(RECIPIENT_2)); + assertEq(registry.getRecipientCount(), 1); } - function test_GetRecipients() public { + // ── when getting recipients ── + + function test_WhenGettingRecipients() external { + // it should return all active recipients registry.queueRecipientAddition(RECIPIENT_1); registry.queueRecipientAddition(RECIPIENT_2); registry.queueRecipientAddition(RECIPIENT_3); @@ -245,61 +337,10 @@ contract RecipientRegistryTest is TestWrapper { assertEq(recipients[2], RECIPIENT_3); } - function test_RevertOnInvalidRecipient() public { - vm.expectRevert(); - registry.queueRecipientAddition(address(0)); - } - - function test_RevertOnDuplicateRecipient() public { - registry.queueRecipientAddition(RECIPIENT_1); - registry.processQueue(); - - vm.expectRevert(); - registry.queueRecipientAddition(RECIPIENT_1); - } - - function test_RevertOnDuplicateQueuedAddition() public { - registry.queueRecipientAddition(RECIPIENT_1); + // ── when checking access control ── - vm.expectRevert(IRecipientRegistry.RecipientAlreadyQueued.selector); - registry.queueRecipientAddition(RECIPIENT_1); - } - - function test_RevertOnRemovalOfNonExistent() public { - vm.expectRevert(); - registry.queueRecipientRemoval(RECIPIENT_1); - } - - function test_RevertOnDuplicateQueuedRemoval() public { - registry.queueRecipientAddition(RECIPIENT_1); - registry.processQueue(); - - registry.queueRecipientRemoval(RECIPIENT_1); - - vm.expectRevert(IRecipientRegistry.RecipientAlreadyQueued.selector); - registry.queueRecipientRemoval(RECIPIENT_1); - } - - function test_AdditionAndRemovalQueuesAreIndependent() public { - registry.queueRecipientAddition(RECIPIENT_1); - registry.processQueue(); - - registry.queueRecipientAddition(RECIPIENT_2); - registry.queueRecipientRemoval(RECIPIENT_1); - - assertTrue(registry.isQueuedForAddition(RECIPIENT_2)); - assertTrue(registry.isQueuedForRemoval(RECIPIENT_1)); - assertFalse(registry.isQueuedForAddition(RECIPIENT_1)); - assertFalse(registry.isQueuedForRemoval(RECIPIENT_2)); - - registry.processQueue(); - - assertFalse(registry.isRecipient(RECIPIENT_1)); - assertTrue(registry.isRecipient(RECIPIENT_2)); - assertEq(registry.getRecipientCount(), 1); - } - - function test_OnlyOwnerCanQueue() public { + function test_WhenCheckingAccessControl_ShouldOnlyAllowOwnerToQueue() external { + // it should only allow owner to queue vm.prank(address(0xdead)); vm.expectRevert(); registry.queueRecipientAddition(RECIPIENT_1); @@ -309,7 +350,8 @@ contract RecipientRegistryTest is TestWrapper { registry.queueRecipientRemoval(RECIPIENT_1); } - function test_OnlyOwnerCanClearQueues() public { + function test_WhenCheckingAccessControl_ShouldOnlyAllowOwnerToClearQueues() external { + // it should only allow owner to clear queues registry.queueRecipientAddition(RECIPIENT_1); vm.prank(address(0xdead)); @@ -321,7 +363,8 @@ contract RecipientRegistryTest is TestWrapper { registry.clearRemovalQueue(); } - function test_AnyoneCanProcessQueue() public { + function test_WhenCheckingAccessControl_ShouldAllowAnyoneToProcessQueue() external { + // it should allow anyone to process queue registry.queueRecipientAddition(RECIPIENT_1); vm.prank(address(0xdead)); @@ -330,13 +373,10 @@ contract RecipientRegistryTest is TestWrapper { assertTrue(registry.isRecipient(RECIPIENT_1)); } - function test_EmptyQueueProcess() public { - // Should not revert on empty queue - registry.processQueue(); - assertEq(registry.getRecipientCount(), 0); - } + // ── when performing large scale operations ── - function test_LargeScaleOperations() public { + function test_WhenPerformingLargeScaleOperations() external { + // it should handle 100 additions and 50 removals // Add many recipients uint256 count = 100; for (uint256 i = 1; i <= count; i++) { diff --git a/test/RecipientRegistry.tree b/test/RecipientRegistry.tree new file mode 100644 index 0000000..ea3c9ee --- /dev/null +++ b/test/RecipientRegistry.tree @@ -0,0 +1,47 @@ +RecipientRegistryTest +├── when initializing +│ └── it should set owner and zero recipients +├── when queuing a recipient for addition +│ ├── given the recipient is valid +│ │ ├── it should add to queue and emit RecipientQueued +│ │ └── it should support multiple additions +│ ├── given the recipient is address zero +│ │ └── it should revert +│ ├── given the recipient is already active +│ │ └── it should revert +│ └── given the recipient is already queued +│ └── it should revert with RecipientAlreadyQueued +├── when queuing a recipient for removal +│ ├── given the recipient is active +│ │ └── it should add to removal queue and emit RecipientQueued +│ ├── given the recipient is not active +│ │ └── it should revert +│ └── given the recipient is already queued for removal +│ └── it should revert with RecipientAlreadyQueued +├── when processing the queue +│ ├── given the queue has additions +│ │ └── it should add recipients and emit events +│ ├── given the queue has removals +│ │ └── it should remove recipients and emit events +│ ├── given the queue has mixed operations +│ │ └── it should process additions and removals together +│ └── given the queue is empty +│ └── it should not revert +├── when clearing the addition queue +│ └── it should remove all queued additions +├── when clearing the removal queue +│ └── it should remove all queued removals +├── when re-queuing after removal +│ └── it should allow re-adding a previously removed recipient +├── when re-queuing after clearing +│ └── it should allow re-adding after clearing addition queue +├── when checking queue independence +│ └── it should keep addition and removal queues independent +├── when getting recipients +│ └── it should return all active recipients +├── when checking access control +│ ├── it should only allow owner to queue +│ ├── it should only allow owner to clear queues +│ └── it should allow anyone to process queue +└── when performing large scale operations + └── it should handle 100 additions and 50 removals \ No newline at end of file diff --git a/test/TimeWeightedVotingPower.t.sol b/test/TimeWeightedVotingPower.t.sol index a00ce23..20e6431 100644 --- a/test/TimeWeightedVotingPower.t.sol +++ b/test/TimeWeightedVotingPower.t.sol @@ -68,26 +68,26 @@ contract TimeWeightedVotingPowerTest is Test { strategy = new TimeWeightedVotingPower(IVotesCheckpoints(address(token)), ICycleModule(address(cycleModule))); } - // ============ Constructor Tests ============ + // ============ when constructing ============ - function testConstructorSetsState() public view { + function test_WhenConstructing_ItShouldSetVotingTokenAndCycleModule() public view { assertEq(address(strategy.votingToken()), address(token)); assertEq(address(strategy.cycleModule()), address(cycleModule)); } - function testConstructorRevertsInvalidToken() public { + function test_RevertWhen_Constructing_InvalidToken() public { vm.expectRevert(TimeWeightedVotingPower.InvalidToken.selector); new TimeWeightedVotingPower(IVotesCheckpoints(address(0)), ICycleModule(address(cycleModule))); } - function testConstructorRevertsInvalidCycleModule() public { + function test_RevertWhen_Constructing_InvalidCycleModule() public { vm.expectRevert(TimeWeightedVotingPower.InvalidCycleModule.selector); new TimeWeightedVotingPower(IVotesCheckpoints(address(token)), ICycleModule(address(0))); } - // ============ Lossless Calculation Tests ============ + // ============ when calculating exact checkpoint integration ============ - function testExactCheckpointIntegration() public { + function test_WhenCalculatingExactCheckpointIntegration_ItShouldComputeCorrectTimeWeightedAverage() public { vm.roll(10); token.mint(user1, 1_000_000); @@ -107,7 +107,9 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 1_250_000); } - function testConstantBalanceFullCycle() public { + // ============ when balance is constant for full cycle ============ + + function test_WhenBalanceIsConstantForFullCycle_ItShouldReturnProportionalPower() public { // Mint early in cycle vm.roll(10); token.mint(user1, 100 ether); @@ -123,7 +125,9 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, expected); } - function testFlashLoanProtection() public { + // ============ when a flash loan occurs ============ + + function test_WhenFlashLoanOccurs_ItShouldDiluteFlashLoanPower() public { // User has held 1 ether for a long time vm.roll(10); token.mint(user1, 1 ether); @@ -149,7 +153,7 @@ contract TimeWeightedVotingPowerTest is Test { assertLt(power, 10 ether, "Flash loan should not give significant power"); } - function testFlashLoanExactMath() public { + function test_WhenFlashLoanOccurs_ItShouldComputeExactAreaUnderCurve() public { // Verify the exact area-under-curve for a flash loan scenario vm.roll(10); token.mint(user1, 10 ether); @@ -169,19 +173,25 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 19 ether); } - function testZeroBalanceReturnsZero() public { + // ============ when balance is zero ============ + + function test_WhenBalanceIsZero_ItShouldReturnZeroPower() public { vm.roll(200); uint256 power = strategy.getCurrentVotingPower(user1); assertEq(power, 0); } - function testPowerAtCycleStart() public { + // ============ when at cycle start ============ + + function test_WhenAtCycleStart_ItShouldReturnZeroPower() public { vm.roll(1); uint256 power = strategy.getCurrentVotingPower(user1); assertEq(power, 0, "Power should be 0 at cycle start"); } - function testGetVotingPowerForPeriod() public { + // ============ when querying power for a specific period ============ + + function test_WhenQueryingPowerForSpecificPeriod_ItShouldReturnCorrectPowerForFullPeriod() public { vm.roll(10); token.mint(user1, 100 ether); @@ -192,21 +202,21 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 100 ether); } - function testGetVotingPowerForPeriodRevertsStartAfterEnd() public { + function test_RevertWhen_QueryingPowerForSpecificPeriod_StartEqualsEnd() public { vm.roll(100); vm.expectRevert(TimeWeightedVotingPower.StartAfterEnd.selector); strategy.getVotingPowerForPeriod(user1, 50, 50); } - function testGetVotingPowerForPeriodRevertsFuturePeriod() public { + function test_RevertWhen_QueryingPowerForSpecificPeriod_FuturePeriod() public { vm.roll(100); vm.expectRevert(TimeWeightedVotingPower.FuturePeriod.selector); strategy.getVotingPowerForPeriod(user1, 50, 200); } - // ============ Checkpoint-walking edge cases ============ + // ============ when balance changes mid-period ============ - function testMidPeriodBalanceChange() public { + function test_WhenBalanceChangesMidPeriod_ItShouldComputeWeightedAverage() public { vm.roll(10); token.mint(user1, 50 ether); @@ -222,7 +232,9 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 75 ether); } - function testMultipleCheckpointsInPeriod() public { + // ============ when multiple checkpoints exist in period ============ + + function test_WhenMultipleCheckpointsExistInPeriod_ItShouldWalkAllCheckpointsCorrectly() public { vm.roll(10); token.mint(user1, 100 ether); @@ -242,18 +254,22 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 200 ether); } - function testCheckpointBeforePeriodStart() public { - // Token acquired well before the period — should count as constant + // ============ when checkpoint is before period start ============ + + function test_WhenCheckpointIsBeforePeriodStart_ItShouldUseBalanceAtPeriodStart() public { + // Token acquired well before the period -- should count as constant vm.roll(5); token.mint(user1, 100 ether); vm.roll(200); - // Period [100, 200) — user had 100 ether the whole time + // Period [100, 200) -- user had 100 ether the whole time uint256 power = strategy.getVotingPowerForPeriod(user1, 100, 200); assertEq(power, 100 ether); } - function testCheckpointAfterPeriodStart() public { + // ============ when checkpoint is after period start ============ + + function test_WhenCheckpointIsAfterPeriodStart_ItShouldWeightFromCheckpointOnward() public { // No tokens at period start, acquired mid-period vm.roll(50); token.mint(user1, 100 ether); @@ -267,7 +283,9 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 60 ether); } - function testBalanceDecreaseInPeriod() public { + // ============ when balance decreases in period ============ + + function test_WhenBalanceDecreasesInPeriod_ItShouldAccountForDecreasedBalance() public { vm.roll(10); token.mint(user1, 100 ether); @@ -285,9 +303,9 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 75 ether); } - // ============ Cycle Boundary Tests ============ + // ============ when crossing cycle boundaries ============ - function testCycleBoundaryHandling() public { + function test_WhenCrossingCycleBoundaries_ItShouldUseNewCycleStartAsPeriodStart() public { vm.roll(10); token.mint(user1, 100 ether); @@ -305,7 +323,7 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power, 100 ether); } - function testLookbackDerivedFromCycleLength() public { + function test_WhenCrossingCycleBoundaries_ItShouldReflectLookbackFromCycleLength() public { // getCurrentVotingPower uses cycle start, not a separate lookback // So the effective lookback is always from cycle start to now vm.roll(10); @@ -330,9 +348,9 @@ contract TimeWeightedVotingPowerTest is Test { assertGt(power2, 98 ether); } - // ============ Multiple Users ============ + // ============ when multiple users hold tokens ============ - function testMultipleUsersIndependent() public { + function test_WhenMultipleUsersHoldTokens_ItShouldCalculateIndependentPowerPerUser() public { vm.roll(10); token.mint(user1, 100 ether); @@ -353,9 +371,9 @@ contract TimeWeightedVotingPowerTest is Test { assertEq(power2, expected2); } - // ============ Gas ============ + // ============ when measuring gas ============ - function testGasWithFewCheckpoints() public { + function test_WhenMeasuringGas_ItShouldUseLowGasWithFewCheckpoints() public { vm.roll(10); token.mint(user1, 100 ether); diff --git a/test/TimeWeightedVotingPower.tree b/test/TimeWeightedVotingPower.tree new file mode 100644 index 0000000..31ae3c3 --- /dev/null +++ b/test/TimeWeightedVotingPower.tree @@ -0,0 +1,37 @@ +TimeWeightedVotingPowerTest +├── when constructing +│ ├── it should set voting token and cycle module +│ ├── it should revert with invalid token +│ └── it should revert with invalid cycle module +├── when calculating exact checkpoint integration +│ └── it should compute correct time-weighted average +├── when balance is constant for full cycle +│ └── it should return proportional power +├── when a flash loan occurs +│ ├── it should dilute flash loan power +│ └── it should compute exact area under curve +├── when balance is zero +│ └── it should return zero power +├── when at cycle start +│ └── it should return zero power +├── when querying power for a specific period +│ ├── it should return correct power for full period +│ ├── it should revert when start equals end +│ └── it should revert for future period +├── when balance changes mid-period +│ └── it should compute weighted average +├── when multiple checkpoints exist in period +│ └── it should walk all checkpoints correctly +├── when checkpoint is before period start +│ └── it should use balance at period start +├── when checkpoint is after period start +│ └── it should weight from checkpoint onward +├── when balance decreases in period +│ └── it should account for decreased balance +├── when crossing cycle boundaries +│ ├── it should use new cycle start as period start +│ └── it should reflect lookback from cycle length +├── when multiple users hold tokens +│ └── it should calculate independent power per user +└── when measuring gas + └── it should use low gas with few checkpoints \ No newline at end of file diff --git a/test/VotingModule.t.sol b/test/VotingModule.t.sol index 4442ab1..c3119b9 100644 --- a/test/VotingModule.t.sol +++ b/test/VotingModule.t.sol @@ -147,7 +147,19 @@ contract VotingModuleTest is Test { return abi.encodePacked(r, s, v); } - function testInitialization() public view { + // Helper functions + + function _createVoteDigest(address voter, uint256[] memory points, uint256 nonce) internal view returns (bytes32) { + bytes32 voteTypehash = keccak256("Vote(address voter,bytes32 pointsHash,uint256 nonce)"); + + bytes32 structHash = keccak256(abi.encode(voteTypehash, voter, keccak256(abi.encodePacked(points)), nonce)); + + return keccak256(abi.encodePacked("\x19\x01", votingModule.DOMAIN_SEPARATOR(), structHash)); + } + + // ============ when initializing ============ + + function test_WhenInitializing_ItShouldSetMaxPointsAndStrategies() public view { assertEq(votingModule.maxPoints(), MAX_POINTS); assertEq(cycleModule.getCurrentCycle(), 1); @@ -157,7 +169,9 @@ contract VotingModuleTest is Test { assertEq(address(votingModule.recipientRegistry()), address(recipientRegistry)); } - function testDirectVoting() public { + // ============ when casting a direct vote ============ + + function test_WhenCastingDirectVote_ItShouldRecordVoteAndUpdateDistribution() public { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -174,7 +188,9 @@ contract VotingModuleTest is Test { assertEq(projectDist.length, 3); } - function testSignatureVoting() public { + // ============ when casting a vote with signature ============ + + function test_WhenCastingVoteWithSignature_ItShouldVerifySignatureAndEmitVoteCast() public { uint256[] memory points = new uint256[](3); points[0] = 40; points[1] = 35; @@ -200,7 +216,9 @@ contract VotingModuleTest is Test { assertTrue(votingModule.isNonceUsed(voter1, nonce)); } - function testBatchVoting() public { + // ============ when casting batch votes ============ + + function test_WhenCastingBatchVotes_ItShouldRecordAllVotesAndEmitBatchVotesCast() public { address[] memory voters = new address[](2); voters[0] = voter1; voters[1] = voter2; @@ -242,7 +260,9 @@ contract VotingModuleTest is Test { assertTrue(votingModule.hasVotedInCurrentCycle(voter2), "Voter2 should have voted in current cycle"); } - function testVoteRecasting() public { + // ============ when recasting a vote ============ + + function test_WhenRecastingVote_ItShouldReplacePreviousVoteDistribution() public { uint256[] memory points1 = new uint256[](3); points1[0] = 50; points1[1] = 30; @@ -294,7 +314,9 @@ contract VotingModuleTest is Test { assertEq(dist2[2], (15 * votingPower * 1e18) / totalPoints2 / 1e18, "Third project should have new allocation"); } - function testNonceReplayProtection() public { + // ============ when replaying a nonce ============ + + function test_RevertWhen_ReplayingNonce_NonceAlreadyUsed() public { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -315,7 +337,9 @@ contract VotingModuleTest is Test { votingModule.castVoteWithSignature(voter1, points, nonce, signature); } - function testInvalidSignature() public { + // ============ when using an invalid signature ============ + + function test_RevertWhen_UsingInvalidSignature_InvalidSignature() public { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -333,7 +357,9 @@ contract VotingModuleTest is Test { votingModule.castVoteWithSignature(voter1, points, nonce, signature); } - function testZeroVotingPower() public { + // ============ when voting with zero power ============ + + function test_WhenVotingWithZeroPower_ItShouldRecordVoteWithZeroEffect() public { // Create account with no tokens uint256 noTokensVoterPrivateKey = 0x999; address noTokensVoter = vm.addr(noTokensVoterPrivateKey); @@ -354,7 +380,9 @@ contract VotingModuleTest is Test { ); } - function testExceedsMaxPoints() public { + // ============ when exceeding max points ============ + + function test_RevertWhen_ExceedingMaxPoints_ExceedsMaxPoints() public { uint256[] memory points = new uint256[](3); points[0] = MAX_POINTS + 1; // Exceeds max points[1] = 50; @@ -366,7 +394,9 @@ contract VotingModuleTest is Test { votingModule.castVoteWithSignature(voter1, points, nonce, signature); } - function testZeroVotePoints() public { + // ============ when all points are zero ============ + + function test_RevertWhen_AllPointsAreZero_ZeroVotePoints() public { uint256[] memory points = new uint256[](3); points[0] = 0; points[1] = 0; @@ -378,7 +408,9 @@ contract VotingModuleTest is Test { votingModule.castVoteWithSignature(voter1, points, nonce, signature); } - function testValidateSignature() public view { + // ============ when validating a signature ============ + + function test_WhenValidatingSignature_ItShouldReturnTrueForValidSignature() public view { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -393,12 +425,28 @@ contract VotingModuleTest is Test { // Should return true for valid signature assertTrue(votingModule.validateSignature(voter1, points, nonce, signature)); + } + + function test_WhenValidatingSignature_ItShouldReturnFalseForWrongVoter() public view { + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + + uint256 nonce = 1; + + // Create valid signature + bytes32 digest = _createVoteDigest(voter1, points, nonce); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(voter1PrivateKey, digest); + bytes memory signature = abi.encodePacked(r, s, v); // Should return false for wrong voter assertFalse(votingModule.validateSignature(voter2, points, nonce, signature)); } - function testGetVotingPower() public view { + // ============ when querying voting power ============ + + function test_WhenQueryingVotingPower_ItShouldReturnCorrectPowerPerTokenHoldings() public view { uint256 power = votingModule.getVotingPower(voter1); assertGt(power, 0); @@ -407,7 +455,9 @@ contract VotingModuleTest is Test { assertGt(power, power2); } - function testNewCycle() public { + // ============ when advancing to a new cycle ============ + + function test_WhenAdvancingToNewCycle_ItShouldResetVoteTrackingForNewCycle() public { // Vote in cycle 1 uint256[] memory points = new uint256[](3); points[0] = 50; @@ -440,7 +490,9 @@ contract VotingModuleTest is Test { assertEq(cycleModule.getCurrentCycle(), 2); } - function testNonceSkipping() public { + // ============ when skipping nonces ============ + + function test_WhenSkippingNonces_ItShouldAllowNonSequentialNonceUsage() public { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -463,14 +515,4 @@ contract VotingModuleTest is Test { assertFalse(votingModule.isNonceUsed(voter1, 3)); assertFalse(votingModule.isNonceUsed(voter1, 4)); } - - // Helper functions - - function _createVoteDigest(address voter, uint256[] memory points, uint256 nonce) internal view returns (bytes32) { - bytes32 voteTypehash = keccak256("Vote(address voter,bytes32 pointsHash,uint256 nonce)"); - - bytes32 structHash = keccak256(abi.encode(voteTypehash, voter, keccak256(abi.encodePacked(points)), nonce)); - - return keccak256(abi.encodePacked("\x19\x01", votingModule.DOMAIN_SEPARATOR(), structHash)); - } } diff --git a/test/VotingModule.tree b/test/VotingModule.tree new file mode 100644 index 0000000..d97367e --- /dev/null +++ b/test/VotingModule.tree @@ -0,0 +1,30 @@ +VotingModuleTest +├── when initializing +│ └── it should set max points and strategies +├── when casting a direct vote +│ └── it should record vote and update distribution +├── when casting a vote with signature +│ └── it should verify signature and emit VoteCast +├── when casting batch votes +│ └── it should record all votes and emit BatchVotesCast +├── when recasting a vote +│ └── it should replace the previous vote distribution +├── when replaying a nonce +│ └── it should revert with NonceAlreadyUsed +├── when using an invalid signature +│ └── it should revert with InvalidSignature +├── when voting with zero power +│ └── it should record vote with zero effect +├── when exceeding max points +│ └── it should revert with ExceedsMaxPoints +├── when all points are zero +│ └── it should revert with ZeroVotePoints +├── when validating a signature +│ ├── it should return true for valid signature +│ └── it should return false for wrong voter +├── when querying voting power +│ └── it should return correct power per token holdings +├── when advancing to a new cycle +│ └── it should reset vote tracking for new cycle +└── when skipping nonces + └── it should allow non-sequential nonce usage \ No newline at end of file diff --git a/test/VotingModuleSimple.t.sol b/test/VotingModuleSimple.t.sol index c7af800..5c28ed0 100644 --- a/test/VotingModuleSimple.t.sol +++ b/test/VotingModuleSimple.t.sol @@ -167,7 +167,18 @@ contract VotingModuleSimpleTest is Test { } } - function testInitialization() public view { + // Helper function + function _createVoteDigest(address voter, uint256[] memory points, uint256 nonce) internal view returns (bytes32) { + bytes32 voteTypehash = keccak256("Vote(address voter,bytes32 pointsHash,uint256 nonce)"); + + bytes32 structHash = keccak256(abi.encode(voteTypehash, voter, keccak256(abi.encodePacked(points)), nonce)); + + return keccak256(abi.encodePacked("\x19\x01", votingModule.DOMAIN_SEPARATOR(), structHash)); + } + + // ============ when initializing ============ + + function test_WhenInitializing_ItShouldSetMaxPointsAndStrategies() public view { assertEq(votingModule.maxPoints(), MAX_POINTS); assertEq(cycleModule.getCurrentCycle(), 1); @@ -176,7 +187,9 @@ contract VotingModuleSimpleTest is Test { assertEq(address(strategies[0]), address(tokenStrategy)); } - function testDirectVoting() public { + // ============ when casting a direct vote ============ + + function test_WhenCastingDirectVote_ItShouldRecordVoteAndUpdateDistribution() public { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -195,7 +208,9 @@ contract VotingModuleSimpleTest is Test { assertEq(projectDist.length, 3); } - function testSignatureVoting() public { + // ============ when casting a vote with signature ============ + + function test_WhenCastingVoteWithSignature_ItShouldVerifySignatureAndRecordVote() public { uint256[] memory points = new uint256[](3); points[0] = 40; points[1] = 35; @@ -222,7 +237,9 @@ contract VotingModuleSimpleTest is Test { assertTrue(votingModule.isNonceUsed(voter1, nonce)); } - function testNonceReplayProtection() public { + // ============ when replaying a nonce ============ + + function test_RevertWhen_ReplayingNonce_NonceAlreadyUsed() public { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -243,7 +260,9 @@ contract VotingModuleSimpleTest is Test { votingModule.castVoteWithSignature(voter1, points, nonce, signature); } - function testIncorrectRecipientCount() public { + // ============ when using incorrect recipient count ============ + + function test_RevertWhen_UsingIncorrectRecipientCount_TooFewPoints() public { // Try to vote with wrong number of points (2 instead of 3) uint256[] memory points = new uint256[](2); points[0] = 50; @@ -256,7 +275,9 @@ contract VotingModuleSimpleTest is Test { bytes memory signature = abi.encodePacked(r, s, v); vm.expectRevert(IVotingModule.InvalidPointsDistribution.selector); votingModule.castVoteWithSignature(voter1, points, nonce, signature); + } + function test_RevertWhen_UsingIncorrectRecipientCount_TooManyPoints() public { // Try with 4 points (too many) uint256[] memory points2 = new uint256[](4); points2[0] = 25; @@ -273,7 +294,9 @@ contract VotingModuleSimpleTest is Test { votingModule.castVoteWithSignature(voter1, points2, nonce2, signature2); } - function testValidRecipientCount() public { + // ============ when using valid recipient count ============ + + function test_WhenUsingValidRecipientCount_ItShouldAcceptAndRecordVote() public { // Vote with correct number of points (3 recipients) uint256[] memory points = new uint256[](3); points[0] = 50; @@ -295,7 +318,9 @@ contract VotingModuleSimpleTest is Test { assertEq(votingModule.getExpectedPointsLength(), 3); } - function testRecipientRegistryUpdate() public { + // ============ when recipient registry updates ============ + + function test_WhenRecipientRegistryUpdates_ItShouldAcceptVotesMatchingNewRecipientCount() public { // Add a new recipient address[] memory newRecipients = new address[](4); newRecipients[0] = address(0x1111); @@ -323,7 +348,9 @@ contract VotingModuleSimpleTest is Test { assertEq(projectDist.length, 4); } - function testInvalidSignature() public { + // ============ when using an invalid signature ============ + + function test_RevertWhen_UsingInvalidSignature_InvalidSignature() public { uint256[] memory points = new uint256[](3); points[0] = 50; points[1] = 30; @@ -340,13 +367,4 @@ contract VotingModuleSimpleTest is Test { vm.expectRevert(IVotingModule.InvalidSignature.selector); votingModule.castVoteWithSignature(voter1, points, nonce, signature); } - - // Helper function - function _createVoteDigest(address voter, uint256[] memory points, uint256 nonce) internal view returns (bytes32) { - bytes32 voteTypehash = keccak256("Vote(address voter,bytes32 pointsHash,uint256 nonce)"); - - bytes32 structHash = keccak256(abi.encode(voteTypehash, voter, keccak256(abi.encodePacked(points)), nonce)); - - return keccak256(abi.encodePacked("\x19\x01", votingModule.DOMAIN_SEPARATOR(), structHash)); - } } diff --git a/test/VotingModuleSimple.tree b/test/VotingModuleSimple.tree new file mode 100644 index 0000000..662b76a --- /dev/null +++ b/test/VotingModuleSimple.tree @@ -0,0 +1,18 @@ +VotingModuleSimpleTest +├── when initializing +│ └── it should set max points and strategies +├── when casting a direct vote +│ └── it should record vote and update distribution +├── when casting a vote with signature +│ └── it should verify signature and record vote +├── when replaying a nonce +│ └── it should revert with NonceAlreadyUsed +├── when using incorrect recipient count +│ ├── it should revert with too few points +│ └── it should revert with too many points +├── when using valid recipient count +│ └── it should accept and record vote +├── when recipient registry updates +│ └── it should accept votes matching new recipient count +└── when using an invalid signature + └── it should revert with InvalidSignature \ No newline at end of file diff --git a/test/VotingRecipientRegistry.t.sol b/test/VotingRecipientRegistry.t.sol index 0d7e68d..a13ef59 100644 --- a/test/VotingRecipientRegistry.t.sol +++ b/test/VotingRecipientRegistry.t.sol @@ -32,7 +32,8 @@ contract VotingRecipientRegistryTest is TestWrapper { registry.initialize(ADMIN, initial, 7 days); } - function test_Initialize() public view { + function test_WhenInitializing() external view { + // it should set admin recipients and expiry assertEq(registry.owner(), ADMIN); assertEq(registry.getRecipientCount(), 3); assertTrue(registry.isRecipient(RECIPIENT_1)); @@ -40,7 +41,9 @@ contract VotingRecipientRegistryTest is TestWrapper { assertTrue(registry.isRecipient(RECIPIENT_3)); } - function test_ProposeAddition() public { + function test_WhenProposingAnAddition() external { + // it should create proposal with auto-vote from proposer + // it should snapshot required votes at creation vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -63,7 +66,8 @@ contract VotingRecipientRegistryTest is TestWrapper { assertTrue(registry.hasVoted(proposalId, RECIPIENT_1)); } - function test_VoteOnProposal() public { + function test_WhenVotingOnAProposal() external { + // it should increment vote count vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -77,7 +81,8 @@ contract VotingRecipientRegistryTest is TestWrapper { assertTrue(registry.hasVoted(proposalId, RECIPIENT_2)); } - function test_UnanimousVoteExecutesAutomatically() public { + function test_WhenUnanimousVoteIsReached() external { + // it should auto-execute and queue recipient vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -108,7 +113,8 @@ contract VotingRecipientRegistryTest is TestWrapper { assertFalse(registry.isQueuedForAddition(NEW_RECIPIENT)); // No longer in queue } - function test_ManualExecuteProposal() public { + function test_WhenManuallyExecutingAProposal() external { + // it should queue recipient after enough votes // Add a fourth recipient first so we can test manual execution vm.prank(RECIPIENT_1); uint256 addProposal = registry.proposeAddition(NEW_RECIPIENT); @@ -145,7 +151,9 @@ contract VotingRecipientRegistryTest is TestWrapper { assertTrue(registry.isRecipient(address(0x99))); // Now added } - function test_ProposeRemoval() public { + function test_WhenProposingARemoval() external { + // it should create removal proposal + // it should require fewer votes than addition vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeRemoval(RECIPIENT_3); @@ -158,7 +166,8 @@ contract VotingRecipientRegistryTest is TestWrapper { assertEq(requiredVotes, 2); // Snapshotted at creation: 3 - 1 for removal } - function test_RemovalRequiresFewerVotes() public { + function test_WhenRemovalReachesThreshold() external { + // it should auto-execute and queue for removal vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeRemoval(RECIPIENT_3); @@ -186,7 +195,9 @@ contract VotingRecipientRegistryTest is TestWrapper { assertFalse(registry.isQueuedForRemoval(RECIPIENT_3)); // No longer queued } - function test_ProposalExpiry() public { + function test_WhenAProposalExpires() external { + // it should reject votes on expired proposals + // it should reject execution of expired proposals vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -205,13 +216,15 @@ contract VotingRecipientRegistryTest is TestWrapper { registry.executeProposal(proposalId); } - function test_RevertOnNonRecipientPropose() public { + function test_WhenANonRecipientProposes() external { + // it should revert with NotARecipient vm.prank(NON_RECIPIENT); vm.expectRevert(VotingRecipientRegistry.NotARecipient.selector); registry.proposeAddition(NEW_RECIPIENT); } - function test_RevertOnNonRecipientVote() public { + function test_WhenANonRecipientVotes() external { + // it should revert with NotEligibleVoter vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -220,7 +233,8 @@ contract VotingRecipientRegistryTest is TestWrapper { registry.vote(proposalId); } - function test_RevertOnDoubleVote() public { + function test_WhenDoubleVoting() external { + // it should revert with AlreadyVoted vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -229,13 +243,15 @@ contract VotingRecipientRegistryTest is TestWrapper { registry.vote(proposalId); } - function test_RevertOnInvalidProposal() public { + function test_WhenVotingOnInvalidProposal() external { + // it should revert with ProposalNotFound vm.prank(RECIPIENT_1); vm.expectRevert(VotingRecipientRegistry.ProposalNotFound.selector); registry.vote(999); } - function test_RevertOnExecutedProposal() public { + function test_WhenVotingOnExecutedProposal() external { + // it should revert with ProposalAlreadyExecuted // Create and execute a proposal vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -252,19 +268,22 @@ contract VotingRecipientRegistryTest is TestWrapper { registry.vote(proposalId); } - function test_RevertOnProposingExistingRecipient() public { + function test_WhenProposingExistingRecipient() external { + // it should revert with RecipientAlreadyExists vm.prank(RECIPIENT_1); vm.expectRevert(IRecipientRegistry.RecipientAlreadyExists.selector); registry.proposeAddition(RECIPIENT_2); } - function test_RevertOnRemovingNonExistent() public { + function test_WhenRemovingNonExistentRecipient() external { + // it should revert with RecipientNotFound vm.prank(RECIPIENT_1); vm.expectRevert(IRecipientRegistry.RecipientNotFound.selector); registry.proposeRemoval(NEW_RECIPIENT); } - function test_RevertOnNotEnoughVotes() public { + function test_WhenExecutingWithoutEnoughVotes() external { + // it should revert with NotEnoughVotes vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -276,7 +295,8 @@ contract VotingRecipientRegistryTest is TestWrapper { registry.executeProposal(proposalId); } - function test_NewRecipientCanVoteAfterAdded() public { + function test_WhenNewRecipientVotesAfterBeingAdded() external { + // it should allow new recipient to participate // Add new recipient vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); @@ -300,7 +320,8 @@ contract VotingRecipientRegistryTest is TestWrapper { assertEq(registry.getRequiredVotes(newProposalId), 4); } - function test_RevertOnEmptyInitialRecipients() public { + function test_WhenInitializingWithEmptyRecipients() external { + // it should revert with NoRecipients VotingRecipientRegistry newRegistry = new VotingRecipientRegistry(); address[] memory empty = new address[](0); @@ -308,12 +329,13 @@ contract VotingRecipientRegistryTest is TestWrapper { newRegistry.initialize(ADMIN, empty, 7 days); } - function test_ProposalExpiryConfiguration() public view { - // Test that proposal expiry is set correctly during initialization + function test_WhenConfiguringProposalExpiry_ShouldReturnConfiguredExpiry() external view { + // it should return configured expiry assertEq(registry.proposalExpiry(), 7 days); } - function test_SetProposalExpiry() public { + function test_WhenConfiguringProposalExpiry_ShouldAllowAdminToUpdateExpiry() external { + // it should allow admin to update expiry uint256 newExpiry = 3 days; vm.prank(ADMIN); @@ -324,7 +346,8 @@ contract VotingRecipientRegistryTest is TestWrapper { assertEq(registry.proposalExpiry(), newExpiry); } - function test_RevertOnInvalidProposalExpiryInitialize() public { + function test_RevertWhen_ConfiguringProposalExpiry_ZeroExpiryInInitialize() external { + // it should revert on zero expiry in initialize VotingRecipientRegistry newRegistry = new VotingRecipientRegistry(); address[] memory initial = new address[](1); initial[0] = RECIPIENT_1; @@ -333,19 +356,22 @@ contract VotingRecipientRegistryTest is TestWrapper { newRegistry.initialize(ADMIN, initial, 0); } - function test_RevertOnInvalidProposalExpiryUpdate() public { + function test_RevertWhen_ConfiguringProposalExpiry_ZeroExpiryInUpdate() external { + // it should revert on zero expiry in update vm.prank(ADMIN); vm.expectRevert(VotingRecipientRegistry.InvalidProposalExpiry.selector); registry.setProposalExpiry(0); } - function test_OnlyAdminCanSetProposalExpiry() public { + function test_RevertWhen_ConfiguringProposalExpiry_NonAdminSetsExpiry() external { + // it should only allow admin to set expiry vm.prank(RECIPIENT_1); vm.expectRevert(); registry.setProposalExpiry(3 days); } - function test_RequiredVotesUnchangedAfterRecipientSetChanges() public { + function test_WhenRequiredVotesAreSnapshotted() external { + // it should not change after recipient set changes // Create an addition proposal while there are 3 recipients (requires 3 votes) vm.prank(RECIPIENT_1); uint256 proposalId = registry.proposeAddition(NEW_RECIPIENT); diff --git a/test/VotingRecipientRegistry.tree b/test/VotingRecipientRegistry.tree new file mode 100644 index 0000000..82425d4 --- /dev/null +++ b/test/VotingRecipientRegistry.tree @@ -0,0 +1,48 @@ +VotingRecipientRegistryTest +├── when initializing +│ └── it should set admin recipients and expiry +├── when proposing an addition +│ ├── it should create proposal with auto-vote from proposer +│ └── it should snapshot required votes at creation +├── when voting on a proposal +│ └── it should increment vote count +├── when unanimous vote is reached +│ └── it should auto-execute and queue recipient +├── when manually executing a proposal +│ └── it should queue recipient after enough votes +├── when proposing a removal +│ ├── it should create removal proposal +│ └── it should require fewer votes than addition +├── when removal reaches threshold +│ └── it should auto-execute and queue for removal +├── when a proposal expires +│ ├── it should reject votes on expired proposals +│ └── it should reject execution of expired proposals +├── when a non-recipient proposes +│ └── it should revert with NotARecipient +├── when a non-recipient votes +│ └── it should revert with NotEligibleVoter +├── when double voting +│ └── it should revert with AlreadyVoted +├── when voting on invalid proposal +│ └── it should revert with ProposalNotFound +├── when voting on executed proposal +│ └── it should revert with ProposalAlreadyExecuted +├── when proposing existing recipient +│ └── it should revert with RecipientAlreadyExists +├── when removing non-existent recipient +│ └── it should revert with RecipientNotFound +├── when executing without enough votes +│ └── it should revert with NotEnoughVotes +├── when new recipient votes after being added +│ └── it should allow new recipient to participate +├── when initializing with empty recipients +│ └── it should revert with NoRecipients +├── when configuring proposal expiry +│ ├── it should return configured expiry +│ ├── it should allow admin to update expiry +│ ├── it should revert on zero expiry in initialize +│ ├── it should revert on zero expiry in update +│ └── it should only allow admin to set expiry +└── when required votes are snapshotted + └── it should not change after recipient set changes \ No newline at end of file diff --git a/test/VotingStreakNFTModule.t.sol b/test/VotingStreakNFTModule.t.sol index a9b2556..f493415 100644 --- a/test/VotingStreakNFTModule.t.sol +++ b/test/VotingStreakNFTModule.t.sol @@ -97,11 +97,9 @@ contract VotingStreakNFTModuleTest is Test { harness = VotingStreakBasisPointsModuleHarness(address(new ERC1967Proxy(address(impl), initData))); } - // ============ Test Cases ============ + // ============ when voting consecutively ============ - /// @notice Test: Consecutive votes in cycles 1, 2, 3 increment streak to 3 - /// @dev AAA Pattern: Arrange -> Act -> Assert - function test_ConsecutiveVotesIncrementStreak() public { + function test_WhenVotingConsecutively_ShouldIncrementStreak() public { // Arrange uint256[] memory points = new uint256[](2); points[0] = 50; @@ -134,9 +132,9 @@ contract VotingStreakNFTModuleTest is Test { assertEq(lastVoteCycle3, 3, "lastVoteCycle should be 3"); } - /// @notice Test: Streak break resets streak from 1 to 1 - /// @dev Voting in cycle 1, skipping cycle 2, voting in cycle 3 should reset streak - function test_StreakBreakResetsStreak() public { + // ============ when missing a cycle ============ + + function test_WhenMissingACycle_ShouldResetStreakToZero() public { // Arrange uint256[] memory points = new uint256[](2); points[0] = 50; @@ -160,9 +158,9 @@ contract VotingStreakNFTModuleTest is Test { assertEq(lastVoteCycle, 3, "lastVoteCycle should be 3"); } - /// @notice Test: 10 consecutive votes mints an NFT - /// @dev After reaching 10 consecutive votes, mockNft balance should be 1 - function test_TenVotesMintsNFT() public { + // ============ when reaching ten consecutive votes ============ + + function test_WhenReachingTenConsecutiveVotes_ShouldMintAnNFT() public { // Arrange uint256[] memory points = new uint256[](2); points[0] = 50; @@ -190,15 +188,9 @@ contract VotingStreakNFTModuleTest is Test { assertEq(lastVoteCycle, 10, "lastVoteCycle should be 10"); } + // ============ when recasting a vote in same cycle ============ - - - - - - /// @notice Test: Recasting vote in same cycle does not increment streak - /// @dev User votes twice in cycle 1; streak should remain 1 - function test_ReCastVoteDoesNotIncrementStreak() public { + function test_WhenRecastingAVoteInSameCycle_ShouldNotIncrementStreak() public { // Arrange uint256[] memory points = new uint256[](2); points[0] = 50; @@ -225,9 +217,9 @@ contract VotingStreakNFTModuleTest is Test { assertEq(lastVoteCycleAfterRecast, 1, "lastVoteCycle should still be 1"); } - /// @notice Test: Only owner can set NFT contract - /// @dev Non-owner should revert when calling setNFTContract - function test_RevertIf_NotOwnerSetsNft() public { + // ============ when non-owner sets NFT contract ============ + + function test_RevertWhen_NonOwnerSetsNFTContract() public { // Arrange address newNftAddress = address(0x9999); @@ -245,11 +237,9 @@ contract VotingStreakNFTModuleTest is Test { assertEq(address(retrievedNft), newNftAddress, "NFT contract should be updated by owner"); } - // ============ Additional Edge Case Tests ============ + // ============ when rebuilding a broken streak ============ - /// @notice Test: User can vote again after streak is properly reset through pattern - /// @dev Verifies that streak tracking works correctly across multiple reset cycles - function test_StreakResetAndRebuild() public { + function test_WhenRebuildingABrokenStreak_ShouldTrackNewStreakFromZero() public { // Arrange uint256[] memory points = new uint256[](2); points[0] = 50; @@ -281,9 +271,9 @@ contract VotingStreakNFTModuleTest is Test { assertEq(streak3, 5, "Streak should build to 5 again"); } - /// @notice Test: Multiple users can track streaks independently - /// @dev Verifies that streak state is properly isolated per user - function test_MultipleUsersIndependentStreaks() public { + // ============ when multiple users vote ============ + + function test_WhenMultipleUsersVote_ShouldTrackIndependentStreaks() public { // Arrange uint256[] memory points = new uint256[](2); points[0] = 50; @@ -317,5 +307,4 @@ contract VotingStreakNFTModuleTest is Test { assertEq(finalStreak1, 2, "user1 should maintain streak of 2"); assertEq(finalStreak2, 1, "user2 should have streak of 1"); } - } diff --git a/test/VotingStreakNFTModule.tree b/test/VotingStreakNFTModule.tree new file mode 100644 index 0000000..412d7e9 --- /dev/null +++ b/test/VotingStreakNFTModule.tree @@ -0,0 +1,15 @@ +VotingStreakNFTModuleTest +├── when voting consecutively +│ └── it should increment streak +├── when missing a cycle +│ └── it should reset streak to zero +├── when reaching ten consecutive votes +│ └── it should mint an NFT +├── when recasting a vote in same cycle +│ └── it should not increment streak +├── when non-owner sets NFT contract +│ └── it should revert +├── when rebuilding a broken streak +│ └── it should track new streak from zero +└── when multiple users vote + └── it should track independent streaks \ No newline at end of file diff --git a/test/automation/AutomationBase.t.sol b/test/automation/AutomationBase.t.sol index 3410d86..a8a7d35 100644 --- a/test/automation/AutomationBase.t.sol +++ b/test/automation/AutomationBase.t.sol @@ -95,20 +95,26 @@ contract AutomationBaseTest is Test { distributionManager.setAvailableYield(2000); } - function testChainlinkCheckUpkeep() public { + // ============ when checking Chainlink upkeep ============ + + function test_WhenCheckingChainlinkUpkeep_ShouldReturnFalseWhenTooSoon() public { // Initially should not need upkeep (too soon) (bool upkeepNeeded,) = chainlinkAutomation.checkUpkeep(""); assertFalse(upkeepNeeded); + } + function test_WhenCheckingChainlinkUpkeep_ShouldReturnTrueWhenReady() public { // Advance blocks vm.roll(block.number + 101); // Now should need upkeep - (upkeepNeeded,) = chainlinkAutomation.checkUpkeep(""); + (bool upkeepNeeded,) = chainlinkAutomation.checkUpkeep(""); assertTrue(upkeepNeeded); } - function testChainlinkPerformUpkeep() public { + // ============ when performing Chainlink upkeep ============ + + function test_WhenPerformingChainlinkUpkeep_ShouldExecuteDistributionAndEmitEvent() public { // Advance blocks to make distribution ready vm.roll(block.number + 101); @@ -128,67 +134,47 @@ contract AutomationBaseTest is Test { assertEq(distributionManager.currentCycleNumber(), 2); } - // function testGelatoChecker() public { - // // Initially should not be executable (too soon) - // (bool canExec, bytes memory execPayload) = gelatoAutomation.checker(); - // assertFalse(canExec); - - // // Advance blocks - // vm.roll(block.number + 101); - - // // Now should be executable - // (canExec, execPayload) = gelatoAutomation.checker(); - // assertTrue(canExec); - // assertGt(execPayload.length, 0); - // } - - // function testGelatoExecute() public { - // // Advance blocks to make distribution ready - // vm.roll(block.number + 101); - - // // Check if executable - // (bool canExec,) = gelatoAutomation.checker(); - // assertTrue(canExec); - - // // Execute - // vm.expectEmit(true, false, false, true); - // emit AutomationExecuted(gelatoExecutor, block.number); - - // vm.prank(gelatoExecutor); - // gelatoAutomation.execute(""); + // ============ when resolving distribution conditions ============ - // // Verify distribution was called - // assertEq(distributionModule.distributeCallCount(), 1); - // assertEq(distributionManager.currentCycleNumber(), 2); - // } - - function testResolveDistributionConditions() public { - // Test: Not enough blocks passed + function test_WhenResolvingDistributionConditions_ShouldFailWhenNotEnoughBlocksPassed() public { bool isReady = chainlinkAutomation.isDistributionReady(); assertFalse(isReady); + } + function test_WhenResolvingDistributionConditions_ShouldFailWhenNoVotes() public { vm.roll(block.number + 101); - // Test: No votes distributionManager.setCurrentVotes(0); - isReady = chainlinkAutomation.isDistributionReady(); + bool isReady = chainlinkAutomation.isDistributionReady(); assertFalse(isReady); + } + + function test_WhenResolvingDistributionConditions_ShouldFailWhenInsufficientYield() public { + vm.roll(block.number + 101); - // Test: Insufficient yield distributionManager.setCurrentVotes(100); distributionManager.setAvailableYield(500); - isReady = chainlinkAutomation.isDistributionReady(); + bool isReady = chainlinkAutomation.isDistributionReady(); assertFalse(isReady); + } + + function test_WhenResolvingDistributionConditions_ShouldFailWhenSystemDisabled() public { + vm.roll(block.number + 101); - // Test: System disabled + distributionManager.setCurrentVotes(100); distributionManager.setAvailableYield(2000); distributionManager.setEnabled(false); - isReady = chainlinkAutomation.isDistributionReady(); + bool isReady = chainlinkAutomation.isDistributionReady(); assertFalse(isReady); + } + + function test_WhenResolvingDistributionConditions_ShouldPassWhenAllConditionsMet() public { + vm.roll(block.number + 101); - // Test: All conditions met + distributionManager.setCurrentVotes(100); + distributionManager.setAvailableYield(2000); distributionManager.setEnabled(true); - isReady = chainlinkAutomation.isDistributionReady(); + bool isReady = chainlinkAutomation.isDistributionReady(); assertTrue(isReady); // Test automation data is returned when ready @@ -196,13 +182,17 @@ contract AutomationBaseTest is Test { assertGt(data.length, 0); } - function testExecutionRevertsWhenNotResolved() public { + // ============ when execution is not resolved ============ + + function test_RevertWhen_ExecutionIsNotResolved_ShouldRevertWithNotResolved() public { // Try to execute when conditions not met vm.expectRevert(AbstractAutomation.NotResolved.selector); chainlinkAutomation.executeDistribution(); } - function testCycleManagerIntegration() public { + // ============ when integrating with cycle manager ============ + + function test_WhenIntegratingWithCycleManager_ShouldAdvanceCycleAfterExecution() public { // Check initial state assertEq(distributionManager.currentCycleNumber(), 1); assertEq(distributionManager.currentVotes(), 100); @@ -219,7 +209,9 @@ contract AutomationBaseTest is Test { assertEq(distributionManager.getLastDistributionBlock(), block.number); } - function testCycleInfo() public { + // ============ when querying cycle info ============ + + function test_WhenQueryingCycleInfo_ShouldReturnCorrectCycleBoundaries() public { (uint256 cycleNum, uint256 startBlock, uint256 endBlock) = distributionManager.getCycleInfo(); assertEq(cycleNum, 1); assertEq(startBlock, block.number); @@ -236,37 +228,23 @@ contract AutomationBaseTest is Test { assertEq(endBlock, block.number + 100); } - // function testBothAutomationTypesWork() public { - // // Test Chainlink automation - // vm.roll(block.number + 101); - // distributionManager.setCurrentVotes(100); - // distributionManager.setAvailableYield(2000); - - // vm.prank(chainlinkKeeper); - // chainlinkAutomation.performUpkeep(""); - // assertEq(distributionModule.distributeCallCount(), 1); - - // // // Test Gelato automation - // // vm.roll(block.number + 101); - // // distributionManager.setCurrentVotes(100); - // // distributionManager.setAvailableYield(2000); - - // // vm.prank(gelatoExecutor); - // // gelatoAutomation.execute(""); - // // assertEq(distributionModule.distributeCallCount(), 2); - // } + // ============ when checking minimum yield ============ - function testMinYieldRequired() public { + function test_WhenCheckingMinimumYield_ShouldFailBelowMinimum() public { vm.roll(block.number + 101); // Set yield below minimum distributionManager.setAvailableYield(999); bool isReady = chainlinkAutomation.isDistributionReady(); assertFalse(isReady); + } + + function test_WhenCheckingMinimumYield_ShouldPassAtMinimum() public { + vm.roll(block.number + 101); // Set yield at minimum distributionManager.setAvailableYield(1000); - isReady = chainlinkAutomation.isDistributionReady(); + bool isReady = chainlinkAutomation.isDistributionReady(); assertTrue(isReady); } } diff --git a/test/automation/AutomationBase.tree b/test/automation/AutomationBase.tree new file mode 100644 index 0000000..6c46cd8 --- /dev/null +++ b/test/automation/AutomationBase.tree @@ -0,0 +1,21 @@ +AutomationBaseTest +├── when checking Chainlink upkeep +│ ├── it should return false when too soon +│ └── it should return true when ready +├── when performing Chainlink upkeep +│ └── it should execute distribution and emit event +├── when resolving distribution conditions +│ ├── it should fail when not enough blocks passed +│ ├── it should fail when no votes +│ ├── it should fail when insufficient yield +│ ├── it should fail when system disabled +│ └── it should pass when all conditions met +├── when execution is not resolved +│ └── it should revert with NotResolved +├── when integrating with cycle manager +│ └── it should advance cycle after execution +├── when querying cycle info +│ └── it should return correct cycle boundaries +└── when checking minimum yield + ├── it should fail below minimum + └── it should pass at minimum \ No newline at end of file