From bb15aa0312b5de0553f798ba038736f5d6c2eff9 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Mon, 30 Nov 2020 16:43:50 +0000 Subject: [PATCH 1/2] Quiet/make more accurate fee information in sends Fixes #736. Prior to this commit, the 'relay fee floor' was being shown at INFO level in the command line output, but not the actual feerate paid (in direct send scenarios; the coinjoin scenario is rather more complex since fees are not known before negotiation flow, but estimates are printed). After this commit, the output at INFO level for direct sends shows specifically the feerate and then the actual fee, with min relay feerate relegated to DEBUG messages only, for cases of manual feerate setting. There is also some minor cleanup in comments and coinjoin fee estimate messages are removed for non-coinjoins. --- jmclient/jmclient/blockchaininterface.py | 24 ++++++++++++++++-------- scripts/sendpayment.py | 12 +++++++----- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index caf67c2..a7536f1 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/jmclient/jmclient/blockchaininterface.py @@ -406,20 +406,27 @@ class BitcoinCoreInterface(BlockchainInterface): return result def estimate_fee_per_kb(self, N): + """ The argument N may be either a number of blocks target, + for estimation of feerate by Core, or a number of satoshis + per kilo-vbyte (see `fee_per_kb_has_been_manually_set` for + how this is distinguished). + In the latter case, it is prevented from falling below the + minimum allowed feerate for relay on the Bitcoin network. + """ if super().fee_per_kb_has_been_manually_set(N): - # use the local bitcoin core relay fee as floor to avoid relay problems + # use the local bitcoin core relay fee as floor to avoid relay + # problems btc_relayfee = -1 rpc_result = self._rpc('getnetworkinfo', None) btc_relayfee = rpc_result.get('relayfee', btc_relayfee) if btc_relayfee > 0: relayfee_in_sat = int(Decimal(1e8) * Decimal(btc_relayfee)) - log.info("Using this min relay fee as tx fee floor: " + - btc.fee_per_kb_to_str(relayfee_in_sat)) - return int(max(relayfee_in_sat, random.uniform(N * float(0.8), N * float(1.2)))) else: # cannot get valid relayfee: fall back to 1000 sat/kbyte - log.info("Using this min relay fee as tx fee floor " + - "(fallback): " + btc.fee_per_kb_to_str(1000)) - return int(max(1000, random.uniform(N * float(0.8), N * float(1.2)))) + relayfee_in_sat = 1000 + log.debug("Using this min relay fee as tx feerate per kB floor: " + + btc.fee_per_kb_to_str(1000)) + return int(max(relayfee_in_sat, random.uniform(N * float(0.8), + N * float(1.2)))) # Special bitcoin core case: sometimes the highest priority # cannot be estimated in that case the 2nd highest priority @@ -438,7 +445,8 @@ class BitcoinCoreInterface(BlockchainInterface): if retval == -1: retval = int(Decimal(1e8) * Decimal(estimate)) - log.info("Using tx fee: " + btc.fee_per_kb_to_str(retval)) + log.info("Using bitcoin network feerate (per kB in sats): " +\ + btc.fee_per_kb_to_str(retval)) return retval def get_current_block_height(self): diff --git a/scripts/sendpayment.py b/scripts/sendpayment.py index 285f153..78bb250 100755 --- a/scripts/sendpayment.py +++ b/scripts/sendpayment.py @@ -172,17 +172,19 @@ def main(): # the sync call here will now be a no-op: wallet_service.startService() - # Dynamically estimate a realistic fee. + # Dynamically estimate a realistic fee, for coinjoins. # At this point we do not know even the number of our own inputs, so # we guess conservatively with 2 inputs and 2 outputs each. - fee_per_cp_guess = estimate_tx_fee(2, 2, txtype=wallet_service.get_txtype()) - log.debug("Estimated miner/tx fee for each cj participant: " + str( - fee_per_cp_guess)) + if options.makercount != 0: + fee_per_cp_guess = estimate_tx_fee(2, 2, + txtype=wallet_service.get_txtype()) + log.debug("Estimated miner/tx fee for each cj participant: " + str( + fee_per_cp_guess)) # From the estimated tx fees, check if the expected amount is a # significant value compared the the cj amount; currently enabled # only for single join (the predominant, non-advanced case) - if options.schedule == '': + if options.schedule == '' and options.makercount != 0: total_cj_amount = amount if total_cj_amount == 0: total_cj_amount = wallet_service.get_balance_by_mixdepth()[options.mixdepth] From 8cdf3e0a4ac218327fb9928d81273b2ca311fc12 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Tue, 1 Dec 2020 16:25:45 +0000 Subject: [PATCH 2/2] address comments of @kristapsk and @PulpCattel --- jmclient/jmclient/blockchaininterface.py | 43 +++++++++++++++--------- jmclient/jmclient/wallet.py | 3 ++ scripts/sendpayment.py | 4 +-- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index a7536f1..cf1e802 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/jmclient/jmclient/blockchaininterface.py @@ -412,19 +412,24 @@ class BitcoinCoreInterface(BlockchainInterface): how this is distinguished). In the latter case, it is prevented from falling below the minimum allowed feerate for relay on the Bitcoin network. + In case of failure to connect, None is returned. + In case of failure to source a specific minimum fee relay rate + (which is used to sanity check user's chosen fee rate), 1000 + is used. + In case of failure to source a feerate estimate for targeted + number of blocks, a default of 10000 is returned. """ if super().fee_per_kb_has_been_manually_set(N): # use the local bitcoin core relay fee as floor to avoid relay # problems - btc_relayfee = -1 rpc_result = self._rpc('getnetworkinfo', None) - btc_relayfee = rpc_result.get('relayfee', btc_relayfee) - if btc_relayfee > 0: - relayfee_in_sat = int(Decimal(1e8) * Decimal(btc_relayfee)) - else: # cannot get valid relayfee: fall back to 1000 sat/kbyte - relayfee_in_sat = 1000 - log.debug("Using this min relay fee as tx feerate per kB floor: " + - btc.fee_per_kb_to_str(1000)) + if not rpc_result: + # in case of connection error: + return None + btc_relayfee = rpc_result.get('relayfee', 1000) + relayfee_in_sat = int(Decimal(1e8) * Decimal(btc_relayfee)) + log.debug("Using this min relay fee as tx feerate floor: " + + btc.fee_per_kb_to_str(relayfee_in_sat)) return int(max(relayfee_in_sat, random.uniform(N * float(0.8), N * float(1.2)))) @@ -433,19 +438,25 @@ class BitcoinCoreInterface(BlockchainInterface): # should be used instead of falling back to hardcoded values tries = 2 if N == 1 else 1 - estimate = -1 - retval = -1 for i in range(tries): rpc_result = self._rpc('estimatesmartfee', [N + i]) - estimate = rpc_result.get('feerate', estimate) - if estimate > 0: + if not rpc_result: + # in case of connection error: + return None + estimate = rpc_result.get('feerate') + # `estimatesmartfee` will currently return in the format + # `{'errors': ['Insufficient data or no feerate found'], 'blocks': N}` + # if it is not able to make an estimate. We insist that + # the 'feerate' key is found and contains a positive value: + if estimate and estimate > 0: + retval = int(Decimal(1e8) * Decimal(estimate)) break - else: # estimate <= 0 + else: # cannot get a valid estimate after `tries` tries: + log.warn("Could not source a fee estimate from Core, " + + "falling back to default.") retval = 10000 - if retval == -1: - retval = int(Decimal(1e8) * Decimal(estimate)) - log.info("Using bitcoin network feerate (per kB in sats): " +\ + log.info("Using bitcoin network feerate: " +\ btc.fee_per_kb_to_str(retval)) return retval diff --git a/jmclient/jmclient/wallet.py b/jmclient/jmclient/wallet.py index 2752079..8c7e394 100644 --- a/jmclient/jmclient/wallet.py +++ b/jmclient/jmclient/wallet.py @@ -81,6 +81,9 @@ def estimate_tx_fee(ins, outs, txtype='p2pkh', extra_bytes=0): "without blockchain source.") fee_per_kb = jm_single().bc_interface.estimate_fee_per_kb( jm_single().config.getint("POLICY","tx_fees")) + if fee_per_kb is None: + raise RuntimeError("Cannot estimate fee per kB, possibly" + + " a failure of connection to the blockchain.") absurd_fee = jm_single().config.getint("POLICY", "absurd_fee_per_kb") if fee_per_kb > absurd_fee: #This error is considered critical; for safety reasons, shut down. diff --git a/scripts/sendpayment.py b/scripts/sendpayment.py index 78bb250..d343832 100755 --- a/scripts/sendpayment.py +++ b/scripts/sendpayment.py @@ -178,8 +178,8 @@ def main(): if options.makercount != 0: fee_per_cp_guess = estimate_tx_fee(2, 2, txtype=wallet_service.get_txtype()) - log.debug("Estimated miner/tx fee for each cj participant: " + str( - fee_per_cp_guess)) + log.debug("Estimated miner/tx fee for each cj participant: " + + btc.amount_to_str(fee_per_cp_guess)) # From the estimated tx fees, check if the expected amount is a # significant value compared the the cj amount; currently enabled