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
This commit is contained in:
parent
71f8f2b467
commit
4063c1a9fa
2 changed files with 29 additions and 63 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue