Fixing small stuff

fastlane_bot/bot.py

@@ -119,15 +119,18 @@ def __post_init__(self):
         if self.ConfigObj is None:
             self.ConfigObj = Config()
 
+        # TODO: self.c, self.C and self.ConfigObj are all the same
         self.c = self.ConfigObj
 
         assert (
             self.polling_interval is None
         ), "polling_interval is now a parameter to run"
 
+        # TODO: Why do we need TxRouteHandlerClass as a parameter?
         if self.TxRouteHandlerClass is None:
             self.TxRouteHandlerClass = TxRouteHandler
 
+        # TODO: Why do we need TxHelpersClass as a parameter?
         if self.TxHelpersClass is None:
             self.TxHelpersClass = TxHelpers(ConfigObj=self.ConfigObj)
         assert issubclass(
@@ -138,7 +141,7 @@ def __post_init__(self):
         self.RUN_FLASHLOAN_TOKENS = [*self.ConfigObj.CHAIN_FLASHLOAN_TOKENS.values()]
 
     @property
-    def C(self) -> Any:
+    def C(self) -> Config:
         """
         Convenience method self.ConfigObj
         """
@@ -193,6 +196,7 @@ def get_curves(self) -> CPCContainer:
                     f"[bot.get_curves] MUST FIX INVALID CURVE {p} [{e}]\n"
                 )
             except TypeError as e:
+                # TODO: remove if
                 if fastlane_bot.__version__ not in ["3.0.31", "3.0.32"]:
                     self.ConfigObj.logger.error(
                         f"[bot.get_curves] MUST FIX DECIMAL ERROR CURVE {p} [{e}]\n"
@@ -206,13 +210,16 @@ def get_curves(self) -> CPCContainer:
                     f"[bot.get_curves] MUST FIX DECIMALS MISSING [{e}]\n"
                 )
             except Exception as e:
+                # TODO: unexpected Exception should possible raise
                 self.ConfigObj.logger.error(
-                    f"[bot.get_curves] error converting pool to curve {p}\n[ERR={e}]\n\n"
+                    f"[bot.get_curves] MUST FIX UNEXPECTED ERROR converting pool to curve {p}\n[ERR={e}]\n\n"
                 )
 
         return CPCContainer(curves)
 
 
+# TODO: Why do we need a class hierarchy here?
+
 @dataclass
 class CarbonBot(CarbonBotBase):
     """
@@ -454,7 +461,7 @@ def _run(
         randomizer=int,
         data_validator=True,
         replay_mode: bool = False,
-    ) -> Any:
+    ) -> Optional[str]: 
         """
         Runs the bot.
 
@@ -473,7 +480,8 @@ def _run(
 
         Returns
         -------
-        Transaction hash.
+        Optional[str]
+            Transaction hash or None
 
         """
         arbitrage = self._find_arbitrage(flashloan_tokens=flashloan_tokens, CCm=CCm, arb_mode=arb_mode, randomizer=randomizer)
@@ -890,9 +898,15 @@ def _handle_trade_instructions(
         CCm: CPCContainer,
         arb_mode: str,
         r: Any
-    ) -> Any:
+    ) -> Optional[str]:
         """
-        Handles the trade instructions.
+        Creates and executes the trade instructions
+
+        This is the workhorse function that chains all the different actions that
+        are necessary to create trade instructions and to ultimately execute them.
+
+        Execution is done via call to ``tx_helpers.validate_and_submit_transaction``
+        and the return value of this function (the txhash) is returned to the caller.
 
         Parameters
         ----------
@@ -905,8 +919,8 @@ def _handle_trade_instructions(
 
         Returns
         -------
-        Any
-            The result.
+        Optional[str]
+            Transaction hash or None
         """
         (
             best_profit,
@@ -1182,75 +1196,13 @@ def log_flashloan_details(
             f"[bot.log_flashloan_details] Trade Instructions: \n {best_trade_instructions_dic}"
         )
 
-    def validate_mode(self, mode: str):
-        """
-        Validate the mode. If the mode is None, set it to RUN_CONTINUOUS.
-        """
-        if mode is None:
-            mode = self.RUN_CONTINUOUS
-        assert mode in [
-            self.RUN_SINGLE,
-            self.RUN_CONTINUOUS,
-        ], f"Unknown mode {mode} [possible values: {self.RUN_SINGLE}, {self.RUN_CONTINUOUS}]"
-        return mode
-
-    def setup_polling_interval(self, polling_interval: int):
-        """
-        Setup the polling interval. If the polling interval is None, set it to RUN_POLLING_INTERVAL.
-        """
-        if self.polling_interval is None:
-            self.polling_interval = (
-                polling_interval
-                if polling_interval is not None
-                else self.RUN_POLLING_INTERVAL
-            )
-
-    def setup_flashloan_tokens(self, flashloan_tokens):
-        """
-        Setup the flashloan tokens. If flashloan_tokens is None, set it to RUN_FLASHLOAN_TOKENS.
-        """
-        return (
-            flashloan_tokens
-            if flashloan_tokens is not None
-            else self.RUN_FLASHLOAN_TOKENS
-        )
-
-    def setup_CCm(self, CCm: CPCContainer) -> CPCContainer:
-        """
-        Setup the CCm. If CCm is None, retrieve and filter curves.
-
-        Parameters
-        ----------
-        CCm: CPCContainer
-            The CPCContainer object
-
-        Returns
-        -------
-        CPCContainer
-            The filtered CPCContainer object
-        """
-        if CCm is None:
-            CCm = self.get_curves()
-            if self.ConfigObj.ARB_CONTRACT_VERSION < 10:
-                filter_out_weth = [
-                    x
-                    for x in CCm
-                    if (x.params.exchange in self.ConfigObj.CARBON_V1_FORKS)
-                    & (
-                        (x.params.tkny_addr == self.ConfigObj.WETH_ADDRESS)
-                        or (x.params.tknx_addr == self.ConfigObj.WETH_ADDRESS)
-                    )
-                ]
-                CCm = CPCContainer([x for x in CCm if x not in filter_out_weth])
-        return CCm
-
     def run_continuous_mode(
         self,
         flashloan_tokens: List[str],
         arb_mode: str,
         run_data_validator: bool,
         randomizer: int,
-    ):
+    ) -> None:
         """
         Run the bot in continuous mode.
 
@@ -1275,10 +1227,20 @@ def run_continuous_mode(
                     self.ConfigObj.logger.info(f"Arbitrage executed [hash={tx_hash}]")
 
                 time.sleep(self.polling_interval)
+
             except self.NoArbAvailable as e:
+                # TODO: Why is "no arb available" an exception? this is the normal case
+                # in fact, no arb possible should be indicated by tx_hash being None
                 self.ConfigObj.logger.debug(f"[bot:run:continuous] {e}")
             except Exception as e:
+                # TODO: Here we are just squashing all errors; we should have a log which
+                # exceptions we expect here; ideally none -- meaning we should try to weed
+                # them out; if we can't, we should catch and log them separately
+                # There should be a mode where unexpected exceptions terminate the bot;
+                # otherwise they should at least be logged prominently
+                self.ConfigObj.logger.error(f"[bot:run:continuous] ****** UNEXPECTED EXCEPTION -- MUST FIX ******")
                 self.ConfigObj.logger.error(f"[bot:run:continuous] {e}")
+                self.ConfigObj.logger.error(f"[bot:run:continuous] ****** END UNEXPECTED EXCEPTION ******")
                 time.sleep(self.polling_interval)
 
     def run_single_mode(
@@ -1290,7 +1252,7 @@ def run_single_mode(
         randomizer: int,
         replay_mode: bool = False,
         tenderly_fork: str = None,
-    ):
+    ) -> None:
         """
         Run the bot in single mode.
 
@@ -1311,6 +1273,9 @@ def run_single_mode(
         try:
             if replay_mode:
                 self._ensure_connection(tenderly_fork)
+                # TODO: This changes the state of the object by changing the provider to 
+                # the tenderly fork. This side effect here should be avoided and rather
+                # managed properly
 
             tx_hash = self._run(
                 flashloan_tokens=flashloan_tokens,
@@ -1320,6 +1285,10 @@ def run_single_mode(
                 randomizer=randomizer,
                 replay_mode=replay_mode,
             )
+            # TODO: check the below; the return value of _run should be a tx_hash (it is 
+            # unfortunately indicated as Any, and at one point in the chain as Dict[str])
+            # However -- I can see no instance where tx_hash[0] would be a meaningful value
+            # because tx_hash does not seem to be a list
             if tx_hash and tx_hash[0]:
                 self.ConfigObj.logger.info(
                     f"[bot.run_single_mode] Arbitrage executed [hash={tx_hash}]"
@@ -1328,12 +1297,14 @@ def run_single_mode(
                 # Write the tx hash to a file in the logging_path directory
                 if self.logging_path:
                     filename = f"successful_tx_hash_{datetime.now().strftime('%Y-%m-%d_%H-%M-%S')}.txt"
-                    print(f"Writing tx_hash hash {tx_hash} to {filename}")
+                    #print(f"Writing tx_hash hash {tx_hash} to {filename}")
+                    # TODO: either remove or convert to logging
                     with open(f"{self.logging_path}/{filename}", "w") as f:
 
                         # if isinstance(tx_hash[0], AttributeDict):
                         #     f.write(str(tx_hash[0]))
                         # else:
+                        # TODO: tx_hash here is in my view a string so this seems like old code
                         for record in tx_hash:
                             f.write("\n")
                             f.write("\n")
@@ -1387,22 +1358,22 @@ def run(
         replay_mode: bool = False,
         tenderly_fork: str = None,
         replay_from_block: int = None,
-    ):
+    ) -> None:
         """
         Runs the bot.
 
         Parameters
         ----------
         flashloan_tokens: List[str]
-            The flashloan tokens (optional; default: self.RUN_FLASHLOAN_TOKENS)
+            The flashloan tokens (optional; default: RUN_FLASHLOAN_TOKENS)
         CCm: CPCContainer
-            The complete market data container (optional; default: database via self.get_curves())
+            The complete market data container (optional; default: database via get_curves())
         polling_interval: int
-            the polling interval in seconds (default: 60 via self.RUN_POLLING_INTERVAL)
+            the polling interval in seconds (default: 60 via RUN_POLLING_INTERVAL)
         mode: RN_SINGLE or RUN_CONTINUOUS
             whether to run the bot one-off or continuously (default: RUN_CONTINUOUS)
         arb_mode: str
-            the arbitrage mode (default: None)
+            the arbitrage mode (default: None; can be set depending on arbmode)
         run_data_validator: bool
             whether to run the data validator (default: False)
         randomizer: int
@@ -1418,14 +1389,13 @@ def run(
 
         Returns
         -------
-        str
-            The transaction hash.
+        None
         """
 
-        mode = self.validate_mode(mode)
-        self.setup_polling_interval(polling_interval)
-        flashloan_tokens = self.setup_flashloan_tokens(flashloan_tokens)
-        CCm = self.setup_CCm(CCm)
+        mode = mode or self.RUN_CONTINUOUS
+        self.polling_interval = polling_interval or self.RUN_POLLING_INTERVAL
+        flashloan_tokens = flashloan_tokens or self.RUN_FLASHLOAN_TOKENS
+        CCm = CCm or self.get_curves()
         self.logging_path = logging_path
         self.replay_from_block = replay_from_block
 
@@ -1436,11 +1406,14 @@ def run(
                 f"[bot.run] Transactions will be required to pass data validation for {arb_mode}"
             )
 
-        if mode == "continuous":
+        if mode == self.RUN_CONTINUOUS:
             self.run_continuous_mode(
-                flashloan_tokens, arb_mode, run_data_validator, randomizer
+                flashloan_tokens, 
+                arb_mode, 
+                run_data_validator, 
+                randomizer,
             )
-        else:
+        elif mode == self.RUN_SINGLE:
             self.run_single_mode(
                 flashloan_tokens,
                 CCm,
@@ -1450,3 +1423,5 @@ def run(
                 replay_mode,
                 tenderly_fork,
             )
+        else:
+            raise ValueError(f"Unknown mode {mode} [possible values: RUN_SINGLE, RUN_CONTINUOUS]")

fastlane_bot/helpers/txhelpers.py

@@ -137,12 +137,18 @@ def validate_and_submit_transaction(
         safety_override: bool = False,
         log_object: Dict[str, Any] = None,
         flashloan_struct: List[Dict[str, int or str]] = None,
-    ) -> Optional[Dict[str, Any]]:
+    ) -> Optional[str]:
         """
         Validates and submits a transaction to the arb contract.
 
         Parameters
         ----------
+        TODO: Add parameters
+
+        Returns
+        -------
+        Optional[str]
+            The transaction hash of the submitted transaction or None
 
         """
 
@@ -473,13 +479,13 @@ def build_tx(
         tx_details["value"] = value
         return tx_details
 
-    def submit_regular_transaction(self, signed_tx) -> str:
+    def submit_regular_transaction(self, signed_tx) -> Optional[str]:
         """
         Submits the transaction to the blockchain.
 
         :param signed_tx: the signed transaction to be submitted to the blockchain
 
-        returns: the transaction hash of the submitted transaction
+        :returns: the transaction hash (str) of the submitted transaction (None if timed out)
         """
 
         self.ConfigObj.logger.info(
@@ -488,14 +494,14 @@ def submit_regular_transaction(self, signed_tx) -> str:
 
         return self._submit_transaction(self.web3.eth.send_raw_transaction(signed_tx.rawTransaction))
 
-    def submit_private_transaction(self, signed_tx, block_number: int) -> str:
+    def submit_private_transaction(self, signed_tx, block_number: int) -> Optional[str]:
         """
         Submits the transaction privately through Alchemy -> Flashbots RPC to mitigate frontrunning.
 
         :param signed_tx: the signed transaction to be submitted to the blockchain
         :param block_number: the current block number
 
-        returns: The transaction receipt, or None if the transaction failed
+        :returns: the transaction hash (str) of the submitted transaction (None if timed out)
         """
 
         self.ConfigObj.logger.info(
@@ -528,11 +534,18 @@ def submit_private_transaction(self, signed_tx, block_number: int) -> str:
             )
             return None
 
-    def _submit_transaction(self, tx_hash) -> str:
+    # TODO: should be renamed to _wait_for_transaction_receipt
+    def _submit_transaction(self, tx_hash) -> Optional[str]:
+        """
+        waits for the transaction receipt
+
+        :param tx_hash: the transaction hash of the receipt to wait for
+        :returns: the transaction hash (str) of the submitted transaction or None (TimeExhausted)
+        """
         try:
             tx_receipt = self.web3.eth.wait_for_transaction_receipt(tx_hash)
             assert tx_hash == tx_receipt["transactionHash"]
-            return tx_hash
+            return tx_hash.hex() # HexBytes -> str [TODO-KEVIN: Please check]
         except TimeExhausted as e:
             self.ConfigObj.logger.info(
                 f"[helpers.txhelpers._submit_transaction] Transaction timeout (stuck in mempool); moving on"