Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Soloseng/ToB-audit-Insufficient-event-generation #11285

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading