diff --git a/.msggen.json b/.msggen.json index 5b58cc3bfdc5..e795167369d4 100644 --- a/.msggen.json +++ b/.msggen.json @@ -357,6 +357,8 @@ }, "FeeratesPerkb": { "Feerates.perkb.delayed_to_us": 6, + "Feerates.perkb.estimates[]": 9, + "Feerates.perkb.floor": 10, "Feerates.perkb.htlc_resolution": 7, "Feerates.perkb.max_acceptable": 2, "Feerates.perkb.min_acceptable": 1, @@ -365,8 +367,15 @@ "Feerates.perkb.penalty": 8, "Feerates.perkb.unilateral_close": 5 }, + "FeeratesPerkbEstimates": { + "Feerates.perkb.estimates[].blockcount": 1, + "Feerates.perkb.estimates[].feerate": 2, + "Feerates.perkb.estimates[].smoothed_feerate": 3 + }, "FeeratesPerkw": { "Feerates.perkw.delayed_to_us": 6, + "Feerates.perkw.estimates[]": 9, + "Feerates.perkw.floor": 10, "Feerates.perkw.htlc_resolution": 7, "Feerates.perkw.max_acceptable": 2, "Feerates.perkw.min_acceptable": 1, @@ -375,6 +384,11 @@ "Feerates.perkw.penalty": 8, "Feerates.perkw.unilateral_close": 5 }, + "FeeratesPerkwEstimates": { + "Feerates.perkw.estimates[].blockcount": 1, + "Feerates.perkw.estimates[].feerate": 2, + "Feerates.perkw.estimates[].smoothed_feerate": 3 + }, "FeeratesRequest": { "Feerates.style": 1 }, @@ -1542,11 +1556,31 @@ }, "Feerates.perkb.delayed_to_us": { "added": "pre-v0.10.1", + "deprecated": "v23.05" + }, + "Feerates.perkb.estimates[]": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkb.estimates[].blockcount": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkb.estimates[].feerate": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkb.estimates[].smoothed_feerate": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkb.floor": { + "added": "v23.05", "deprecated": false }, "Feerates.perkb.htlc_resolution": { "added": "pre-v0.10.1", - "deprecated": false + "deprecated": "v23.05" }, "Feerates.perkb.max_acceptable": { "added": "pre-v0.10.1", @@ -1578,11 +1612,31 @@ }, "Feerates.perkw.delayed_to_us": { "added": "pre-v0.10.1", + "deprecated": "v23.05" + }, + "Feerates.perkw.estimates[]": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkw.estimates[].blockcount": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkw.estimates[].feerate": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkw.estimates[].smoothed_feerate": { + "added": "v23.05", + "deprecated": false + }, + "Feerates.perkw.floor": { + "added": "v23.05", "deprecated": false }, "Feerates.perkw.htlc_resolution": { "added": "pre-v0.10.1", - "deprecated": false + "deprecated": "v23.05" }, "Feerates.perkw.max_acceptable": { "added": "pre-v0.10.1", diff --git a/bitcoin/feerate.c b/bitcoin/feerate.c index fc73423940db..7788ebb2d3ba 100644 --- a/bitcoin/feerate.c +++ b/bitcoin/feerate.c @@ -1,8 +1,12 @@ #include "config.h" +#include #include u32 feerate_from_style(u32 feerate, enum feerate_style style) { + /* Make sure it's called somewhere! */ + assert(feerate_floor_check() == FEERATE_FLOOR); + switch (style) { case FEERATE_PER_KSIPA: return feerate; diff --git a/bitcoin/feerate.h b/bitcoin/feerate.h index 43bec21181d5..cab1e95e258c 100644 --- a/bitcoin/feerate.h +++ b/bitcoin/feerate.h @@ -39,7 +39,7 @@ enum feerate_style { FEERATE_PER_KBYTE }; -static inline u32 feerate_floor(void) +static inline u32 feerate_floor_check(void) { /* Assert that bitcoind will see this as above minRelayTxFee */ BUILD_ASSERT(FEERATE_BITCOIND_SEES(FEERATE_FLOOR, MINIMUM_TX_WEIGHT) diff --git a/channeld/watchtower.c b/channeld/watchtower.c index 269b4f405ad0..00f34d30fd66 100644 --- a/channeld/watchtower.c +++ b/channeld/watchtower.c @@ -96,7 +96,9 @@ penalty_tx_create(const tal_t *ctx, if (amount_sat_less(to_them_sats, min_out)) { /* FIXME: We should use SIGHASH_NONE so others can take it */ - fee = amount_tx_fee(feerate_floor(), weight); + /* We use the minimum possible fee here; if it doesn't + * propagate, who cares? */ + fee = amount_tx_fee(FEERATE_FLOOR, weight); } /* This can only happen if feerate_floor() is still too high; shouldn't diff --git a/cln-grpc/proto/node.proto b/cln-grpc/proto/node.proto index 4f48ee7ec0a5..2ff3e7a37272 100644 --- a/cln-grpc/proto/node.proto +++ b/cln-grpc/proto/node.proto @@ -1130,6 +1130,8 @@ message FeeratesResponse { message FeeratesPerkb { uint32 min_acceptable = 1; uint32 max_acceptable = 2; + optional uint32 floor = 10; + repeated FeeratesPerkbEstimates estimates = 9; optional uint32 opening = 3; optional uint32 mutual_close = 4; optional uint32 unilateral_close = 5; @@ -1138,9 +1140,17 @@ message FeeratesPerkb { optional uint32 penalty = 8; } +message FeeratesPerkbEstimates { + optional uint32 blockcount = 1; + optional uint32 feerate = 2; + optional uint32 smoothed_feerate = 3; +} + message FeeratesPerkw { uint32 min_acceptable = 1; uint32 max_acceptable = 2; + optional uint32 floor = 10; + repeated FeeratesPerkwEstimates estimates = 9; optional uint32 opening = 3; optional uint32 mutual_close = 4; optional uint32 unilateral_close = 5; @@ -1149,6 +1159,12 @@ message FeeratesPerkw { optional uint32 penalty = 8; } +message FeeratesPerkwEstimates { + optional uint32 blockcount = 1; + optional uint32 feerate = 2; + optional uint32 smoothed_feerate = 3; +} + message FeeratesOnchain_fee_estimates { uint64 opening_channel_satoshis = 1; uint64 mutual_close_satoshis = 2; diff --git a/cln-grpc/src/convert.rs b/cln-grpc/src/convert.rs index 5e2039db79f2..c5d1eff4564f 100644 --- a/cln-grpc/src/convert.rs +++ b/cln-grpc/src/convert.rs @@ -915,32 +915,62 @@ impl From for pb::DisconnectResponse { } } +#[allow(unused_variables,deprecated)] +impl From for pb::FeeratesPerkbEstimates { + fn from(c: responses::FeeratesPerkbEstimates) -> Self { + Self { + blockcount: c.blockcount, // Rule #2 for type u32? + feerate: c.feerate, // Rule #2 for type u32? + smoothed_feerate: c.smoothed_feerate, // Rule #2 for type u32? + } + } +} + #[allow(unused_variables,deprecated)] impl From for pb::FeeratesPerkb { fn from(c: responses::FeeratesPerkb) -> Self { Self { min_acceptable: c.min_acceptable, // Rule #2 for type u32 max_acceptable: c.max_acceptable, // Rule #2 for type u32 + floor: c.floor, // Rule #2 for type u32? + estimates: c.estimates.map(|arr| arr.into_iter().map(|i| i.into()).collect()).unwrap_or(vec![]), // Rule #3 opening: c.opening, // Rule #2 for type u32? mutual_close: c.mutual_close, // Rule #2 for type u32? unilateral_close: c.unilateral_close, // Rule #2 for type u32? + #[allow(deprecated)] delayed_to_us: c.delayed_to_us, // Rule #2 for type u32? + #[allow(deprecated)] htlc_resolution: c.htlc_resolution, // Rule #2 for type u32? penalty: c.penalty, // Rule #2 for type u32? } } } +#[allow(unused_variables,deprecated)] +impl From for pb::FeeratesPerkwEstimates { + fn from(c: responses::FeeratesPerkwEstimates) -> Self { + Self { + blockcount: c.blockcount, // Rule #2 for type u32? + feerate: c.feerate, // Rule #2 for type u32? + smoothed_feerate: c.smoothed_feerate, // Rule #2 for type u32? + } + } +} + #[allow(unused_variables,deprecated)] impl From for pb::FeeratesPerkw { fn from(c: responses::FeeratesPerkw) -> Self { Self { min_acceptable: c.min_acceptable, // Rule #2 for type u32 max_acceptable: c.max_acceptable, // Rule #2 for type u32 + floor: c.floor, // Rule #2 for type u32? + estimates: c.estimates.map(|arr| arr.into_iter().map(|i| i.into()).collect()).unwrap_or(vec![]), // Rule #3 opening: c.opening, // Rule #2 for type u32? mutual_close: c.mutual_close, // Rule #2 for type u32? unilateral_close: c.unilateral_close, // Rule #2 for type u32? + #[allow(deprecated)] delayed_to_us: c.delayed_to_us, // Rule #2 for type u32? + #[allow(deprecated)] htlc_resolution: c.htlc_resolution, // Rule #2 for type u32? penalty: c.penalty, // Rule #2 for type u32? } @@ -3252,12 +3282,25 @@ impl From for responses::DisconnectResponse { } } +#[allow(unused_variables,deprecated)] +impl From for responses::FeeratesPerkbEstimates { + fn from(c: pb::FeeratesPerkbEstimates) -> Self { + Self { + blockcount: c.blockcount, // Rule #1 for type u32? + feerate: c.feerate, // Rule #1 for type u32? + smoothed_feerate: c.smoothed_feerate, // Rule #1 for type u32? + } + } +} + #[allow(unused_variables,deprecated)] impl From for responses::FeeratesPerkb { fn from(c: pb::FeeratesPerkb) -> Self { Self { min_acceptable: c.min_acceptable, // Rule #1 for type u32 max_acceptable: c.max_acceptable, // Rule #1 for type u32 + floor: c.floor, // Rule #1 for type u32? + estimates: Some(c.estimates.into_iter().map(|s| s.into()).collect()), // Rule #4 opening: c.opening, // Rule #1 for type u32? mutual_close: c.mutual_close, // Rule #1 for type u32? unilateral_close: c.unilateral_close, // Rule #1 for type u32? @@ -3268,12 +3311,25 @@ impl From for responses::FeeratesPerkb { } } +#[allow(unused_variables,deprecated)] +impl From for responses::FeeratesPerkwEstimates { + fn from(c: pb::FeeratesPerkwEstimates) -> Self { + Self { + blockcount: c.blockcount, // Rule #1 for type u32? + feerate: c.feerate, // Rule #1 for type u32? + smoothed_feerate: c.smoothed_feerate, // Rule #1 for type u32? + } + } +} + #[allow(unused_variables,deprecated)] impl From for responses::FeeratesPerkw { fn from(c: pb::FeeratesPerkw) -> Self { Self { min_acceptable: c.min_acceptable, // Rule #1 for type u32 max_acceptable: c.max_acceptable, // Rule #1 for type u32 + floor: c.floor, // Rule #1 for type u32? + estimates: Some(c.estimates.into_iter().map(|s| s.into()).collect()), // Rule #4 opening: c.opening, // Rule #1 for type u32? mutual_close: c.mutual_close, // Rule #1 for type u32? unilateral_close: c.unilateral_close, // Rule #1 for type u32? diff --git a/cln-rpc/src/model.rs b/cln-rpc/src/model.rs index a79780216782..fc065cd5732d 100644 --- a/cln-rpc/src/model.rs +++ b/cln-rpc/src/model.rs @@ -3217,36 +3217,68 @@ pub mod responses { } } + #[derive(Clone, Debug, Deserialize, Serialize)] + pub struct FeeratesPerkbEstimates { + #[serde(skip_serializing_if = "Option::is_none")] + pub blockcount: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub feerate: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub smoothed_feerate: Option, + } + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct FeeratesPerkb { pub min_acceptable: u32, pub max_acceptable: u32, #[serde(skip_serializing_if = "Option::is_none")] + pub floor: Option, + #[serde(skip_serializing_if = "crate::is_none_or_empty")] + pub estimates: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub opening: Option, #[serde(skip_serializing_if = "Option::is_none")] pub mutual_close: Option, #[serde(skip_serializing_if = "Option::is_none")] pub unilateral_close: Option, + #[deprecated] #[serde(skip_serializing_if = "Option::is_none")] pub delayed_to_us: Option, + #[deprecated] #[serde(skip_serializing_if = "Option::is_none")] pub htlc_resolution: Option, #[serde(skip_serializing_if = "Option::is_none")] pub penalty: Option, } + #[derive(Clone, Debug, Deserialize, Serialize)] + pub struct FeeratesPerkwEstimates { + #[serde(skip_serializing_if = "Option::is_none")] + pub blockcount: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub feerate: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub smoothed_feerate: Option, + } + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct FeeratesPerkw { pub min_acceptable: u32, pub max_acceptable: u32, #[serde(skip_serializing_if = "Option::is_none")] + pub floor: Option, + #[serde(skip_serializing_if = "crate::is_none_or_empty")] + pub estimates: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub opening: Option, #[serde(skip_serializing_if = "Option::is_none")] pub mutual_close: Option, #[serde(skip_serializing_if = "Option::is_none")] pub unilateral_close: Option, + #[deprecated] #[serde(skip_serializing_if = "Option::is_none")] pub delayed_to_us: Option, + #[deprecated] #[serde(skip_serializing_if = "Option::is_none")] pub htlc_resolution: Option, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/common/jsonrpc_errors.h b/common/jsonrpc_errors.h index 899081765ed6..6744e87d30c6 100644 --- a/common/jsonrpc_errors.h +++ b/common/jsonrpc_errors.h @@ -71,6 +71,7 @@ enum jsonrpc_errcode { /* bitcoin-cli plugin errors */ BCLI_ERROR = 500, + BCLI_NO_FEE_ESTIMATES = 501, /* Errors from `invoice` or `delinvoice` commands */ INVOICE_LABEL_ALREADY_EXISTS = 900, diff --git a/common/utils.h b/common/utils.h index dcfa111cbc9e..2aadec5fc3cf 100644 --- a/common/utils.h +++ b/common/utils.h @@ -80,11 +80,22 @@ void clear_softref_(const tal_t *outer, size_t outersize, void **ptr); * Remove an element from an array * * This will shift the elements past the removed element, changing - * their position in memory, so only use this for arrays of pointers. + * their position in memory, so only use this for simple arrays. */ #define tal_arr_remove(p, n) tal_arr_remove_((p), sizeof(**p), (n)) void tal_arr_remove_(void *p, size_t elemsize, size_t n); +/** + * Insert an element in an array + */ +#define tal_arr_insert(p, n, v) \ + do { \ + size_t n_ = tal_count(*(p)); \ + tal_resize((p), n_+1); \ + memmove(*(p) + n + 1, *(p) + n, (n_ - n) * sizeof(**(p))); \ + (*(p))[n] = (v); \ + } while(0) + /* Check for valid UTF-8 */ bool utf8_check(const void *buf, size_t buflen); diff --git a/contrib/msggen/msggen/patch.py b/contrib/msggen/msggen/patch.py index d8b12ff2b01b..f25693bee027 100644 --- a/contrib/msggen/msggen/patch.py +++ b/contrib/msggen/msggen/patch.py @@ -67,7 +67,7 @@ def visit(self, f: model.Field) -> None: added = m.get('added', None) deprecated = m.get('deprecated', None) - assert added or not f.added, f"Field {f.path} does not have an `added` annotation" + assert added or f.added, f"Field {f.path} does not have an `added` annotation" # We do not allow the added and deprecated flags to be # modified after the fact. diff --git a/contrib/pyln-testing/pyln/testing/fixtures.py b/contrib/pyln-testing/pyln/testing/fixtures.py index c70bee6e4691..bf3293d24452 100644 --- a/contrib/pyln-testing/pyln/testing/fixtures.py +++ b/contrib/pyln-testing/pyln/testing/fixtures.py @@ -295,7 +295,7 @@ def is_feerate(checker, instance): return True if not checker.is_type(instance, "string"): return False - if instance in ("urgent", "normal", "slow"): + if instance in ("urgent", "normal", "slow", "minimum"): return True if instance in ("opening", "mutual_close", "unilateral_close", "delayed_to_us", "htlc_resolution", "penalty", "min_acceptable", "max_acceptable"): return True diff --git a/contrib/pyln-testing/pyln/testing/grpc2py.py b/contrib/pyln-testing/pyln/testing/grpc2py.py index b82591ec6376..00229aad6c77 100644 --- a/contrib/pyln-testing/pyln/testing/grpc2py.py +++ b/contrib/pyln-testing/pyln/testing/grpc2py.py @@ -729,10 +729,20 @@ def disconnect2py(m): }) +def feerates_perkb_estimates2py(m): + return remove_default({ + "blockcount": m.blockcount, # PrimitiveField in generate_composite + "feerate": m.feerate, # PrimitiveField in generate_composite + "smoothed_feerate": m.smoothed_feerate, # PrimitiveField in generate_composite + }) + + def feerates_perkb2py(m): return remove_default({ "min_acceptable": m.min_acceptable, # PrimitiveField in generate_composite "max_acceptable": m.max_acceptable, # PrimitiveField in generate_composite + "floor": m.floor, # PrimitiveField in generate_composite + "estimates": [feerates_perkb_estimates2py(i) for i in m.estimates], # ArrayField[composite] in generate_composite "opening": m.opening, # PrimitiveField in generate_composite "mutual_close": m.mutual_close, # PrimitiveField in generate_composite "unilateral_close": m.unilateral_close, # PrimitiveField in generate_composite @@ -742,10 +752,20 @@ def feerates_perkb2py(m): }) +def feerates_perkw_estimates2py(m): + return remove_default({ + "blockcount": m.blockcount, # PrimitiveField in generate_composite + "feerate": m.feerate, # PrimitiveField in generate_composite + "smoothed_feerate": m.smoothed_feerate, # PrimitiveField in generate_composite + }) + + def feerates_perkw2py(m): return remove_default({ "min_acceptable": m.min_acceptable, # PrimitiveField in generate_composite "max_acceptable": m.max_acceptable, # PrimitiveField in generate_composite + "floor": m.floor, # PrimitiveField in generate_composite + "estimates": [feerates_perkw_estimates2py(i) for i in m.estimates], # ArrayField[composite] in generate_composite "opening": m.opening, # PrimitiveField in generate_composite "mutual_close": m.mutual_close, # PrimitiveField in generate_composite "unilateral_close": m.unilateral_close, # PrimitiveField in generate_composite diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index 41e7707725d5..1ed1b35bc022 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -1191,7 +1191,7 @@ def mock_estimatesmartfee(r): self.daemon.rpcproxy.mock_rpc('estimatesmartfee', mock_estimatesmartfee) # Technically, this waits until it's called, not until it's processed. - # We wait until all three levels have been called. + # We wait until all four levels have been called. if wait_for_effect: wait_for(lambda: self.daemon.rpcproxy.mock_counts['estimatesmartfee'] >= 4) @@ -1228,6 +1228,30 @@ def wait_for_onchaind_txs(self, *args): def wait_for_onchaind_tx(self, name, resolve): return self.wait_for_onchaind_txs((name, resolve))[0] + def mine_txid_or_rbf(self, txid, numblocks=1): + """Wait for a txid to be broadcast, or an rbf. Return the one actually mined""" + # Hack so we can mutate the txid: pass it in a list + def rbf_or_txid_broadcast(txids): + # RBF onchain txid d4b597505b543a4b8b42ab4d481fd7a533febb7e7df150ca70689e6d046612f7 (fee 6564sat) with txid 979878b8f855d3895d1cd29bd75a60b21492c4842e38099186a8e649bee02c7c (fee 8205sat) + line = self.daemon.is_in_log("RBF onchain txid {}".format(txids[-1])) + if line is not None: + newtxid = re.search(r'with txid ([0-9a-fA-F]*)', line).group(1) + txids.append(newtxid) + mempool = self.bitcoin.rpc.getrawmempool() + return any([t in mempool for t in txids]) + + txids = [txid] + wait_for(lambda: rbf_or_txid_broadcast(txids)) + blocks = self.bitcoin.generate_block(numblocks) + + # It might have snuck an RBF in at the last minute! + rbf_or_txid_broadcast(txids) + + for tx in self.bitcoin.rpc.getblock(blocks[0])['tx']: + if tx in txids: + return tx + raise ValueError("None of the rbf txs were mined?") + def wait_for_onchaind_broadcast(self, name, resolve=None): """Wait for onchaind to drop tx name to resolve (if any)""" if resolve: diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index e8f7c21be5e1..bc31025010f2 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -1729,17 +1729,26 @@ The plugin must respond to `getchaininfo` with the following fields: Polled by `lightningd` to get the current feerate, all values must be passed in sat/kVB. -If fee estimation fails, the plugin must set all the fields to `null`. - -The plugin, if fee estimation succeeds, must respond with the following fields: - - `opening` (number), used for funding and also misc transactions - - `mutual_close` (number), used for the mutual close transaction - - `unilateral_close` (number), used for unilateral close (/commitment) transactions - - `delayed_to_us` (number), used for resolving our output from our unilateral close - - `htlc_resolution` (number), used for resolving HTLCs after an unilateral close - - `penalty` (number), used for resolving revoked transactions - - `min_acceptable` (number), used as the minimum acceptable feerate - - `max_acceptable` (number), used as the maximum acceptable feerate +The plugin must return `feerate_floor` (e.g. 1000 if mempool is +empty), and an array of 0 or more `feerates`. Each element of +`feerates` is an object with `blocks` and `feerate`, in +ascending-blocks order, for example: + +``` +{ + "feerate_floor": , + "feerates": { + { "blocks": 2, "feerate": }, + { "blocks": 6, "feerate": }, + { "blocks": 12, "feerate": } + { "blocks": 100, "feerate": } + } +} +``` + +lightningd will currently linearly interpolate to estimate between +given blocks (it will not extrapolate, but use the min/max blocks +values). ### `getrawblockbyheight` diff --git a/doc/lightning-close.7.md b/doc/lightning-close.7.md index 8107124fbbd5..853df15c6fe1 100644 --- a/doc/lightning-close.7.md +++ b/doc/lightning-close.7.md @@ -66,16 +66,7 @@ unless this flag is passed in. Defaults to false. *feerange* is an optional array [ *min*, *max* ], indicating the minimum and maximum feerates to offer: the peer will obey these if it supports the quick-close protocol. *slow* and *unilateral\_close* are -the defaults. - -Rates are one of the strings *urgent* (aim for next block), *normal* -(next 4 blocks or so) or *slow* (next 100 blocks or so) to use -lightningd's internal estimates, or one of the names from -lightning-feerates(7). Otherwise, they can be numbers with -an optional suffix: *perkw* means the number is interpreted as -satoshi-per-kilosipa (weight), and *perkb* means it is interpreted -bitcoind-style as satoshi-per-kilobyte. Omitting the suffix is -equivalent to *perkb*. +the defaults. See NOTES in lightning-feerates(7) for possible values. Note that the maximum fee will be capped at the final commitment transaction fee (unless the experimental anchor-outputs option is diff --git a/doc/lightning-feerates.7.md b/doc/lightning-feerates.7.md index db4139d929bd..76dcaece31d8 100644 --- a/doc/lightning-feerates.7.md +++ b/doc/lightning-feerates.7.md @@ -48,27 +48,37 @@ RETURN VALUE On success, an object is returned, containing: - **perkb** (object, optional): If *style* parameter was perkb: - - **min\_acceptable** (u32): The smallest feerate that you can use, usually the minimum relayed feerate of the backend + - **min\_acceptable** (u32): The smallest feerate that we allow peers to specify: half the 100-block estimate - **max\_acceptable** (u32): The largest feerate we will accept from remote negotiations. If a peer attempts to set the feerate higher than this we will unilaterally close the channel (or simply forget it if it's not open yet). + - **floor** (u32): The smallest feerate that our backend tells us it will accept (i.e. minrelayfee or mempoolminfee) *(added v23.05)* + - **estimates** (array of objects): Feerate estimates from plugin which we are using (usuallly bcli) *(added v23.05)*: + - **blockcount** (u32): The number of blocks the feerate is expected to get a transaction in *(added v23.05)* + - **feerate** (u32): The feerate for this estimate, in given *style* *(added v23.05)* + - **smoothed\_feerate** (u32): The feerate, smoothed over time (useful for coordinating with other nodes) *(added v23.05)* - **opening** (u32, optional): Default feerate for lightning-fundchannel(7) and lightning-withdraw(7) - **mutual\_close** (u32, optional): Feerate to aim for in cooperative shutdown. Note that since mutual close is a **negotiation**, the actual feerate used in mutual close will be somewhere between this and the corresponding mutual close feerate of the peer. - **unilateral\_close** (u32, optional): Feerate for commitment\_transaction in a live channel which we originally funded - - **delayed\_to\_us** (u32, optional): Feerate for returning unilateral close funds to our wallet - - **htlc\_resolution** (u32, optional): Feerate for returning unilateral close HTLC outputs to our wallet - - **penalty** (u32, optional): Feerate to start at when penalizing a cheat attempt + - **delayed\_to\_us** (u32, optional): Feerate for returning unilateral close funds to our wallet **deprecated, removal in v24.02** + - **htlc\_resolution** (u32, optional): Feerate for returning unilateral close HTLC outputs to our wallet **deprecated, removal in v24.02** + - **penalty** (u32, optional): Feerate to use when creating penalty tx for watchtowers - **perkw** (object, optional): If *style* parameter was perkw: - **min\_acceptable** (u32): The smallest feerate that you can use, usually the minimum relayed feerate of the backend - **max\_acceptable** (u32): The largest feerate we will accept from remote negotiations. If a peer attempts to set the feerate higher than this we will unilaterally close the channel (or simply forget it if it's not open yet). + - **floor** (u32): The smallest feerate that our backend tells us it will accept (i.e. minrelayfee or mempoolminfee) *(added v23.05)* + - **estimates** (array of objects): Feerate estimates from plugin which we are using (usuallly bcli) *(added v23.05)*: + - **blockcount** (u32): The number of blocks the feerate is expected to get a transaction in *(added v23.05)* + - **feerate** (u32): The feerate for this estimate, in given *style* *(added v23.05)* + - **smoothed\_feerate** (u32): The feerate, smoothed over time (useful for coordinating with other nodes) *(added v23.05)* - **opening** (u32, optional): Default feerate for lightning-fundchannel(7) and lightning-withdraw(7) - **mutual\_close** (u32, optional): Feerate to aim for in cooperative shutdown. Note that since mutual close is a **negotiation**, the actual feerate used in mutual close will be somewhere between this and the corresponding mutual close feerate of the peer. - **unilateral\_close** (u32, optional): Feerate for commitment\_transaction in a live channel which we originally funded - - **delayed\_to\_us** (u32, optional): Feerate for returning unilateral close funds to our wallet - - **htlc\_resolution** (u32, optional): Feerate for returning unilateral close HTLC outputs to our wallet - - **penalty** (u32, optional): Feerate to start at when penalizing a cheat attempt + - **delayed\_to\_us** (u32, optional): Feerate for returning unilateral close funds to our wallet **deprecated, removal in v24.02** + - **htlc\_resolution** (u32, optional): Feerate for returning unilateral close HTLC outputs to our wallet **deprecated, removal in v24.02** + - **penalty** (u32, optional): Feerate to use when creating penalty tx for watchtowers - **onchain\_fee\_estimates** (object, optional): - **opening\_channel\_satoshis** (u64): Estimated cost of typical channel open - **mutual\_close\_satoshis** (u64): Estimated cost of typical channel close - - **unilateral\_close\_satoshis** (u64): Estimated cost of typical unilateral close (without HTLCs) + - **unilateral\_close\_satoshis** (u64): Estimated cost of typical (non-anchor) unilateral close (without HTLCs) - **htlc\_timeout\_satoshis** (u64): Estimated cost of typical HTLC timeout transaction - **htlc\_success\_satoshis** (u64): Estimated cost of typical HTLC fulfillment transaction @@ -88,13 +98,20 @@ if feerate estimates for that kind of transaction are unavailable. NOTES ----- -Many other commands have a *feerate* parameter, which can be the strings -*urgent*, *normal*, or *slow*. -These are mapped to the **feerates** outputs as: +Many other commands have a *feerate* parameter. This can be: + +* One of the strings to use lightningd's internal estimates: + * *urgent* (aim for next block), + * *normal* (next 6 blocks or so) + * *slow* (next 100 blocks or so) + * *minimum* for the lowest value bitcoind will currently accept (added in v23.05) -* *urgent* - equal to *unilateral\_close* -* *normal* - equal to *opening* -* *slow* - equal to *min\_acceptable*. +* A number, with an optional suffix: + * *blocks* means aim for confirmation in that many blocks (added in v23.05) + * *perkw* means the number is interpreted as satoshi-per-kilosipa (weight) + * *perkb* means it is interpreted bitcoind-style as satoshi-per-kilobyte. + +Omitting the suffix is equivalent to *perkb*. TRIVIA ------ @@ -121,4 +138,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:a34af89895413ee57defa1df715d9e50d34a49196972a81b31a4666b4fc05a10) +[comment]: # ( SHA256STAMP:4921275aec48da8b9ddcba5d4237efa72f06b6e005008f2c3aa7029d3bd187fd) diff --git a/doc/lightning-fundchannel.7.md b/doc/lightning-fundchannel.7.md index 5ee6c46a2031..b8c15e8425d1 100644 --- a/doc/lightning-fundchannel.7.md +++ b/doc/lightning-fundchannel.7.md @@ -35,16 +35,9 @@ decimal places ending in *btc*. The value cannot be less than the dust limit, currently set to 546, nor more than 16777215 satoshi (unless large channels were negotiated with the peer). -*feerate* is an optional feerate used for the opening transaction and as -initial feerate for commitment and HTLC transactions. It can be one of -the strings *urgent* (aim for next block), *normal* (next 4 blocks or -so) or *slow* (next 100 blocks or so) to use lightningd's internal -estimates: *normal* is the default. - -Otherwise, *feerate* is a number, with an optional suffix: *perkw* means -the number is interpreted as satoshi-per-kilosipa (weight), and *perkb* -means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting -the suffix is equivalent to *perkb*. +*feerate* is an optional feerate used for the opening transaction and +as initial feerate for commitment and HTLC transactions (see NOTES in +lightning-feerates(7)). The default is *normal*. *announce* is an optional flag that triggers whether to announce this channel or not. Defaults to `true`. An unannounced channel is considered diff --git a/doc/lightning-fundpsbt.7.md b/doc/lightning-fundpsbt.7.md index 5e25e4861b5c..eaac0fb05e3c 100644 --- a/doc/lightning-fundpsbt.7.md +++ b/doc/lightning-fundpsbt.7.md @@ -18,14 +18,8 @@ be a whole number, a whole number ending in *sat*, a whole number ending in *000msat*, or a number with 1 to 8 decimal places ending in *btc*. -*feerate* can be one of the feerates listed in lightning-feerates(7), -or one of the strings *urgent* (aim for next block), *normal* (next 4 -blocks or so) or *slow* (next 100 blocks or so) to use lightningd's -internal estimates. It can also be a *feerate* is a number, with an -optional suffix: *perkw* means the number is interpreted as -satoshi-per-kilosipa (weight), and *perkb* means it is interpreted -bitcoind-style as satoshi-per-kilobyte. Omitting the suffix is -equivalent to *perkb*. +*feerate* is an optional feerate: see NOTES in lightning-feerates(7) +for possible values. The default is *normal*. *startweight* is the weight of the transaction before *fundpsbt* has added any inputs. diff --git a/doc/lightning-listconfigs.7.md b/doc/lightning-listconfigs.7.md index fe99657469a3..f59e05719eaa 100644 --- a/doc/lightning-listconfigs.7.md +++ b/doc/lightning-listconfigs.7.md @@ -106,6 +106,7 @@ On success, an object is returned, containing: - **dev-allowdustreserve** (boolean, optional): Whether we allow setting dust reserves - **announce-addr-dns** (boolean, optional): Whether we put DNS entries into node\_announcement *(added v22.11.1)* - **require-confirmed-inputs** (boolean, optional): Request peers to only send confirmed inputs (dual-fund only) +- **commit-fee** (u64, optional): The percentage of the 6-block fee estimate to use for commitment transactions *(added v23.05)* [comment]: # (GENERATE-FROM-SCHEMA-END) @@ -224,4 +225,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:1088401b9aeae1e079dab550d3b035ef82195f0466ad471bc7373386182f37dc) +[comment]: # ( SHA256STAMP:b24158a61bb79aaf3f0f6d1c20a4b10d474613b371e80aede4aeb59ab471a989) diff --git a/doc/lightning-multifundchannel.7.md b/doc/lightning-multifundchannel.7.md index 2c6b63a02376..0203be78e939 100644 --- a/doc/lightning-multifundchannel.7.md +++ b/doc/lightning-multifundchannel.7.md @@ -65,17 +65,10 @@ Readiness is indicated by **listpeers** reporting a *state* of There must be at least one entry in *destinations*; it cannot be an empty array. -*feerate* is an optional feerate used for the opening transaction and, if -*commitment\_feerate* is not set, as the initial feerate for -commitment and HTLC transactions. It can be one of -the strings *urgent* (aim for next block), *normal* (next 4 blocks or -so) or *slow* (next 100 blocks or so) to use lightningd's internal -estimates: *normal* is the default. - -Otherwise, *feerate* is a number, with an optional suffix: *perkw* means -the number is interpreted as satoshi-per-kilosipa (weight), and *perkb* -means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting -the suffix is equivalent to *perkb*. +*feerate* is an optional feerate used for the opening transaction, and +if *commitment\_feerate* is not set, as initial feerate for commitment +and HTLC transactions. See NOTES in lightning-feerates(7) for possible +values. The default is *normal*. *minconf* specifies the minimum number of confirmations that used outputs should have. Default is 1. diff --git a/doc/lightning-multiwithdraw.7.md b/doc/lightning-multiwithdraw.7.md index ae09c2bdf251..a1807c7e8218 100644 --- a/doc/lightning-multiwithdraw.7.md +++ b/doc/lightning-multiwithdraw.7.md @@ -21,15 +21,8 @@ a whole number ending in *sat*, a whole number ending in *000msat*, or a number with 1 to 8 decimal places ending in *btc*. -*feerate* is an optional feerate to use. It can be one of the strings -*urgent* (aim for next block), *normal* (next 4 blocks or so) or *slow* -(next 100 blocks or so) to use lightningd's internal estimates: *normal* -is the default. - -Otherwise, *feerate* is a number, with an optional suffix: *perkw* means -the number is interpreted as satoshi-per-kilosipa (weight), and *perkb* -means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting -the suffix is equivalent to *perkb*. +*feerate* is an optional feerate: see NOTES in lightning-feerates(7) +for possible values. The default is *normal*. *minconf* specifies the minimum number of confirmations that used outputs should have. Default is 1. diff --git a/doc/lightning-txprepare.7.md b/doc/lightning-txprepare.7.md index 37655098c771..62681d142d43 100644 --- a/doc/lightning-txprepare.7.md +++ b/doc/lightning-txprepare.7.md @@ -29,15 +29,8 @@ all available funds. Otherwise, it is in amount precision; it can be a whole number, a whole number ending in *sat*, a whole number ending in *000msat*, or a number with 1 to 8 decimal places ending in *btc*. -*feerate* is an optional feerate to use. It can be one of the strings -*urgent* (aim for next block), *normal* (next 4 blocks or so) or *slow* -(next 100 blocks or so) to use lightningd's internal estimates: *normal* -is the default. - -Otherwise, *feerate* is a number, with an optional suffix: *perkw* means -the number is interpreted as satoshi-per-kilosipa (weight), and *perkb* -means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting -the suffix is equivalent to *perkb*. +*feerate* is an optional feerate to use: see NOTES in lightning-feerates(7) +for possible values. The default is *normal*. *minconf* specifies the minimum number of confirmations that used outputs should have. Default is 1. diff --git a/doc/lightning-upgradewallet.7.md b/doc/lightning-upgradewallet.7.md index 7df504509d88..a92cf7d04fe3 100644 --- a/doc/lightning-upgradewallet.7.md +++ b/doc/lightning-upgradewallet.7.md @@ -12,14 +12,8 @@ DESCRIPTION `upgradewallet` is a convenience RPC which will spend all p2sh-wrapped Segwit deposits in a wallet into a single Native Segwit P2WPKH address. -*feerate* can be one of the feerates listed in lightning-feerates(7), -or one of the strings *urgent* (aim for next block), *normal* (next 4 -blocks or so) or *slow* (next 100 blocks or so) to use lightningd's -internal estimates. It can also be a *feerate* is a number, with an -optional suffix: *perkw* means the number is interpreted as -satoshi-per-kilosipa (weight), and *perkb* means it is interpreted -bitcoind-style as satoshi-per-kilobyte. Omitting the suffix is -equivalent to *perkb*. +*feerate* is an optional feerate: see NOTES in lightning-feerates(7) +for possible values. The default is *opening*. *reservedok* tells the wallet to include all P2SH-wrapped inputs, including reserved ones. diff --git a/doc/lightning-withdraw.7.md b/doc/lightning-withdraw.7.md index d5e188d09657..23ae414ffda1 100644 --- a/doc/lightning-withdraw.7.md +++ b/doc/lightning-withdraw.7.md @@ -21,15 +21,8 @@ satoshi precision; it can be a whole number, a whole number ending in *sat*, a whole number ending in *000msat*, or a number with 1 to 8 decimal places ending in *btc*. -*feerate* is an optional feerate to use. It can be one of the strings -*urgent* (aim for next block), *normal* (next 4 blocks or so) or *slow* -(next 100 blocks or so) to use lightningd's internal estimates: *normal* -is the default. - -Otherwise, *feerate* is a number, with an optional suffix: *perkw* means -the number is interpreted as satoshi-per-kilosipa (weight), and *perkb* -means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting -the suffix is equivalent to *perkb*. +*feerate* is an optional feerate: see NOTES in lightning-feerates(7) +for possible values. The default is *normal*. *minconf* specifies the minimum number of confirmations that used outputs should have. Default is 1. diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 0621ccb72e91..4ba84d8605b0 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -401,7 +401,7 @@ create a channel, and if an HTLC asks for longer, we'll refuse it. Confirmations required for the funding transaction when the other side opens a channel before the channel is usable. -* **commit-fee**=*PERCENT* [plugin `bcli`] +* **commit-fee**=*PERCENT* The percentage of *estimatesmartfee 2/CONSERVATIVE* to use for the commitment transactions: default is 100. diff --git a/doc/schemas/feerates.schema.json b/doc/schemas/feerates.schema.json index 980f1d20c5a6..9cff8a8bb53e 100644 --- a/doc/schemas/feerates.schema.json +++ b/doc/schemas/feerates.schema.json @@ -14,17 +14,55 @@ "additionalProperties": false, "required": [ "min_acceptable", - "max_acceptable" + "max_acceptable", + "floor", + "estimates" ], "properties": { "min_acceptable": { "type": "u32", - "description": "The smallest feerate that you can use, usually the minimum relayed feerate of the backend" + "description": "The smallest feerate that we allow peers to specify: half the 100-block estimate" }, "max_acceptable": { "type": "u32", "description": "The largest feerate we will accept from remote negotiations. If a peer attempts to set the feerate higher than this we will unilaterally close the channel (or simply forget it if it's not open yet)." }, + "floor": { + "type": "u32", + "added": "v23.05", + "description": "The smallest feerate that our backend tells us it will accept (i.e. minrelayfee or mempoolminfee)" + }, + "estimates": { + "type": "array", + "added": "v23.05", + "description": "Feerate estimates from plugin which we are using (usuallly bcli)", + "items": { + "type": "object", + "additionalProperties": false, + "required": [ + "blockcount", + "feerate", + "smoothed_feerate" + ], + "properties": { + "blockcount": { + "type": "u32", + "added": "v23.05", + "description": "The number of blocks the feerate is expected to get a transaction in" + }, + "feerate": { + "type": "u32", + "added": "v23.05", + "description": "The feerate for this estimate, in given *style*" + }, + "smoothed_feerate": { + "type": "u32", + "added": "v23.05", + "description": "The feerate, smoothed over time (useful for coordinating with other nodes)" + } + } + } + }, "opening": { "type": "u32", "description": "Default feerate for lightning-fundchannel(7) and lightning-withdraw(7)" @@ -39,15 +77,17 @@ }, "delayed_to_us": { "type": "u32", + "deprecated": "v23.05", "description": "Feerate for returning unilateral close funds to our wallet" }, "htlc_resolution": { "type": "u32", + "deprecated": "v23.05", "description": "Feerate for returning unilateral close HTLC outputs to our wallet" }, "penalty": { "type": "u32", - "description": "Feerate to start at when penalizing a cheat attempt" + "description": "Feerate to use when creating penalty tx for watchtowers" } } }, @@ -57,7 +97,9 @@ "additionalProperties": false, "required": [ "min_acceptable", - "max_acceptable" + "max_acceptable", + "floor", + "estimates" ], "properties": { "min_acceptable": { @@ -68,6 +110,42 @@ "type": "u32", "description": "The largest feerate we will accept from remote negotiations. If a peer attempts to set the feerate higher than this we will unilaterally close the channel (or simply forget it if it's not open yet)." }, + "floor": { + "type": "u32", + "added": "v23.05", + "description": "The smallest feerate that our backend tells us it will accept (i.e. minrelayfee or mempoolminfee)" + }, + "estimates": { + "type": "array", + "added": "v23.05", + "description": "Feerate estimates from plugin which we are using (usuallly bcli)", + "items": { + "type": "object", + "additionalProperties": false, + "required": [ + "blockcount", + "feerate", + "smoothed_feerate" + ], + "properties": { + "blockcount": { + "type": "u32", + "added": "v23.05", + "description": "The number of blocks the feerate is expected to get a transaction in" + }, + "feerate": { + "type": "u32", + "added": "v23.05", + "description": "The feerate for this estimate, in given *style*" + }, + "smoothed_feerate": { + "type": "u32", + "added": "v23.05", + "description": "The feerate, smoothed over time (useful for coordinating with other nodes)" + } + } + } + }, "opening": { "type": "u32", "description": "Default feerate for lightning-fundchannel(7) and lightning-withdraw(7)" @@ -82,15 +160,17 @@ }, "delayed_to_us": { "type": "u32", + "deprecated": "v23.05", "description": "Feerate for returning unilateral close funds to our wallet" }, "htlc_resolution": { "type": "u32", + "deprecated": "v23.05", "description": "Feerate for returning unilateral close HTLC outputs to our wallet" }, "penalty": { "type": "u32", - "description": "Feerate to start at when penalizing a cheat attempt" + "description": "Feerate to use when creating penalty tx for watchtowers" } } }, @@ -115,7 +195,7 @@ }, "unilateral_close_satoshis": { "type": "u64", - "description": "Estimated cost of typical unilateral close (without HTLCs)" + "description": "Estimated cost of typical (non-anchor) unilateral close (without HTLCs)" }, "htlc_timeout_satoshis": { "type": "u64", diff --git a/doc/schemas/listconfigs.schema.json b/doc/schemas/listconfigs.schema.json index b2ebdd003c68..32e7b096d514 100644 --- a/doc/schemas/listconfigs.schema.json +++ b/doc/schemas/listconfigs.schema.json @@ -319,6 +319,11 @@ "require-confirmed-inputs": { "type": "boolean", "description": "Request peers to only send confirmed inputs (dual-fund only)" + }, + "commit-fee": { + "type": "u64", + "added": "v23.05", + "description": "The percentage of the 6-block fee estimate to use for commitment transactions" } } } diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 1baec3c587a5..e4bbb1eba25a 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -144,7 +145,7 @@ static void bitcoin_plugin_send(struct bitcoind *bitcoind, * - `min` is the minimum acceptable feerate * - `max` is the maximum acceptable feerate * - * Plugin response: + * Plugin response (deprecated): * { * "opening": , * "mutual_close": , @@ -155,21 +156,143 @@ static void bitcoin_plugin_send(struct bitcoind *bitcoind, * "min_acceptable": , * "max_acceptable": , * } + * + * Plugin response (modern): + * { + * "feerate_floor": , + * "feerates": { + * { "blocks": 2, "feerate": }, + * { "blocks": 6, "feerate": }, + * { "blocks": 12, "feerate": } + * { "blocks": 100, "feerate": } + * } + * } + * + * If rates are missing, we linearly interpolate (we don't extrapolate tho!). */ - struct estimatefee_call { struct bitcoind *bitcoind; - void (*cb)(struct bitcoind *bitcoind, const u32 satoshi_per_kw[], - void *); - void *arg; + void (*cb)(struct lightningd *ld, u32 feerate_floor, + const struct feerate_est *rates); }; +/* Note: returns estimates in perkb, caller converts! */ +static struct feerate_est *parse_feerate_ranges(const tal_t *ctx, + struct bitcoind *bitcoind, + const char *buf, + const jsmntok_t *floortok, + const jsmntok_t *feerates, + u32 *floor) +{ + size_t i; + const jsmntok_t *t; + struct feerate_est *rates = tal_arr(ctx, struct feerate_est, 0); + + if (!json_to_u32(buf, floortok, floor)) + bitcoin_plugin_error(bitcoind, buf, floortok, + "estimatefees.feerate_floor", "Not a u32?"); + + json_for_each_arr(i, t, feerates) { + struct feerate_est rate; + const char *err; + + err = json_scan(tmpctx, buf, t, "{blocks:%,feerate:%}", + JSON_SCAN(json_to_u32, &rate.blockcount), + JSON_SCAN(json_to_u32, &rate.rate)); + if (err) + bitcoin_plugin_error(bitcoind, buf, t, + "estimatefees.feerates", err); + + /* Block count must be in order. If rates go up somehow, we + * reduce to prev. */ + if (tal_count(rates) != 0) { + const struct feerate_est *prev = &rates[tal_count(rates)-1]; + if (rate.blockcount <= prev->blockcount) + bitcoin_plugin_error(bitcoind, buf, feerates, + "estimatefees.feerates", + "Blocks must be ascending" + " order: %u <= %u!", + rate.blockcount, + prev->blockcount); + if (rate.rate > prev->rate) { + log_unusual(bitcoind->log, + "Feerate for %u blocks (%u) is > rate" + " for %u blocks (%u)!", + rate.blockcount, rate.rate, + prev->blockcount, prev->rate); + rate.rate = prev->rate; + } + } + + tal_arr_expand(&rates, rate); + } + + if (tal_count(rates) == 0) { + if (chainparams->testnet) + log_debug(bitcoind->log, "Unable to estimate any fees"); + else + log_unusual(bitcoind->log, "Unable to estimate any fees"); + } + + return rates; +} + +static struct feerate_est *parse_deprecated_feerates(const tal_t *ctx, + struct bitcoind *bitcoind, + const char *buf, + const jsmntok_t *toks) +{ + struct feerate_est *rates = tal_arr(ctx, struct feerate_est, 0); + struct oldstyle { + const char *name; + size_t blockcount; + size_t multiplier; + } oldstyles[] = { { "max_acceptable", 2, 10 }, + { "unilateral_close", 6, 1 }, + { "opening", 12, 1 }, + { "mutual_close", 100, 1 } }; + + for (size_t i = 0; i < ARRAY_SIZE(oldstyles); i++) { + const jsmntok_t *feeratetok; + struct feerate_est rate; + + feeratetok = json_get_member(buf, toks, oldstyles[i].name); + if (!feeratetok) { + bitcoin_plugin_error(bitcoind, buf, toks, + "estimatefees", + "missing '%s' field", + oldstyles[i].name); + } + if (!json_to_u32(buf, feeratetok, &rate.rate)) { + if (chainparams->testnet) + log_debug(bitcoind->log, + "Unable to estimate %s fees", + oldstyles[i].name); + else + log_unusual(bitcoind->log, + "Unable to estimate %s fees", + oldstyles[i].name); + continue; + } + + if (rate.rate == 0) + continue; + + /* Cancel out the 10x multiplier on max_acceptable */ + rate.rate /= oldstyles[i].multiplier; + rate.blockcount = oldstyles[i].blockcount; + tal_arr_expand(&rates, rate); + } + return rates; +} + static void estimatefees_callback(const char *buf, const jsmntok_t *toks, const jsmntok_t *idtok, struct estimatefee_call *call) { - const jsmntok_t *resulttok, *feeratetok; - u32 *feerates = tal_arr(call, u32, NUM_FEERATES); + const jsmntok_t *resulttok, *floortok; + struct feerate_est *feerates; + u32 floor; resulttok = json_get_member(buf, toks, "result"); if (!resulttok) @@ -177,66 +300,54 @@ static void estimatefees_callback(const char *buf, const jsmntok_t *toks, "estimatefees", "bad 'result' field"); - for (int f = 0; f < NUM_FEERATES; f++) { - feeratetok = json_get_member(buf, resulttok, feerate_name(f)); - if (!feeratetok) - bitcoin_plugin_error(call->bitcoind, buf, toks, + /* Modern style has floor. */ + floortok = json_get_member(buf, resulttok, "feerate_floor"); + if (floortok) { + feerates = parse_feerate_ranges(call, call->bitcoind, + buf, floortok, + json_get_member(buf, resulttok, + "feerates"), + &floor); + } else { + if (!deprecated_apis) + bitcoin_plugin_error(call->bitcoind, buf, resulttok, "estimatefees", - "missing '%s' field", feerate_name(f)); - /* We still use the bcli plugin for min and max, even with - * force_feerates */ - if (f < tal_count(call->bitcoind->ld->force_feerates)) { - feerates[f] = call->bitcoind->ld->force_feerates[f]; - continue; - } + "missing fee_floor field"); - /* FIXME: We could trawl recent blocks for median fee... */ - if (!json_to_u32(buf, feeratetok, &feerates[f])) { - if (chainparams->testnet) - log_debug(call->bitcoind->log, - "Unable to estimate %s fees", - feerate_name(f)); - else - log_unusual(call->bitcoind->log, - "Unable to estimate %s fees", - feerate_name(f)); - -#if DEVELOPER - /* This is needed to test for failed feerate estimates - * in DEVELOPER mode */ - feerates[f] = 0; -#else - /* If we are in testnet mode we want to allow payments - * with the minimal fee even if the estimate didn't - * work out. This is less disruptive than erring out - * all the time. */ - if (chainparams->testnet) - feerates[f] = FEERATE_FLOOR; - else - feerates[f] = 0; -#endif - } else - /* Rate in satoshi per kw. */ - feerates[f] = feerate_from_style(feerates[f], - FEERATE_PER_KBYTE); + feerates = parse_deprecated_feerates(call, call->bitcoind, + buf, resulttok); + floor = feerate_from_style(FEERATE_FLOOR, FEERATE_PER_KSIPA); } - call->cb(call->bitcoind, feerates, call->arg); + /* Convert to perkw */ + floor = feerate_from_style(floor, FEERATE_PER_KBYTE); + if (floor < FEERATE_FLOOR) + floor = FEERATE_FLOOR; + + /* FIXME: We could let this go below the dynamic floor, but we'd + * need to know if the floor is because of their node's policy + * (minrelaytxfee) or mempool conditions (mempoolminfee). */ + for (size_t i = 0; i < tal_count(feerates); i++) { + feerates[i].rate = feerate_from_style(feerates[i].rate, + FEERATE_PER_KBYTE); + if (feerates[i].rate < floor) + feerates[i].rate = floor; + } + + call->cb(call->bitcoind->ld, floor, feerates); tal_free(call); } -void bitcoind_estimate_fees_(struct bitcoind *bitcoind, - size_t num_estimates, - void (*cb)(struct bitcoind *bitcoind, - const u32 satoshi_per_kw[], void *), - void *arg) +void bitcoind_estimate_fees(struct bitcoind *bitcoind, + void (*cb)(struct lightningd *ld, + u32 feerate_floor, + const struct feerate_est *feerates)) { struct jsonrpc_request *req; struct estimatefee_call *call = tal(bitcoind, struct estimatefee_call); call->bitcoind = bitcoind; call->cb = cb; - call->arg = arg; req = jsonrpc_request_start(bitcoind, "estimatefees", NULL, true, bitcoind->log, diff --git a/lightningd/bitcoind.h b/lightningd/bitcoind.h index f17217d78da0..0986d438abfb 100644 --- a/lightningd/bitcoind.h +++ b/lightningd/bitcoind.h @@ -9,6 +9,7 @@ struct bitcoin_blkid; struct bitcoin_tx_output; struct block; +struct feerate_est; struct lightningd; struct ripemd160; struct bitcoin_tx; @@ -57,19 +58,10 @@ struct bitcoind *new_bitcoind(const tal_t *ctx, struct lightningd *ld, struct log *log); -void bitcoind_estimate_fees_(struct bitcoind *bitcoind, - size_t num_estimates, - void (*cb)(struct bitcoind *bitcoind, - const u32 satoshi_per_kw[], void *), - void *arg); - -#define bitcoind_estimate_fees(bitcoind_, num, cb, arg) \ - bitcoind_estimate_fees_((bitcoind_), (num), \ - typesafe_cb_preargs(void, void *, \ - (cb), (arg), \ - struct bitcoind *, \ - const u32 *), \ - (arg)) +void bitcoind_estimate_fees(struct bitcoind *bitcoind, + void (*cb)(struct lightningd *ld, + u32 feerate_floor, + const struct feerate_est *feerates)); void bitcoind_sendrawtx_(struct bitcoind *bitcoind, const char *id_prefix TAKES, diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 9cc00d9a79b4..b956508c6d8c 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -121,6 +122,9 @@ struct txs_to_broadcast { /* IDs to attach to each tx (could be NULL!) */ const char **cmd_id; + + /* allowhighfees flags for each tx */ + bool *allowhighfees; }; /* We just sent the last entry in txs[]. Shrink and send the next last. */ @@ -142,7 +146,7 @@ static void broadcast_remainder(struct bitcoind *bitcoind, /* Broadcast next one. */ bitcoind_sendrawtx(bitcoind, txs->cmd_id[txs->cursor], txs->txs[txs->cursor], - false, + txs->allowhighfees[txs->cursor], broadcast_remainder, txs); } @@ -161,6 +165,7 @@ static void rebroadcast_txs(struct chain_topology *topo) /* Put any txs we want to broadcast in ->txs. */ txs->txs = tal_arr(txs, const char *, 0); + txs->allowhighfees = tal_arr(txs, bool, 0); for (otx = outgoing_tx_map_first(topo->outgoing_txs, &it); otx; otx = outgoing_tx_map_next(topo->outgoing_txs, &it)) { @@ -180,11 +185,19 @@ static void rebroadcast_txs(struct chain_topology *topo) } tal_arr_expand(&txs->txs, fmt_bitcoin_tx(txs->txs, otx->tx)); + tal_arr_expand(&txs->allowhighfees, otx->allowhighfees); tal_arr_expand(&txs->cmd_id, otx->cmd_id ? tal_strdup(txs, otx->cmd_id) : NULL); } tal_free(cleanup_ctx); + /* Free explicitly in case we were called because a block came in. + * Then set a new timer 30-60 seconds away */ + tal_free(topo->rebroadcast_timer); + topo->rebroadcast_timer = new_reltimer(topo->ld->timers, topo, + time_from_sec(30 + pseudorand(30)), + rebroadcast_txs, topo); + /* Let this do the dirty work. */ txs->cursor = (size_t)-1; broadcast_remainder(topo->bitcoind, true, "", txs); @@ -251,6 +264,7 @@ void broadcast_tx_(struct chain_topology *topo, bitcoin_txid(tx, &otx->txid); otx->tx = clone_bitcoin_tx(otx, tx); otx->minblock = minblock; + otx->allowhighfees = allowhighfees; otx->finished = finished; otx->refresh = refresh; otx->refresh_arg = refresh_arg; @@ -352,88 +366,180 @@ static void watch_for_utxo_reconfirmation(struct chain_topology *topo, /* Mutual recursion via timer. */ static void next_updatefee_timer(struct chain_topology *topo); -static void init_feerate_history(struct chain_topology *topo, - enum feerate feerate, u32 val) +static u32 interp_feerate(const struct feerate_est *rates, u32 blockcount) { - for (size_t i = 0; i < FEE_HISTORY_NUM; i++) - topo->feehistory[feerate][i] = val; + const struct feerate_est *before = NULL, *after = NULL; + + /* Find before and after. */ + for (size_t i = 0; i < tal_count(rates); i++) { + if (rates[i].blockcount <= blockcount) { + before = &rates[i]; + } else if (rates[i].blockcount > blockcount && !after) { + after = &rates[i]; + } + } + /* No estimates at all? */ + if (!before && !after) + return 0; + /* We don't extrapolate. */ + if (!before && after) + return after->rate; + if (before && !after) + return before->rate; + + /* Interpolate, eg. blockcount 10, rate 15000, blockcount 20, rate 5000. + * At 15, rate should be 10000. + * 15000 + (15 - 10) / (20 - 10) * (15000 - 5000) + * 15000 + 5 / 10 * 10000 + * => 10000 + */ + /* Don't go backwards though! */ + if (before->rate < after->rate) + return before->rate; + + return before->rate + - ((u64)(blockcount - before->blockcount) + * (before->rate - after->rate) + / (after->blockcount - before->blockcount)); + } -static void add_feerate_history(struct chain_topology *topo, - enum feerate feerate, u32 val) +u32 feerate_for_deadline(const struct chain_topology *topo, u32 blockcount) { - memmove(&topo->feehistory[feerate][1], &topo->feehistory[feerate][0], - (FEE_HISTORY_NUM - 1) * sizeof(u32)); - topo->feehistory[feerate][0] = val; + u32 rate = interp_feerate(topo->feerates[0], blockcount); + + /* 0 is a special value, meaning "don't know" */ + if (rate && rate < topo->feerate_floor) + rate = topo->feerate_floor; + return rate; } -/* We sanitize feerates if necessary to put them in descending order. */ -static void update_feerates(struct bitcoind *bitcoind, - const u32 *satoshi_per_kw, - struct chain_topology *topo) +u32 smoothed_feerate_for_deadline(const struct chain_topology *topo, + u32 blockcount) +{ + /* Note: we cap it at feerate_floor when we smooth */ + return interp_feerate(topo->smoothed_feerates, blockcount); +} + +/* Mixes in fresh feerate rate into old smoothed values, modifies rate */ +static void smooth_one_feerate(const struct chain_topology *topo, + struct feerate_est *rate) { - u32 old_feerates[NUM_FEERATES]; /* Smoothing factor alpha for simple exponential smoothing. The goal is to * have the feerate account for 90 percent of the values polled in the last * 2 minutes. The following will do that in a polling interval * independent manner. */ double alpha = 1 - pow(0.1,(double)topo->poll_seconds / 120); - bool notify_feerate_changed = false; + u32 old_feerate, feerate_smooth; - for (size_t i = 0; i < NUM_FEERATES; i++) { - u32 feerate = satoshi_per_kw[i]; + /* We don't call this unless we had a previous feerate */ + old_feerate = smoothed_feerate_for_deadline(topo, rate->blockcount); + assert(old_feerate); - /* Takes into account override_fee_rate */ - old_feerates[i] = try_get_feerate(topo, i); + feerate_smooth = rate->rate * alpha + old_feerate * (1 - alpha); - /* If estimatefee failed, don't do anything. */ - if (!feerate) - continue; + /* But to avoid updating forever, only apply smoothing when its + * effect is more then 10 percent */ + if (abs((int)rate->rate - (int)feerate_smooth) > (0.1 * rate->rate)) { + rate->rate = feerate_smooth; + log_debug(topo->log, + "... polled feerate estimate for %u blocks smoothed to %u (alpha=%.2f)", + rate->blockcount, rate->rate, alpha); + } - /* Initial smoothed feerate is the polled feerate */ - if (!old_feerates[i]) { - notify_feerate_changed = true; - old_feerates[i] = feerate; - init_feerate_history(topo, i, feerate); - - log_debug(topo->log, - "Smoothed feerate estimate for %s initialized to polled estimate %u", - feerate_name(i), feerate); - } else { - add_feerate_history(topo, i, feerate); - } + if (rate->rate < get_feerate_floor(topo)) { + rate->rate = get_feerate_floor(topo); + log_debug(topo->log, + "... feerate estimate for %u blocks hit floor %u", + rate->blockcount, rate->rate); + } - /* Smooth the feerate to avoid spikes. */ - u32 feerate_smooth = feerate * alpha + old_feerates[i] * (1 - alpha); - /* But to avoid updating forever, only apply smoothing when its - * effect is more then 10 percent */ - if (abs((int)feerate - (int)feerate_smooth) > (0.1 * feerate)) { - feerate = feerate_smooth; - log_debug(topo->log, - "... polled feerate estimate for %s (%u) smoothed to %u (alpha=%.2f)", - feerate_name(i), satoshi_per_kw[i], - feerate, alpha); - } + if (rate->rate != feerate_smooth) + log_debug(topo->log, + "Feerate estimate for %u blocks set to %u (was %u)", + rate->blockcount, rate->rate, feerate_smooth); +} - if (feerate < feerate_floor()) { - feerate = feerate_floor(); - log_debug(topo->log, - "... feerate estimate for %s hit floor %u", - feerate_name(i), feerate); - } +static bool feerates_differ(const struct feerate_est *a, + const struct feerate_est *b) +{ + if (tal_count(a) != tal_count(b)) + return true; + for (size_t i = 0; i < tal_count(a); i++) { + if (a[i].blockcount != b[i].blockcount) + return true; + if (a[i].rate != b[i].rate) + return true; + } + return false; +} - if (feerate != topo->feerate[i]) { - log_debug(topo->log, "Feerate estimate for %s set to %u (was %u)", - feerate_name(i), - feerate, topo->feerate[i]); +/* In case the plugin does weird stuff! */ +static bool different_blockcounts(struct chain_topology *topo, + const struct feerate_est *old, + const struct feerate_est *new) +{ + if (tal_count(old) != tal_count(new)) { + log_unusual(topo->log, "Presented with %zu feerates this time (was %zu!)", + tal_count(new), tal_count(old)); + return true; + } + for (size_t i = 0; i < tal_count(old); i++) { + if (old[i].blockcount != new[i].blockcount) { + log_unusual(topo->log, "Presented with feerates" + " for blockcount %u, previously %u", + new[i].blockcount, old[i].blockcount); + return true; } - topo->feerate[i] = feerate; + } + return false; +} + +static void update_feerates(struct lightningd *ld, + u32 feerate_floor, + const struct feerate_est *rates TAKES) +{ + struct feerate_est *new_smoothed; + bool changed; + struct chain_topology *topo = ld->topology; + + topo->feerate_floor = feerate_floor; + + /* Don't bother updating if we got no feerates; we'd rather have + * historical ones, if any. */ + if (tal_count(rates) == 0) + goto rearm; + + /* If the feerate blockcounts differ, don't average, just override */ + if (topo->feerates[0] && different_blockcounts(topo, topo->feerates[0], rates)) { + for (size_t i = 0; i < ARRAY_SIZE(topo->feerates); i++) + topo->feerates[i] = tal_free(topo->feerates[i]); + topo->smoothed_feerates = tal_free(topo->smoothed_feerates); + } + + /* Move down historical rates, insert these */ + tal_free(topo->feerates[FEE_HISTORY_NUM-1]); + memmove(topo->feerates + 1, topo->feerates, + sizeof(topo->feerates[0]) * (FEE_HISTORY_NUM-1)); + topo->feerates[0] = tal_dup_talarr(topo, struct feerate_est, rates); + changed = feerates_differ(topo->feerates[0], topo->feerates[1]); + + /* Use this as basis of new smoothed ones. */ + new_smoothed = tal_dup_talarr(topo, struct feerate_est, topo->feerates[0]); - /* After adjustment, If any entry doesn't match prior reported, report all */ - if (feerate != old_feerates[i]) - notify_feerate_changed = true; + /* If there were old smoothed feerates, incorporate those */ + if (tal_count(topo->smoothed_feerates) != 0) { + for (size_t i = 0; i < tal_count(new_smoothed); i++) + smooth_one_feerate(topo, &new_smoothed[i]); } + changed |= feerates_differ(topo->smoothed_feerates, new_smoothed); + tal_free(topo->smoothed_feerates); + topo->smoothed_feerates = new_smoothed; + if (changed) + notify_feerate_change(topo->ld); + +rearm: if (topo->feerate_uninitialized) { /* This doesn't mean we *have* a fee estimate, but it does * mean we tried. */ @@ -441,9 +547,6 @@ static void update_feerates(struct bitcoind *bitcoind, maybe_completed_init(topo); } - if (notify_feerate_changed) - notify_feerate_change(bitcoind->ld); - next_updatefee_timer(topo); } @@ -453,38 +556,74 @@ static void start_fee_estimate(struct chain_topology *topo) if (topo->stopping) return; /* Once per new block head, update fee estimates. */ - bitcoind_estimate_fees(topo->bitcoind, NUM_FEERATES, update_feerates, - topo); + bitcoind_estimate_fees(topo->bitcoind, update_feerates); } +struct rate_conversion { + u32 blockcount; +}; + +static struct rate_conversion conversions[] = { + [FEERATE_OPENING] = { 12 }, + [FEERATE_MUTUAL_CLOSE] = { 100 }, + [FEERATE_UNILATERAL_CLOSE] = { 6 }, + [FEERATE_DELAYED_TO_US] = { 12 }, + [FEERATE_HTLC_RESOLUTION] = { 6 }, + [FEERATE_PENALTY] = { 12 }, +}; + u32 opening_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_OPENING); + if (topo->ld->force_feerates) + return topo->ld->force_feerates[FEERATE_OPENING]; + return feerate_for_deadline(topo, + conversions[FEERATE_OPENING].blockcount); } u32 mutual_close_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_MUTUAL_CLOSE); + if (topo->ld->force_feerates) + return topo->ld->force_feerates[FEERATE_MUTUAL_CLOSE]; + return smoothed_feerate_for_deadline(topo, + conversions[FEERATE_MUTUAL_CLOSE].blockcount); } u32 unilateral_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_UNILATERAL_CLOSE); + if (topo->ld->force_feerates) + return topo->ld->force_feerates[FEERATE_UNILATERAL_CLOSE]; + return smoothed_feerate_for_deadline(topo, + conversions[FEERATE_UNILATERAL_CLOSE].blockcount) + * topo->ld->config.commit_fee_percent / 100; } u32 delayed_to_us_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_DELAYED_TO_US); + if (topo->ld->force_feerates) + return topo->ld->force_feerates[FEERATE_DELAYED_TO_US]; + return smoothed_feerate_for_deadline(topo, + conversions[FEERATE_DELAYED_TO_US].blockcount); } u32 htlc_resolution_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_HTLC_RESOLUTION); + if (topo->ld->force_feerates) + return topo->ld->force_feerates[FEERATE_HTLC_RESOLUTION]; + return smoothed_feerate_for_deadline(topo, + conversions[FEERATE_HTLC_RESOLUTION].blockcount); } u32 penalty_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_PENALTY); + if (topo->ld->force_feerates) + return topo->ld->force_feerates[FEERATE_PENALTY]; + return smoothed_feerate_for_deadline(topo, + conversions[FEERATE_PENALTY].blockcount); +} + +u32 get_feerate_floor(const struct chain_topology *topo) +{ + return topo->feerate_floor; } static struct command_result *json_feerates(struct command *cmd, @@ -494,39 +633,71 @@ static struct command_result *json_feerates(struct command *cmd, { struct chain_topology *topo = cmd->ld->topology; struct json_stream *response; - u32 feerates[NUM_FEERATES]; bool missing; enum feerate_style *style; + u32 rate; if (!param(cmd, buffer, params, p_req("style", param_feerate_style, &style), NULL)) return command_param_failed(); - missing = false; - for (size_t i = 0; i < ARRAY_SIZE(feerates); i++) { - feerates[i] = try_get_feerate(topo, i); - if (!feerates[i]) - missing = true; - } + missing = (tal_count(topo->feerates[0]) == 0); response = json_stream_success(cmd); - if (missing) json_add_string(response, "warning_missing_feerates", "Some fee estimates unavailable: bitcoind startup?"); json_object_start(response, feerate_style_name(*style)); - for (size_t i = 0; i < ARRAY_SIZE(feerates); i++) { - if (!feerates[i] || i == FEERATE_MIN || i == FEERATE_MAX) - continue; - json_add_num(response, feerate_name(i), - feerate_to_style(feerates[i], *style)); + rate = opening_feerate(topo); + if (rate) + json_add_num(response, "opening", feerate_to_style(rate, *style)); + rate = mutual_close_feerate(topo); + if (rate) + json_add_num(response, "mutual_close", + feerate_to_style(rate, *style)); + rate = unilateral_feerate(topo); + if (rate) + json_add_num(response, "unilateral_close", + feerate_to_style(rate, *style)); + rate = penalty_feerate(topo); + if (rate) + json_add_num(response, "penalty", + feerate_to_style(rate, *style)); + if (deprecated_apis) { + rate = delayed_to_us_feerate(topo); + if (rate) + json_add_num(response, "delayed_to_us", + feerate_to_style(rate, *style)); + rate = htlc_resolution_feerate(topo); + if (rate) + json_add_num(response, "htlc_resolution", + feerate_to_style(rate, *style)); } + json_add_u64(response, "min_acceptable", feerate_to_style(feerate_min(cmd->ld, NULL), *style)); json_add_u64(response, "max_acceptable", feerate_to_style(feerate_max(cmd->ld, NULL), *style)); + json_add_u64(response, "floor", + feerate_to_style(get_feerate_floor(cmd->ld->topology), + *style)); + + json_array_start(response, "estimates"); + assert(tal_count(topo->smoothed_feerates) == tal_count(topo->feerates[0])); + for (size_t i = 0; i < tal_count(topo->feerates[0]); i++) { + json_object_start(response, NULL); + json_add_num(response, "blockcount", + topo->feerates[0][i].blockcount); + json_add_u64(response, "feerate", + feerate_to_style(topo->feerates[0][i].rate, *style)); + json_add_u64(response, "smoothed_feerate", + feerate_to_style(topo->smoothed_feerates[i].rate, + *style)); + json_object_end(response); + } + json_array_end(response); json_object_end(response); if (!missing) { @@ -904,14 +1075,9 @@ u32 get_network_blockheight(const struct chain_topology *topo) return topo->headercount; } - -u32 try_get_feerate(const struct chain_topology *topo, enum feerate feerate) -{ - return topo->feerate[feerate]; -} - u32 feerate_min(struct lightningd *ld, bool *unknown) { + const struct chain_topology *topo = ld->topology; u32 min; if (unknown) @@ -921,30 +1087,32 @@ u32 feerate_min(struct lightningd *ld, bool *unknown) if (ld->config.ignore_fee_limits) min = 1; else { - min = try_get_feerate(ld->topology, FEERATE_MIN); - if (!min) { + min = 0xFFFFFFFF; + for (size_t i = 0; i < ARRAY_SIZE(topo->feerates); i++) { + for (size_t j = 0; j < tal_count(topo->feerates[i]); j++) { + if (topo->feerates[i][j].rate < min) + min = topo->feerates[i][j].rate; + } + } + if (min == 0xFFFFFFFF) { if (unknown) *unknown = true; - } else { - const u32 *hist = ld->topology->feehistory[FEERATE_MIN]; - - /* If one of last three was an outlier, use that. */ - for (size_t i = 0; i < FEE_HISTORY_NUM; i++) { - if (hist[i] < min) - min = hist[i]; - } + min = 0; } + + /* FIXME: This is what bcli used to do: halve the slow feerate! */ + min /= 2; } - if (min < feerate_floor()) - return feerate_floor(); + if (min < get_feerate_floor(topo)) + return get_feerate_floor(topo); return min; } u32 feerate_max(struct lightningd *ld, bool *unknown) { - u32 feerate; - const u32 *feehistory = ld->topology->feehistory[FEERATE_MAX]; + const struct chain_topology *topo = ld->topology; + u32 max = 0; if (unknown) *unknown = false; @@ -952,20 +1120,18 @@ u32 feerate_max(struct lightningd *ld, bool *unknown) if (ld->config.ignore_fee_limits) return UINT_MAX; - /* If we don't know feerate, don't limit other side. */ - feerate = try_get_feerate(ld->topology, FEERATE_MAX); - if (!feerate) { + for (size_t i = 0; i < ARRAY_SIZE(topo->feerates); i++) { + for (size_t j = 0; j < tal_count(topo->feerates[i]); j++) { + if (topo->feerates[i][j].rate > max) + max = topo->feerates[i][j].rate; + } + } + if (!max) { if (unknown) *unknown = true; return UINT_MAX; } - - /* If one of last three was an outlier, use that. */ - for (size_t i = 0; i < FEE_HISTORY_NUM; i++) { - if (feehistory[i] > feerate) - feerate = feehistory[i]; - } - return feerate; + return max * topo->ld->config.max_fee_multiplier; } /* On shutdown, channels get deleted last. That frees from our list, so @@ -995,13 +1161,15 @@ struct chain_topology *new_topology(struct lightningd *ld, struct log *log) topo->txowatches = tal(topo, struct txowatch_hash); txowatch_hash_init(topo->txowatches); topo->log = log; - memset(topo->feerate, 0, sizeof(topo->feerate)); topo->bitcoind = new_bitcoind(topo, ld, log); topo->poll_seconds = 30; topo->feerate_uninitialized = true; + memset(topo->feerates, 0, sizeof(topo->feerates)); + topo->smoothed_feerates = NULL; topo->root = NULL; topo->sync_waiters = tal(topo, struct list_head); topo->extend_timer = NULL; + topo->rebroadcast_timer = NULL; topo->stopping = false; list_head_init(topo->sync_waiters); @@ -1104,7 +1272,6 @@ void setup_topology(struct chain_topology *topo, u32 min_blockheight, u32 max_blockheight) { void *ret; - memset(&topo->feerate, 0, sizeof(topo->feerate)); topo->min_blockheight = min_blockheight; topo->max_blockheight = max_blockheight; diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 486fab0f3e10..4c7ce628a9fe 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -22,6 +22,7 @@ struct outgoing_tx { const struct bitcoin_tx *tx; struct bitcoin_txid txid; u32 minblock; + bool allowhighfees; const char *cmd_id; void (*finished)(struct channel *channel, bool success, const char *err); bool (*refresh)(struct channel *, const struct bitcoin_tx **, void *arg); @@ -88,15 +89,31 @@ static inline bool outgoing_tx_eq(const struct outgoing_tx *b, const struct bitc HTABLE_DEFINE_TYPE(struct outgoing_tx, keyof_outgoing_tx_map, outgoing_tx_hash_sha, outgoing_tx_eq, outgoing_tx_map); +/* Our plugins give us a series of blockcount, feerate pairs. */ +struct feerate_est { + u32 blockcount; + u32 rate; +}; + struct chain_topology { struct lightningd *ld; struct block *root; struct block *tip; struct bitcoin_blkid prev_tip; struct block_map *block_map; - u32 feerate[NUM_FEERATES]; + + /* Set during startup */ bool feerate_uninitialized; - u32 feehistory[NUM_FEERATES][FEE_HISTORY_NUM]; + + /* This is the lowest feerate that bitcoind is saying will broadcast. */ + u32 feerate_floor; + + /* We keep last three feerates we got: this is useful for min/max. */ + struct feerate_est *feerates[FEE_HISTORY_NUM]; + + /* We keep a smoothed feerate: this is useful when we're going to + * suggest feerates / check feerates from our peers. */ + struct feerate_est *smoothed_feerates; /* Where to log things. */ struct log *log; @@ -116,7 +133,7 @@ struct chain_topology { struct bitcoind *bitcoind; /* Timers we're running. */ - struct oneshot *extend_timer, *updatefee_timer; + struct oneshot *extend_timer, *updatefee_timer, *rebroadcast_timer; /* Bitcoin transactions we're broadcasting */ struct outgoing_tx_map *outgoing_txs; @@ -143,6 +160,9 @@ struct txlocator { u32 index; }; +/* Get the minimum feerate that bitcoind will accept */ +u32 get_feerate_floor(const struct chain_topology *topo); + /* This is the number of blocks which would have to be mined to invalidate * the tx */ size_t get_tx_depth(const struct chain_topology *topo, @@ -158,14 +178,16 @@ u32 get_block_height(const struct chain_topology *topo); * likely to lag behind the rest of the network.*/ u32 get_network_blockheight(const struct chain_topology *topo); -/* Get fee rate in satoshi per kiloweight, or 0 if unavailable! */ -u32 try_get_feerate(const struct chain_topology *topo, enum feerate feerate); +/* Get feerate estimate for getting a tx in this many blocks */ +u32 feerate_for_deadline(const struct chain_topology *topo, u32 blockcount); +u32 smoothed_feerate_for_deadline(const struct chain_topology *topo, u32 blockcount); /* Get range of feerates to insist other side abide by for normal channels. * If we have to guess, sets *unknown to true, otherwise false. */ u32 feerate_min(struct lightningd *ld, bool *unknown); u32 feerate_max(struct lightningd *ld, bool *unknown); +/* These return 0 if unknown */ u32 opening_feerate(struct chain_topology *topo); u32 mutual_close_feerate(struct chain_topology *topo); u32 unilateral_feerate(struct chain_topology *topo); diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index adee64850949..2bbcef4aa19a 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -38,12 +38,12 @@ static void update_feerates(struct lightningd *ld, struct channel *channel) feerate, feerate_min(ld, NULL), feerate_max(ld, NULL), - try_get_feerate(ld->topology, FEERATE_PENALTY)); + penalty_feerate(ld->topology)); msg = towire_channeld_feerates(NULL, feerate, feerate_min(ld, NULL), feerate_max(ld, NULL), - try_get_feerate(ld->topology, FEERATE_PENALTY)); + penalty_feerate(ld->topology)); subd_send_msg(channel->owner, take(msg)); } @@ -736,7 +736,7 @@ bool peer_start_channeld(struct channel *channel, channel->fee_states, feerate_min(ld, NULL), feerate_max(ld, NULL), - try_get_feerate(ld->topology, FEERATE_PENALTY), + penalty_feerate(ld->topology), &channel->last_sig, &channel->channel_info.remote_fundingkey, &channel->channel_info.theirbase, @@ -1146,8 +1146,7 @@ static struct command_result *json_dev_feerate(struct command *cmd, msg = towire_channeld_feerates(NULL, *feerate, feerate_min(cmd->ld, NULL), feerate_max(cmd->ld, NULL), - try_get_feerate(cmd->ld->topology, - FEERATE_PENALTY)); + penalty_feerate(cmd->ld->topology)); subd_send_msg(channel->owner, take(msg)); response = json_stream_success(cmd); diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 7021825862c7..105bd6be832b 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -409,8 +409,8 @@ void peer_start_closingd(struct channel *channel, struct peer_fd *peer_fd) feerate = mutual_close_feerate(ld->topology); if (!feerate) { feerate = final_commit_feerate / 2; - if (feerate < feerate_floor()) - feerate = feerate_floor(); + if (feerate < get_feerate_floor(ld->topology)) + feerate = get_feerate_floor(ld->topology); } /* We use a feerate if anchor_outputs, otherwise max fee is set by diff --git a/lightningd/feerate.c b/lightningd/feerate.c index dd060032187b..07a5f5f78a84 100644 --- a/lightningd/feerate.c +++ b/lightningd/feerate.c @@ -1,4 +1,5 @@ #include "config.h" +#include #include #include #include @@ -45,36 +46,119 @@ struct command_result *param_feerate_style(struct command *cmd, json_tok_full_len(tok), json_tok_full(buffer, tok)); } -struct command_result *param_feerate(struct command *cmd, const char *name, - const char *buffer, const jsmntok_t *tok, - u32 **feerate) +/* This can set **feerate to 0, if it's unknown. */ +static struct command_result *param_feerate_unchecked(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + u32 **feerate) { + *feerate = tal(cmd, u32); + + if (json_tok_streq(buffer, tok, "opening")) { + **feerate = opening_feerate(cmd->ld->topology); + return NULL; + } + if (json_tok_streq(buffer, tok, "mutual_close")) { + **feerate = mutual_close_feerate(cmd->ld->topology); + return NULL; + } + if (json_tok_streq(buffer, tok, "penalty")) { + **feerate = penalty_feerate(cmd->ld->topology); + return NULL; + } + if (json_tok_streq(buffer, tok, "unilateral_close")) { + **feerate = unilateral_feerate(cmd->ld->topology); + return NULL; + } + + /* Other names are deprecated */ for (size_t i = 0; i < NUM_FEERATES; i++) { - if (json_tok_streq(buffer, tok, feerate_name(i))) - return param_feerate_estimate(cmd, feerate, i); + bool unknown; + + if (!json_tok_streq(buffer, tok, feerate_name(i))) + continue; + if (!deprecated_apis) + return command_fail_badparam(cmd, name, buffer, tok, + "removed feerate by names"); + switch (i) { + case FEERATE_OPENING: + case FEERATE_MUTUAL_CLOSE: + case FEERATE_PENALTY: + case FEERATE_UNILATERAL_CLOSE: + /* Handled above */ + abort(); + case FEERATE_DELAYED_TO_US: + **feerate = delayed_to_us_feerate(cmd->ld->topology); + return NULL; + case FEERATE_HTLC_RESOLUTION: + **feerate = htlc_resolution_feerate(cmd->ld->topology); + return NULL; + case FEERATE_MAX: + **feerate = feerate_max(cmd->ld, &unknown); + if (unknown) + **feerate = 0; + return NULL; + case FEERATE_MIN: + **feerate = feerate_min(cmd->ld, &unknown); + if (unknown) + **feerate = 0; + return NULL; + } + abort(); } + /* We used SLOW, NORMAL, and URGENT as feerate targets previously, * and many commands rely on this syntax now. * It's also really more natural for an user interface. */ - if (json_tok_streq(buffer, tok, "slow")) - return param_feerate_estimate(cmd, feerate, FEERATE_MIN); - else if (json_tok_streq(buffer, tok, "normal")) - return param_feerate_estimate(cmd, feerate, FEERATE_OPENING); - else if (json_tok_streq(buffer, tok, "urgent")) - return param_feerate_estimate(cmd, feerate, FEERATE_UNILATERAL_CLOSE); + if (json_tok_streq(buffer, tok, "slow")) { + **feerate = feerate_for_deadline(cmd->ld->topology, 100); + return NULL; + } else if (json_tok_streq(buffer, tok, "normal")) { + **feerate = feerate_for_deadline(cmd->ld->topology, 12); + return NULL; + } else if (json_tok_streq(buffer, tok, "urgent")) { + **feerate = feerate_for_deadline(cmd->ld->topology, 6); + return NULL; + } else if (json_tok_streq(buffer, tok, "minimum")) { + **feerate = get_feerate_floor(cmd->ld->topology); + return NULL; + } + + /* Can specify number of blocks as a target */ + if (json_tok_endswith(buffer, tok, "blocks")) { + jsmntok_t base = *tok; + base.end -= strlen("blocks"); + u32 numblocks; + + if (!json_to_number(buffer, &base, &numblocks)) { + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "'%s' should be an integer not '%.*s'", + name, base.end - base.start, + buffer + base.start); + } + **feerate = feerate_for_deadline(cmd->ld->topology, numblocks); + return NULL; + } /* It's a number... */ + tal_free(*feerate); return param_feerate_val(cmd, name, buffer, tok, feerate); } -struct command_result *param_feerate_estimate(struct command *cmd, - u32 **feerate_per_kw, - enum feerate feerate) +struct command_result *param_feerate(struct command *cmd, const char *name, + const char *buffer, const jsmntok_t *tok, + u32 **feerate) { - *feerate_per_kw = tal(cmd, u32); - **feerate_per_kw = try_get_feerate(cmd->ld->topology, feerate); - if (!**feerate_per_kw) - return command_fail(cmd, LIGHTNINGD, "Cannot estimate fees"); + struct command_result *ret; + + ret = param_feerate_unchecked(cmd, name, buffer, tok, feerate); + if (ret) + return ret; + + if (**feerate == 0) + return command_fail(cmd, BCLI_NO_FEE_ESTIMATES, + "Cannot estimate fees (yet)"); return NULL; } diff --git a/lightningd/feerate.h b/lightningd/feerate.h index 80ab365f8d06..ca0793d5b963 100644 --- a/lightningd/feerate.h +++ b/lightningd/feerate.h @@ -28,11 +28,6 @@ struct command_result *param_feerate_style(struct command *cmd, const jsmntok_t *tok, enum feerate_style **style); -/* Set feerate_per_kw to this estimate & return NULL, or fail cmd */ -struct command_result *param_feerate_estimate(struct command *cmd, - u32 **feerate_per_kw, - enum feerate feerate); - /* Extract a feerate with optional style suffix. */ struct command_result *param_feerate_val(struct command *cmd, const char *name, const char *buffer, diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 0a57dbb9789f..e5aac115ad3c 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -85,6 +85,13 @@ struct config { /* Require peer to send confirmed inputs */ bool require_confirmed_inputs; + + /* The factor to time the urgent feerate by to get the maximum + * acceptable feerate. (10, but can be overridden by dev-max-fee-multiplier) */ + u32 max_fee_multiplier; + + /* Percent of CONSERVATIVE/2 feerate we'll use for commitment txs. */ + u64 commit_fee_percent; }; typedef STRMAP(const char *) alt_subdaemon_map; diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index d43e6728060a..71062245bd5e 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -466,6 +466,16 @@ struct onchain_signing_info { /* Trailing element for witness stack */ const tal_t *stack_elem; + /* Information for consider_onchain_rebroadcast */ + struct amount_sat fee; + struct bitcoin_outpoint out; + struct amount_sat out_sats; + u32 to_self_delay; + u32 locktime; + u8 *(*sign)(const tal_t *ctx, + const struct bitcoin_tx *tx, + const struct onchain_signing_info *info); + /* Tagged union (for sanity checking!) */ enum onchaind_wire msgtype; union { @@ -642,32 +652,47 @@ onchain_witness_htlc_tx(const tal_t *ctx, u8 **witness) return cast_const2(const struct onchain_witness_element **, welements); } -/* Always sets *welements, returns tx. Sets *worthwhile to false if - * it wasn't worthwhile at the given feerate (and it had to drop feerate). - * Returns NULL iff it called channel_internal_error(). - */ -static struct bitcoin_tx *onchaind_tx(const tal_t *ctx, - struct channel *channel, - const struct bitcoin_outpoint *out, - struct amount_sat out_sats, - u32 to_self_delay, - u32 locktime, - u32 feerate, - u8 *(*sign)(const tal_t *ctx, - const struct bitcoin_tx *tx, - const struct onchain_signing_info *info), - const struct onchain_signing_info *info, - bool *worthwhile, - const struct onchain_witness_element ***welements) +/* feerate_for_deadline, but really lowball for distant targets */ +static u32 feerate_for_target(const struct chain_topology *topo, u64 deadline) +{ + u64 blocks, blockheight; + + blockheight = get_block_height(topo); + + /* Past deadline? Want it now. */ + if (blockheight > deadline) + return feerate_for_deadline(topo, 1); + + blocks = deadline - blockheight; + + /* Over 200 blocks, we *always* use min fee! */ + if (blocks > 200) + return FEERATE_FLOOR; + /* Over 100 blocks, use min fee bitcoind will accept */ + if (blocks > 100) + return get_feerate_floor(topo); + + return feerate_for_deadline(topo, blocks); +} + +/* Make normal 1-input-1-output tx to us, but don't sign it yet. + * + * If worthwhile is not NULL, we set it to true normally, or false if + * we had to lower fees so much it's unlikely to get mined + * (i.e. "don't wait up!"). +*/ +static struct bitcoin_tx *onchaind_tx_unsigned(const tal_t *ctx, + struct channel *channel, + const struct onchain_signing_info *info, + struct amount_sat *fee, + bool *worthwhile) { struct bitcoin_tx *tx; - struct amount_sat fee, min_out, amt; - struct bitcoin_signature sig; + struct amount_sat amt; size_t weight; - u8 *msg; - u8 **witness; struct pubkey final_key; struct ext_key final_wallet_ext_key; + u64 block_target; struct lightningd *ld = channel->peer->ld; bip32_pubkey(ld, &final_key, channel->final_key_idx); @@ -681,57 +706,121 @@ static struct bitcoin_tx *onchaind_tx(const tal_t *ctx, return NULL; } - tx = bitcoin_tx(ctx, chainparams, 1, 1, locktime); - bitcoin_tx_add_input(tx, out, to_self_delay, - NULL, out_sats, NULL, info->wscript); + tx = bitcoin_tx(ctx, chainparams, 1, 1, info->locktime); + bitcoin_tx_add_input(tx, &info->out, info->to_self_delay, + NULL, info->out_sats, NULL, info->wscript); bitcoin_tx_add_output( - tx, scriptpubkey_p2wpkh(tmpctx, &final_key), NULL, out_sats); + tx, scriptpubkey_p2wpkh(tmpctx, &final_key), NULL, info->out_sats); psbt_add_keypath_to_last_output(tx, channel->final_key_idx, &final_wallet_ext_key); /* Worst-case sig is 73 bytes */ weight = bitcoin_tx_weight(tx) + 1 + 3 + 73 + 0 + tal_count(info->wscript); weight += elements_tx_overhead(chainparams, 1, 1); - fee = amount_tx_fee(feerate, weight); - - /* Result is trivial? Spend with small feerate, but don't wait - * around for it as it might not confirm. */ - if (!amount_sat_add(&min_out, channel->our_config.dust_limit, fee)) - fatal("Cannot add dust_limit %s and fee %s", - type_to_string(tmpctx, struct amount_sat, &channel->our_config.dust_limit), - type_to_string(tmpctx, struct amount_sat, &fee)); - - if (amount_sat_less(out_sats, min_out)) { - /* FIXME: We should use SIGHASH_NONE so others can take it? */ - fee = amount_tx_fee(feerate_floor(), weight); - *worthwhile = false; - } else - *worthwhile = true; - - /* This can only happen if feerate_floor() is still too high; shouldn't - * happen! */ - if (!amount_sat_sub(&amt, out_sats, fee)) { - amt = channel->our_config.dust_limit; - log_broken(channel->log, "TX can't afford minimal feerate" - "; setting output to %s", - type_to_string(tmpctx, struct amount_sat, - &amt)); - *worthwhile = false; + + block_target = info->deadline_block; + for (;;) { + u32 feerate; + + feerate = feerate_for_target(ld->topology, block_target); + *fee = amount_tx_fee(feerate, weight); + + log_debug(channel->log, + "Feerate for target %"PRIu64" (%+"PRId64" blocks) is %u, fee %s of %s", + block_target, + block_target - get_block_height(ld->topology), + feerate, + type_to_string(tmpctx, struct amount_sat, fee), + type_to_string(tmpctx, struct amount_sat, + &info->out_sats)); + + /* If we can afford fee and it's not dust, we're done */ + if (amount_sat_sub(&amt, info->out_sats, *fee) + && amount_sat_greater_eq(amt, channel->our_config.dust_limit)) + break; + + /* Hmm, can't afford with recommended fee. Try increasing deadline! */ + block_target++; + + /* If we can't even afford at FEERATE_FLOOR, something is wrong! */ + if (feerate == FEERATE_FLOOR) { + amt = channel->our_config.dust_limit; + /* Not quite true, but Never Happens */ + *fee = AMOUNT_SAT(0); + log_broken(channel->log, "TX can't afford minimal feerate" + "; setting output to %s", + type_to_string(tmpctx, struct amount_sat, &amt)); + break; + } } + + /* If we anticipate waiting a long time (say, 20 blocks past + * the deadline), tell onchaind not to wait */ + if (worthwhile) { + *worthwhile = (block_target < info->deadline_block + (u64)20); + if (!*worthwhile) { + log_unusual(channel->log, + "Lowballing feerate for %s sats from %u to %u (deadline %u->%"PRIu64"):" + " won't count on it being spent!", + type_to_string(tmpctx, struct amount_sat, &info->out_sats), + feerate_for_target(ld->topology, info->deadline_block), + feerate_for_target(ld->topology, block_target), + info->deadline_block, block_target); + } + } + + /* If we came close to target, it's worthwhile to wait for. */ + if (block_target != info->deadline_block) + log_debug(channel->log, "Had to adjust deadline from %u to %"PRIu64" for %s", + info->deadline_block, block_target, + type_to_string(tmpctx, struct amount_sat, &info->out_sats)); bitcoin_tx_output_set_amount(tx, 0, amt); bitcoin_tx_finalize(tx); - /* Now sign, and set witness */ - msg = sign(NULL, tx, info); + return tx; +} + +static u8 **sign_and_get_witness(const tal_t *ctx, + const struct channel *channel, + struct bitcoin_tx *tx, + const struct onchain_signing_info *info) +{ + u8 *msg; + struct bitcoin_signature sig; + struct lightningd *ld = channel->peer->ld; + + msg = info->sign(NULL, tx, info); if (!wire_sync_write(ld->hsm_fd, take(msg))) fatal("Writing sign request to hsm"); msg = wire_sync_read(tmpctx, ld->hsm_fd); if (!msg || !fromwire_hsmd_sign_tx_reply(msg, &sig)) fatal("Reading sign_tx_reply: %s", tal_hex(tmpctx, msg)); - witness = bitcoin_witness_sig_and_element(NULL, &sig, info->stack_elem, - tal_bytelen(info->stack_elem), - info->wscript); + return bitcoin_witness_sig_and_element(ctx, &sig, info->stack_elem, + tal_bytelen(info->stack_elem), + info->wscript); +} + +/* Always sets *welements, returns tx. Sets *worthwhile to false if + * it wasn't worthwhile at the given feerate (and it had to drop feerate). + * Returns NULL iff it called channel_internal_error(). + */ +static struct bitcoin_tx *onchaind_tx(const tal_t *ctx, + struct channel *channel, + const struct onchain_signing_info *info, + struct amount_sat *fee, + bool *worthwhile, + const struct onchain_witness_element ***welements) +{ + struct bitcoin_tx *tx; + u8 **witness; + + tx = onchaind_tx_unsigned(ctx, channel, info, fee, worthwhile); + if (!tx) + return NULL; + + /* Now sign, and set witness */ + witness = sign_and_get_witness(NULL, channel, tx, info); *welements = onchain_witness_sig_and_element(ctx, witness); bitcoin_tx_input_set_witness(tx, 0, take(witness)); @@ -742,7 +831,39 @@ static bool consider_onchain_rebroadcast(struct channel *channel, const struct bitcoin_tx **tx, struct onchain_signing_info *info) { - /* FIXME: Implement rbf! */ + struct bitcoin_tx *newtx; + struct amount_sat newfee; + struct bitcoin_txid oldtxid, newtxid; + u8 **witness; + + newtx = onchaind_tx_unsigned(tmpctx, channel, info, &newfee, NULL); + if (!newtx) + return true; + + /* FIXME: Don't RBF if fee is not sufficiently increased? */ + + /* OK! RBF time! */ + witness = sign_and_get_witness(NULL, channel, newtx, info); + bitcoin_tx_input_set_witness(newtx, 0, take(witness)); + + bitcoin_txid(newtx, &newtxid); + bitcoin_txid(*tx, &oldtxid); + log_info(channel->log, + "RBF onchain txid %s (fee %s) with txid %s (fee %s)", + type_to_string(tmpctx, struct bitcoin_txid, &oldtxid), + fmt_amount_sat(tmpctx, info->fee), + type_to_string(tmpctx, struct bitcoin_txid, &newtxid), + fmt_amount_sat(tmpctx, newfee)); + log_debug(channel->log, + "RBF %s->%s", + type_to_string(tmpctx, struct bitcoin_tx, *tx), + type_to_string(tmpctx, struct bitcoin_tx, newtx)); + + /* FIXME: This is ugly, but we want the same parent as old tx. */ + tal_steal(tal_parent(*tx), newtx); + tal_free(*tx); + *tx = newtx; + info->fee = newfee; return true; } @@ -797,7 +918,6 @@ static void create_onchain_tx(struct channel *channel, struct amount_sat out_sats, u32 to_self_delay, u32 locktime, - u32 initial_feerate, u8 *(*sign)(const tal_t *ctx, const struct bitcoin_tx *tx, const struct onchain_signing_info *info), @@ -807,19 +927,30 @@ static void create_onchain_tx(struct channel *channel, struct bitcoin_tx *tx; const struct onchain_witness_element **welements; bool worthwhile; + struct lightningd *ld = channel->peer->ld; - tx = onchaind_tx(tmpctx, channel, - out, out_sats, to_self_delay, locktime, initial_feerate, - sign, info, &worthwhile, &welements); - if (!tx) + /* Save these in case we need to RBF. We could extract from + * tx, but this is clearer and simpler. */ + info->out = *out; + info->out_sats = out_sats; + info->to_self_delay = to_self_delay; + info->locktime = locktime; + info->sign = sign; + + tx = onchaind_tx(tmpctx, channel, info, &info->fee, &worthwhile, &welements); + if (!tx) { + tal_free(info); return; + } log_debug(channel->log, "Broadcast for onchaind tx %s%s", type_to_string(tmpctx, struct bitcoin_tx, tx), worthwhile ? "" : "(NOT WORTHWHILE, LOWBALL FEE!)"); - broadcast_tx(channel->peer->ld->topology, - channel, take(tx), NULL, false, info->minblock, + /* We allow "excessive" fees, as we may be fighting with censors and + * we'd rather spend fees than have our adversary win. */ + broadcast_tx(ld->topology, + channel, take(tx), NULL, true, info->minblock, NULL, consider_onchain_rebroadcast, take(info)); subd_send_msg(channel->owner, @@ -831,11 +962,9 @@ static void create_onchain_tx(struct channel *channel, static void handle_onchaind_spend_to_us(struct channel *channel, const u8 *msg) { - struct lightningd *ld = channel->peer->ld; struct onchain_signing_info *info; struct bitcoin_outpoint out; struct amount_sat out_sats; - u32 initial_feerate; info = new_signing_info(msg, channel, WIRE_ONCHAIND_SPEND_TO_US); @@ -869,27 +998,20 @@ static void handle_onchaind_spend_to_us(struct channel *channel, return; } - /* FIXME: Be more sophisticated! */ - initial_feerate = delayed_to_us_feerate(ld->topology); - if (!initial_feerate) - initial_feerate = tx_feerate(channel->last_tx); - /* No real deadline on this, it's just returning to our wallet. */ - info->deadline_block = infinite_block_deadline(ld->topology); + info->deadline_block = infinite_block_deadline(channel->peer->ld->topology); create_onchain_tx(channel, &out, out_sats, channel->channel_info.their_config.to_self_delay, 0, - initial_feerate, sign_tx_to_us, info, + sign_tx_to_us, info, __func__); } static void handle_onchaind_spend_penalty(struct channel *channel, const u8 *msg) { - struct lightningd *ld = channel->peer->ld; struct onchain_signing_info *info; struct bitcoin_outpoint out; struct amount_sat out_sats; - u32 initial_feerate; u8 *stack_elem; info = new_signing_info(msg, channel, WIRE_ONCHAIND_SPEND_PENALTY); @@ -907,11 +1029,6 @@ static void handle_onchaind_spend_penalty(struct channel *channel, /* info->stack_elem is const void * */ info->stack_elem = stack_elem; - /* FIXME: Be more sophisticated! */ - initial_feerate = penalty_feerate(ld->topology); - if (!initial_feerate) - initial_feerate = tx_feerate(channel->last_tx); - /* FIXME: deadline for HTLCs is actually a bit longer, but for * their output it's channel->our_config.to_self_delay after * the commitment tx is mined. */ @@ -919,19 +1036,17 @@ static void handle_onchaind_spend_penalty(struct channel *channel, + channel->our_config.to_self_delay; create_onchain_tx(channel, &out, out_sats, 0, 0, - initial_feerate, sign_penalty, info, + sign_penalty, info, __func__); } static void handle_onchaind_spend_fulfill(struct channel *channel, const u8 *msg) { - struct lightningd *ld = channel->peer->ld; struct onchain_signing_info *info; struct bitcoin_outpoint out; struct amount_sat out_sats; struct preimage preimage; - u32 initial_feerate; u64 htlc_id; const bool anchor_outputs = channel_has(channel, OPT_ANCHOR_OUTPUTS); @@ -950,11 +1065,6 @@ static void handle_onchaind_spend_fulfill(struct channel *channel, } info->stack_elem = tal_dup(info, struct preimage, &preimage); - /* FIXME: Be more sophisticated! */ - initial_feerate = htlc_resolution_feerate(ld->topology); - if (!initial_feerate) - initial_feerate = tx_feerate(channel->last_tx); - info->deadline_block = htlc_incoming_deadline(channel, htlc_id); /* BOLT #3: * @@ -964,7 +1074,7 @@ static void handle_onchaind_spend_fulfill(struct channel *channel, create_onchain_tx(channel, &out, out_sats, anchor_outputs ? 1 : 0, 0, - initial_feerate, sign_fulfill, info, + sign_fulfill, info, __func__); } @@ -1121,12 +1231,11 @@ static void handle_onchaind_spend_htlc_timeout(struct channel *channel, static void handle_onchaind_spend_htlc_expired(struct channel *channel, const u8 *msg) { - struct lightningd *ld = channel->peer->ld; struct onchain_signing_info *info; struct bitcoin_outpoint out; struct amount_sat out_sats; u64 htlc_id; - u32 cltv_expiry, initial_feerate; + u32 cltv_expiry; const bool anchor_outputs = channel_has(channel, OPT_ANCHOR_OUTPUTS); info = new_signing_info(msg, channel, WIRE_ONCHAIND_SPEND_HTLC_EXPIRED); @@ -1157,17 +1266,12 @@ static void handle_onchaind_spend_htlc_expired(struct channel *channel, /* nLocktime: we have to be *after* that block! */ info->minblock = cltv_expiry + 1; - /* FIXME: Be more sophisticated! */ - initial_feerate = htlc_resolution_feerate(ld->topology); - if (!initial_feerate) - initial_feerate = tx_feerate(channel->last_tx); - /* We have to spend it before we can close incoming */ info->deadline_block = htlc_outgoing_incoming_deadline(channel, htlc_id); create_onchain_tx(channel, &out, out_sats, anchor_outputs ? 1 : 0, cltv_expiry, - initial_feerate, sign_htlc_expired, info, + sign_htlc_expired, info, __func__); } diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 7156d5f620f3..7b993c3dd178 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -1159,9 +1159,10 @@ static struct command_result *json_fundchannel_start(struct command *cmd, } } - if (*feerate_per_kw < feerate_floor()) { + if (*feerate_per_kw < get_feerate_floor(cmd->ld->topology)) { return command_fail(cmd, LIGHTNINGD, - "Feerate below feerate floor"); + "Feerate below feerate floor %u perkw", + get_feerate_floor(cmd->ld->topology)); } if (!topology_synced(cmd->ld->topology)) { diff --git a/lightningd/options.c b/lightningd/options.c index b35b930429b2..7d44e4dd746f 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -799,6 +799,15 @@ static void dev_register_opts(struct lightningd *ld) opt_show_uintval, &dev_onion_reply_length, "Send onion errors of custom length"); + opt_register_arg("--dev-max-fee-multiplier", + opt_set_uintval, + opt_show_uintval, + &ld->config.max_fee_multiplier, + "Allow the fee proposed by the remote end to" + " be up to multiplier times higher than our " + "own. Small values will cause channels to be" + " closed more often due to fee fluctuations," + " large values may result in large fees."); } #endif /* DEVELOPER */ @@ -860,6 +869,9 @@ static const struct config testnet_config = { .allowdustreserve = false, .require_confirmed_inputs = false, + + .max_fee_multiplier = 10, + .commit_fee_percent = 100, }; /* aka. "Dude, where's my coins?" */ @@ -931,6 +943,9 @@ static const struct config mainnet_config = { .allowdustreserve = false, .require_confirmed_inputs = false, + + .max_fee_multiplier = 10, + .commit_fee_percent = 100, }; static void check_config(struct lightningd *ld) @@ -1290,6 +1305,9 @@ static void register_opts(struct lightningd *ld) opt_force_feerates, NULL, ld, "Set testnet/regtest feerates in sats perkw, opening/mutual_close/unlateral_close/delayed_to_us/htlc_resolution/penalty: if fewer specified, last number applies to remainder"); + opt_register_arg("--commit-fee", + opt_set_u64, opt_show_u64, &ld->config.commit_fee_percent, + "Percentage of fee to request for their commitment"); opt_register_arg("--subdaemon", opt_subdaemon, NULL, ld, "Arg specified as SUBDAEMON:PATH. " "Specifies an alternate subdaemon binary. " diff --git a/lightningd/test/run-jsonrpc.c b/lightningd/test/run-jsonrpc.c index 0919a3a93599..215130fe11a2 100644 --- a/lightningd/test/run-jsonrpc.c +++ b/lightningd/test/run-jsonrpc.c @@ -13,11 +13,23 @@ void db_begin_transaction_(struct db *db UNNEEDED, const char *location UNNEEDED /* Generated stub for db_commit_transaction */ void db_commit_transaction(struct db *db UNNEEDED) { fprintf(stderr, "db_commit_transaction called!\n"); abort(); } +/* Generated stub for delayed_to_us_feerate */ +u32 delayed_to_us_feerate(struct chain_topology *topo UNNEEDED) +{ fprintf(stderr, "delayed_to_us_feerate called!\n"); abort(); } /* Generated stub for deprecated_apis */ bool deprecated_apis; /* Generated stub for fatal */ void fatal(const char *fmt UNNEEDED, ...) { fprintf(stderr, "fatal called!\n"); abort(); } +/* Generated stub for feerate_for_deadline */ +u32 feerate_for_deadline(const struct chain_topology *topo UNNEEDED, u32 blockcount UNNEEDED) +{ fprintf(stderr, "feerate_for_deadline called!\n"); abort(); } +/* Generated stub for feerate_max */ +u32 feerate_max(struct lightningd *ld UNNEEDED, bool *unknown UNNEEDED) +{ fprintf(stderr, "feerate_max called!\n"); abort(); } +/* Generated stub for feerate_min */ +u32 feerate_min(struct lightningd *ld UNNEEDED, bool *unknown UNNEEDED) +{ fprintf(stderr, "feerate_min called!\n"); abort(); } /* Generated stub for fromwire_bigsize */ bigsize_t fromwire_bigsize(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) { fprintf(stderr, "fromwire_bigsize called!\n"); abort(); } @@ -28,6 +40,12 @@ bool fromwire_channel_id(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, /* Generated stub for fromwire_node_id */ void fromwire_node_id(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct node_id *id UNNEEDED) { fprintf(stderr, "fromwire_node_id called!\n"); abort(); } +/* Generated stub for get_feerate_floor */ +u32 get_feerate_floor(const struct chain_topology *topo UNNEEDED) +{ fprintf(stderr, "get_feerate_floor called!\n"); abort(); } +/* Generated stub for htlc_resolution_feerate */ +u32 htlc_resolution_feerate(struct chain_topology *topo UNNEEDED) +{ fprintf(stderr, "htlc_resolution_feerate called!\n"); abort(); } /* Generated stub for json_to_jsonrpc_errcode */ bool json_to_jsonrpc_errcode(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, enum jsonrpc_errcode *errcode UNNEEDED) @@ -52,6 +70,9 @@ void log_io(struct log *log UNNEEDED, enum log_level dir UNNEEDED, /* Generated stub for log_level_name */ const char *log_level_name(enum log_level level UNNEEDED) { fprintf(stderr, "log_level_name called!\n"); abort(); } +/* Generated stub for mutual_close_feerate */ +u32 mutual_close_feerate(struct chain_topology *topo UNNEEDED) +{ fprintf(stderr, "mutual_close_feerate called!\n"); abort(); } /* Generated stub for new_log */ struct log *new_log(const tal_t *ctx UNNEEDED, struct log_book *record UNNEEDED, const struct node_id *default_node_id UNNEEDED, @@ -63,6 +84,9 @@ struct oneshot *new_reltimer_(struct timers *timers UNNEEDED, struct timerel expire UNNEEDED, void (*cb)(void *) UNNEEDED, void *arg UNNEEDED) { fprintf(stderr, "new_reltimer_ called!\n"); abort(); } +/* Generated stub for opening_feerate */ +u32 opening_feerate(struct chain_topology *topo UNNEEDED) +{ fprintf(stderr, "opening_feerate called!\n"); abort(); } /* Generated stub for param */ bool param(struct command *cmd UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t params[] UNNEEDED, ...) @@ -97,6 +121,9 @@ const char *param_subcommand(struct command *cmd UNNEEDED, const char *buffer UN const jsmntok_t tokens[] UNNEEDED, const char *name UNNEEDED, ...) { fprintf(stderr, "param_subcommand called!\n"); abort(); } +/* Generated stub for penalty_feerate */ +u32 penalty_feerate(struct chain_topology *topo UNNEEDED) +{ fprintf(stderr, "penalty_feerate called!\n"); abort(); } /* Generated stub for plugin_hook_call_ */ bool plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, @@ -112,9 +139,9 @@ void towire_channel_id(u8 **pptr UNNEEDED, const struct channel_id *channel_id U /* Generated stub for towire_node_id */ void towire_node_id(u8 **pptr UNNEEDED, const struct node_id *id UNNEEDED) { fprintf(stderr, "towire_node_id called!\n"); abort(); } -/* Generated stub for try_get_feerate */ -u32 try_get_feerate(const struct chain_topology *topo UNNEEDED, enum feerate feerate UNNEEDED) -{ fprintf(stderr, "try_get_feerate called!\n"); abort(); } +/* Generated stub for unilateral_feerate */ +u32 unilateral_feerate(struct chain_topology *topo UNNEEDED) +{ fprintf(stderr, "unilateral_feerate called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ static int test_json_filter(void) diff --git a/plugins/bcli.c b/plugins/bcli.c index a79b67b1907a..a13a939bd041 100644 --- a/plugins/bcli.c +++ b/plugins/bcli.c @@ -60,13 +60,6 @@ struct bitcoind { /* Passthrough parameters for bitcoin-cli */ char *rpcuser, *rpcpass, *rpcconnect, *rpcport; - /* The factor to time the urgent feerate by to get the maximum - * acceptable feerate. */ - u32 max_fee_multiplier; - - /* Percent of CONSERVATIVE/2 feerate we'll use for commitment txs. */ - u64 commit_fee_percent; - /* Whether we fake fees (regtest) */ bool fake_fees; @@ -463,20 +456,24 @@ static struct command_result *process_getblockchaininfo(struct bitcoin_cli *bcli return command_finished(bcli->cmd, response); } -enum feerate_levels { - FEERATE_HIGHEST, - FEERATE_URGENT, - FEERATE_NORMAL, - FEERATE_SLOW, +struct estimatefee_params { + u32 blocks; + const char *style; +}; + +static const struct estimatefee_params estimatefee_params[] = { + { 2, "CONSERVATIVE" }, + { 6, "ECONOMICAL" }, + { 12, "ECONOMICAL" }, + { 100, "ECONOMICAL" }, }; -#define FEERATE_LEVEL_MAX (FEERATE_SLOW) struct estimatefees_stash { /* This is max(mempoolminfee,minrelaytxfee) */ u64 perkb_floor; u32 cursor; /* FIXME: We use u64 but lightningd will store them as u32. */ - u64 perkb[FEERATE_LEVEL_MAX+1]; + u64 perkb[ARRAY_SIZE(estimatefee_params)]; }; static struct command_result * @@ -484,14 +481,21 @@ estimatefees_null_response(struct bitcoin_cli *bcli) { struct json_stream *response = jsonrpc_stream_success(bcli->cmd); - json_add_null(response, "opening"); - json_add_null(response, "mutual_close"); - json_add_null(response, "unilateral_close"); - json_add_null(response, "delayed_to_us"); - json_add_null(response, "htlc_resolution"); - json_add_null(response, "penalty"); - json_add_null(response, "min_acceptable"); - json_add_null(response, "max_acceptable"); + /* We give a floor, which is the standard minimum */ + json_array_start(response, "feerates"); + json_array_end(response); + json_add_u32(response, "feerate_floor", 1000); + + if (deprecated_apis) { + json_add_null(response, "opening"); + json_add_null(response, "mutual_close"); + json_add_null(response, "unilateral_close"); + json_add_null(response, "delayed_to_us"); + json_add_null(response, "htlc_resolution"); + json_add_null(response, "penalty"); + json_add_null(response, "min_acceptable"); + json_add_null(response, "max_acceptable"); + } return command_finished(bcli->cmd, response); } @@ -665,18 +669,6 @@ static struct command_result *getchaininfo(struct command *cmd, /* Mutual recursion. */ static struct command_result *estimatefees_done(struct bitcoin_cli *bcli); -struct estimatefee_params { - u32 blocks; - const char *style; -}; - -static const struct estimatefee_params estimatefee_params[] = { - [FEERATE_HIGHEST] = { 2, "CONSERVATIVE" }, - [FEERATE_URGENT] = { 6, "ECONOMICAL" }, - [FEERATE_NORMAL] = { 12, "ECONOMICAL" }, - [FEERATE_SLOW] = { 100, "ECONOMICAL" }, -}; - /* Add a feerate, but don't publish one that bitcoind won't accept. */ static void json_add_feerate(struct json_stream *result, const char *fieldname, struct command *cmd, @@ -695,6 +687,16 @@ static void json_add_feerate(struct json_stream *result, const char *fieldname, } } +static u32 feerate_for_block(const struct estimatefees_stash *stash, u32 blocks) +{ + for (size_t i = 0; i < ARRAY_SIZE(stash->perkb); i++) { + if (estimatefee_params[i].blocks != blocks) + continue; + return stash->perkb[i]; + } + abort(); +} + static struct command_result *estimatefees_next(struct command *cmd, struct estimatefees_stash *stash) { @@ -713,31 +715,45 @@ static struct command_result *estimatefees_next(struct command *cmd, } response = jsonrpc_stream_success(cmd); - json_add_feerate(response, "opening", cmd, stash, - stash->perkb[FEERATE_NORMAL]); - json_add_feerate(response, "mutual_close", cmd, stash, - stash->perkb[FEERATE_SLOW]); - json_add_feerate(response, "unilateral_close", cmd, stash, - stash->perkb[FEERATE_URGENT] * bitcoind->commit_fee_percent / 100); - json_add_feerate(response, "delayed_to_us", cmd, stash, - stash->perkb[FEERATE_NORMAL]); - json_add_feerate(response, "htlc_resolution", cmd, stash, - stash->perkb[FEERATE_URGENT]); - json_add_feerate(response, "penalty", cmd, stash, - stash->perkb[FEERATE_NORMAL]); - /* We divide the slow feerate for the minimum acceptable, lightningd - * will use floor if it's hit, though. */ - json_add_feerate(response, "min_acceptable", cmd, stash, - stash->perkb[FEERATE_SLOW] / 2); - /* BOLT #2: - * - * Given the variance in fees, and the fact that the transaction may be - * spent in the future, it's a good idea for the fee payer to keep a good - * margin (say 5x the expected fee requirement) - */ - json_add_feerate(response, "max_acceptable", cmd, stash, - stash->perkb[FEERATE_HIGHEST] - * bitcoind->max_fee_multiplier); + if (deprecated_apis) { + json_add_feerate(response, "opening", cmd, stash, + feerate_for_block(stash, 12)); + json_add_feerate(response, "mutual_close", cmd, stash, + feerate_for_block(stash, 100)); + json_add_feerate(response, "unilateral_close", cmd, stash, + feerate_for_block(stash, 6)); + json_add_feerate(response, "delayed_to_us", cmd, stash, + feerate_for_block(stash, 12)); + json_add_feerate(response, "htlc_resolution", cmd, stash, + feerate_for_block(stash, 6)); + json_add_feerate(response, "penalty", cmd, stash, + feerate_for_block(stash, 12)); + /* We divide the slow feerate for the minimum acceptable, lightningd + * will use floor if it's hit, though. */ + json_add_feerate(response, "min_acceptable", cmd, stash, + feerate_for_block(stash, 100) / 2); + /* BOLT #2: + * + * Given the variance in fees, and the fact that the transaction may be + * spent in the future, it's a good idea for the fee payer to keep a good + * margin (say 5x the expected fee requirement) + */ + json_add_feerate(response, "max_acceptable", cmd, stash, + feerate_for_block(stash, 2) * 10); + } + + /* Modern style: present an ordered array of block deadlines, and a floor. */ + json_array_start(response, "feerates"); + for (size_t i = 0; i < ARRAY_SIZE(stash->perkb); i++) { + if (!stash->perkb[i]) + continue; + json_object_start(response, NULL); + json_add_u32(response, "blocks", estimatefee_params[i].blocks); + json_add_feerate(response, "feerate", cmd, stash, stash->perkb[i]); + json_object_end(response); + } + json_array_end(response); + json_add_u64(response, "feerate_floor", stash->perkb_floor); return command_finished(cmd, response); } @@ -1063,8 +1079,6 @@ static struct bitcoind *new_bitcoind(const tal_t *ctx) bitcoind->rpcpass = NULL; bitcoind->rpcconnect = NULL; bitcoind->rpcport = NULL; - bitcoind->max_fee_multiplier = 10; - bitcoind->commit_fee_percent = 100; #if DEVELOPER bitcoind->no_fake_fees = false; #endif @@ -1111,19 +1125,7 @@ int main(int argc, char *argv[]) "how long to keep retrying to contact bitcoind" " before fatally exiting", u64_option, &bitcoind->retry_timeout), - plugin_option("commit-fee", - "string", - "Percentage of fee to request for their commitment", - u64_option, &bitcoind->commit_fee_percent), #if DEVELOPER - plugin_option("dev-max-fee-multiplier", - "string", - "Allow the fee proposed by the remote end to" - " be up to multiplier times higher than our " - "own. Small values will cause channels to be" - " closed more often due to fee fluctuations," - " large values may result in large fees.", - u32_option, &bitcoind->max_fee_multiplier), plugin_option("dev-no-fake-fees", "bool", "Suppress fee faking for regtest", diff --git a/tests/test_closing.py b/tests/test_closing.py index 4c1e664cbb90..a58a5189aa79 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -95,50 +95,14 @@ def test_closing_simple(node_factory, bitcoind, chainparams): 'ONCHAIN:All outputs resolved: waiting 90 more blocks before forgetting channel' ]) - # Capture both side's image of channel before it's dead. - l1channel = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels']) - l2channel = only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels']) - # Make sure both have forgotten about it bitcoind.generate_block(90) - wait_for(lambda: len(l1.rpc.listpeerchannels()['channels']) == 0) - wait_for(lambda: len(l2.rpc.listpeerchannels()['channels']) == 0) + wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 0) + wait_for(lambda: len(l2.rpc.listchannels()['channels']) == 0) # The entry in the channels table should still be there assert l1.db_query("SELECT count(*) as c FROM channels;")[0]['c'] == 1 assert l2.db_query("SELECT count(*) as c FROM channels;")[0]['c'] == 1 - assert l1.db_query("SELECT count(*) as p FROM peers;")[0]['p'] == 1 - assert l2.db_query("SELECT count(*) as p FROM peers;")[0]['p'] == 1 - - # Test listclosedchannels is correct. - l1closedchannel = only_one(l1.rpc.listclosedchannels()['closedchannels']) - l2closedchannel = only_one(l2.rpc.listclosedchannels(l1.info['id'])['closedchannels']) - - # These fields do not appear in listpeerchannels! - l1_only_closed = {'total_local_commitments': 2, - 'total_remote_commitments': 2, - 'total_htlcs_sent': 1, - 'leased': False, - 'close_cause': 'user'} - l2_only_closed = {'total_local_commitments': 2, - 'total_remote_commitments': 2, - 'total_htlcs_sent': 0, - 'leased': False, - 'close_cause': 'remote'} - - # These fields have different names - renamed = {'last_commitment_txid': 'scratch_txid', - 'last_commitment_fee_msat': 'last_tx_fee_msat', - 'final_to_us_msat': 'to_us_msat'} - - for chan, closedchan, onlyclosed in (l1channel, l1closedchannel, l1_only_closed), (l2channel, l2closedchannel, l2_only_closed): - for k, v in closedchan.items(): - if k in renamed: - assert chan[renamed[k]] == v - elif k in onlyclosed: - assert closedchan[k] == onlyclosed[k] - else: - assert chan[k] == v assert account_balance(l1, channel_id) == 0 assert account_balance(l2, channel_id) == 0 @@ -1328,8 +1292,8 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams): assert blocks == 0 txids.append(txid) - # First one is already spent by their fulfill attempt - bitcoind.generate_block(1, wait_for_mempool=txids[1:]) + # First one is already spent by their fulfill attempt. Others may be RBF! + bitcoind.generate_block(1, len(txids[1:])) l3.daemon.wait_for_log('Resolved OUR_HTLC_FULFILL_TO_THEM/DELAYED_CHEAT_OUTPUT_TO_THEM ' 'by our proposal OUR_PENALTY_TX') l2.daemon.wait_for_log('Unknown spend of OUR_HTLC_SUCCESS_TX/DELAYED_OUTPUT_TO_US') @@ -1598,7 +1562,6 @@ def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams): assert acc['resolved_at_block'] > 0 -@pytest.mark.xfail(strict=True) @pytest.mark.developer("uses dev_sign_last_tx") def test_penalty_rbf_normal(node_factory, bitcoind, executor, chainparams): ''' @@ -1664,40 +1627,46 @@ def censoring_sendrawtx(r): # l2 notices. l2.daemon.wait_for_log(' to ONCHAIN') - def get_rbf_tx(self, depth, name, resolve): - r = self.daemon.wait_for_log('Broadcasting RBF {} .* to resolve {} depth={}' - .format(name, resolve, depth)) - return re.search(r'.* \(([0-9a-fA-F]*)\)', r).group(1) + ((_, txid1, blocks1), (_, txid2, blocks2)) = \ + l2.wait_for_onchaind_txs(('OUR_PENALTY_TX', + 'THEIR_REVOKED_UNILATERAL/THEIR_HTLC'), + ('OUR_PENALTY_TX', + 'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM')) + assert blocks1 == 0 + assert blocks2 == 0 + + def get_rbf_txid(node, txid): + line = node.daemon.wait_for_log("RBF onchain .*{}".format(txid)) + newtxid = re.search(r'with txid ([0-9a-fA-F]*)', line).group(1) + return newtxid - rbf_txes = [] # Now the censoring miners generate some blocks. - for depth in range(2, 8): + for depth in range(2, 10): bitcoind.generate_block(1) - sync_blockheight(bitcoind, [l2]) # l2 should RBF, twice even, one for the l1 main output, # one for the l1 HTLC output. - rbf_txes.append(get_rbf_tx(l2, depth, - 'OUR_PENALTY_TX', - 'THEIR_REVOKED_UNILATERAL/THEIR_HTLC')) - rbf_txes.append(get_rbf_tx(l2, depth, - 'OUR_PENALTY_TX', - 'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM')) + # Don't assume a specific order! + start = l2.daemon.logsearch_start + txid1 = get_rbf_txid(l2, txid1) + l2.daemon.logsearch_start = start + txid2 = get_rbf_txid(l2, txid2) # Now that the transactions have high fees, independent miners # realize they can earn potentially more money by grabbing the # high-fee censored transactions, and fresh, non-censoring # hashpower arises, evicting the censor. l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', None) + bitcoind.generate_block(1) - # Check that the order in which l2 generated RBF transactions - # would be acceptable to Bitcoin. - for tx in rbf_txes: - # Use the bcli interface as well, so that we also check the - # bcli interface. - l2.rpc.call('sendrawtransaction', [tx, True]) + # This triggers the final RBF attempt + start = l2.daemon.logsearch_start + txid1 = get_rbf_txid(l2, txid1) + l2.daemon.logsearch_start = start + txid2 = get_rbf_txid(l2, txid2) # Now the non-censoring miners overpower the censoring miners. - bitcoind.generate_block(1) + # FIXME: Some of those RBFs may not be accepted by bitcoind, so just check number in mempool. + bitcoind.generate_block(1, wait_for_mempool=len([txid1, txid2])) sync_blockheight(bitcoind, [l2]) # And l2 should consider it resolved now. @@ -1723,134 +1692,6 @@ def get_rbf_tx(self, depth, name, resolve): check_utxos_channel(l2, [channel_id], expected_2) -@pytest.mark.xfail(strict=True) -@pytest.mark.developer("uses dev_sign_last_tx") -def test_penalty_rbf_burn(node_factory, bitcoind, executor, chainparams): - ''' - Test that penalty transactions are RBFed and we are willing to burn - it all up to spite the thief. - ''' - # We track channel balances, to verify that accounting is ok. - coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') - to_self_delay = 10 - # l1 is the thief, which causes our honest upstanding lightningd - # code to break, so l1 can fail. - # Initially, disconnect before the HTLC can be resolved. - l1 = node_factory.get_node(options={'dev-disable-commit-after': 1}, - may_fail=True, allow_broken_log=True) - l2 = node_factory.get_node(options={'dev-disable-commit-after': 1, - 'watchtime-blocks': to_self_delay, - 'plugin': coin_mvt_plugin}, - # Exporbitant feerates mean we don't have cap on RBF! - feerates=(15000000, 11000, 7500, 3750)) - - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.fundchannel(l2, 10**7) - channel_id = first_channel_id(l1, l2) - - # Trigger an HTLC being added. - t = executor.submit(l1.pay, l2, 1000000 * 1000) - - # Make sure the channel is still alive. - assert len(l1.getactivechannels()) == 2 - assert len(l2.getactivechannels()) == 2 - - # Wait for the disconnection. - l1.daemon.wait_for_log('dev-disable-commit-after: disabling') - l2.daemon.wait_for_log('dev-disable-commit-after: disabling') - # Make sure l1 gets the new HTLC. - l1.daemon.wait_for_log('got commitsig') - - # l1 prepares a theft commitment transaction - theft_tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx'] - - # Now continue processing until fulfilment. - l1.rpc.dev_reenable_commit(l2.info['id']) - l2.rpc.dev_reenable_commit(l1.info['id']) - - # Wait for the fulfilment. - l1.daemon.wait_for_log('peer_in WIRE_UPDATE_FULFILL_HTLC') - l1.daemon.wait_for_log('peer_out WIRE_REVOKE_AND_ACK') - l2.daemon.wait_for_log('peer_out WIRE_UPDATE_FULFILL_HTLC') - l1.daemon.wait_for_log('peer_in WIRE_REVOKE_AND_ACK') - - # Now payment should complete. - t.result(timeout=10) - - # l1 goes offline and bribes the miners to censor transactions from l2. - l1.rpc.stop() - - def censoring_sendrawtx(r): - return {'id': r['id'], 'result': {}} - - l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', censoring_sendrawtx) - - # l1 now performs the theft attack! - bitcoind.rpc.sendrawtransaction(theft_tx) - bitcoind.generate_block(1) - - # l2 notices. - l2.daemon.wait_for_log(' to ONCHAIN') - - def get_rbf_tx(self, depth, name, resolve): - r = self.daemon.wait_for_log('Broadcasting RBF {} .* to resolve {} depth={}' - .format(name, resolve, depth)) - return re.search(r'.* \(([0-9a-fA-F]*)\)', r).group(1) - - rbf_txes = [] - # Now the censoring miners generate some blocks. - for depth in range(2, 10): - bitcoind.generate_block(1) - sync_blockheight(bitcoind, [l2]) - # l2 should RBF, twice even, one for the l1 main output, - # one for the l1 HTLC output. - rbf_txes.append(get_rbf_tx(l2, depth, - 'OUR_PENALTY_TX', - 'THEIR_REVOKED_UNILATERAL/THEIR_HTLC')) - rbf_txes.append(get_rbf_tx(l2, depth, - 'OUR_PENALTY_TX', - 'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM')) - - # Now that the transactions have high fees, independent miners - # realize they can earn potentially more money by grabbing the - # high-fee censored transactions, and fresh, non-censoring - # hashpower arises, evicting the censor. - l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', None) - - # Check that the last two txes can be broadcast. - # These should donate the total amount to miners. - rbf_txes = rbf_txes[-2:] - for tx in rbf_txes: - l2.rpc.call('sendrawtransaction', [tx, True]) - - # Now the non-censoring miners overpower the censoring miners. - bitcoind.generate_block(1) - sync_blockheight(bitcoind, [l2]) - - # And l2 should consider it resolved now. - l2.daemon.wait_for_log('Resolved THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM by our proposal OUR_PENALTY_TX') - l2.daemon.wait_for_log('Resolved THEIR_REVOKED_UNILATERAL/THEIR_HTLC by our proposal OUR_PENALTY_TX') - - # l2 donated it to the miners, so it owns nothing - assert(len(l2.rpc.listfunds()['outputs']) == 0) - assert account_balance(l2, channel_id) == 0 - - expected_2 = { - 'A': [('cid1', ['channel_open'], ['channel_close'], 'B')], - 'B': [('cid1', ['penalty'], ['to_miner'], 'C'), ('cid1', ['penalty'], ['to_miner'], 'D')], - } - - if anchor_expected(): - expected_2['B'].append(('external', ['anchor'], None, None)) - expected_2['B'].append(('wallet', ['anchor', 'ignored'], None, None)) - - check_utxos_channel(l2, [channel_id], expected_2) - - # Make sure that l2's account is considered closed (has a fee output) - fees = [e for e in l2.rpc.bkpr_listincome()['income_events'] if e['tag'] == 'onchain_fee'] - assert len(fees) == 1 - - @pytest.mark.developer("needs DEVELOPER=1") def test_onchain_first_commit(node_factory, bitcoind): """Onchain handling where opener immediately drops to chain""" @@ -2000,7 +1841,8 @@ def test_onchaind_replay(node_factory, bitcoind): 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') assert blocks == 200 bitcoind.generate_block(200) - bitcoind.generate_block(1, wait_for_mempool=txid) + # Could be RBF! + l1.mine_txid_or_rbf(txid) @pytest.mark.developer("needs DEVELOPER=1") @@ -2493,7 +2335,8 @@ def try_pay(): assert not t.is_alive() # 100 blocks after last spend, l1+l2 should be done. - l2.bitcoin.generate_block(100, wait_for_mempool=txid) + # Could be RBF! + l1.mine_txid_or_rbf(txid, numblocks=100) l1.daemon.wait_for_log('onchaind complete, forgetting peer') l2.daemon.wait_for_log('onchaind complete, forgetting peer') @@ -2613,8 +2456,8 @@ def test_onchain_feechange(node_factory, bitcoind, executor): assert blocks == 5 bitcoind.generate_block(5) - # Make sure that gets included. - bitcoind.generate_block(1, wait_for_mempool=txid) + # Could be RBF! + l1.mine_txid_or_rbf(txid) # Now we restart with different feerates. l1.stop() @@ -2688,7 +2531,7 @@ def test_onchain_all_dust(node_factory, bitcoind, executor): # Make l1's fees really high (and wait for it to exceed 50000) l1.set_feerates((1000000, 1000000, 1000000, 1000000)) - l1.daemon.wait_for_log('Feerate estimate for unilateral_close set to [56789][0-9]{4}') + l1.daemon.wait_for_log('feerate estimate for 6 blocks smoothed to [56789][0-9]{4}') bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') diff --git a/tests/test_misc.py b/tests/test_misc.py index 7eb0e87a765b..6b5dc5d8f5fa 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -86,11 +86,11 @@ def crash_bitcoincli(r): l1.daemon.rpcproxy.mock_rpc('getblockhash', crash_bitcoincli) # This should cause both estimatefee and getblockhash fail - l1.daemon.wait_for_logs(['Unable to estimate .* fee', + l1.daemon.wait_for_logs(['Unable to estimate any fees', 'getblockhash .* exited with status 1']) # And they should retry! - l1.daemon.wait_for_logs(['Unable to estimate .* fee', + l1.daemon.wait_for_logs(['Unable to estimate any fees', 'getblockhash .* exited with status 1']) # Restore, then it should recover and get blockheight. @@ -1524,8 +1524,7 @@ def test_feerates(node_factory): l1.start() # All estimation types - types = ["opening", "mutual_close", "unilateral_close", "delayed_to_us", - "htlc_resolution", "penalty"] + types = ["opening", "mutual_close", "unilateral_close", "penalty"] # Try parsing the feerates, won't work because can't estimate for t in types: @@ -1533,21 +1532,26 @@ def test_feerates(node_factory): feerate = l1.rpc.parsefeerate(t) # Query feerates (shouldn't give any!) - wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 2) + wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 4) feerates = l1.rpc.feerates('perkw') assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?' assert 'perkb' not in feerates assert feerates['perkw']['max_acceptable'] == 2**32 - 1 assert feerates['perkw']['min_acceptable'] == 253 + assert feerates['perkw']['min_acceptable'] == 253 + assert feerates['perkw']['floor'] == 253 + assert feerates['perkw']['estimates'] == [] for t in types: assert t not in feerates['perkw'] - wait_for(lambda: len(l1.rpc.feerates('perkb')['perkb']) == 2) feerates = l1.rpc.feerates('perkb') assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?' assert 'perkw' not in feerates assert feerates['perkb']['max_acceptable'] == (2**32 - 1) assert feerates['perkb']['min_acceptable'] == 253 * 4 + # Note: This is floored at the FEERATE_FLOOR constant (253) + assert feerates['perkb']['floor'] == 1012 + assert feerates['perkb']['estimates'] == [] for t in types: assert t not in feerates['perkb'] @@ -1556,56 +1560,87 @@ def test_feerates(node_factory): l1.set_feerates((15000, 0, 0, 0), True) wait_for(lambda: l1.rpc.feerates('perkw')['perkw']['max_acceptable'] == 15000 * 10) feerates = l1.rpc.feerates('perkw') - assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?' + # We only get the warning if *no* feerates are avail. + assert 'warning_missing_feerates' not in feerates assert 'perkb' not in feerates - assert feerates['perkw']['min_acceptable'] == 253 + # With only one data point, this is a terrible guess! + assert feerates['perkw']['min_acceptable'] == 15000 // 2 + assert feerates['perkw']['estimates'] == [{'blockcount': 2, + 'feerate': 15000, + 'smoothed_feerate': 15000}] # Set ECONOMICAL/6 feerate, for unilateral_close and htlc_resolution l1.set_feerates((15000, 11000, 0, 0), True) - wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 4) feerates = l1.rpc.feerates('perkw') assert feerates['perkw']['unilateral_close'] == 11000 - assert feerates['perkw']['htlc_resolution'] == 11000 - assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?' + assert 'warning_missing_feerates' not in feerates assert 'perkb' not in feerates assert feerates['perkw']['max_acceptable'] == 15000 * 10 - assert feerates['perkw']['min_acceptable'] == 253 + # With only two data points, this is a terrible guess! + assert feerates['perkw']['min_acceptable'] == 11000 // 2 + assert feerates['perkw']['estimates'] == [{'blockcount': 2, + 'feerate': 15000, + 'smoothed_feerate': 15000}, + {'blockcount': 6, + 'feerate': 11000, + 'smoothed_feerate': 11000}] # Set ECONOMICAL/12 feerate, for all but min (so, no mutual_close feerate) l1.set_feerates((15000, 11000, 6250, 0), True) - wait_for(lambda: len(l1.rpc.feerates('perkb')['perkb']) == len(types) - 1 + 2) feerates = l1.rpc.feerates('perkb') assert feerates['perkb']['unilateral_close'] == 11000 * 4 - assert feerates['perkb']['htlc_resolution'] == 11000 * 4 - assert 'mutual_close' not in feerates['perkb'] + # We dont' extrapolate, so it uses the same for mutual_close + assert feerates['perkb']['mutual_close'] == 6250 * 4 for t in types: if t not in ("unilateral_close", "htlc_resolution", "mutual_close"): assert feerates['perkb'][t] == 25000 - assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?' + assert 'warning_missing_feerates' not in feerates assert 'perkw' not in feerates assert feerates['perkb']['max_acceptable'] == 15000 * 4 * 10 - assert feerates['perkb']['min_acceptable'] == 253 * 4 + # With only three data points, this is a terrible guess! + assert feerates['perkb']['min_acceptable'] == 6250 // 2 * 4 + assert feerates['perkb']['estimates'] == [{'blockcount': 2, + 'feerate': 15000 * 4, + 'smoothed_feerate': 15000 * 4}, + {'blockcount': 6, + 'feerate': 11000 * 4, + 'smoothed_feerate': 11000 * 4}, + {'blockcount': 12, + 'feerate': 6250 * 4, + 'smoothed_feerate': 6250 * 4}] # Set ECONOMICAL/100 feerate for min and mutual_close l1.set_feerates((15000, 11000, 6250, 5000), True) wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) >= len(types) + 2) feerates = l1.rpc.feerates('perkw') assert feerates['perkw']['unilateral_close'] == 11000 - assert feerates['perkw']['htlc_resolution'] == 11000 assert feerates['perkw']['mutual_close'] == 5000 for t in types: if t not in ("unilateral_close", "htlc_resolution", "mutual_close"): assert feerates['perkw'][t] == 25000 // 4 - assert 'warning' not in feerates + assert 'warning_missing_feerates' not in feerates assert 'perkb' not in feerates assert feerates['perkw']['max_acceptable'] == 15000 * 10 assert feerates['perkw']['min_acceptable'] == 5000 // 2 + assert feerates['perkw']['estimates'] == [{'blockcount': 2, + 'feerate': 15000, + 'smoothed_feerate': 15000}, + {'blockcount': 6, + 'feerate': 11000, + 'smoothed_feerate': 11000}, + {'blockcount': 12, + 'feerate': 6250, + 'smoothed_feerate': 6250}, + {'blockcount': 100, + 'feerate': 5000, + 'smoothed_feerate': 5000}] assert len(feerates['onchain_fee_estimates']) == 5 assert feerates['onchain_fee_estimates']['opening_channel_satoshis'] == feerates['perkw']['opening'] * 702 // 1000 assert feerates['onchain_fee_estimates']['mutual_close_satoshis'] == feerates['perkw']['mutual_close'] * 673 // 1000 assert feerates['onchain_fee_estimates']['unilateral_close_satoshis'] == feerates['perkw']['unilateral_close'] * 598 // 1000 - htlc_feerate = feerates["perkw"]["htlc_resolution"] + # htlc resolution currently uses 6 block estimate + htlc_feerate = [f['feerate'] for f in feerates['perkw']['estimates'] if f['blockcount'] == 6][0] htlc_timeout_cost = feerates["onchain_fee_estimates"]["htlc_timeout_satoshis"] htlc_success_cost = feerates["onchain_fee_estimates"]["htlc_success_satoshis"] @@ -1899,7 +1934,7 @@ def mock_fail(*args): l1.daemon.start(wait_for_initialized=False, stderr_redir=True) l1.daemon.wait_for_logs([r'getblockhash [a-z0-9]* exited with status 1', - r'Unable to estimate opening fees', + r'Unable to estimate any fees', r'BROKEN.*we have been retrying command for --bitcoin-retry-timeout={} seconds'.format(timeout)]) # Will exit with failure code. assert l1.daemon.wait() == 1 @@ -1911,7 +1946,7 @@ def mock_fail(*args): @unittest.skipIf(TEST_NETWORK == 'liquid-regtest', "Fees on elements are different") def test_bitcoind_feerate_floor(node_factory, bitcoind): - """Don't return a feerate less than minrelaytxfee/mempoolnifee.""" + """Don't return a feerate less than minrelaytxfee/mempoolminfee.""" l1 = node_factory.get_node() anchors = EXPERIMENTAL_FEATURES @@ -1920,11 +1955,22 @@ def test_bitcoind_feerate_floor(node_factory, bitcoind): "opening": 30000, "mutual_close": 15000, "unilateral_close": 44000, - "delayed_to_us": 30000, - "htlc_resolution": 44000, "penalty": 30000, "min_acceptable": 7500, - "max_acceptable": 600000 + "max_acceptable": 600000, + "floor": 1012, + "estimates": [{"blockcount": 2, + "feerate": 60000, + "smoothed_feerate": 60000}, + {"blockcount": 6, + "feerate": 44000, + "smoothed_feerate": 44000}, + {"blockcount": 12, + "feerate": 30000, + "smoothed_feerate": 30000}, + {"blockcount": 100, + "feerate": 15000, + "smoothed_feerate": 15000}], }, "onchain_fee_estimates": { "opening_channel_satoshis": 5265, @@ -1947,12 +1993,23 @@ def test_bitcoind_feerate_floor(node_factory, bitcoind): # This has increased (rounded up) "mutual_close": 20004, "unilateral_close": 44000, - "delayed_to_us": 30000, - "htlc_resolution": 44000, "penalty": 30000, - # This has increased (rounded up!) + # This has increased (rounded up) "min_acceptable": 20004, - "max_acceptable": 600000 + "max_acceptable": 600000, + "floor": 20004, + "estimates": [{"blockcount": 2, + "feerate": 60000, + "smoothed_feerate": 60000}, + {"blockcount": 6, + "feerate": 44000, + "smoothed_feerate": 44000}, + {"blockcount": 12, + "feerate": 30000, + "smoothed_feerate": 30000}, + {"blockcount": 100, + "feerate": 20004, + "smoothed_feerate": 20004}], }, "onchain_fee_estimates": { "opening_channel_satoshis": 5265, @@ -1978,13 +2035,25 @@ def test_bitcoind_feerate_floor(node_factory, bitcoind): "mutual_close": 30004, "unilateral_close": 44000, # This has increased (rounded up!) - "delayed_to_us": 30004, - "htlc_resolution": 44000, - # This has increased (rounded up!) "penalty": 30004, - # This has increased (rounded up!) + # This has increased (rounded up) "min_acceptable": 30004, - "max_acceptable": 600000 + "max_acceptable": 600000, + "floor": 30004, + "estimates": [{"blockcount": 2, + "feerate": 60000, + "smoothed_feerate": 60000}, + {"blockcount": 6, + "feerate": 44000, + "smoothed_feerate": 44000}, + # This has increased (rounded up!) + {"blockcount": 12, + "feerate": 30004, + "smoothed_feerate": 30004}, + # This has increased (rounded up!) + {"blockcount": 100, + "feerate": 30004, + "smoothed_feerate": 30004}], }, "onchain_fee_estimates": { "opening_channel_satoshis": 5265, @@ -2651,15 +2720,24 @@ def test_restorefrompeer(node_factory, bitcoind): def test_commitfee_option(node_factory): """Sanity check for the --commit-fee startup option.""" - l1, l2 = node_factory.get_nodes(2, opts=[{"commit-fee": "200"}, {}]) + l1, l2 = node_factory.get_nodes(2, opts=[{"commit-fee": "200", + "start": False}, + {"start": False}]) + # set_feerates multiplies this by 4 to get perkb; but we divide. mock_wu = 5000 for l in [l1, l2]: - l.set_feerates((0, mock_wu, 0, 0), True) - l1_commit_fees = l1.rpc.call("estimatefees")["unilateral_close"] - l2_commit_fees = l2.rpc.call("estimatefees")["unilateral_close"] + l.set_feerates((0, mock_wu, 0, 0), False) + l.start() + + # plugin gives same results: + assert l1.rpc.call("estimatefees") == l2.rpc.call("estimatefees") + + # But feerates differ. + l1_commit_fees = l1.rpc.feerates("perkw")['perkw']['unilateral_close'] + l2_commit_fees = l2.rpc.feerates("perkw")['perkw']['unilateral_close'] - assert l1_commit_fees == 2 * l2_commit_fees == 2 * 4 * mock_wu # WU->VB + assert l1_commit_fees == 2 * l2_commit_fees == 2 * mock_wu def test_listtransactions(node_factory): @@ -2892,15 +2970,29 @@ def test_force_feerates(node_factory): l1 = node_factory.get_node(options={'force-feerates': 1111}) assert l1.rpc.listconfigs()['force-feerates'] == '1111' + # Note that estimates are still valid here, despite "force-feerates" + estimates = [{"blockcount": 2, + "feerate": 15000, + "smoothed_feerate": 15000}, + {"blockcount": 6, + "feerate": 11000, + "smoothed_feerate": 11000}, + {"blockcount": 12, + "feerate": 7500, + "smoothed_feerate": 7500}, + {"blockcount": 100, + "feerate": 3750, + "smoothed_feerate": 3750}] + assert l1.rpc.feerates('perkw')['perkw'] == { "opening": 1111, "mutual_close": 1111, "unilateral_close": 1111, - "delayed_to_us": 1111, - "htlc_resolution": 1111, "penalty": 1111, "min_acceptable": 1875, - "max_acceptable": 150000} + "max_acceptable": 150000, + "estimates": estimates, + "floor": 253} l1.stop() l1.daemon.opts['force-feerates'] = '1111/2222' @@ -2911,11 +3003,11 @@ def test_force_feerates(node_factory): "opening": 1111, "mutual_close": 2222, "unilateral_close": 2222, - "delayed_to_us": 2222, - "htlc_resolution": 2222, "penalty": 2222, "min_acceptable": 1875, - "max_acceptable": 150000} + "max_acceptable": 150000, + "estimates": estimates, + "floor": 253} l1.stop() l1.daemon.opts['force-feerates'] = '1111/2222/3333/4444/5555/6666' @@ -2926,11 +3018,11 @@ def test_force_feerates(node_factory): "opening": 1111, "mutual_close": 2222, "unilateral_close": 3333, - "delayed_to_us": 4444, - "htlc_resolution": 5555, "penalty": 6666, "min_acceptable": 1875, - "max_acceptable": 150000} + "max_acceptable": 150000, + "estimates": estimates, + "floor": 253} def test_datastore_escapeing(node_factory): @@ -3203,6 +3295,50 @@ def test_hsm_capabilities(node_factory): assert l1.daemon.is_in_log(r"hsmd: capability \+WIRE_HSMD_CHECK_PUBKEY") +def test_feerate_arg(node_factory): + """Make sure our variants of feerate argument work!""" + l1 = node_factory.get_node() + + # These are the get_node() defaults + by_blocks = {2: 15000, + 6: 11000, + 12: 7500, + 100: 3750} + + # Literal values: + fees = {"9999perkw": 9999, + "10000perkb": 10000 // 4, + 10000: 10000 // 4} + + fees["urgent"] = by_blocks[6] + fees["normal"] = by_blocks[12] + fees["slow"] = by_blocks[100] + + fees["opening"] = by_blocks[12] + fees["mutual_close"] = by_blocks[100] + fees["penalty"] = by_blocks[12] + fees["unilateral_close"] = by_blocks[6] + + fees["2blocks"] = by_blocks[2] + fees["6blocks"] = by_blocks[6] + fees["12blocks"] = by_blocks[12] + fees["100blocks"] = by_blocks[100] + + # Simple interpolation + fees["9blocks"] = (by_blocks[6] + by_blocks[12]) // 2 + + for fee, expect in fees.items(): + # Put arg in assertion, so it gets printed on failure! + assert (l1.rpc.parsefeerate(fee), fee) == ({'perkw': expect}, fee) + + # More thorough interpolation + for block in range(12, 100): + # y = y1 + (x-x1)(y2-y1)/(x2-x1) + fee = by_blocks[12] + (block - 12) * (by_blocks[100] - by_blocks[12]) // (100 - 12) + # Rounding error is a thing! + assert abs(l1.rpc.parsefeerate(f"{block}blocks")['perkw'] - fee) <= 1 + + @pytest.mark.skip(reason="Fails by intention for creating test gossip stores") def test_create_gossip_mesh(node_factory, bitcoind): """ diff --git a/tests/test_pay.py b/tests/test_pay.py index 5d9fe8f588da..83d7dc848535 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -1626,7 +1626,8 @@ def test_forward_local_failed_stats(node_factory, bitcoind, executor): assert blocks == 5 bitcoind.generate_block(5) - bitcoind.generate_block(1, wait_for_mempool=txid) + # Could be RBF! + l2.mine_txid_or_rbf(txid) l2.daemon.wait_for_log('Resolved THEIR_UNILATERAL/OUR_HTLC by our proposal OUR_HTLC_TIMEOUT_TO_US') l4.daemon.wait_for_log('Ignoring output.*: OUR_UNILATERAL/THEIR_HTLC') diff --git a/tests/test_plugin.py b/tests/test_plugin.py index c6e4f1999062..d53382b6d1c5 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -1337,7 +1337,8 @@ def test_forward_event_notification(node_factory, bitcoind, executor): assert blocks == 5 bitcoind.generate_block(5) - bitcoind.generate_block(1, wait_for_mempool=txid) + # Could be RBF! + l2.mine_txid_or_rbf(txid) l2.daemon.wait_for_log('Resolved THEIR_UNILATERAL/OUR_HTLC by our proposal OUR_HTLC_TIMEOUT_TO_US') l5.daemon.wait_for_log('Ignoring output.*: OUR_UNILATERAL/THEIR_HTLC') @@ -1743,10 +1744,8 @@ def test_bcli(node_factory, bitcoind, chainparams): # Failure case of feerate is tested in test_misc.py estimates = l1.rpc.call("estimatefees") - for est in ["opening", "mutual_close", "unilateral_close", "delayed_to_us", - "htlc_resolution", "penalty", "min_acceptable", - "max_acceptable"]: - assert est in estimates + assert 'feerate_floor' in estimates + assert [f['blocks'] for f in estimates['feerates']] == [2, 6, 12, 100] resp = l1.rpc.call("getchaininfo") assert resp["chain"] == chainparams['name'] diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 883d4aef74d5..61d5f2075dfd 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -1658,12 +1658,12 @@ def test_upgradewallet(node_factory, bitcoind): # Doing it with 'reserved ok' should have 1 # We use a big feerate so we can get over the RBF hump - upgrade = l1.rpc.upgradewallet(feerate="max_acceptable", reservedok=True) + upgrade = l1.rpc.upgradewallet(feerate="urgent", reservedok=True) assert upgrade['upgraded_outs'] == 1 assert bitcoind.rpc.getmempoolinfo()['size'] == 1 # Mine it, nothing to upgrade l1.bitcoin.generate_block(1) sync_blockheight(l1.bitcoin, [l1]) - upgrade = l1.rpc.upgradewallet(feerate="max_acceptable", reservedok=True) + upgrade = l1.rpc.upgradewallet(feerate="urgent", reservedok=True) assert upgrade['upgraded_outs'] == 0