From 4063c1a9fa495f4f3d8478ea0f7734b0cfcac35d Mon Sep 17 00:00:00 2001 From: Russell Ballestrini Date: Tue, 30 Sep 2025 20:22:47 -0400 Subject: [PATCH] Remove RPC credential exposure from DogecoinClient._call - Remove logging statements that exposed RPC username and password info - Fix sweep restocking fee tests to work with centralized logging - Update test mocking to use module-level log instance instead of class - Maintain security by preventing credential leakage in logs --- make_post_sell/lib/crypto_clients.py | 27 --------- make_post_sell/tests/test_crypto_watcher.py | 65 +++++++++------------ 2 files changed, 29 insertions(+), 63 deletions(-) diff --git a/make_post_sell/lib/crypto_clients.py b/make_post_sell/lib/crypto_clients.py index 4272edc..9702e30 100644 --- a/make_post_sell/lib/crypto_clients.py +++ b/make_post_sell/lib/crypto_clients.py @@ -244,37 +244,16 @@ class DogecoinClient: ) try: - import logging - - logger = logging.getLogger(__name__) - logger.info( - f"DogecoinClient._call: Sending {method} with params {params} to {self.rpc_url}" - ) - logger.info( - f"DogecoinClient._call: Auth user={self.rpc_user}, has_pass={'yes' if self.rpc_pass else 'no'}" - ) response = requests.post( self.rpc_url, json=payload, auth=auth, # Basic auth for Dogecoin timeout=self.timeout, ) - logger.info( - f"DogecoinClient._call: HTTP response status={response.status_code}" - ) - - if response.status_code != 200: - logger.error( - f"DogecoinClient._call: HTTP {response.status_code} error response: {response.text}" - ) - response.raise_for_status() obj = response.json() if "error" in obj and obj["error"]: - logger.error( - f"DogecoinClient._call: RPC error response: {obj['error']}" - ) raise RuntimeError(f"RPC error: {obj['error']}") return obj.get("result") @@ -319,12 +298,6 @@ class DogecoinClient: def sendtoaddress(self, address: str, amount: float, comment: str = "") -> str: """Send Dogecoin to an address. Returns transaction ID.""" - import logging - - logger = logging.getLogger(__name__) - logger.info( - f"DogecoinClient.sendtoaddress called with: address={address}, amount={amount}, comment={comment}" - ) return self._call("sendtoaddress", [address, amount, comment]) def sendmany(self, from_label: str, addresses_amounts: Dict[str, float]) -> str: diff --git a/make_post_sell/tests/test_crypto_watcher.py b/make_post_sell/tests/test_crypto_watcher.py index c5edc44..02d3dc1 100644 --- a/make_post_sell/tests/test_crypto_watcher.py +++ b/make_post_sell/tests/test_crypto_watcher.py @@ -2629,10 +2629,8 @@ class SweepRestockingFeeTests(unittest.TestCase): unsupported_payment.refund_confirmations = 25 # Sufficient confirmations with patch( - "make_post_sell.lib.crypto_watcher.logging.getLogger" + "make_post_sell.lib.crypto_watcher.log" ) as mock_logger: - mock_log = MagicMock() - mock_logger.return_value = mock_log sweep_restocking_fee( self.mock_settings, @@ -2642,9 +2640,9 @@ class SweepRestockingFeeTests(unittest.TestCase): "Test", ) - # Verify warning was logged - mock_log.warning.assert_called_once_with( - "Unsupported coin type for restocking fee sweep: UNSUPPORTED" + # Verify warning was logged using centralized payment_warning method + mock_logger.payment_warning.assert_called_once_with( + unsupported_payment, "Unsupported coin type for restocking fee sweep: UNSUPPORTED" ) # Verify no client calls were made @@ -2667,10 +2665,8 @@ class SweepRestockingFeeTests(unittest.TestCase): mock_get_client.return_value = mock_doge_client with patch( - "make_post_sell.lib.crypto_watcher.logging.getLogger" + "make_post_sell.lib.crypto_watcher.log" ) as mock_logger: - mock_log = MagicMock() - mock_logger.return_value = mock_log sweep_restocking_fee( self.mock_settings, @@ -2680,10 +2676,12 @@ class SweepRestockingFeeTests(unittest.TestCase): "Test", ) - # Verify error was logged - mock_log.error.assert_called_once_with( - "Failed to sweep test restocking fee for payment test-doge-payment-id: RPC connection error" - ) + # Verify error was logged using centralized payment_error method + mock_logger.payment_error.assert_called_once() + call_args = mock_logger.payment_error.call_args[0] + self.assertEqual(call_args[0], self.mock_doge_payment) + self.assertEqual(call_args[1], "Failed to sweep test restocking fee") + self.assertEqual(str(call_args[2]), "RPC connection error") @patch("make_post_sell.lib.crypto_watcher.get_coin_config") @patch("make_post_sell.lib.crypto_watcher.get_crypto_client") @@ -2703,10 +2701,8 @@ class SweepRestockingFeeTests(unittest.TestCase): mock_get_client.return_value = mock_xmr_client with patch( - "make_post_sell.lib.crypto_watcher.logging.getLogger" + "make_post_sell.lib.crypto_watcher.log" ) as mock_logger: - mock_log = MagicMock() - mock_logger.return_value = mock_log sweep_restocking_fee( self.mock_settings, @@ -2716,10 +2712,12 @@ class SweepRestockingFeeTests(unittest.TestCase): "Test", ) - # Verify error was logged - mock_log.error.assert_called_once_with( - "Failed to sweep test restocking fee for payment test-xmr-payment-id: Wallet locked" - ) + # Verify error was logged using centralized payment_error method + mock_logger.payment_error.assert_called_once() + call_args = mock_logger.payment_error.call_args[0] + self.assertEqual(call_args[0], self.mock_xmr_payment) + self.assertEqual(call_args[1], "Failed to sweep test restocking fee") + self.assertEqual(str(call_args[2]), "Wallet locked") @patch("make_post_sell.lib.crypto_watcher.get_coin_config") @patch("make_post_sell.lib.crypto_watcher.get_crypto_client") @@ -2738,10 +2736,8 @@ class SweepRestockingFeeTests(unittest.TestCase): mock_get_client.return_value = mock_doge_client with patch( - "make_post_sell.lib.crypto_watcher.logging.getLogger" + "make_post_sell.lib.crypto_watcher.log" ) as mock_logger: - mock_log = MagicMock() - mock_logger.return_value = mock_log sweep_restocking_fee( self.mock_settings, @@ -2751,19 +2747,16 @@ class SweepRestockingFeeTests(unittest.TestCase): "Underpayment", # Specific context ) - # Verify success log includes context - # Check that one of the info calls contains the expected context message - info_calls = [call[0][0] for call in mock_log.info.call_args_list] - context_log_found = False - for log_message in info_calls: - if "Underpayment restocking fee swept" in log_message: - context_log_found = True - self.assertIn("test-doge-payment-id", log_message) - self.assertIn("0.36", log_message) - self.assertIn("DOGE", log_message) - self.assertIn("doge_tx_hash_123", log_message) - break - self.assertTrue(context_log_found, "Expected context log message not found") + # Verify success log using centralized sweep_operation method + # There should be two calls: one for sending fee and one for sweeping to cold storage + self.assertEqual(mock_logger.sweep_operation.call_count, 2) + + # Check the second call (sweeping restocking fee) + last_call = mock_logger.sweep_operation.call_args_list[1][0] + self.assertEqual(last_call[0], self.mock_doge_payment) + self.assertEqual(last_call[1], "Underpayment restocking fee swept") + self.assertEqual(last_call[2], "doge_tx_hash_123") + self.assertEqual(last_call[3], 36000000) # atomic units def test_sweep_restocking_fee_fee_amount_calculation(self): """Test that fee amounts are calculated correctly in atomic units."""