Skip to content

Commit

Permalink
Soloseng/ToB-audit-Insufficient-event-generation (#11285)
Browse files Browse the repository at this point in the history
* ++ events

* PR feedback

* updated & fixed tests
  • Loading branch information
soloseng authored Dec 4, 2024
1 parent 6fde928 commit 336a600
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 1 deletion.
18 changes: 18 additions & 0 deletions packages/protocol/contracts-0.8/common/EpochManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ contract EpochManager is
uint256 delegatedPayment
);

/**
* @notice Emitted when group is marked for processing.
* @param group Address of the group to be processed.
* @param epochNumber The epoch number for which the group gets marked for processing.
*/
event GroupMarkedForProcessing(address indexed group, uint256 indexed epochNumber);

/**
* @notice Emitted when group is processed.
* @param group Address of the processed group.
* @param epochNumber The epoch number for which the group gets processed.
*/
event GroupProcessed(address indexed group, uint256 indexed epochNumber);

/**
* @notice Throws if called by other than EpochManagerEnabler contract.
*/
Expand Down Expand Up @@ -245,6 +259,7 @@ contract EpochManager is
groupScore
);
processedGroups[group] = epochRewards == 0 ? type(uint256).max : epochRewards;
emit GroupMarkedForProcessing(group, currentEpochNumber);
}
}
}
Expand Down Expand Up @@ -291,6 +306,8 @@ contract EpochManager is
delete processedGroups[group];
toProcessGroups--;

emit GroupProcessed(group, currentEpochNumber);

if (toProcessGroups == 0) {
_finishEpochHelper(_epochProcessing, election);
}
Expand Down Expand Up @@ -348,6 +365,7 @@ contract EpochManager is
}

delete processedGroups[groups[i]];
emit GroupProcessed(groups[i], currentEpochNumber);
}

_finishEpochHelper(_epochProcessing, election);
Expand Down
16 changes: 16 additions & 0 deletions packages/protocol/contracts-0.8/common/FeeCurrencyDirectory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@ contract FeeCurrencyDirectory is
mapping(address => CurrencyConfig) public currencies;
address[] private currencyList;

/**
* @notice Emitted when currency config is set.
* @param token Address of the added currency token.
* @param oracle Address of the currency token oracle.
* @param intrinsicGas The intrinsic gas value for transactions.
*/
event CurrencyConfigSet(address indexed token, address indexed oracle, uint256 intrinsicGas);

/**
* @notice Emitted when currency is removed.
* @param token Address of the removed currency token.
*/
event CurrencyRemoved(address indexed token);

constructor(bool test) Initializable(test) {}

/**
Expand Down Expand Up @@ -43,6 +57,7 @@ contract FeeCurrencyDirectory is

currencies[token] = CurrencyConfig({ oracle: oracle, intrinsicGas: intrinsicGas });
currencyList.push(token);
emit CurrencyConfigSet(token, oracle, intrinsicGas);
}

/**
Expand All @@ -58,6 +73,7 @@ contract FeeCurrencyDirectory is
delete currencies[token];
currencyList[index] = currencyList[currencyList.length - 1];
currencyList.pop();
emit CurrencyRemoved(token);
}

/**
Expand Down
41 changes: 40 additions & 1 deletion packages/protocol/test-sol/unit/common/EpochManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ contract EpochManagerTest is Test, TestConstants, Utils08 {
address indexed beneficiary,
uint256 delegatedPayment
);

event EpochProcessingStarted(uint256 indexed epochNumber);
event EpochDurationSet(uint256 indexed newEpochDuration);
event OracleAddressSet(address indexed newOracleAddress);
event GroupMarkedForProcessing(address indexed group, uint256 indexed epochNumber);
event GroupProcessed(address indexed group, uint256 indexed epochNumber);

function setUp() public virtual {
epochManager = new EpochManager_WithMocks();
Expand Down Expand Up @@ -640,6 +641,23 @@ contract EpochManagerTest_finishNextEpochProcess is EpochManagerTest {
assertEq(newElected[i], afterElected[i]);
}
}

function test_Emits_GroupProcessedEvent() public {
(
address[] memory groups,
address[] memory lessers,
address[] memory greaters
) = getGroupsWithLessersAndGreaters();

epochManager.startNextEpochProcess();

for (uint i = 0; i < groups.length; i++) {
vm.expectEmit(true, true, true, true);
emit GroupProcessed(groups[i], firstEpochNumber);
}

epochManager.finishNextEpochProcess(groups, lessers, greaters);
}
}

contract EpochManagerTest_setToProcessGroups is EpochManagerTest {
Expand Down Expand Up @@ -707,6 +725,19 @@ contract EpochManagerTest_setToProcessGroups is EpochManagerTest {
assertEq(EpochManager(address(epochManager)).processedGroups(group), groupEpochRewards);
}
}

function test_Emits_GroupMarkedForProcessingEvent() public {
(address[] memory groups, , ) = getGroupsWithLessersAndGreaters();

epochManager.startNextEpochProcess();

for (uint i = 0; i < groups.length; i++) {
vm.expectEmit(true, true, true, true);
emit GroupMarkedForProcessing(groups[i], firstEpochNumber);
}

epochManager.setToProcessGroups();
}
}

contract EpochManagerTest_processGroup is EpochManagerTest {
Expand Down Expand Up @@ -829,6 +860,14 @@ contract EpochManagerTest_processGroup is EpochManagerTest {
assertEq(signers[i], afterSigners[i]);
}
}

function test_Emits_GroupProcessed() public {
epochManager.startNextEpochProcess();
epochManager.setToProcessGroups();
vm.expectEmit(true, true, true, true);
emit GroupProcessed(group, firstEpochNumber);
epochManager.processGroup(group, address(0), address(0));
}
}

contract EpochManagerTest_getEpochByNumber is EpochManagerTest {
Expand Down
19 changes: 19 additions & 0 deletions packages/protocol/test-sol/unit/common/FeeCurrencyDirectory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ contract FeeCurrencyDirectoryTestBase is Test {
MockOracle oracle;
address nonOwner;
address owner;
event CurrencyConfigSet(address indexed token, address indexed oracle, uint256 intrinsicGas);
event CurrencyRemoved(address indexed token);

function setUp() public virtual {
owner = address(this);
Expand All @@ -33,6 +35,16 @@ contract TestSetCurrencyConfig is FeeCurrencyDirectoryTestBase {
assertEq(config.intrinsicGas, intrinsicGas);
}

function test_Emits_CurrencyConfigSetEvent() public {
address token = address(1);
uint256 intrinsicGas = 21000;

vm.expectEmit(true, true, true, true);
emit CurrencyConfigSet(token, address(oracle), intrinsicGas);

directory.setCurrencyConfig(token, address(oracle), intrinsicGas);
}

function test_Reverts_WhenNonOwnerSetsCurrencyConfig() public {
address token = address(2);
uint256 intrinsicGas = 21000;
Expand Down Expand Up @@ -78,6 +90,13 @@ contract TestRemoveCurrencies is FeeCurrencyDirectoryTestBase {
assertEq(config.oracle, address(0));
}

function test_Emits_CurrencyRemovedEvent() public {
address token = address(4);
vm.expectEmit(true, true, true, true);
emit CurrencyRemoved(token);
directory.removeCurrencies(token, 0);
}

function test_Reverts_WhenNonOwnerRemovesCurrencies() public {
address token = address(4);
vm.prank(nonOwner);
Expand Down

0 comments on commit 336a600

Please sign in to comment.