From 2805e2ee296a8fa63d3a6ae9f0fc5c757384586f Mon Sep 17 00:00:00 2001 From: Benjamin Smith Date: Wed, 25 Jan 2023 14:29:08 +0100 Subject: [PATCH] [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 | 30 ++---------------------------- src/models/split_transfers.py | 21 +++++++++++++++++---- src/models/transfer.py | 11 ++++++----- src/slack.py | 31 +++++++++++++++++++++++++++++++ src/utils/print_store.py | 7 ++++--- tests/unit/test_models.py | 19 +++++++++++++++++++ 6 files changed, 79 insertions(+), 40 deletions(-) create mode 100644 src/slack.py diff --git a/src/fetch/transfer_file.py b/src/fetch/transfer_file.py index 91be87ff..0cb84896 100644 --- a/src/fetch/transfer_file.py +++ b/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, ) diff --git a/src/models/split_transfers.py b/src/models/split_transfers.py index 3f440b74..19447468 100644 --- a/src/models/split_transfers.py +++ b/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: diff --git a/src/models/transfer.py b/src/models/transfer.py index bdcd3a37..69cf6ea7 100644 --- a/src/models/transfer.py +++ b/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 diff --git a/src/slack.py b/src/slack.py new file mode 100644 index 00000000..52c95da1 --- /dev/null +++ b/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, + ) diff --git a/src/utils/print_store.py b/src/utils/print_store.py index 5c831a7b..5c9d788c 100644 --- a/src/utils/print_store.py +++ b/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: diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 5fa0e4e2..546dfa10 100644 --- a/tests/unit/test_models.py +++ b/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):