From c79ed493aa189f173143a891bd0bc04909d33b25 Mon Sep 17 00:00:00 2001 From: tecnovert Date: Wed, 22 Jan 2025 18:07:20 +0200 Subject: [PATCH] Add estimated tx fee to amount check when posting bid. Add more log messages around balance checks. --- .github/workflows/ci.yml | 6 +- basicswap/basicswap.py | 114 ++++++++------ basicswap/bin/run.py | 4 +- basicswap/interface/btc.py | 4 + basicswap/interface/dcr/dcr.py | 4 + basicswap/interface/doge.py | 4 + basicswap/interface/part.py | 12 ++ basicswap/interface/xmr.py | 5 + tests/basicswap/test_btc_xmr.py | 258 ++++++++++++++++---------------- 9 files changed, 235 insertions(+), 176 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bb93fa7..66de431 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,9 +56,9 @@ jobs: - name: Running test_xmr run: | export PYTHONPATH=$(pwd) - export PARTICL_BINDIR="$BIN_DIR/particl"; - export BITCOIN_BINDIR="$BIN_DIR/bitcoin"; - export XMR_BINDIR="$BIN_DIR/monero"; + export PARTICL_BINDIR="$BIN_DIR/particl" + export BITCOIN_BINDIR="$BIN_DIR/bitcoin" + export XMR_BINDIR="$BIN_DIR/monero" pytest tests/basicswap/test_btc_xmr.py::TestBTC -k "test_003_api or test_02_a_leader_recover_a_lock_tx" - name: Running test_encrypted_xmr_reload run: | diff --git a/basicswap/basicswap.py b/basicswap/basicswap.py index 7f67079..8685173 100644 --- a/basicswap/basicswap.py +++ b/basicswap/basicswap.py @@ -1974,6 +1974,32 @@ class BasicSwap(BaseApp): if not offer.rate_negotiable: ensure(offer.rate == bid_rate, "Bid rate must match offer rate.") + def ensureWalletCanSend( + self, ci, swap_type, ensure_balance: int, estimated_fee: int, for_offer=True + ) -> None: + balance_msg: str = ( + f"{ci.format_amount(ensure_balance)} {ci.coin_name()} with estimated fee {ci.format_amount(estimated_fee)}" + ) + self.log.debug(f"Ensuring wallet can send {balance_msg}.") + try: + if ci.interface_type() in self.scriptless_coins: + ci.ensureFunds(ensure_balance + estimated_fee) + else: + pi = self.pi(swap_type) + _ = pi.getFundedInitiateTxTemplate(ci, ensure_balance, False) + # TODO: Save the prefunded tx so the fee can't change, complicates multiple offers at the same time. + except Exception as e: + type_str = "offer" if for_offer else "bid" + err_msg = f"Insufficient funds for {type_str} of {balance_msg}." + if self.debug: + self.log.error(f"ensureWalletCanSend failed {e}") + current_balance: int = ci.getSpendableBalance() + err_msg += ( + f" Debug: Spendable balance: {ci.format_amount(current_balance)}." + ) + self.log.error(err_msg) + raise ValueError(err_msg) + def getOfferAddressTo(self, extra_options) -> str: if "addr_send_to" in extra_options: return extra_options["addr_send_to"] @@ -2007,25 +2033,25 @@ class BasicSwap(BaseApp): except Exception: raise ValueError("Unknown coin to type") - valid_for_seconds: int = extra_options.get("valid_for_seconds", 60 * 60) - amount_to: int = extra_options.get( - "amount_to", int((amount * rate) // ci_from.COIN()) - ) - - # Recalculate the rate so it will match the bid rate - rate = ci_from.make_int(amount_to / amount, r=1) - self.validateSwapType(coin_from_t, coin_to_t, swap_type) - self.validateOfferAmounts( - coin_from_t, coin_to_t, amount, amount_to, min_bid_amount - ) self.validateOfferLockValue( swap_type, coin_from_t, coin_to_t, lock_type, lock_value ) + + valid_for_seconds: int = extra_options.get("valid_for_seconds", 60 * 60) self.validateOfferValidTime( swap_type, coin_from_t, coin_to_t, valid_for_seconds ) + amount_to: int = extra_options.get( + "amount_to", int((amount * rate) // ci_from.COIN()) + ) + self.validateOfferAmounts( + coin_from_t, coin_to_t, amount, amount_to, min_bid_amount + ) + # Recalculate the rate so it will match the bid rate + rate: int = ci_from.make_int(amount_to / amount, r=1) + offer_addr_to = self.getOfferAddressTo(extra_options) reverse_bid: bool = self.is_reverse_ads_bid(coin_from, coin_to) @@ -2066,8 +2092,8 @@ class BasicSwap(BaseApp): ) if "from_fee_override" in extra_options: - msg_buf.fee_rate_from = make_int( - extra_options["from_fee_override"], self.ci(coin_from).exp() + msg_buf.fee_rate_from = ci_from.make_int( + extra_options["from_fee_override"] ) else: # TODO: conf_target = ci_from.settings.get('conf_target', 2) @@ -2077,12 +2103,10 @@ class BasicSwap(BaseApp): fee_rate, fee_src = self.getFeeRateForCoin(coin_from, conf_target) if "from_fee_multiplier_percent" in extra_options: fee_rate *= extra_options["fee_multiplier"] / 100.0 - msg_buf.fee_rate_from = make_int(fee_rate, self.ci(coin_from).exp()) + msg_buf.fee_rate_from = ci_from.make_int(fee_rate) if "to_fee_override" in extra_options: - msg_buf.fee_rate_to = make_int( - extra_options["to_fee_override"], self.ci(coin_to).exp() - ) + msg_buf.fee_rate_to = ci_to.make_int(extra_options["to_fee_override"]) else: # TODO: conf_target = ci_to.settings.get('conf_target', 2) conf_target = 2 @@ -2091,7 +2115,7 @@ class BasicSwap(BaseApp): fee_rate, fee_src = self.getFeeRateForCoin(coin_to, conf_target) if "to_fee_multiplier_percent" in extra_options: fee_rate *= extra_options["fee_multiplier"] / 100.0 - msg_buf.fee_rate_to = make_int(fee_rate, self.ci(coin_to).exp()) + msg_buf.fee_rate_to = ci_to.make_int(fee_rate) if swap_type == SwapTypes.XMR_SWAP: xmr_offer = XmrOffer() @@ -2116,27 +2140,19 @@ class BasicSwap(BaseApp): msg_buf.fee_rate_to ) # Unused: TODO - Set priority? - ensure_balance: int = int(amount) - if coin_from in self.scriptless_coins: + # If a prefunded txn is not used, check that the wallet balance can cover the tx fee. + if "prefunded_itx" not in extra_options: # TODO: Better tx size estimate, xmr_swap_b_lock_tx_vsize could be larger than xmr_swap_b_lock_spend_tx_vsize estimated_fee: int = ( - msg_buf.fee_rate_from - * ci_from.xmr_swap_b_lock_spend_tx_vsize() - / 1000 + msg_buf.fee_rate_from * ci_from.est_lock_tx_vsize() // 1000 ) - ci_from.ensureFunds(msg_buf.amount_from + estimated_fee) - else: - # If a prefunded txn is not used, check that the wallet balance can cover the tx fee. - if "prefunded_itx" not in extra_options: - pi = self.pi(SwapTypes.XMR_SWAP) - _ = pi.getFundedInitiateTxTemplate(ci_from, ensure_balance, False) - # TODO: Save the prefunded tx so the fee can't change, complicates multiple offers at the same time. + self.ensureWalletCanSend(ci_from, swap_type, int(amount), estimated_fee) - # TODO: Send proof of funds with offer - # proof_of_funds_hash = getOfferProofOfFundsHash(msg_buf, offer_addr) - # proof_addr, proof_sig, proof_utxos = self.getProofOfFunds( - # coin_from_t, ensure_balance, proof_of_funds_hash - # ) + # TODO: Send proof of funds with offer + # proof_of_funds_hash = getOfferProofOfFundsHash(msg_buf, offer_addr) + # proof_addr, proof_sig, proof_utxos = self.getProofOfFunds( + # coin_from_t, ensure_balance, proof_of_funds_hash + # ) offer_bytes = msg_buf.to_bytes() payload_hex = str.format("{:02x}", MessageTypes.OFFER) + offer_bytes.hex() @@ -2174,6 +2190,8 @@ class BasicSwap(BaseApp): was_sent=True, bid_reversed=bid_reversed, security_token=security_token, + from_feerate=msg_buf.fee_rate_from, + to_feerate=msg_buf.fee_rate_to, ) offer.setState(OfferStates.OFFER_SENT) @@ -3525,14 +3543,12 @@ class BasicSwap(BaseApp): self.checkCoinsReady(coin_from, coin_to) - balance_to: int = ci_to.getSpendableBalance() - ensure( - balance_to > amount_to, - "{} spendable balance is too low: {} < {}".format( - ci_to.coin_name(), - ci_to.format_amount(balance_to), - ci_to.format_amount(amount_to), - ), + # TODO: Better tx size estimate + fee_rate, fee_src = self.getFeeRateForCoin(coin_to, conf_target=2) + fee_rate_to = ci_to.make_int(fee_rate) + estimated_fee: int = fee_rate_to * ci_to.est_lock_tx_vsize() // 1000 + self.ensureWalletCanSend( + ci_to, offer.swap_type, int(amount_to), estimated_fee, for_offer=False ) reverse_bid: bool = self.is_reverse_ads_bid(coin_from, coin_to) @@ -4069,6 +4085,18 @@ class BasicSwap(BaseApp): ci_from = self.ci(coin_from) ci_to = self.ci(coin_to) + # TODO: Better tx size estimate + fee_rate, fee_src = self.getFeeRateForCoin(coin_to, conf_target=2) + fee_rate_from = ci_to.make_int(fee_rate) + estimated_fee: int = fee_rate_from * ci_to.est_lock_tx_vsize() // 1000 + self.ensureWalletCanSend( + ci_to, + offer.swap_type, + offer.amount_from, + estimated_fee, + for_offer=False, + ) + if xmr_swap.contract_count is None: xmr_swap.contract_count = self.getNewContractId(use_cursor) diff --git a/basicswap/bin/run.py b/basicswap/bin/run.py index 3400941..7e15779 100755 --- a/basicswap/bin/run.py +++ b/basicswap/bin/run.py @@ -440,7 +440,9 @@ def runClient(fp, data_dir, chain, start_only_coins): swap_client.log.info(f"Starting {display_name} daemon") filename: str = getCoreBinName(coin_id, v, c + "d") - extra_opts = getCoreBinArgs(coin_id, v, use_tor_proxy=swap_client.use_tor_proxy) + extra_opts = getCoreBinArgs( + coin_id, v, use_tor_proxy=swap_client.use_tor_proxy + ) daemons.append( startDaemon(v["datadir"], v["bindir"], filename, opts=extra_opts) ) diff --git a/basicswap/interface/btc.py b/basicswap/interface/btc.py index 4b8bc8b..0caa59b 100644 --- a/basicswap/interface/btc.py +++ b/basicswap/interface/btc.py @@ -217,6 +217,10 @@ class BTCInterface(Secp256k1Interface): rv += output.nValue return rv + @staticmethod + def est_lock_tx_vsize() -> int: + return 110 + @staticmethod def xmr_swap_a_lock_spend_tx_vsize() -> int: return 147 diff --git a/basicswap/interface/dcr/dcr.py b/basicswap/interface/dcr/dcr.py index 614a73d..9bd6843 100644 --- a/basicswap/interface/dcr/dcr.py +++ b/basicswap/interface/dcr/dcr.py @@ -211,6 +211,10 @@ class DCRInterface(Secp256k1Interface): def txoType(): return CTxOut + @staticmethod + def est_lock_tx_vsize() -> int: + return 224 + @staticmethod def xmr_swap_a_lock_spend_tx_vsize() -> int: return 327 diff --git a/basicswap/interface/doge.py b/basicswap/interface/doge.py index caa5b49..4ef829d 100644 --- a/basicswap/interface/doge.py +++ b/basicswap/interface/doge.py @@ -24,6 +24,10 @@ class DOGEInterface(BTCInterface): def coin_type(): return Coins.DOGE + @staticmethod + def est_lock_tx_vsize() -> int: + return 192 + @staticmethod def xmr_swap_b_lock_spend_tx_vsize() -> int: return 192 diff --git a/basicswap/interface/part.py b/basicswap/interface/part.py index d7ef2b5..80db020 100644 --- a/basicswap/interface/part.py +++ b/basicswap/interface/part.py @@ -66,6 +66,10 @@ class PARTInterface(BTCInterface): def txVersion() -> int: return 0xA0 + @staticmethod + def est_lock_tx_vsize() -> int: + return 138 + @staticmethod def xmr_swap_a_lock_spend_tx_vsize() -> int: return 200 @@ -195,6 +199,10 @@ class PARTInterfaceBlind(PARTInterface): def balance_type(): return BalanceTypes.BLIND + @staticmethod + def est_lock_tx_vsize() -> int: + return 980 + @staticmethod def xmr_swap_a_lock_spend_tx_vsize() -> int: return 1032 @@ -1220,6 +1228,10 @@ class PARTInterfaceAnon(PARTInterface): def balance_type(): return BalanceTypes.ANON + @staticmethod + def est_lock_tx_vsize() -> int: + return 1153 + @staticmethod def xmr_swap_a_lock_spend_tx_vsize() -> int: raise ValueError("Not possible") diff --git a/basicswap/interface/xmr.py b/basicswap/interface/xmr.py index b947643..0ce3d5c 100644 --- a/basicswap/interface/xmr.py +++ b/basicswap/interface/xmr.py @@ -71,6 +71,11 @@ class XMRInterface(CoinInterface): def xmr_swap_a_lock_spend_tx_vsize() -> int: raise ValueError("Not possible") + @staticmethod + def est_lock_tx_vsize() -> int: + # TODO: Estimate with ringsize + return 1604 + @staticmethod def xmr_swap_b_lock_spend_tx_vsize() -> int: # TODO: Estimate with ringsize diff --git a/tests/basicswap/test_btc_xmr.py b/tests/basicswap/test_btc_xmr.py index 9deebc3..54d0145 100644 --- a/tests/basicswap/test_btc_xmr.py +++ b/tests/basicswap/test_btc_xmr.py @@ -10,9 +10,6 @@ import random import logging import unittest -from basicswap.db import ( - Concepts, -) from basicswap.basicswap import ( BidStates, Coins, @@ -23,6 +20,9 @@ from basicswap.basicswap_util import ( TxLockTypes, EventLogTypes, ) +from basicswap.db import ( + Concepts, +) from basicswap.util import ( make_int, ) @@ -32,10 +32,10 @@ from tests.basicswap.util import ( ) from tests.basicswap.common import ( abandon_all_swaps, + wait_for_balance, wait_for_bid, wait_for_event, wait_for_offer, - wait_for_balance, wait_for_unspent, wait_for_none_active, BTC_BASE_RPC_PORT, @@ -640,6 +640,129 @@ class TestFunctions(BaseTest): wait_for=(self.extra_wait_time + 180), ) + def do_test_08_insufficient_funds(self, coin_from, coin_to): + logging.info( + "---------- Test {} to {} Insufficient Funds".format( + coin_from.name, coin_to.name + ) + ) + swap_clients = self.swap_clients + reverse_bid: bool = swap_clients[0].is_reverse_ads_bid(coin_from, coin_to) + + id_offerer: int = self.node_c_id + id_bidder: int = self.node_b_id + + self.prepare_balance( + coin_from, + 10.0, + 1800 + id_offerer, + 1801 if coin_from in (Coins.XMR,) else 1800, + ) + jsw = read_json_api(1800 + id_offerer, "wallets") + balance_from_before: float = self.getBalance(jsw, coin_from) + self.prepare_balance( + coin_to, + balance_from_before + 1, + 1800 + id_bidder, + 1801 if coin_to in (Coins.XMR,) else 1800, + ) + + swap_clients = self.swap_clients + ci_from = swap_clients[id_offerer].ci(coin_from) + ci_to = swap_clients[id_bidder].ci(coin_to) + + amt_swap: int = ci_from.make_int(balance_from_before, r=1) + rate_swap: int = ci_to.make_int(2.0, r=1) + + try: + offer_id = swap_clients[id_offerer].postOffer( + coin_from, + coin_to, + amt_swap, + rate_swap, + amt_swap, + SwapTypes.XMR_SWAP, + auto_accept_bids=True, + ) + except Exception as e: + assert "Insufficient funds" in str(e) + else: + assert False, "Should fail" + + # Test that postbid errors when offer is for the full balance + id_offerer_test_bid = id_bidder + id_bidder_test_bid = id_offerer + amt_swap_test_bid_to: int = ci_from.make_int(balance_from_before, r=1) + amt_swap_test_bid_from: int = ci_to.make_int(1.0) + offer_id = swap_clients[id_offerer_test_bid].postOffer( + coin_to, + coin_from, + amt_swap_test_bid_from, + 0, + amt_swap_test_bid_from, + SwapTypes.XMR_SWAP, + extra_options={"amount_to": amt_swap_test_bid_to}, + ) + wait_for_offer(test_delay_event, swap_clients[id_bidder_test_bid], offer_id) + try: + bid_id = swap_clients[id_bidder_test_bid].postBid( + offer_id, amt_swap_test_bid_from + ) + except Exception as e: + assert "Insufficient funds" in str(e) + else: + assert False, "Should fail" + + amt_swap -= ci_from.make_int(1) + offer_id = swap_clients[id_offerer].postOffer( + coin_from, + coin_to, + amt_swap, + rate_swap, + amt_swap, + SwapTypes.XMR_SWAP, + auto_accept_bids=True, + ) + wait_for_offer(test_delay_event, swap_clients[id_bidder], offer_id) + + # First bid should work + bid_id = swap_clients[id_bidder].postXmrBid(offer_id, amt_swap) + wait_for_bid( + test_delay_event, + swap_clients[id_offerer], + bid_id, + ( + (BidStates.SWAP_COMPLETED, BidStates.XMR_SWAP_NOSCRIPT_COIN_LOCKED) + if reverse_bid + else (BidStates.BID_ACCEPTED, BidStates.XMR_SWAP_SCRIPT_COIN_LOCKED) + ), + wait_for=120, + ) + + # Should be out of funds for second bid (over remaining offer value causes a hard auto accept fail) + bid_id = swap_clients[id_bidder].postXmrBid(offer_id, amt_swap) + wait_for_bid( + test_delay_event, + swap_clients[id_offerer], + bid_id, + BidStates.BID_AACCEPT_FAIL, + wait_for=40, + ) + event = wait_for_event( + test_delay_event, + swap_clients[id_offerer], + Concepts.BID, + bid_id, + event_type=EventLogTypes.AUTOMATION_CONSTRAINT, + ) + assert "Over remaining offer value" in event.event_msg + try: + swap_clients[id_offerer].acceptBid(bid_id) + except Exception as e: + assert "Insufficient funds" in str(e) or "Balance too low" in str(e) + else: + assert False, "Should fail" + class BasicSwapTest(TestFunctions): @@ -1714,133 +1837,10 @@ class BasicSwapTest(TestFunctions): swap_clients[0].setMockTimeOffset(0) def test_08_insufficient_funds(self): - tla_from = self.test_coin_from.name - logging.info("---------- Test {} Insufficient Funds".format(tla_from)) - swap_clients = self.swap_clients - coin_from = self.test_coin_from - coin_to = Coins.XMR - - self.prepare_balance(coin_from, 10.0, 1802, 1800) - - id_offerer: int = self.node_c_id - id_bidder: int = self.node_b_id - - swap_clients = self.swap_clients - ci_from = swap_clients[id_offerer].ci(coin_from) - ci_to = swap_clients[id_bidder].ci(coin_to) - - jsw = read_json_api(1800 + id_offerer, "wallets") - balance_from_before: float = self.getBalance(jsw, coin_from) - - amt_swap: int = ci_from.make_int(balance_from_before, r=1) - rate_swap: int = ci_to.make_int(2.0, r=1) - - try: - offer_id = swap_clients[id_offerer].postOffer( - coin_from, - coin_to, - amt_swap, - rate_swap, - amt_swap, - SwapTypes.XMR_SWAP, - auto_accept_bids=True, - ) - except Exception as e: - assert "Insufficient funds" in str(e) - else: - assert False, "Should fail" - amt_swap -= ci_from.make_int(1) - offer_id = swap_clients[id_offerer].postOffer( - coin_from, - coin_to, - amt_swap, - rate_swap, - amt_swap, - SwapTypes.XMR_SWAP, - auto_accept_bids=True, - ) - wait_for_offer(test_delay_event, swap_clients[id_bidder], offer_id) - - # First bid should work - bid_id = swap_clients[id_bidder].postXmrBid(offer_id, amt_swap) - wait_for_bid( - test_delay_event, - swap_clients[id_offerer], - bid_id, - (BidStates.BID_ACCEPTED, BidStates.XMR_SWAP_SCRIPT_COIN_LOCKED), - wait_for=40, - ) - - # Should be out of funds for second bid (over remaining offer value causes a hard auto accept fail) - bid_id = swap_clients[id_bidder].postXmrBid(offer_id, amt_swap) - wait_for_bid( - test_delay_event, - swap_clients[id_offerer], - bid_id, - BidStates.BID_AACCEPT_FAIL, - wait_for=40, - ) - try: - swap_clients[id_offerer].acceptBid(bid_id) - except Exception as e: - assert "Insufficient funds" in str(e) - else: - assert False, "Should fail" + self.do_test_08_insufficient_funds(self.test_coin_from, Coins.XMR) def test_08_insufficient_funds_rev(self): - tla_from = self.test_coin_from.name - logging.info("---------- Test {} Insufficient Funds (reverse)".format(tla_from)) - swap_clients = self.swap_clients - coin_from = Coins.XMR - coin_to = self.test_coin_from - - self.prepare_balance(coin_to, 10.0, 1802, 1800) - - id_offerer: int = self.node_b_id - id_bidder: int = self.node_c_id - - swap_clients = self.swap_clients - ci_from = swap_clients[id_offerer].ci(coin_from) - ci_to = swap_clients[id_bidder].ci(coin_to) - - jsw = read_json_api(1800 + id_bidder, "wallets") - balance_to_before: float = self.getBalance(jsw, coin_to) - - amt_swap: int = ci_from.make_int(balance_to_before, r=1) - rate_swap: int = ci_to.make_int(1.0, r=1) - - amt_swap -= 1 - offer_id = swap_clients[id_offerer].postOffer( - coin_from, - coin_to, - amt_swap, - rate_swap, - amt_swap, - SwapTypes.XMR_SWAP, - auto_accept_bids=True, - ) - wait_for_offer(test_delay_event, swap_clients[id_bidder], offer_id) - - bid_id = swap_clients[id_bidder].postXmrBid(offer_id, amt_swap) - - event = wait_for_event( - test_delay_event, - swap_clients[id_bidder], - Concepts.BID, - bid_id, - event_type=EventLogTypes.ERROR, - wait_for=60, - ) - assert "Insufficient funds" in event.event_msg - - wait_for_bid( - test_delay_event, - swap_clients[id_bidder], - bid_id, - BidStates.BID_ERROR, - sent=True, - wait_for=20, - ) + self.do_test_08_insufficient_funds(Coins.XMR, self.test_coin_from) class TestBTC(BasicSwapTest):