Data Streams v0.3 On-Chain Audit Feedback (#10658)

* Additional even emitting + sanity checks + version

* Update setConfigFromSource to force config count

* Cherry picked changes from bugfix/MERC-1618

* Wrappers + gas

* Fixed issue with getAvailableRewardPoolIds

* Update snapshot

* M-1 item addressed

* M-4 Remove constrain on adding or removing new reward recipients

---------

Co-authored-by: Michael Fletcher <[email protected]>
Co-authored-by: Michael Fletcher <[email protected]>

contracts/gas-snapshots/llo-feeds.gas-snapshot

@@ -99,7 +99,7 @@ RewardManagerClaimTest:test_eventIsEmittedUponClaim() (gas: 86069)
 RewardManagerClaimTest:test_eventIsNotEmittedUponUnsuccessfulClaim() (gas: 24700)
 RewardManagerClaimTest:test_recipientsClaimMultipleDeposits() (gas: 383214)
 RewardManagerClaimTest:test_singleRecipientClaimMultipleDeposits() (gas: 136289)
-RewardManagerNoRecipientSet:test_claimAllRecipientsAfterRecipientsSet() (gas: 489469)
+RewardManagerNoRecipientSet:test_claimAllRecipientsAfterRecipientsSet() (gas: 489377)
 RewardManagerPayRecipientsTest:test_addFundsToPoolAsNonOwnerOrFeeManager() (gas: 11428)
 RewardManagerPayRecipientsTest:test_addFundsToPoolAsOwner() (gas: 53876)
 RewardManagerPayRecipientsTest:test_payAllRecipients() (gas: 249472)
@@ -133,39 +133,31 @@ RewardManagerRecipientClaimMultiplePoolsTest:test_recipientsClaimMultipleDeposit
 RewardManagerRecipientClaimMultiplePoolsTest:test_singleRecipientClaimMultipleDeposits() (gas: 136328)
 RewardManagerRecipientClaimUnevenWeightTest:test_allRecipientsClaimingReceiveExpectedAmount() (gas: 198450)
 RewardManagerRecipientClaimUnevenWeightTest:test_allRecipientsClaimingReceiveExpectedAmountWithSmallDeposit() (gas: 218320)
-RewardManagerSetRecipientsTest:test_eventIsEmittedUponSetRecipients() (gas: 191821)
-RewardManagerSetRecipientsTest:test_setRecipientContainsDuplicateRecipients() (gas: 126091)
-RewardManagerSetRecipientsTest:test_setRewardRecipientFromManagerAddress() (gas: 193972)
+RewardManagerSetRecipientsTest:test_eventIsEmittedUponSetRecipients() (gas: 191729)
+RewardManagerSetRecipientsTest:test_setRecipientContainsDuplicateRecipients() (gas: 126058)
+RewardManagerSetRecipientsTest:test_setRewardRecipientFromManagerAddress() (gas: 193880)
 RewardManagerSetRecipientsTest:test_setRewardRecipientFromNonOwnerOrFeeManagerAddress() (gas: 21452)
-RewardManagerSetRecipientsTest:test_setRewardRecipientTwice() (gas: 193416)
-RewardManagerSetRecipientsTest:test_setRewardRecipientWeights() (gas: 180711)
+RewardManagerSetRecipientsTest:test_setRewardRecipientTwice() (gas: 193324)
+RewardManagerSetRecipientsTest:test_setRewardRecipientWeights() (gas: 180630)
 RewardManagerSetRecipientsTest:test_setRewardRecipientWithZeroAddress() (gas: 90224)
-RewardManagerSetRecipientsTest:test_setRewardRecipientWithZeroWeight() (gas: 183724)
-RewardManagerSetRecipientsTest:test_setRewardRecipients() (gas: 185681)
+RewardManagerSetRecipientsTest:test_setRewardRecipients() (gas: 185567)
 RewardManagerSetRecipientsTest:test_setRewardRecipientsIsEmpty() (gas: 87113)
-RewardManagerSetRecipientsTest:test_setSingleRewardRecipient() (gas: 110394)
+RewardManagerSetRecipientsTest:test_setSingleRewardRecipient() (gas: 110414)
 RewardManagerSetupTest:test_eventEmittedUponFeeManagerUpdate() (gas: 21388)
-RewardManagerSetupTest:test_eventEmittedUponFeePaid() (gas: 259213)
-RewardManagerSetupTest:test_rejectsZeroLinkAddressOnConstruction() (gas: 59432)
+RewardManagerSetupTest:test_eventEmittedUponFeePaid() (gas: 259121)
+RewardManagerSetupTest:test_rejectsZeroLinkAddressOnConstruction() (gas: 59411)
 RewardManagerSetupTest:test_setFeeManagerZeroAddress() (gas: 17038)
-RewardManagerUpdateRewardRecipientsMultiplePoolsTest:test_updatePrimaryRecipientWeights() (gas: 373719)
-RewardManagerUpdateRewardRecipientsTest:test_eventIsEmittedUponUpdateRecipients() (gas: 279303)
-RewardManagerUpdateRewardRecipientsTest:test_onlyAdminCanUpdateRecipients() (gas: 19749)
-RewardManagerUpdateRewardRecipientsTest:test_partialUpdateRecipientWeights() (gas: 218995)
-RewardManagerUpdateRewardRecipientsTest:test_updateAllRecipientsWithSameAddressAndWeight() (gas: 273125)
-RewardManagerUpdateRewardRecipientsTest:test_updatePartialRecipientsToSubset() (gas: 245331)
-RewardManagerUpdateRewardRecipientsTest:test_updatePartialRecipientsWithExcessiveWeight() (gas: 259403)
-RewardManagerUpdateRewardRecipientsTest:test_updatePartialRecipientsWithSameAddressAndWeight() (gas: 149004)
-RewardManagerUpdateRewardRecipientsTest:test_updatePartialRecipientsWithUnderWeightSet() (gas: 259477)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientWeights() (gas: 369200)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientWithNewZeroAddress() (gas: 258619)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsContainsDuplicateRecipients() (gas: 288757)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsToDifferentLargerSet() (gas: 261428)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsToDifferentPartialSet() (gas: 259406)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsToDifferentSet() (gas: 260816)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsToDifferentSetWithInvalidWeights() (gas: 259546)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsUpdateAndRemoveExistingForLargerSet() (gas: 251938)
-RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsUpdateAndRemoveExistingForSmallerSet() (gas: 250223)
+RewardManagerUpdateRewardRecipientsMultiplePoolsTest:test_updatePrimaryRecipientWeights() (gas: 373535)
+RewardManagerUpdateRewardRecipientsTest:test_eventIsEmittedUponUpdateRecipients() (gas: 279096)
+RewardManagerUpdateRewardRecipientsTest:test_onlyAdminCanUpdateRecipients() (gas: 19704)
+RewardManagerUpdateRewardRecipientsTest:test_partialUpdateRecipientWeights() (gas: 218880)
+RewardManagerUpdateRewardRecipientsTest:test_updateAllRecipientsWithSameAddressAndWeight() (gas: 272941)
+RewardManagerUpdateRewardRecipientsTest:test_updatePartialRecipientsWithExcessiveWeight() (gas: 259196)
+RewardManagerUpdateRewardRecipientsTest:test_updatePartialRecipientsWithSameAddressAndWeight() (gas: 148912)
+RewardManagerUpdateRewardRecipientsTest:test_updatePartialRecipientsWithUnderWeightSet() (gas: 259270)
+RewardManagerUpdateRewardRecipientsTest:test_updateRecipientWeights() (gas: 368972)
+RewardManagerUpdateRewardRecipientsTest:test_updateRecipientWithNewZeroAddress() (gas: 270802)
+RewardManagerUpdateRewardRecipientsTest:test_updateRecipientsContainsDuplicateRecipients() (gas: 288572)
 VerificationdeactivateConfigWhenThereAreMultipleDigestsTest:test_correctlyRemovesAMiddleDigest() (gas: 24177)
 VerificationdeactivateConfigWhenThereAreMultipleDigestsTest:test_correctlyRemovesTheFirstDigest() (gas: 24144)
 VerificationdeactivateConfigWhenThereAreMultipleDigestsTest:test_correctlyUnsetsDigestsInSequence() (gas: 44109)

contracts/src/v0.8/llo-feeds/dev/RewardManager.sol

@@ -104,7 +104,9 @@ contract RewardManager is IRewardManager, ConfirmedOwner, TypeAndVersionInterfac
     uint256 totalFeeAmount;
     for (uint256 i; i < payments.length; ++i) {
       unchecked {
-        //the total amount for any ERC20 asset cannot exceed 2^256 - 1
+        //the total amount for any ERC-20 asset cannot exceed 2^256 - 1
+        //see https://github.com/OpenZeppelin/openzeppelin-contracts/blob/36bf1e46fa811f0f07d38eb9cfbc69a955f300ce/contracts/token/ERC20/ERC20.sol#L266
+        //for example implementation.
         s_totalRewardRecipientFees[payments[i].poolId] += payments[i].amount;
 
         //tally the total payable fees
@@ -210,8 +212,6 @@ contract RewardManager is IRewardManager, ConfirmedOwner, TypeAndVersionInterfac
 
       //ensure the reward recipient address is not zero
       if (recipientAddress == address(0)) revert InvalidAddress();
-      //ensure the weight is not zero
-      if (recipientWeight == 0) revert InvalidWeights();
 
       //save/overwrite the weight for the reward recipient
       s_rewardRecipientWeights[poolId][recipientAddress] = recipientWeight;
@@ -243,9 +243,6 @@ contract RewardManager is IRewardManager, ConfirmedOwner, TypeAndVersionInterfac
       //get the existing weight
       uint256 existingWeight = s_rewardRecipientWeights[poolId][recipientAddress];
 
-      //if the existing weight is 0, the recipient isn't part of this configuration
-      if (existingWeight == 0) revert InvalidAddress();
-
       //if a recipient is updated, the rewards must be claimed first as they can't claim previous fees at the new weight
       _claimRewards(newRewardRecipients[i].addr, poolIds);
 

contracts/src/v0.8/llo-feeds/test/reward-manager/RewardManager.setRecipients.t.sol

@@ -32,24 +32,6 @@ contract RewardManagerSetRecipientsTest is BaseRewardManagerTest {
     setRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
   }
 
-  function test_setRewardRecipientWithZeroWeight() public {
-    //array of recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](5);
-
-    //init each recipient with even weights
-    recipients[0] = Common.AddressAndWeight(DEFAULT_RECIPIENT_1, ONE_PERCENT * 25);
-    recipients[1] = Common.AddressAndWeight(DEFAULT_RECIPIENT_2, ONE_PERCENT * 25);
-    recipients[2] = Common.AddressAndWeight(DEFAULT_RECIPIENT_3, ONE_PERCENT * 25);
-    recipients[3] = Common.AddressAndWeight(DEFAULT_RECIPIENT_4, ONE_PERCENT * 25);
-    recipients[4] = Common.AddressAndWeight(DEFAULT_RECIPIENT_5, 0);
-
-    //Zero weight should fail
-    vm.expectRevert(INVALID_WEIGHT_ERROR_SELECTOR);
-
-    //set the recipients
-    setRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
   function test_setRewardRecipientWithZeroAddress() public {
     //array of recipients
     Common.AddressAndWeight[] memory recipients = getPrimaryRecipients();

contracts/src/v0.8/llo-feeds/test/reward-manager/RewardManager.updateRewardRecipients.t.sol

@@ -106,146 +106,6 @@ contract RewardManagerUpdateRewardRecipientsTest is BaseRewardManagerTest {
     updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
   }
 
-  function test_updateRecipientsToDifferentSet() public {
-    //create a list of containing recipients from the primary configured set, and new recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](getPrimaryRecipients().length + 4);
-    for (uint256 i; i < getPrimaryRecipients().length; i++) {
-      //copy the recipient and set the weight to 0 which implies the recipient is being replaced
-      recipients[i] = Common.AddressAndWeight(getPrimaryRecipients()[i].addr, 0);
-    }
-
-    //add the new recipients individually
-    recipients[4] = Common.AddressAndWeight(DEFAULT_RECIPIENT_5, ONE_PERCENT * 25);
-    recipients[5] = Common.AddressAndWeight(DEFAULT_RECIPIENT_6, ONE_PERCENT * 25);
-    recipients[6] = Common.AddressAndWeight(DEFAULT_RECIPIENT_7, ONE_PERCENT * 25);
-    recipients[7] = Common.AddressAndWeight(DEFAULT_RECIPIENT_8, ONE_PERCENT * 25);
-
-    //should revert as it's not possible to new recipients
-    vm.expectRevert(INVALID_ADDRESS_ERROR_SELECTOR);
-
-    //updating a recipient should force the funds to be paid out for the primary recipients
-    updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
-  function test_updateRecipientsToDifferentPartialSet() public {
-    //create a list of containing recipients from the primary configured set, and new recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](getPrimaryRecipients().length + 2);
-    for (uint256 i; i < getPrimaryRecipients().length; i++) {
-      //copy the recipient and set the weight to 0 which implies the recipient is being replaced
-      recipients[i] = Common.AddressAndWeight(getPrimaryRecipients()[i].addr, 0);
-    }
-
-    //add the new recipients individually
-    recipients[4] = Common.AddressAndWeight(DEFAULT_RECIPIENT_5, FIFTY_PERCENT);
-    recipients[5] = Common.AddressAndWeight(DEFAULT_RECIPIENT_6, FIFTY_PERCENT);
-
-    //should revert as it's not possible to add new recipients
-    vm.expectRevert(INVALID_ADDRESS_ERROR_SELECTOR);
-
-    //updating a recipient should force the funds to be paid out for the primary recipients
-    updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
-  function test_updateRecipientsToDifferentLargerSet() public {
-    //create a list of containing recipients from the primary configured set, and new recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](getPrimaryRecipients().length + 5);
-    for (uint256 i; i < getPrimaryRecipients().length; i++) {
-      //copy the recipient and set the weight to 0 which implies the recipient is being replaced
-      recipients[i] = Common.AddressAndWeight(getPrimaryRecipients()[i].addr, 0);
-    }
-
-    //add the new recipients individually
-    recipients[4] = Common.AddressAndWeight(DEFAULT_RECIPIENT_5, TEN_PERCENT * 2);
-    recipients[5] = Common.AddressAndWeight(DEFAULT_RECIPIENT_6, TEN_PERCENT * 2);
-    recipients[6] = Common.AddressAndWeight(DEFAULT_RECIPIENT_7, TEN_PERCENT * 2);
-    recipients[7] = Common.AddressAndWeight(DEFAULT_RECIPIENT_8, TEN_PERCENT * 2);
-    recipients[8] = Common.AddressAndWeight(DEFAULT_RECIPIENT_9, TEN_PERCENT * 2);
-
-    //should revert as changing the recipient set is not allowed
-    vm.expectRevert(INVALID_ADDRESS_ERROR_SELECTOR);
-
-    //updating a recipient should force the funds to be paid out for the primary recipients
-    updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
-  function test_updateRecipientsUpdateAndRemoveExistingForLargerSet() public {
-    //create a list of containing recipients from the primary configured set, and new recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](9);
-
-    //update the existing recipients
-    recipients[0] = Common.AddressAndWeight(getPrimaryRecipients()[0].addr, 0);
-    recipients[1] = Common.AddressAndWeight(getPrimaryRecipients()[1].addr, 0);
-    recipients[2] = Common.AddressAndWeight(getPrimaryRecipients()[2].addr, TEN_PERCENT * 3);
-    recipients[3] = Common.AddressAndWeight(getPrimaryRecipients()[3].addr, TEN_PERCENT * 3);
-
-    //add the new recipients individually
-    recipients[4] = Common.AddressAndWeight(DEFAULT_RECIPIENT_5, TEN_PERCENT);
-    recipients[5] = Common.AddressAndWeight(DEFAULT_RECIPIENT_6, TEN_PERCENT);
-    recipients[6] = Common.AddressAndWeight(DEFAULT_RECIPIENT_7, TEN_PERCENT);
-    recipients[7] = Common.AddressAndWeight(DEFAULT_RECIPIENT_8, TEN_PERCENT);
-    recipients[8] = Common.AddressAndWeight(DEFAULT_RECIPIENT_9, TEN_PERCENT);
-
-    //should revert as it's not possible to add new recipients
-    vm.expectRevert(INVALID_ADDRESS_ERROR_SELECTOR);
-
-    //updating a recipient should force the funds to be paid out for the primary recipients
-    updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
-  function test_updateRecipientsUpdateAndRemoveExistingForSmallerSet() public {
-    //create a list of containing recipients from the primary configured set, and new recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](5);
-
-    //update the existing recipients
-    recipients[0] = Common.AddressAndWeight(getPrimaryRecipients()[0].addr, 0);
-    recipients[1] = Common.AddressAndWeight(getPrimaryRecipients()[1].addr, 0);
-    recipients[2] = Common.AddressAndWeight(getPrimaryRecipients()[2].addr, TEN_PERCENT * 3);
-    recipients[3] = Common.AddressAndWeight(getPrimaryRecipients()[3].addr, TEN_PERCENT * 2);
-
-    //add the new recipients individually
-    recipients[4] = Common.AddressAndWeight(DEFAULT_RECIPIENT_5, TEN_PERCENT * 5);
-
-    //should revert as it's not possible to add new recipients
-    vm.expectRevert(INVALID_ADDRESS_ERROR_SELECTOR);
-
-    //updating a recipient should force the funds to be paid out for the primary recipients
-    updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
-  function test_updateRecipientsToDifferentSetWithInvalidWeights() public {
-    //create a list of containing recipients from the primary configured set, and new recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](getPrimaryRecipients().length + 2);
-    for (uint256 i; i < getPrimaryRecipients().length; i++) {
-      //copy the recipient and set the weight to 0 which implies the recipient is being replaced
-      recipients[i] = Common.AddressAndWeight(getPrimaryRecipients()[i].addr, 0);
-    }
-
-    //add the new recipients individually
-    recipients[4] = Common.AddressAndWeight(DEFAULT_RECIPIENT_5, TEN_PERCENT * 5);
-    recipients[5] = Common.AddressAndWeight(DEFAULT_RECIPIENT_6, TEN_PERCENT);
-
-    //should revert as the addresses are not in the configured set
-    vm.expectRevert(INVALID_ADDRESS_ERROR_SELECTOR);
-
-    //updating a recipient should force the funds to be paid out for the primary recipients
-    updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
-  function test_updatePartialRecipientsToSubset() public {
-    //create a list of containing recipients from the primary configured set, and new recipients
-    Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](4);
-    recipients[0] = Common.AddressAndWeight(DEFAULT_RECIPIENT_1, 0);
-    recipients[1] = Common.AddressAndWeight(DEFAULT_RECIPIENT_2, 0);
-    recipients[2] = Common.AddressAndWeight(DEFAULT_RECIPIENT_3, TEN_PERCENT * 5);
-    recipients[3] = Common.AddressAndWeight(DEFAULT_RECIPIENT_4, TEN_PERCENT * 5);
-
-    //should revert as changing the recipients is not allowed
-    vm.expectRevert(INVALID_WEIGHT_ERROR_SELECTOR);
-
-    //updating a recipient should force the funds to be paid out for the primary recipients
-    updateRewardRecipients(PRIMARY_POOL_ID, recipients, ADMIN);
-  }
-
   function test_updatePartialRecipientsWithUnderWeightSet() public {
     //create a list of containing recipients from the primary configured set, and new recipients
     Common.AddressAndWeight[] memory recipients = new Common.AddressAndWeight[](4);