No service fee on penalties (#424)

With this PR, if a solver receives a negative reward for an accounting
week, that amount is not multiplied by `reward_scaling`.

Before the change, ~negative~ rewards were multiplied by
`reward_scaling` independent of the sign of the reward. For positive
rewards, this does what it should, as a fraction of the reward is
withheld from the solver and becomes part of a bounty budget. For
negative rewards, the calculation implied that part of the penalty is
withheld and it reduces the bounty budget.

With the change, negative _batch_ rewards are not multiplied by
`reward_scaling`. Quote rewards are left as is.

The proposed implementation changes some code in 3 places. Ideally there
would only be one change required. Note, that quote rewards are always
non-negative and no change in the formula is required.

Alternatively, the total cow reward could be used to decide on the
scaling, instead of handling batch and quote rewards separately.

Tests have not been adapted yet.

---------

Co-authored-by: Haris Angelidakis <[email protected]>

src/fetch/payouts.py

@@ -136,11 +136,19 @@ def total_outgoing_eth(self) -> int:
 
     def total_cow_reward(self) -> int:
         """Total outgoing COW token reward"""
-        return int(self.reward_scaling() * self.primary_reward_cow)
+        return (
+            int(self.reward_scaling() * self.primary_reward_cow)
+            if self.primary_reward_cow > 0
+            else self.primary_reward_cow
+        )
 
     def total_eth_reward(self) -> int:
         """Total outgoing ETH reward"""
-        return int(self.reward_scaling() * self.primary_reward_eth)
+        return (
+            int(self.reward_scaling() * self.primary_reward_eth)
+            if self.primary_reward_eth > 0
+            else self.primary_reward_eth
+        )
 
     def reward_scaling(self) -> Fraction:
         """Scaling factor for service fee
@@ -149,7 +157,9 @@ def reward_scaling(self) -> Fraction:
 
     def total_service_fee(self) -> Fraction:
         """Total service fee charged from rewards"""
-        return self.service_fee * (self.primary_reward_cow + self.quote_reward_cow)
+        return self.service_fee * (
+            max(self.primary_reward_cow, 0) + self.quote_reward_cow
+        )
 
     def is_overdraft(self) -> bool:
         """

tests/unit/test_payouts.py

@@ -605,6 +605,17 @@ def test_performance_reward_service_fee(self):
             num_quotes=num_quotes,
             service_fee=service_fee,
         )
+
+        self.assertTrue(
+            test_datum.total_cow_reward(),
+            test_datum.total_eth_reward() * self.conversion_rate,
+        )  # ensure consistency of cow and eth batch rewards
+        self.assertEqual(
+            test_datum.total_service_fee(),
+            int(
+                primary_reward * self.conversion_rate * service_fee
+            ),  # only quote rewards enter
+        )
         self.assertFalse(test_datum.is_overdraft())
         self.assertEqual(
             test_datum.as_payouts(),
@@ -621,26 +632,116 @@ def test_performance_reward_service_fee(self):
     def test_quote_reward_service_fee(self):
         """Sevice fee reduces COW reward."""
         primary_reward, num_quotes, service_fee = 0, 100, Fraction(15, 100)
+        reward_per_quote = 6 * 10**18
+
         test_datum = self.sample_record(
             primary_reward=primary_reward,
             slippage=0,
             num_quotes=num_quotes,
             service_fee=service_fee,
         )
+
+        self.assertEqual(
+            test_datum.total_service_fee(),
+            int(
+                reward_per_quote * num_quotes * service_fee
+            ),  # only quote rewards enter
+        )
+        self.assertFalse(test_datum.is_overdraft())
+        self.assertEqual(
+            test_datum.as_payouts(),
+            [
+                Transfer(
+                    token=self.cow_token,
+                    recipient=self.reward_target,
+                    amount_wei=int(reward_per_quote * num_quotes * (1 - service_fee)),
+                ),
+            ],
+        )
+
+    def test_positive_reward_service_fee(self):
+        """Sevice fee reduces COW reward."""
+        primary_reward = 10**18  # positive reward
+        num_quotes = 100
+        service_fee = Fraction(15, 100)
+        reward_per_quote = 6 * 10**18
+
+        test_datum = self.sample_record(
+            primary_reward=primary_reward,
+            slippage=0,
+            num_quotes=num_quotes,
+            service_fee=service_fee,
+        )
+
+        self.assertEqual(
+            test_datum.total_service_fee(),
+            int(
+                (primary_reward * self.conversion_rate + reward_per_quote * num_quotes)
+                * service_fee
+            ),
+        )
         self.assertFalse(test_datum.is_overdraft())
         self.assertEqual(
             test_datum.as_payouts(),
             [
+                Transfer(
+                    token=self.cow_token,
+                    recipient=self.reward_target,
+                    amount_wei=int(reward_per_quote * num_quotes * (1 - service_fee)),
+                ),
                 Transfer(
                     token=self.cow_token,
                     recipient=self.reward_target,
                     amount_wei=int(
-                        6000000000000000000 * num_quotes * (1 - service_fee)
+                        primary_reward * self.conversion_rate * (1 - service_fee)
                     ),
                 ),
             ],
         )
 
+    def test_negative_reward_service_fee(self):
+        """Sevice fee reduces COW quote reward but not reduce a negative batch reward."""
+        primary_reward = -(10**18)  # negative reward
+        slippage = 2 * 10**18  # to avoid overdraft
+        num_quotes = 100
+        service_fee = Fraction(15, 100)
+        reward_per_quote = 6 * 10**18
+
+        test_datum = self.sample_record(
+            primary_reward=primary_reward,
+            slippage=slippage,
+            num_quotes=num_quotes,
+            service_fee=service_fee,
+        )
+
+        self.assertTrue(
+            test_datum.total_cow_reward(),
+            test_datum.total_eth_reward() * self.conversion_rate,
+        )  # ensure consistency of cow and eth batch rewards
+        self.assertEqual(
+            test_datum.total_service_fee(),
+            int(
+                reward_per_quote * num_quotes * service_fee
+            ),  # only quote rewards enter
+        )
+        self.assertFalse(test_datum.is_overdraft())
+        self.assertEqual(
+            test_datum.as_payouts(),
+            [
+                Transfer(
+                    token=self.cow_token,
+                    recipient=self.reward_target,
+                    amount_wei=int(reward_per_quote * num_quotes * (1 - service_fee)),
+                ),
+                Transfer(
+                    token=None,
+                    recipient=self.buffer_accounting_target,
+                    amount_wei=slippage
+                    + primary_reward,  # no multiplication by 1 - service_fee
+                ),
+            ],
+        )
+
 
 if __name__ == "__main__":
     unittest.main()