[Easy] Improved Logging & Alerting (#175)

Redirects are now split into COW and ETH (with ETH Redirects being equivalent to grouped positive slippage). This also fixes the issue that the redirect message was exceeding max character limit for a single slack message and will now fit again into a "code block" (cf attached screenshots)

2. We now include both the +ve and -ve totals in the "Totals" summary

src/fetch/transfer_file.py

@@ -12,7 +12,6 @@
 from eth_typing.ethpm import URI
 from gnosis.eth.ethereum_client import EthereumClient
 from slack.web.client import WebClient
-from slack.web.slack_response import SlackResponse
 
 from src.constants import (
     SAFE_ADDRESS,
@@ -25,6 +24,7 @@
 from src.fetch.dune import DuneFetcher
 from src.models.transfer import Transfer, CSVTransfer
 from src.multisend import post_multisend, prepend_unwrap_if_necessary
+from src.slack import post_to_slack
 from src.utils.print_store import Category
 from src.utils.script_args import generic_script_init
 
@@ -50,32 +50,6 @@ def manual_propose(dune: DuneFetcher) -> None:
     )
 
 
-def post_to_slack(
-    slack_client: WebClient, channel: str, message: str, sub_messages: dict[str, str]
-) -> None:
-    """Posts message to Slack channel and sub message inside thread of first message"""
-    response = slack_client.chat_postMessage(
-        channel=channel,
-        text=message,
-        # Do not show link preview!
-        # https://api.slack.com/reference/messaging/link-unfurling
-        unfurl_media=False,
-    )
-    # This assertion is only for type safety,
-    # since previous function can also return a Future
-    assert isinstance(response, SlackResponse)
-    # Post logs in thread.
-    for category, log in sub_messages.items():
-        slack_client.chat_postMessage(
-            channel=channel,
-            format="mrkdwn",
-            text=f"{category}:\n```{log}```",
-            # According to https://api.slack.com/methods/conversations.replies
-            thread_ts=response.get("ts"),
-            unfurl_media=False,
-        )
-
-
 def auto_propose(dune: DuneFetcher, slack_client: WebClient, dry_run: bool) -> None:
     """
     Entry point auto creation of rewards payout transaction.
@@ -118,7 +92,7 @@ def auto_propose(dune: DuneFetcher, slack_client: WebClient, dry_run: bool) -> N
 if __name__ == "__main__":
     args = generic_script_init(description="Fetch Complete Reimbursement")
     args.dune.log_saver.print(
-        f"The data being aggregated here is available for visualization at\n"
+        f"The data aggregated can be visualized at\n"
         f"{args.dune.period.dashboard_url()}",
         category=Category.GENERAL,
     )

src/models/split_transfers.py

@@ -56,6 +56,7 @@ def _process_native_transfers(
         """
         Draining the `unprocessed_native` (ETH) transfers into processed
         versions as `eth_transfers`. Processing adjusts for negative slippage by deduction.
+        Returns: total negative slippage
         """
         penalty_total = 0
         while self.unprocessed_native:
@@ -89,7 +90,12 @@ def _process_rewards(
         self,
         redirect_map: dict[Address, Vouch],
         positive_slippage: list[SolverSlippage],
-    ) -> None:
+    ) -> int:
+        """
+        Draining the `unprocessed_cow` (COW) transfers into processed versions
+        as `cow_transfers`. Processing accounts for overdraft and positive slippage.
+        Returns: total positive slippage
+        """
         price_day = self.period.end - timedelta(days=1)
         while self.unprocessed_cow:
             transfer = self.unprocessed_cow.pop(0)
@@ -120,14 +126,17 @@ def _process_rewards(
         # We do not need to worry about any controversy between overdraft
         # and positive slippage adjustments, because positive/negative slippage
         # is disjoint between solvers.
+        total_positive_slippage = 0
         while positive_slippage:
             slippage = positive_slippage.pop()
             assert (
                 slippage.amount_wei > 0
             ), f"Expected positive slippage got {slippage.amount_wei}"
+            total_positive_slippage += slippage.amount_wei
             slippage_transfer = Transfer.from_slippage(slippage)
             slippage_transfer.redirect_to(redirect_map, self.log_saver)
             self.eth_transfers.append(slippage_transfer)
+        return total_positive_slippage
 
     def process(
         self,
@@ -141,19 +150,23 @@ def process(
         so that any overdraft from slippage can be carried over and deducted from
         the COW rewards.
         """
-        penalty_total = self._process_native_transfers(
+        total_penalty = self._process_native_transfers(
             indexed_slippage=index_by(
                 slippages.solvers_with_negative_total, "solver_address"
             )
         )
+        self.log_saver.print(
+            f"Total Negative Slippage (ETH): {total_penalty / 10**18:.4f}",
+            category=Category.TOTALS,
+        )
         # Note that positive and negative slippage is DISJOINT.
         # So no overdraft computations will overlap with the positive slippage perturbations.
-        self._process_rewards(
+        total_positive_slippage = self._process_rewards(
             cow_redirects,
             positive_slippage=slippages.solvers_with_positive_total,
         )
         self.log_saver.print(
-            f"Total Slippage deducted (ETH): {penalty_total / 10**18}",
+            f"Total Positive Slippage (ETH): {total_positive_slippage / 10**18:.4f}",
             category=Category.TOTALS,
         )
         if self.overdrafts:

src/models/transfer.py

@@ -81,8 +81,8 @@ def summarize(transfers: list[Transfer]) -> str:
             t.amount_wei for t in transfers if t.token_type == TokenType.ERC20
         )
         return (
-            f"Total ETH Funds needed: {eth_total / 10 ** 18}\n"
-            f"Total COW Funds needed: {cow_total / 10 ** 18}\n"
+            f"Total ETH Funds needed: {eth_total / 10 ** 18:.4f}\n"
+            f"Total COW Funds needed: {cow_total / 10 ** 18:.4f}\n"
         )
 
     @staticmethod
@@ -200,9 +200,10 @@ def redirect_to(
             # Redirect COW rewards to reward target specific by VouchRegistry
             redirect_address = redirects[recipient].reward_target
             log_saver.print(
-                f"Redirecting {recipient} Transfer of {self.token or 'ETH'}"
-                f"({self.amount}) to {redirect_address}",
-                category=Category.REDIRECT,
+                f"Redirecting {recipient} Transfer of {self.amount} to {redirect_address}",
+                category=Category.ETH_REDIRECT
+                if self.token is None
+                else Category.COW_REDIRECT,
             )
             self.receiver = redirect_address
 

src/slack.py

@@ -0,0 +1,31 @@
+"""
+Basic Slack Post functionality. Sends a message thread to a specified channel.
+"""
+from slack.web.client import WebClient
+from slack.web.slack_response import SlackResponse
+
+
+def post_to_slack(
+    slack_client: WebClient, channel: str, message: str, sub_messages: dict[str, str]
+) -> None:
+    """Posts message to Slack channel and sub message inside thread of first message"""
+    response = slack_client.chat_postMessage(
+        channel=channel,
+        text=message,
+        # Do not show link preview!
+        # https://api.slack.com/reference/messaging/link-unfurling
+        unfurl_media=False,
+    )
+    # This assertion is only for type safety,
+    # since previous function can also return a Future
+    assert isinstance(response, SlackResponse)
+    # Post logs in thread.
+    for category, log in sub_messages.items():
+        slack_client.chat_postMessage(
+            channel=channel,
+            format="mrkdwn",
+            text=f"{category}:\n```{log}```",
+            # According to https://api.slack.com/methods/conversations.replies
+            thread_ts=response.get("ts"),
+            unfurl_media=False,
+        )

src/utils/print_store.py

@@ -8,11 +8,12 @@
 class Category(Enum):
     """Known Categories for PrintStore"""
 
-    GENERAL = ""
+    GENERAL = "Overview"
     TOTALS = "Totals"
     OVERDRAFT = "Overdraft"
-    REDIRECT = "Redirect"
-    SLIPPAGE = "Slippage"
+    COW_REDIRECT = "COW Redirects"
+    ETH_REDIRECT = "ETH Redirects (Positive Slippage)"
+    SLIPPAGE = "Negative Slippage"
 
 
 class PrintStore:

tests/unit/test_models.py

@@ -345,6 +345,25 @@ def test_as_multisend_tx(self):
             ),
         )
 
+    def test_summarize(self):
+        receiver = Address.from_int(1)
+        eth_amount = 123456789101112131415
+        cow_amount = 9999999999999999999999999
+        result = Transfer.summarize(
+            [
+                Transfer(token=None, receiver=receiver, amount_wei=eth_amount),
+                Transfer(
+                    token=Token(COW_TOKEN_ADDRESS),
+                    receiver=receiver,
+                    amount_wei=cow_amount,
+                ),
+            ]
+        )
+        self.assertEqual(
+            result,
+            "Total ETH Funds needed: 123.4568\nTotal COW Funds needed: 10000000.0000\n",
+        )
+
 
 class TestAccountingPeriod(unittest.TestCase):
     def test_str(self):