diff --git a/contracts/src/v0.8/feeds/DualAggregator.sol b/contracts/src/v0.8/feeds/DualAggregator.sol index f166d401bbd..04ac7a90b0a 100644 --- a/contracts/src/v0.8/feeds/DualAggregator.sol +++ b/contracts/src/v0.8/feeds/DualAggregator.sol @@ -386,9 +386,8 @@ contract DualAggregator is OCR2Abstract, Ownable2StepMsgSender, AggregatorV2V3In /// @return the aggregatorRoundId of the next round. Note: The report for this round may have been /// transmitted (but not yet mined) *before* requestNewRound() was even called. There is *no* /// guarantee of causality between the request and the report at aggregatorRoundId. - // TODO: i think we can remove this function entirely function requestNewRound() external returns (uint80) { - if (!(msg.sender == owner() || s_requesterAccessController.hasAccess(msg.sender, msg.data))) { + if (msg.sender != owner() && !s_requesterAccessController.hasAccess(msg.sender, msg.data)) { revert OnlyOwnerAndRequesterCanCall(); } @@ -1158,7 +1157,7 @@ contract DualAggregator is OCR2Abstract, Ownable2StepMsgSender, AggregatorV2V3In uint24 accountingGas ) external { AccessControllerInterface access = s_billingAccessController; - if (!(msg.sender == owner() || access.hasAccess(msg.sender, msg.data))) { + if (msg.sender != owner() && !access.hasAccess(msg.sender, msg.data)) { revert OnlyOwnerAndBillingAdminCanCall(); } _payOracles(); @@ -1212,7 +1211,7 @@ contract DualAggregator is OCR2Abstract, Ownable2StepMsgSender, AggregatorV2V3In function withdrawPayment( address transmitter ) external { - if (!(msg.sender == s_payees[transmitter])) revert OnlyPayeeCanWithdraw(); + if (msg.sender != s_payees[transmitter]) revert OnlyPayeeCanWithdraw(); _payOracle(transmitter); } @@ -1307,7 +1306,7 @@ contract DualAggregator is OCR2Abstract, Ownable2StepMsgSender, AggregatorV2V3In /// @param amount maximum amount to withdraw, denominated in LINK-wei. /// @dev access control provided by billingAccessController function withdrawFunds(address recipient, uint256 amount) external { - if (!(msg.sender == owner() || s_billingAccessController.hasAccess(msg.sender, msg.data))) { + if (msg.sender != owner() && !s_billingAccessController.hasAccess(msg.sender, msg.data)) { revert OnlyOwnerAndBillingAdminCanCall(); } uint256 linkDue = _totalLinkDue(); @@ -1501,7 +1500,7 @@ contract DualAggregator is OCR2Abstract, Ownable2StepMsgSender, AggregatorV2V3In address payee = payees[i]; address currentPayee = s_payees[transmitter]; bool zeroedOut = currentPayee == address(0); - if (!(zeroedOut || currentPayee == payee)) revert PayeeAlreadySent(); + if (!zeroedOut && currentPayee != payee) revert PayeeAlreadySent(); s_payees[transmitter] = payee; if (currentPayee != payee) { diff --git a/contracts/src/v0.8/feeds/test/DualAggregator.t.sol b/contracts/src/v0.8/feeds/test/DualAggregator.t.sol index dcef6f9e0b9..47b461419fb 100644 --- a/contracts/src/v0.8/feeds/test/DualAggregator.t.sol +++ b/contracts/src/v0.8/feeds/test/DualAggregator.t.sol @@ -70,6 +70,10 @@ contract DualAggregatorHarness is DualAggregator { return _getSyncPrimaryRound(); } + function exposed_hotVars() external view returns (HotVars memory) { + return s_hotVars; + } + // helper function to define the latest round ids function setLatestRoundIds(uint32 _latestAggregatorRoundId, uint32 _latestSecondaryRoundId) public { s_hotVars.latestAggregatorRoundId = _latestAggregatorRoundId; @@ -504,8 +508,43 @@ contract GetRequesterAccessController is DualAggregatorBaseTest { } } -// TODO: determine if we need this method still -contract RequestNewRound is ConfiguredDualAggregatorBaseTest {} +contract RequestNewRound is ConfiguredDualAggregatorBaseTest { + event RoundRequested(address indexed requester, bytes32 configDigest, uint32 epoch, uint8 round); + + address internal constant USER = address(777); + + function test_RevertIf_NotRequester() public { + _changePrank(USER); + + vm.mockCall( + REQUESTER_ACCESS_CONTROLLER_ADDRESS, + abi.encodeWithSelector(AccessControllerInterface.hasAccess.selector, USER), + abi.encode(false) + ); + + vm.expectRevert(DualAggregator.OnlyOwnerAndRequesterCanCall.selector); + s_aggregator.requestNewRound(); + } + + function test_ShouldRequestNewRound() public { + uint256 currentRoundId = s_aggregator.exposed_hotVars().latestAggregatorRoundId; + uint40 currentEpochAndRound = s_aggregator.exposed_hotVars().latestEpochAndRound; + + vm.mockCall( + REQUESTER_ACCESS_CONTROLLER_ADDRESS, + abi.encodeWithSelector(AccessControllerInterface.hasAccess.selector, USER), + abi.encode(true) + ); + + _changePrank(USER); + + vm.expectEmit(); + emit RoundRequested(USER, s_configDigest, uint32(currentEpochAndRound >> 8), uint8(currentEpochAndRound)); + uint256 newRoundId = s_aggregator.requestNewRound(); + + assertEq(newRoundId, currentRoundId + 1, "round id not incremented"); + } +} contract Transmit is ConfiguredDualAggregatorBaseTest { uint32 constant CUTOFF_TIME = 40;