From 8d95917e7c2e990cb299cf8c7bb81f9bc1cb9cb1 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 6 Sep 2018 18:40:11 +0200 Subject: [PATCH 01/36] chainparams: Add max_funding_satoshi and max_payment_msat to chainparams --- bitcoin/chainparams.c | 20 ++++++++++++++++++++ bitcoin/chainparams.h | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/bitcoin/chainparams.c b/bitcoin/chainparams.c index fe4a1c64537e..be1793facf6a 100644 --- a/bitcoin/chainparams.c +++ b/bitcoin/chainparams.c @@ -12,6 +12,8 @@ const struct chainparams networks[] = { .cli = "bitcoin-cli", .cli_args = NULL, .dust_limit = 546, + .max_funding_satoshi = (1 << 24) - 1, + .max_payment_msat = 0xFFFFFFFFULL, /* "Lightning Charge Powers Developers & Blockstream Store" */ .when_lightning_became_cool = 504500, .testnet = false}, @@ -23,6 +25,8 @@ const struct chainparams networks[] = { .cli = "bitcoin-cli", .cli_args = "-regtest", .dust_limit = 546, + .max_funding_satoshi = (1 << 24) - 1, + .max_payment_msat = 0xFFFFFFFFULL, .when_lightning_became_cool = 1, .testnet = true}, {.index = 2, @@ -33,6 +37,8 @@ const struct chainparams networks[] = { .cli = "bitcoin-cli", .cli_args = "-testnet", .dust_limit = 546, + .max_funding_satoshi = (1 << 24) - 1, + .max_payment_msat = 0xFFFFFFFFULL, .testnet = true}, {.index = 3, .network_name = "litecoin", @@ -42,6 +48,8 @@ const struct chainparams networks[] = { .cli = "litecoin-cli", .cli_args = NULL, .dust_limit = 100000, + .max_funding_satoshi = 60 * ((1 << 24) - 1), + .max_payment_msat = 60 * 0xFFFFFFFFULL, .when_lightning_became_cool = 1320000, .testnet = false}, {.index = 4, @@ -52,6 +60,8 @@ const struct chainparams networks[] = { .cli = "litecoin-cli", .cli_args = "-testnet", .dust_limit = 100000, + .max_funding_satoshi = 60 * ((1 << 24) - 1), + .max_payment_msat = 60 * 0xFFFFFFFFULL, .when_lightning_became_cool = 1, .testnet = true} }; @@ -75,6 +85,16 @@ const struct chainparams *chainparams_by_index(const int index) } } +const struct chainparams *chainparams_by_chainhash(const struct bitcoin_blkid *chain_hash) +{ + for (size_t i = 0; i < ARRAY_SIZE(networks); i++) { + if (bitcoin_blkid_eq(chain_hash, &networks[i].genesis_blockhash)) { + return &networks[i]; + } + } + return NULL; +} + const struct chainparams *chainparams_by_bip173(const char *bip173_name) { for (size_t i = 0; i < ARRAY_SIZE(networks); i++) { diff --git a/bitcoin/chainparams.h b/bitcoin/chainparams.h index 92a0d2d6dc32..9070a1cd9597 100644 --- a/bitcoin/chainparams.h +++ b/bitcoin/chainparams.h @@ -15,6 +15,8 @@ struct chainparams { const char *cli; const char *cli_args; const u64 dust_limit; + const u64 max_funding_satoshi; + const u64 max_payment_msat; const u32 when_lightning_became_cool; /* Whether this is a test network or not */ @@ -40,4 +42,10 @@ const struct chainparams *chainparams_by_index(const int index); * This lets us decode BOLT11 addresses. */ const struct chainparams *chainparams_by_bip173(const char *bip173_name); + +/** + * chainparams_by_chainhash - Helper to get a network by its genesis blockhash + */ +const struct chainparams *chainparams_by_chainhash(const struct bitcoin_blkid *chain_hash); + #endif /* LIGHTNING_BITCOIN_CHAINPARAMS_H */ From 2402c524cc55d39508a91d716cc8df913b050ef7 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 6 Sep 2018 18:44:47 +0200 Subject: [PATCH 02/36] channeld: Keep track of the chainparams for the chain we are using --- channeld/channeld.c | 4 +++- channeld/full_channel.c | 6 +++++- channeld/full_channel.h | 1 + channeld/test/run-full_channel.c | 9 +++++++-- common/initial_channel.c | 4 ++++ common/initial_channel.h | 5 +++++ openingd/openingd.c | 2 ++ 7 files changed, 27 insertions(+), 4 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 7441c06ccccd..028228ac4ba0 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2614,7 +2614,9 @@ static void init_channel(struct peer *peer) /* channel_id is set from funding txout */ derive_channel_id(&peer->channel_id, &funding_txid, funding_txout); - peer->channel = new_full_channel(peer, &funding_txid, funding_txout, + peer->channel = new_full_channel(peer, + &peer->chain_hash, + &funding_txid, funding_txout, funding_satoshi, local_msatoshi, feerate_per_kw, diff --git a/channeld/full_channel.c b/channeld/full_channel.c index e8b395a41240..d5d930c07a33 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -22,6 +23,7 @@ #include "gen_full_channel_error_names.h" struct channel *new_full_channel(const tal_t *ctx, + const struct bitcoin_blkid *chain_hash, const struct bitcoin_txid *funding_txid, unsigned int funding_txout, u64 funding_satoshis, @@ -35,7 +37,9 @@ struct channel *new_full_channel(const tal_t *ctx, const struct pubkey *remote_funding_pubkey, enum side funder) { - struct channel *channel = new_initial_channel(ctx, funding_txid, + struct channel *channel = new_initial_channel(ctx, + chain_hash, + funding_txid, funding_txout, funding_satoshis, local_msatoshi, diff --git a/channeld/full_channel.h b/channeld/full_channel.h index b8ef156c8860..103de0a8f145 100644 --- a/channeld/full_channel.h +++ b/channeld/full_channel.h @@ -27,6 +27,7 @@ * Returns state, or NULL if malformed. */ struct channel *new_full_channel(const tal_t *ctx, + const struct bitcoin_blkid *chain_hash, const struct bitcoin_txid *funding_txid, unsigned int funding_txout, u64 funding_satoshis, diff --git a/channeld/test/run-full_channel.c b/channeld/test/run-full_channel.c index 58e8bb3914ba..f8098acf1e34 100644 --- a/channeld/test/run-full_channel.c +++ b/channeld/test/run-full_channel.c @@ -336,6 +336,7 @@ int main(void) const struct htlc **htlc_map, **htlcs; const u8 *funding_wscript, **wscripts; size_t i; + const struct chainparams *chainparams = chainparams_for_network("bitcoin"); secp256k1_ctx = wally_get_secp_context(); setup_tmpctx(); @@ -443,7 +444,9 @@ int main(void) to_local_msat = 7000000000; to_remote_msat = 3000000000; feerate_per_kw[LOCAL] = feerate_per_kw[REMOTE] = 15000; - lchannel = new_full_channel(tmpctx, &funding_txid, funding_output_index, + lchannel = new_full_channel(tmpctx, + &chainparams->genesis_blockhash, + &funding_txid, funding_output_index, funding_amount_satoshi, to_local_msat, feerate_per_kw, local_config, @@ -452,7 +455,9 @@ int main(void) &local_funding_pubkey, &remote_funding_pubkey, LOCAL); - rchannel = new_full_channel(tmpctx, &funding_txid, funding_output_index, + rchannel = new_full_channel(tmpctx, + &chainparams->genesis_blockhash, + &funding_txid, funding_output_index, funding_amount_satoshi, to_remote_msat, feerate_per_kw, remote_config, diff --git a/common/initial_channel.c b/common/initial_channel.c index f2ad81c18004..94a93e977074 100644 --- a/common/initial_channel.c +++ b/common/initial_channel.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -8,6 +9,7 @@ #include struct channel *new_initial_channel(const tal_t *ctx, + const struct bitcoin_blkid *chain_hash, const struct bitcoin_txid *funding_txid, unsigned int funding_txout, u64 funding_satoshis, @@ -58,6 +60,8 @@ struct channel *new_initial_channel(const tal_t *ctx, channel->commitment_number_obscurer = commit_number_obscurer(&channel->basepoints[funder].payment, &channel->basepoints[!funder].payment); + channel->chainparams = chainparams_by_chainhash(chain_hash); + assert(channel->chainparams != NULL); return channel; } diff --git a/common/initial_channel.h b/common/initial_channel.h index 9ea5df5732b1..a3ad474545c7 100644 --- a/common/initial_channel.h +++ b/common/initial_channel.h @@ -57,6 +57,9 @@ struct channel { /* What it looks like to each side. */ struct channel_view view[NUM_SIDES]; + + /* Chain params to check against */ + const struct chainparams *chainparams; }; /* Some requirements are self-specified (eg. my dust limit), others @@ -125,6 +128,7 @@ static inline u16 to_self_delay(const struct channel *channel, enum side side) /** * new_initial_channel: Given initial fees and funding, what is initial state? * @ctx: tal context to allocate return value from. + * @chain_hash: Which blockchain are we talking about? * @funding_txid: The commitment transaction id. * @funding_txout: The commitment transaction output number. * @funding_satoshis: The commitment transaction amount. @@ -142,6 +146,7 @@ static inline u16 to_self_delay(const struct channel *channel, enum side side) * Returns channel, or NULL if malformed. */ struct channel *new_initial_channel(const tal_t *ctx, + const struct bitcoin_blkid *chain_hash, const struct bitcoin_txid *funding_txid, unsigned int funding_txout, u64 funding_satoshis, diff --git a/openingd/openingd.c b/openingd/openingd.c index 0aab661fbe52..4d0b2c3c1749 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -503,6 +503,7 @@ static u8 *funder_channel(struct state *state, bitcoin_txid(funding, &state->funding_txid); state->channel = new_initial_channel(state, + &state->chainparams->genesis_blockhash, &state->funding_txid, state->funding_txout, state->funding_satoshis, @@ -842,6 +843,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) type_to_string(msg, struct channel_id, &id_in)); state->channel = new_initial_channel(state, + &chain_hash, &state->funding_txid, state->funding_txout, state->funding_satoshis, From 0128bc73629e66ea2f7a55f42d1cd8b62cb0a2f0 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 6 Sep 2018 18:50:09 +0200 Subject: [PATCH 03/36] channeld: Use the chainparams to check msatoshi and funding_satoshi --- channeld/full_channel.c | 2 +- lightningd/opening_control.c | 5 +++-- openingd/openingd.c | 7 ++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/channeld/full_channel.c b/channeld/full_channel.c index d5d930c07a33..98a22b5f8e19 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -363,7 +363,7 @@ static enum channel_add_err add_htlc(struct channel *channel, * - for channels with `chain_hash` identifying the Bitcoin blockchain: * - MUST set the four most significant bytes of `amount_msat` to 0. */ - if (htlc->msatoshi & 0xFFFFFFFF00000000ULL) { + if (htlc->msatoshi > channel->chainparams->max_payment_msat) { return CHANNEL_ERR_MAX_HTLC_VALUE_EXCEEDED; } diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index b03b9120d683..42f0b819a25a 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -766,6 +766,7 @@ static void json_fund_channel(struct command *cmd, struct channel *channel; u32 *feerate_per_kw; u8 *msg; + u64 max_funding_satoshi = get_chainparams(cmd->ld)->max_funding_satoshi; fc->cmd = cmd; fc->uc = NULL; @@ -777,7 +778,7 @@ static void json_fund_channel(struct command *cmd, NULL)) return; - if (!json_tok_wtx(&fc->wtx, buffer, sattok, MAX_FUNDING_SATOSHI)) + if (!json_tok_wtx(&fc->wtx, buffer, sattok, max_funding_satoshi)) return; if (!feerate_per_kw) { @@ -820,7 +821,7 @@ static void json_fund_channel(struct command *cmd, BITCOIN_SCRIPTPUBKEY_P2WSH_LEN)) return; - assert(fc->wtx.amount <= MAX_FUNDING_SATOSHI); + assert(fc->wtx.amount <= max_funding_satoshi); peer->uncommitted_channel->fc = tal_steal(peer->uncommitted_channel, fc); fc->uc = peer->uncommitted_channel; diff --git a/openingd/openingd.c b/openingd/openingd.c index 4d0b2c3c1749..424f09cc5cee 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -353,9 +353,10 @@ static u8 *funder_channel(struct state *state, temporary_channel_id(&state->channel_id); - if (state->funding_satoshis > MAX_FUNDING_SATOSHI) + if (state->funding_satoshis > state->chainparams->max_funding_satoshi) status_failed(STATUS_FAIL_MASTER_IO, - "funding_satoshis must be < 2^24, not %"PRIu64, + "funding_satoshis must be < %"PRIu64", not %"PRIu64, + state->chainparams->max_funding_satoshi, state->funding_satoshis); /* BOLT #2: @@ -722,7 +723,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * * The receiving node ... MUST fail the channel if `funding-satoshis` * is greater than or equal to 2^24 */ - if (state->funding_satoshis > MAX_FUNDING_SATOSHI) { + if (state->funding_satoshis > state->chainparams->max_funding_satoshi) { negotiation_failed(state, false, "funding_satoshis %"PRIu64" too large", state->funding_satoshis); From 2d7e603ac11a769880841003d1c7dac3f6490802 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 6 Sep 2018 18:52:11 +0200 Subject: [PATCH 04/36] chainparams: Move the BOLT2 quote to the chainparams where we set it --- bitcoin/chainparams.c | 6 ++++++ wire/peer_wire.h | 8 -------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/bitcoin/chainparams.c b/bitcoin/chainparams.c index be1793facf6a..cb081718ae55 100644 --- a/bitcoin/chainparams.c +++ b/bitcoin/chainparams.c @@ -12,6 +12,12 @@ const struct chainparams networks[] = { .cli = "bitcoin-cli", .cli_args = NULL, .dust_limit = 546, + /* BOLT #2: + * + * The sending node: + *... + * - MUST set `funding_satoshis` to less than 2^24 satoshi. + */ .max_funding_satoshi = (1 << 24) - 1, .max_payment_msat = 0xFFFFFFFFULL, /* "Lightning Charge Powers Developers & Blockstream Store" */ diff --git a/wire/peer_wire.h b/wire/peer_wire.h index e1d68029d256..28dd4cec43cf 100644 --- a/wire/peer_wire.h +++ b/wire/peer_wire.h @@ -29,12 +29,4 @@ bool extract_channel_id(const u8 *in_pkt, struct channel_id *channel_id); * the network, as detailed within [BOLT #7] */ #define CHANNEL_FLAGS_ANNOUNCE_CHANNEL 1 - -/* BOLT #2: - * - * The sending node: - *... - * - MUST set `funding_satoshis` to less than 2^24 satoshi. - */ -#define MAX_FUNDING_SATOSHI ((1 << 24) - 1) #endif /* LIGHTNING_WIRE_PEER_WIRE_H */ From f417dfa0e1f20de45421186dd5920c0ba3d2b16b Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sat, 8 Sep 2018 13:37:29 +0200 Subject: [PATCH 05/36] chainparams: Always retrieve chainparams by the chain_hash --- bitcoin/chainparams.c | 9 --------- bitcoin/chainparams.h | 8 -------- lightningd/opening_control.c | 3 ++- openingd/opening_wire.csv | 4 ++-- openingd/openingd.c | 6 +++--- 5 files changed, 7 insertions(+), 23 deletions(-) diff --git a/bitcoin/chainparams.c b/bitcoin/chainparams.c index cb081718ae55..33a70340a533 100644 --- a/bitcoin/chainparams.c +++ b/bitcoin/chainparams.c @@ -82,15 +82,6 @@ const struct chainparams *chainparams_for_network(const char *network_name) return NULL; } -const struct chainparams *chainparams_by_index(const int index) -{ - if (index >= ARRAY_SIZE(networks) || index < 0) { - return NULL; - } else { - return &networks[index]; - } -} - const struct chainparams *chainparams_by_chainhash(const struct bitcoin_blkid *chain_hash) { for (size_t i = 0; i < ARRAY_SIZE(networks); i++) { diff --git a/bitcoin/chainparams.h b/bitcoin/chainparams.h index 9070a1cd9597..2e9071172eb7 100644 --- a/bitcoin/chainparams.h +++ b/bitcoin/chainparams.h @@ -28,14 +28,6 @@ struct chainparams { */ const struct chainparams *chainparams_for_network(const char *network_name); -/** - * chainparams_by_index - Helper to get a network by its numeric index - * - * We may not want to pass the network name through to subdaemons, so - * we allows lookup by index. - */ -const struct chainparams *chainparams_by_index(const int index); - /** * chainparams_by_bip173 - Helper to get a network by its bip173 name * diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 42f0b819a25a..8d43890843b8 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -730,7 +730,8 @@ void peer_start_openingd(struct peer *peer, */ uc->minimum_depth = peer->ld->config.anchor_confirms; - msg = towire_opening_init(NULL, get_chainparams(peer->ld)->index, + msg = towire_opening_init(NULL, + &get_chainparams(peer->ld)->genesis_blockhash, &uc->our_config, max_to_self_delay, min_effective_htlc_capacity_msat, diff --git a/openingd/opening_wire.csv b/openingd/opening_wire.csv index 51ae367777ba..5a25577c5b35 100644 --- a/openingd/opening_wire.csv +++ b/openingd/opening_wire.csv @@ -3,8 +3,8 @@ #include opening_init,6000 -# Which network are we configured for (as index into the chainparams)? -opening_init,,network_index,u32 +# Which network are we configured for? +opening_init,,chain_hash,struct bitcoin_blkid # Base configuration we'll offer (channel reserve will vary with amount) opening_init,,our_config,struct channel_config # Minimum/maximum configuration values we'll accept diff --git a/openingd/openingd.c b/openingd/openingd.c index 424f09cc5cee..b17e0eea3b89 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -1095,7 +1095,7 @@ int main(int argc, char *argv[]) u8 *msg, *inner; struct pollfd pollfd[3]; struct state *state = tal(NULL, struct state); - u32 network_index; + struct bitcoin_blkid chain_hash; struct secret *none; subdaemon_setup(argc, argv); @@ -1104,7 +1104,7 @@ int main(int argc, char *argv[]) msg = wire_sync_read(tmpctx, REQ_FD); if (!fromwire_opening_init(tmpctx, msg, - &network_index, + &chain_hash, &state->localconf, &state->max_to_self_delay, &state->min_effective_htlc_capacity_msat, @@ -1125,7 +1125,7 @@ int main(int argc, char *argv[]) fail_if_all_error(inner); } - state->chainparams = chainparams_by_index(network_index); + state->chainparams = chainparams_by_chainhash(&chain_hash); /* Initially we're not associated with a channel, but * handle_peer_gossip_or_error wants this. */ memset(&state->channel_id, 0, sizeof(state->channel_id)); From e10cde351655f6aa09a34f656a6afe964d2ea1e0 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sat, 8 Sep 2018 13:39:46 +0200 Subject: [PATCH 06/36] chainparams: Remove index from chainparams We no longer use it to reference chainparams, so we can remove it completely. --- bitcoin/chainparams.c | 15 +++++---------- bitcoin/chainparams.h | 1 - 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/bitcoin/chainparams.c b/bitcoin/chainparams.c index 33a70340a533..ed0716b1cabc 100644 --- a/bitcoin/chainparams.c +++ b/bitcoin/chainparams.c @@ -4,8 +4,7 @@ #include const struct chainparams networks[] = { - {.index = 0, - .network_name = "bitcoin", + {.network_name = "bitcoin", .bip173_name = "bc", .genesis_blockhash = {{{.u.u8 = {0x6f, 0xe2, 0x8c, 0x0a, 0xb6, 0xf1, 0xb3, 0x72, 0xc1, 0xa6, 0xa2, 0x46, 0xae, 0x63, 0xf7, 0x4f, 0x93, 0x1e, 0x83, 0x65, 0xe1, 0x5a, 0x08, 0x9c, 0x68, 0xd6, 0x19, 0x00, 0x00, 0x00, 0x00, 0x00}}}}, .rpc_port = 8332, @@ -23,8 +22,7 @@ const struct chainparams networks[] = { /* "Lightning Charge Powers Developers & Blockstream Store" */ .when_lightning_became_cool = 504500, .testnet = false}, - {.index = 1, - .network_name = "regtest", + {.network_name = "regtest", .bip173_name = "bcrt", .genesis_blockhash = {{{.u.u8 = {0x06, 0x22, 0x6e, 0x46, 0x11, 0x1a, 0x0b, 0x59, 0xca, 0xaf, 0x12, 0x60, 0x43, 0xeb, 0x5b, 0xbf, 0x28, 0xc3, 0x4f, 0x3a, 0x5e, 0x33, 0x2a, 0x1f, 0xc7, 0xb2, 0xb7, 0x3c, 0xf1, 0x88, 0x91, 0x0f}}}}, .rpc_port = 18332, @@ -35,8 +33,7 @@ const struct chainparams networks[] = { .max_payment_msat = 0xFFFFFFFFULL, .when_lightning_became_cool = 1, .testnet = true}, - {.index = 2, - .network_name = "testnet", + {.network_name = "testnet", .bip173_name = "tb", .genesis_blockhash = {{{.u.u8 = {0x43, 0x49, 0x7f, 0xd7, 0xf8, 0x26, 0x95, 0x71, 0x08, 0xf4, 0xa3, 0x0f, 0xd9, 0xce, 0xc3, 0xae, 0xba, 0x79, 0x97, 0x20, 0x84, 0xe9, 0x0e, 0xad, 0x01, 0xea, 0x33, 0x09, 0x00, 0x00, 0x00, 0x00}}}}, .rpc_port = 18332, @@ -46,8 +43,7 @@ const struct chainparams networks[] = { .max_funding_satoshi = (1 << 24) - 1, .max_payment_msat = 0xFFFFFFFFULL, .testnet = true}, - {.index = 3, - .network_name = "litecoin", + {.network_name = "litecoin", .bip173_name = "ltc", .genesis_blockhash = {{{.u.u8 = {0xe2, 0xbf, 0x04, 0x7e, 0x7e, 0x5a, 0x19, 0x1a, 0xa4, 0xef, 0x34, 0xd3, 0x14, 0x97, 0x9d, 0xc9, 0x98, 0x6e, 0x0f, 0x19, 0x25, 0x1e, 0xda, 0xba, 0x59, 0x40, 0xfd, 0x1f, 0xe3, 0x65, 0xa7, 0x12 }}}}, .rpc_port = 9332, @@ -58,8 +54,7 @@ const struct chainparams networks[] = { .max_payment_msat = 60 * 0xFFFFFFFFULL, .when_lightning_became_cool = 1320000, .testnet = false}, - {.index = 4, - .network_name = "litecoin-testnet", + {.network_name = "litecoin-testnet", .bip173_name = "tltc", .genesis_blockhash = {{{.u.u8 = { 0xa0, 0x29, 0x3e, 0x4e, 0xeb, 0x3d, 0xa6, 0xe6, 0xf5, 0x6f, 0x81, 0xed, 0x59, 0x5f, 0x57, 0x88, 0x0d, 0x1a, 0x21, 0x56, 0x9e, 0x13, 0xee, 0xfd, 0xd9, 0x51, 0x28, 0x4b, 0x5a, 0x62, 0x66, 0x49 }}}}, .rpc_port = 19332, diff --git a/bitcoin/chainparams.h b/bitcoin/chainparams.h index 2e9071172eb7..44e762101747 100644 --- a/bitcoin/chainparams.h +++ b/bitcoin/chainparams.h @@ -7,7 +7,6 @@ #include struct chainparams { - const int index; const char *network_name; const char *bip173_name; const struct bitcoin_blkid genesis_blockhash; From dc88c35d7f599dcd8a67958d3f77409336fe0222 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sat, 8 Sep 2018 13:44:49 +0200 Subject: [PATCH 07/36] channeld: Do not fail if we get a chain_hash we don't know --- common/initial_channel.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/initial_channel.c b/common/initial_channel.c index 94a93e977074..ad65ec591c9f 100644 --- a/common/initial_channel.c +++ b/common/initial_channel.c @@ -61,7 +61,8 @@ struct channel *new_initial_channel(const tal_t *ctx, = commit_number_obscurer(&channel->basepoints[funder].payment, &channel->basepoints[!funder].payment); channel->chainparams = chainparams_by_chainhash(chain_hash); - assert(channel->chainparams != NULL); + if (channel->chainparams == NULL) + return tal_free(channel); return channel; } From d9ea2e6b454ee564bb44738e69d023ab4d3cb834 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Mon, 3 Sep 2018 15:44:15 +0200 Subject: [PATCH 08/36] master: Move JSON-RPC setup below PID-file creation If we run two daemons on the same directory we'd be getting the failure from trying to listen to the same file before we'd hit the pid-file error, which was causing confusion. --- lightningd/lightningd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 6d09f1e97e4f..6a02d2b701da 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -664,10 +664,6 @@ int main(int argc, char *argv[]) setup_topology(ld->topology, &ld->timers, min_blockheight, max_blockheight); - /*~ Create RPC socket: now lightning-cli can send us JSON RPC commands - * over a UNIX domain socket specified by `ld->rpc_filename`. */ - setup_jsonrpc(ld, ld->rpc_filename); - /*~ We defer --daemon until we've completed most initialization: that * way we'll exit with an error rather than silently exiting 0, then * realizing we can't start and forcing the confused user to read the @@ -679,6 +675,10 @@ int main(int argc, char *argv[]) * changes our pid! */ pidfile_create(ld); + /*~ Create RPC socket: now lightning-cli can send us JSON RPC commands + * over a UNIX domain socket specified by `ld->rpc_filename`. */ + setup_jsonrpc(ld, ld->rpc_filename); + /*~ Activate connect daemon. Needs to be after the initialization of * chaintopology, otherwise peers may connect and ask for * uninitialized data. */ From b861e44f360b605b7f69558d2300b82306a5f9f3 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sun, 9 Sep 2018 22:33:57 +0200 Subject: [PATCH 09/36] docker: Add missing dependencies to builder for `a2x` --- contrib/Dockerfile.builder | 2 ++ contrib/Dockerfile.builder.i386 | 2 ++ 2 files changed, 4 insertions(+) diff --git a/contrib/Dockerfile.builder b/contrib/Dockerfile.builder index 737088071681..5d4a0d6c7317 100644 --- a/contrib/Dockerfile.builder +++ b/contrib/Dockerfile.builder @@ -13,6 +13,7 @@ RUN apt-get -qq update && \ automake \ clang \ cppcheck \ + docbook-xml \ shellcheck \ eatmydata \ software-properties-common \ @@ -34,6 +35,7 @@ RUN apt-get -qq update && \ shellcheck \ libxml2-utils \ wget \ + xsltproc \ zlib1g-dev && \ rm -rf /var/lib/apt/lists/* diff --git a/contrib/Dockerfile.builder.i386 b/contrib/Dockerfile.builder.i386 index 478c00259b3d..9284253534ba 100644 --- a/contrib/Dockerfile.builder.i386 +++ b/contrib/Dockerfile.builder.i386 @@ -13,6 +13,7 @@ RUN apt-get -qq update && \ automake \ clang \ cppcheck \ + docbook-xml \ shellcheck \ eatmydata \ software-properties-common \ @@ -34,6 +35,7 @@ RUN apt-get -qq update && \ shellcheck \ libxml2-utils \ wget \ + xsltproc \ zlib1g-dev && \ rm -rf /var/lib/apt/lists/* From 79da1b9aa259f21b293cb954c3edfd4cf9939dfc Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 11 Sep 2018 22:31:31 +0200 Subject: [PATCH 10/36] pytest: Keep the test directory even if the failure is in the fixtures --- tests/fixtures.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 85cfcb1d24dc..a5a6bac79d98 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -50,12 +50,13 @@ def directory(request, test_base_dir, test_name): # Auto set value if it isn't in the dict yet __attempts[test_name] = __attempts.get(test_name, 0) + 1 directory = os.path.join(test_base_dir, "{}_{}".format(test_name, __attempts[test_name])) + request.node.has_errors = False yield directory # This uses the status set in conftest.pytest_runtest_makereport to # determine whether we succeeded or failed. - if request.node.rep_call.outcome == 'passed': + if not request.node.has_errors and request.node.rep_call.outcome == 'passed': shutil.rmtree(directory) else: logging.debug("Test execution failed, leaving the test directory {} intact.".format(directory)) @@ -100,36 +101,42 @@ def bitcoind(directory): @pytest.fixture -def node_factory(directory, test_name, bitcoind, executor): +def node_factory(request, directory, test_name, bitcoind, executor): nf = NodeFactory(test_name, bitcoind, executor, directory=directory) yield nf err_count = 0 ok = nf.killall([not n.may_fail for n in nf.nodes]) + + def check_errors(request, err_count, msg): + """Just a simple helper to format a message, set flags on request and then raise + """ + if err_count: + request.node.has_errors = True + raise ValueError(msg.format(err_count)) + if VALGRIND: for node in nf.nodes: err_count += printValgrindErrors(node) - if err_count: - raise ValueError("{} nodes reported valgrind errors".format(err_count)) + check_errors(request, err_count, "{} nodes reported valgrind errors") for node in nf.nodes: err_count += printCrashLog(node) - if err_count: - raise ValueError("{} nodes had crash.log files".format(err_count)) + check_errors(request, err_count, "{} nodes had crash.log files") + for node in nf.nodes: err_count += checkReconnect(node) - if err_count: - raise ValueError("{} nodes had unexpected reconnections".format(err_count)) + check_errors(request, err_count, "{} nodes had unexpected reconnections") + for node in nf.nodes: err_count += checkBadGossipOrder(node) - if err_count: - raise ValueError("{} nodes had bad gossip order".format(err_count)) + check_errors(request, err_count, "{} nodes had bad gossip order") for node in nf.nodes: err_count += checkBadReestablish(node) - if err_count: - raise ValueError("{} nodes had bad reestablish".format(err_count)) + check_errors(request, err_count, "{} nodes had bad reestablish") if not ok: + request.node.has_errors = True raise Exception("At least one lightning exited with unexpected non-zero return code") From f1e931f7bb9aa98b217a012ffba9c180f4f86689 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 12 Sep 2018 01:12:19 +0200 Subject: [PATCH 11/36] pytest: Fix flaky test_logging File was rotated away but didn't wait for the first line to be actually written. --- tests/test_misc.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_misc.py b/tests/test_misc.py index ed377055ba8c..a61d0c879e4b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -931,10 +931,12 @@ def test_logging(node_factory): wait_for(lambda: os.path.exists(logpath)) log1 = open(logpath_moved).readlines() - log2 = open(logpath).readlines() - assert log1[-1].endswith("Ending log due to SIGHUP\n") - assert log2[0].endswith("Started log due to SIGHUP\n") + + def check_new_log(): + log2 = open(logpath).readlines() + return len(log2) > 1 and log2[0].endswith("Started log due to SIGHUP\n") + wait_for(check_new_log) @unittest.skipIf(VALGRIND and not DEVELOPER, From bdb841644660e4ea669c9d34380775492ca2660b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Sep 2018 10:04:54 +0930 Subject: [PATCH 12/36] lightningd: split pidfile handling. We want to try it before --daemon, in case we error, but we don't know the pid yet, so we split into 'lock' and 'write'. Signed-off-by: Rusty Russell --- lightningd/lightningd.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 6a02d2b701da..fc9c74645222 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -493,9 +493,8 @@ static void daemonize_but_keep_dir(struct lightningd *ld) * file-lock a pidfile. This not only prevents accidentally running multiple * daemons on the same database at once, but lets nosy sysadmins see what pid * the currently-running daemon is supposed to be. */ -static void pidfile_create(const struct lightningd *ld) +static int pidfile_create(const struct lightningd *ld) { - char *pid; int pid_fd; /* Create PID file */ @@ -508,6 +507,18 @@ static void pidfile_create(const struct lightningd *ld) /* Problem locking file */ err(1, "lightningd already running? Error locking PID file"); + /*~ As closing the file will remove the lock, we need to keep it open; + * the OS will close it implicitly when we exit for any reason. */ + return pid_fd; +} + +/*~ Writing the pid into the lockfile provides a useful clue to users as to + * what created it; however, we can't do that until we've got a stable process + * id, and if --daemon is specified, that's quite late. */ +static void pidfile_write(const struct lightningd *ld, int pid_fd) +{ + char *pid; + /*~ Note that tal_fmt() is what asprintf() dreams of being. */ pid = tal_fmt(tmpctx, "%d\n", getpid()); /*~ CCAN's write_all writes to a file descriptor, looping if necessary @@ -516,9 +527,6 @@ static void pidfile_create(const struct lightningd *ld) * which write() is when FORTIFY_SOURCE is defined, so we're allowed * to ignore the result without jumping through hoops. */ write_all(pid_fd, pid, strlen(pid)); - - /*~ As closing the file will remove the lock, we need to keep it open; - * the OS will close it implicitly when we exit for any reason. */ } /*~ ccan/io allows overriding the poll() function that is the very core @@ -550,7 +558,7 @@ int main(int argc, char *argv[]) { struct lightningd *ld; u32 min_blockheight, max_blockheight; - int connectd_gossipd_fd; + int connectd_gossipd_fd, pid_fd; /*~ What happens in strange locales should stay there. */ setup_locale(); @@ -664,6 +672,14 @@ int main(int argc, char *argv[]) setup_topology(ld->topology, &ld->timers, min_blockheight, max_blockheight); + /*~ Now create the PID file: this errors out if there's already a + * daemon running, so we call before trying to create an RPC socket. */ + pid_fd = pidfile_create(ld); + + /*~ Create RPC socket: now lightning-cli can send us JSON RPC commands + * over a UNIX domain socket specified by `ld->rpc_filename`. */ + setup_jsonrpc(ld, ld->rpc_filename); + /*~ We defer --daemon until we've completed most initialization: that * way we'll exit with an error rather than silently exiting 0, then * realizing we can't start and forcing the confused user to read the @@ -671,13 +687,8 @@ int main(int argc, char *argv[]) if (ld->daemon) daemonize_but_keep_dir(ld); - /*~ Now create the PID file: this has to be after daemonize, since that - * changes our pid! */ - pidfile_create(ld); - - /*~ Create RPC socket: now lightning-cli can send us JSON RPC commands - * over a UNIX domain socket specified by `ld->rpc_filename`. */ - setup_jsonrpc(ld, ld->rpc_filename); + /*~ We have to do this after daemonize, since that changes our pid! */ + pidfile_write(ld, pid_fd); /*~ Activate connect daemon. Needs to be after the initialization of * chaintopology, otherwise peers may connect and ask for From 4a1bc0f90c16c850ba5ae7b688a3d4d080d666e3 Mon Sep 17 00:00:00 2001 From: alan8325 Date: Mon, 10 Sep 2018 23:19:13 -0700 Subject: [PATCH 13/36] Add files via upload --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 12b7b548a1b3..2cbe86433a41 100644 --- a/README.md +++ b/README.md @@ -213,6 +213,7 @@ that `state` is `CHANNELD_NORMAL`; after 6 confirmations you can use * `FUNDING_SPEND_SEEN` means we've seen the funding transaction spent. * `ONCHAIN` means that the `lightning_onchaind` is tracking the onchain closing of the channel. +* `AWAITING_UNILATERAL` means that we're waiting for a unilateral close to hit the blockchain. All these states have more information about what's going on in the `status` field in `listpeers`. From 704d30edce04c7f46ba982e2433d8b4b6610a5a6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 12 Sep 2018 05:27:11 +0930 Subject: [PATCH 14/36] ping: complete JSON RPC ping commands even if one ping gets no response. We would never complete further ping commands if we had < responses than pings. Oops. Fixes: #1928 Signed-off-by: Rusty Russell --- gossipd/gossip_wire.csv | 1 + gossipd/gossipd.c | 8 +++-- lightningd/Makefile | 2 +- lightningd/gossip_control.c | 6 +++- lightningd/lightningd.c | 1 + lightningd/lightningd.h | 2 ++ lightningd/ping.c | 63 +++++++++++++++++++++++++++++++------ lightningd/ping.h | 9 ++++++ 8 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 lightningd/ping.h diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 5e2abc5a43b8..45e11c494c4f 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -52,6 +52,7 @@ gossip_ping,,num_pong_bytes,u16 gossip_ping,,len,u16 gossip_ping_reply,3108 +gossip_ping_reply,,id,struct pubkey # False if id in gossip_ping was unknown. gossip_ping_reply,,sent,bool # 0 == no pong expected diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index e2dcfcad0a1b..45c00be6a4c9 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -648,7 +648,7 @@ static void handle_pong(struct peer *peer, const u8 *pong) } daemon_conn_send(&peer->daemon->master, - take(towire_gossip_ping_reply(NULL, true, + take(towire_gossip_ping_reply(NULL, &peer->id, true, tal_count(pong)))); } @@ -1557,7 +1557,8 @@ static struct io_plan *ping_req(struct io_conn *conn, struct daemon *daemon, peer = find_peer(daemon, &id); if (!peer) { daemon_conn_send(&daemon->master, - take(towire_gossip_ping_reply(NULL, false, 0))); + take(towire_gossip_ping_reply(NULL, &id, + false, 0))); goto out; } @@ -1581,7 +1582,8 @@ static struct io_plan *ping_req(struct io_conn *conn, struct daemon *daemon, */ if (num_pong_bytes >= 65532) daemon_conn_send(&daemon->master, - take(towire_gossip_ping_reply(NULL, true, 0))); + take(towire_gossip_ping_reply(NULL, &id, + true, 0))); else peer->num_pings_outstanding++; diff --git a/lightningd/Makefile b/lightningd/Makefile index 540c2cf3748c..ce554d7760d5 100644 --- a/lightningd/Makefile +++ b/lightningd/Makefile @@ -75,12 +75,12 @@ LIGHTNINGD_SRC := \ lightningd/payalgo.c \ lightningd/peer_control.c \ lightningd/peer_htlcs.c \ + lightningd/ping.c \ lightningd/subd.c \ lightningd/watch.c # Source files without corresponding headers LIGHTNINGD_SRC_NOHDR := \ - lightningd/ping.c \ lightningd/memdump.c LIGHTNINGD_OBJS := $(LIGHTNINGD_SRC:.c=.o) $(LIGHTNINGD_SRC_NOHDR:.c=.o) diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index f75de7001807..83357b59e71a 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -121,7 +122,6 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_GETNODES_REPLY: case WIRE_GOSSIP_GETROUTE_REPLY: case WIRE_GOSSIP_GETCHANNELS_REPLY: - case WIRE_GOSSIP_PING_REPLY: case WIRE_GOSSIP_SCIDS_REPLY: case WIRE_GOSSIP_QUERY_CHANNEL_RANGE_REPLY: case WIRE_GOSSIP_RESOLVE_CHANNEL_REPLY: @@ -131,6 +131,10 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_LOCAL_CHANNEL_CLOSE: break; + case WIRE_GOSSIP_PING_REPLY: + ping_reply(gossip, msg); + break; + case WIRE_GOSSIP_GET_TXOUT: get_txout(gossip, msg); break; diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index fc9c74645222..336ce6e77785 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -176,6 +176,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) list_head_init(&ld->waitsendpay_commands); list_head_init(&ld->sendpay_commands); list_head_init(&ld->close_commands); + list_head_init(&ld->ping_commands); /*~ Tal also explicitly supports arrays: it stores the number of * elements, which can be accessed with tal_count() (or tal_bytelen() diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index cdd90b188562..bfc2b58a35b1 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -162,6 +162,8 @@ struct lightningd { struct list_head sendpay_commands; /* Outstanding close commands. */ struct list_head close_commands; + /* Outstanding ping commands. */ + struct list_head ping_commands; /* Maintained by invoices.c */ struct invoices *invoices; diff --git a/lightningd/ping.c b/lightningd/ping.c index c533e96b096b..23cb4f251a87 100644 --- a/lightningd/ping.c +++ b/lightningd/ping.c @@ -10,28 +10,71 @@ #include #include #include +#include #include -static void ping_reply(struct subd *subd, const u8 *msg, const int *fds UNUSED, - struct command *cmd) +struct ping_command { + struct list_node list; + struct pubkey id; + struct command *cmd; +}; + +static struct ping_command *find_ping_cmd(struct lightningd *ld, + const struct pubkey *id) +{ + struct ping_command *i; + + list_for_each(&ld->ping_commands, i, list) { + if (pubkey_eq(id, &i->id)) + return i; + } + return NULL; +} + +static void destroy_ping_command(struct ping_command *pc) +{ + list_del(&pc->list); +} + +static struct ping_command *new_ping_command(const tal_t *ctx, + struct lightningd *ld, + const struct pubkey *peer_id, + struct command *cmd) +{ + struct ping_command *pc = tal(ctx, struct ping_command); + + pc->id = *peer_id; + pc->cmd = cmd; + list_add_tail(&ld->ping_commands, &pc->list); + tal_add_destructor(pc, destroy_ping_command); + + return pc; +} + +void ping_reply(struct subd *subd, const u8 *msg) { u16 totlen; bool ok, sent = true; + struct pubkey id; + struct ping_command *pc; log_debug(subd->ld->log, "Got ping reply!"); - ok = fromwire_gossip_ping_reply(msg, &sent, &totlen); + ok = fromwire_gossip_ping_reply(msg, &id, &sent, &totlen); + + pc = find_ping_cmd(subd->ld, &id); + assert(pc); if (!ok) - command_fail(cmd, LIGHTNINGD, "Bad reply message"); + command_fail(pc->cmd, LIGHTNINGD, "Bad reply message"); else if (!sent) - command_fail(cmd, LIGHTNINGD, "Unknown peer"); + command_fail(pc->cmd, LIGHTNINGD, "Unknown peer"); else { - struct json_result *response = new_json_result(cmd); + struct json_result *response = new_json_result(pc->cmd); json_object_start(response, NULL); json_add_num(response, "totlen", totlen); json_object_end(response); - command_success(cmd, response); + command_success(pc->cmd, response); } } @@ -76,10 +119,12 @@ static void json_ping(struct command *cmd, return; } + /* parent is cmd, so when we complete cmd, we free this. */ + new_ping_command(cmd, cmd->ld, id, cmd); + /* gossipd handles all pinging, even if it's in another daemon. */ msg = towire_gossip_ping(NULL, id, *pongbytes, *len); - subd_req(cmd->ld->gossip, cmd->ld->gossip, - take(msg), -1, 0, ping_reply, cmd); + subd_send_msg(cmd->ld->gossip, take(msg)); command_still_pending(cmd); } diff --git a/lightningd/ping.h b/lightningd/ping.h new file mode 100644 index 000000000000..fec5c90ee242 --- /dev/null +++ b/lightningd/ping.h @@ -0,0 +1,9 @@ +#ifndef LIGHTNING_LIGHTNINGD_PING_H +#define LIGHTNING_LIGHTNINGD_PING_H +#include "config.h" +#include + +struct subd; +void ping_reply(struct subd *subd, const u8 *msg); + +#endif /* LIGHTNING_LIGHTNINGD_PING_H */ From c2e56fbb1b2698619de8cb0e989247fcaf88220f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 12 Sep 2018 05:51:48 +0930 Subject: [PATCH 15/36] wallet: fix Makefile to include correct dependencies. It didn't depend on its own headers, it should also depend on lightningd/lightningd.h. Signed-off-by: Rusty Russell --- wallet/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/wallet/Makefile b/wallet/Makefile index 4c82a78db2e4..6f5c9e28e9e0 100644 --- a/wallet/Makefile +++ b/wallet/Makefile @@ -15,7 +15,9 @@ WALLET_LIB_OBJS := $(WALLET_LIB_SRC:.c=.o) WALLET_LIB_HEADERS := $(WALLET_LIB_SRC:.c=.h) # Make sure these depend on everything. -ALL_OBJS += $(LIGHTNINGD_OBJS) +ALL_OBJS += $(WALLET_LIB_OBJS) + +$(WALLET_LIB_OBJS): $(LIGHTNINGD_HEADERS_NOGEN) $(WALLET_LIB_HEADERS) check-whitespace: $(WALLET_LIB_SRC:%=check-whitespace/%) $(WALLET_LIB_HEADERS:%=check-whitespace/%) check-source: $(WALLET_LIB_SRC:%=check-src-include-order/%) From 30f129252d29f4ec7c9ed96ecf178a8f2322a6ec Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 12 Sep 2018 05:52:49 +0930 Subject: [PATCH 16/36] wallet: include Makefile from lightningd/Makefile so that lightning headers defined. Signed-off-by: Rusty Russell --- Makefile | 1 - lightningd/Makefile | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f2324f8d2eb2..48b900822d38 100644 --- a/Makefile +++ b/Makefile @@ -186,7 +186,6 @@ include external/Makefile include bitcoin/Makefile include common/Makefile include wire/Makefile -include wallet/Makefile include hsmd/Makefile include gossipd/Makefile include openingd/Makefile diff --git a/lightningd/Makefile b/lightningd/Makefile index ce554d7760d5..918b289f3faa 100644 --- a/lightningd/Makefile +++ b/lightningd/Makefile @@ -100,6 +100,8 @@ LIGHTNINGD_HEADERS_GEN = \ ALL_GEN_HEADERS += $(LIGHTNINGD_HEADERS_GEN) +include wallet/Makefile + # All together in one convenient var LIGHTNINGD_HEADERS = $(LIGHTNINGD_HEADERS_NOGEN) $(LIGHTNINGD_HEADERS_GEN) $(EXTERNAL_HEADERS) $(WIRE_HEADERS) $(BITCOIN_HEADERS) $(COMMON_HEADERS) $(WALLET_LIB_HEADERS) $(LIGHTNINGD_HSM_CLIENT_HEADERS) From bcbcf2f0aea89ed702e232d21ce0555707b64250 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 12 Sep 2018 05:53:31 +0930 Subject: [PATCH 17/36] lightningd: fix Makefile to remove cruft. Everything depends on common headers etc, and the HSM_CLIENT_HEADERS was removed quite a while ago. Signed-off-by: Rusty Russell --- lightningd/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightningd/Makefile b/lightningd/Makefile index 918b289f3faa..682fe33a724f 100644 --- a/lightningd/Makefile +++ b/lightningd/Makefile @@ -103,7 +103,7 @@ ALL_GEN_HEADERS += $(LIGHTNINGD_HEADERS_GEN) include wallet/Makefile # All together in one convenient var -LIGHTNINGD_HEADERS = $(LIGHTNINGD_HEADERS_NOGEN) $(LIGHTNINGD_HEADERS_GEN) $(EXTERNAL_HEADERS) $(WIRE_HEADERS) $(BITCOIN_HEADERS) $(COMMON_HEADERS) $(WALLET_LIB_HEADERS) $(LIGHTNINGD_HSM_CLIENT_HEADERS) +LIGHTNINGD_HEADERS = $(LIGHTNINGD_HEADERS_NOGEN) $(LIGHTNINGD_HEADERS_GEN) $(WALLET_LIB_HEADERS) $(LIGHTNINGD_OBJS): $(LIGHTNINGD_HEADERS) From 3372228cad14aaaaeb3575d75d38a54e3981eadb Mon Sep 17 00:00:00 2001 From: arowser Date: Thu, 13 Sep 2018 17:58:13 +0800 Subject: [PATCH 18/36] add "io" to -log-level usage --- lightningd/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightningd/log.c b/lightningd/log.c index 1175baa6ce38..ee89e9a4ad14 100644 --- a/lightningd/log.c +++ b/lightningd/log.c @@ -520,7 +520,7 @@ char *arg_log_to_file(const char *arg, struct lightningd *ld) void opt_register_logging(struct lightningd *ld) { opt_register_arg("--log-level", arg_log_level, show_log_level, ld->log, - "log level (debug, info, unusual, broken)"); + "log level (io, debug, info, unusual, broken)"); opt_register_arg("--log-prefix", arg_log_prefix, show_log_prefix, ld->log, "log prefix"); From f505a9418b95386271116c087660c5a1247334b5 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 21 Aug 2018 14:19:20 +0200 Subject: [PATCH 19/36] pytest: Fix lint error --- tests/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index cc2fe9373798..7bbc32b12e84 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -233,7 +233,8 @@ def __getattr__(self, name): # Create a callable to do the actual call proxy = BitcoinProxy(btc_conf_file=self.__btc_conf_file__) - f = lambda *args: proxy._call(name, *args) + def f(*args): + return proxy._call(name, *args) # Make debuggers show rather than > From 0a5c45e8b1263583b9c8db847268d6c398608100 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 21 Aug 2018 23:14:03 +0200 Subject: [PATCH 20/36] docker: Prepare builder to include flask and cherrypy This is in preparation for the next commit. --- contrib/Dockerfile.builder | 2 +- contrib/Dockerfile.builder.i386 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/Dockerfile.builder b/contrib/Dockerfile.builder index 5d4a0d6c7317..76827596671a 100644 --- a/contrib/Dockerfile.builder +++ b/contrib/Dockerfile.builder @@ -51,4 +51,4 @@ RUN cd /tmp/ && \ rm -rf bitcoin.tar.gz /tmp/bitcoin-$BITCOIN_VERSION RUN pip3 install --upgrade pip && \ - python3 -m pip install python-bitcoinlib==0.7.0 pytest==3.0.5 setuptools==36.6.0 pytest-test-groups==1.0.3 flake8==3.5.0 pytest-rerunfailures==3.1 ephemeral-port-reserve==1.1.0 pytest-xdist==1.22.2 flaky==3.4.0 + python3 -m pip install python-bitcoinlib==0.7.0 pytest==3.0.5 setuptools==36.6.0 pytest-test-groups==1.0.3 flake8==3.5.0 pytest-rerunfailures==3.1 ephemeral-port-reserve==1.1.0 pytest-xdist==1.22.2 flaky==3.4.0 CherryPy==17.3.0 Flask==1.0.2 diff --git a/contrib/Dockerfile.builder.i386 b/contrib/Dockerfile.builder.i386 index 9284253534ba..814c50a240e8 100644 --- a/contrib/Dockerfile.builder.i386 +++ b/contrib/Dockerfile.builder.i386 @@ -51,4 +51,4 @@ RUN cd /tmp/ && \ rm -rf bitcoin.tar.gz /tmp/bitcoin-$BITCOIN_VERSION RUN pip3 install --upgrade pip && \ - python3 -m pip install python-bitcoinlib==0.7.0 pytest==3.0.5 setuptools==36.6.0 pytest-test-groups==1.0.3 flake8==3.5.0 pytest-rerunfailures==3.1 ephemeral-port-reserve==1.1.0 pytest-xdist==1.22.2 flaky==3.4.0 + python3 -m pip install python-bitcoinlib==0.7.0 pytest==3.0.5 setuptools==36.6.0 pytest-test-groups==1.0.3 flake8==3.5.0 pytest-rerunfailures==3.1 ephemeral-port-reserve==1.1.0 pytest-xdist==1.22.2 flaky==3.4.0 CherryPy==17.3.0 Flask==1.0.2 From e132dffa0b1eede001d42aa480ce00325fefcf2b Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 21 Aug 2018 22:26:02 +0200 Subject: [PATCH 21/36] pytest: Add an RPC proxy inbetween bitcoind and bitcoin-cli This is a simple reverse proxy that `bitcoin-cli` can talk to when invoked by `lightningd`. It allows us to trace `bitcoin-cli` calls, and intercept calls to mock the replies, better than the current bash-script based method. --- tests/btcproxy.py | 81 ++++++++++++++++++++++++++++++++++++++++++ tests/fixtures.py | 4 +-- tests/requirements.txt | 2 ++ tests/test_misc.py | 2 -- tests/utils.py | 9 +++-- 5 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 tests/btcproxy.py diff --git a/tests/btcproxy.py b/tests/btcproxy.py new file mode 100644 index 000000000000..80d54d7a3a18 --- /dev/null +++ b/tests/btcproxy.py @@ -0,0 +1,81 @@ +""" A bitcoind proxy that allows instrumentation and canned responses +""" +from flask import Flask, request +from bitcoin.rpc import JSONRPCError +from bitcoin.rpc import RawProxy as BitcoinProxy +from utils import BitcoinD +from cheroot.wsgi import Server +from cheroot.wsgi import PathInfoDispatcher + +import decimal +import json +import logging +import os +import threading + + +class DecimalEncoder(json.JSONEncoder): + """By default json.dumps does not handle Decimals correctly, so we override it's handling + """ + def default(self, o): + if isinstance(o, decimal.Decimal): + return str(o) + return super(DecimalEncoder, self).default(o) + + +class ProxiedBitcoinD(BitcoinD): + def __init__(self, bitcoin_dir, proxyport=0): + BitcoinD.__init__(self, bitcoin_dir, rpcport=None) + self.app = Flask("BitcoindProxy") + self.app.add_url_rule("/", "API entrypoint", self.proxy, methods=['POST']) + self.proxyport = proxyport + + def proxy(self): + r = json.loads(request.data.decode('ASCII')) + conf_file = os.path.join(self.bitcoin_dir, 'bitcoin.conf') + brpc = BitcoinProxy(btc_conf_file=conf_file) + + try: + reply = { + "result": brpc._call(r['method'], *r['params']), + "error": None, + "id": r['id'] + } + except JSONRPCError as e: + reply = { + "error": e.error, + "id": r['id'] + } + return json.dumps(reply, cls=DecimalEncoder) + + def start(self): + d = PathInfoDispatcher({'/': self.app}) + self.server = Server(('0.0.0.0', self.proxyport), d) + self.proxy_thread = threading.Thread(target=self.server.start) + self.proxy_thread.daemon = True + self.proxy_thread.start() + BitcoinD.start(self) + + # Now that bitcoind is running on the real rpcport, let's tell all + # future callers to talk to the proxyport. We use the bind_addr as a + # signal that the port is bound and accepting connections. + while self.server.bind_addr[1] == 0: + pass + self.proxiedport = self.rpcport + self.rpcport = self.server.bind_addr[1] + logging.debug("bitcoind reverse proxy listening on {}, forwarding to {}".format( + self.rpcport, self.proxiedport + )) + + def stop(self): + BitcoinD.stop(self) + self.server.stop() + self.proxy_thread.join() + + +# The main entrypoint is mainly used to test the proxy. It is not used during +# lightningd testing. +if __name__ == "__main__": + p = ProxiedBitcoinD(bitcoin_dir='/tmp/bitcoind-test/', proxyport=5000) + p.start() + p.proxy_thread.join() diff --git a/tests/fixtures.py b/tests/fixtures.py index a5a6bac79d98..bd900da2ffe0 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -1,4 +1,5 @@ from concurrent import futures +from btcproxy import ProxiedBitcoinD from utils import NodeFactory import logging @@ -8,7 +9,6 @@ import shutil import sys import tempfile -import utils with open('config.vars') as configfile: @@ -69,7 +69,7 @@ def test_name(request): @pytest.fixture def bitcoind(directory): - bitcoind = utils.BitcoinD(bitcoin_dir=directory, rpcport=None) + bitcoind = ProxiedBitcoinD(bitcoin_dir=directory) try: bitcoind.start() except Exception: diff --git a/tests/requirements.txt b/tests/requirements.txt index b0193f03a282..818937667dfe 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -3,3 +3,5 @@ ephemeral-port-reserve==1.1.0 pytest-forked==0.2 pytest-xdist==1.22.2 flaky==3.4.0 +CherryPy==17.3.0 +Flask==1.0.2 diff --git a/tests/test_misc.py b/tests/test_misc.py index a61d0c879e4b..c067930e8fc2 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -570,8 +570,6 @@ def test_listconfigs(node_factory, bitcoind): configs = l1.rpc.listconfigs() # See utils.py - assert configs['bitcoin-datadir'] == bitcoind.bitcoin_dir - assert configs['lightning-dir'] == l1.daemon.lightning_dir assert configs['allow-deprecated-apis'] is False assert configs['network'] == 'regtest' assert configs['ignore-fee-limits'] is False diff --git a/tests/utils.py b/tests/utils.py index 7bbc32b12e84..8bc383f85686 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -287,7 +287,7 @@ def generate_block(self, numblocks=1): class LightningD(TailableProc): - def __init__(self, lightning_dir, bitcoin_dir, port=9735, random_hsm=False, node_id=0): + def __init__(self, lightning_dir, bitcoin_dir, port=9735, random_hsm=False, node_id=0, bitcoin_rpcport=18332): TailableProc.__init__(self, lightning_dir) self.lightning_dir = lightning_dir self.port = port @@ -296,12 +296,14 @@ def __init__(self, lightning_dir, bitcoin_dir, port=9735, random_hsm=False, node self.opts = LIGHTNINGD_CONFIG.copy() opts = { - 'bitcoin-datadir': bitcoin_dir, 'lightning-dir': lightning_dir, 'addr': '127.0.0.1:{}'.format(port), 'allow-deprecated-apis': 'false', 'network': 'regtest', 'ignore-fee-limits': 'false', + 'bitcoin-rpcport': bitcoin_rpcport, + 'bitcoin-rpcuser': BITCOIND_CONFIG['rpcuser'], + 'bitcoin-rpcpassword': BITCOIND_CONFIG['rpcpassword'], } for k, v in opts.items(): @@ -689,7 +691,8 @@ def get_node(self, disconnect=None, options=None, may_fail=False, may_reconnect= socket_path = os.path.join(lightning_dir, "lightning-rpc").format(node_id) daemon = LightningD( lightning_dir, self.bitcoind.bitcoin_dir, - port=port, random_hsm=random_hsm, node_id=node_id + port=port, random_hsm=random_hsm, node_id=node_id, + bitcoin_rpcport=self.bitcoind.rpcport ) # If we have a disconnect string, dump it to a file for daemon. if disconnect: From 88186020e0b0eacd7a1bdf4c9fe976fa7f106954 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 29 Aug 2018 19:17:56 +0200 Subject: [PATCH 22/36] pytest: Implement method mocking for ProxiedBitcoinRpc --- tests/btcproxy.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/btcproxy.py b/tests/btcproxy.py index 80d54d7a3a18..7b33f4e4b132 100644 --- a/tests/btcproxy.py +++ b/tests/btcproxy.py @@ -29,11 +29,20 @@ def __init__(self, bitcoin_dir, proxyport=0): self.app = Flask("BitcoindProxy") self.app.add_url_rule("/", "API entrypoint", self.proxy, methods=['POST']) self.proxyport = proxyport + self.mocks = {} def proxy(self): r = json.loads(request.data.decode('ASCII')) conf_file = os.path.join(self.bitcoin_dir, 'bitcoin.conf') brpc = BitcoinProxy(btc_conf_file=conf_file) + method = r['method'] + + # If we have set a mock for this method reply with that instead of + # forwarding the request. + if method in self.mocks and type(method) == dict: + return self.mocks[method] + elif method in self.mocks and callable(self.mocks[method]): + return self.mocks[method](r) try: reply = { @@ -72,6 +81,19 @@ def stop(self): self.server.stop() self.proxy_thread.join() + def mock_rpc(self, method, response=None): + """Mock the response to a future RPC call of @method + + The response can either be a dict with the full JSON-RPC response, or a + function that returns such a response. If the response is None the mock + is removed and future calls will be passed through to bitcoind again. + + """ + if response is not None: + self.mocks[method] = response + elif method in self.mocks: + del self.mocks[method] + # The main entrypoint is mainly used to test the proxy. It is not used during # lightningd testing. From 74f228deb8649e734243a60215442349d449b819 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 30 Aug 2018 19:50:42 +0200 Subject: [PATCH 23/36] btcproxy: Unpack batched JSON-RPC calls and issue them separately --- tests/btcproxy.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/btcproxy.py b/tests/btcproxy.py index 7b33f4e4b132..9265a21fcaa7 100644 --- a/tests/btcproxy.py +++ b/tests/btcproxy.py @@ -31,8 +31,7 @@ def __init__(self, bitcoin_dir, proxyport=0): self.proxyport = proxyport self.mocks = {} - def proxy(self): - r = json.loads(request.data.decode('ASCII')) + def _handle_request(self, r): conf_file = os.path.join(self.bitcoin_dir, 'bitcoin.conf') brpc = BitcoinProxy(btc_conf_file=conf_file) method = r['method'] @@ -55,6 +54,16 @@ def proxy(self): "error": e.error, "id": r['id'] } + return reply + + def proxy(self): + r = json.loads(request.data.decode('ASCII')) + + if isinstance(r, list): + reply = [self._handle_request(subreq) for subreq in r] + else: + reply = self._handle_request(subreq) + return json.dumps(reply, cls=DecimalEncoder) def start(self): From 2dabc5af93e0e6d11f47f3286ddcd970367a7bc4 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 30 Aug 2018 21:38:28 +0200 Subject: [PATCH 24/36] pytest: Set correct header in mock bitcoind --- tests/btcproxy.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/btcproxy.py b/tests/btcproxy.py index 9265a21fcaa7..e0416c43fb75 100644 --- a/tests/btcproxy.py +++ b/tests/btcproxy.py @@ -8,6 +8,7 @@ from cheroot.wsgi import PathInfoDispatcher import decimal +import flask import json import logging import os @@ -62,9 +63,11 @@ def proxy(self): if isinstance(r, list): reply = [self._handle_request(subreq) for subreq in r] else: - reply = self._handle_request(subreq) + reply = self._handle_request(r) - return json.dumps(reply, cls=DecimalEncoder) + response = flask.Response(json.dumps(reply, cls=DecimalEncoder)) + response.headers['Content-Type'] = 'application/json' + return response def start(self): d = PathInfoDispatcher({'/': self.app}) From aa80a330f1e10a82ce09c66e38195887c95a2618 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 4 Sep 2018 16:00:09 +0200 Subject: [PATCH 25/36] pytest: Remove auto-proxying in favor of a per-node btc proxy --- tests/btcproxy.py | 31 ++++++++++--------------------- tests/fixtures.py | 5 ++--- tests/utils.py | 16 ++++++++++------ 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/tests/btcproxy.py b/tests/btcproxy.py index e0416c43fb75..30486a22e450 100644 --- a/tests/btcproxy.py +++ b/tests/btcproxy.py @@ -3,7 +3,6 @@ from flask import Flask, request from bitcoin.rpc import JSONRPCError from bitcoin.rpc import RawProxy as BitcoinProxy -from utils import BitcoinD from cheroot.wsgi import Server from cheroot.wsgi import PathInfoDispatcher @@ -24,16 +23,17 @@ def default(self, o): return super(DecimalEncoder, self).default(o) -class ProxiedBitcoinD(BitcoinD): - def __init__(self, bitcoin_dir, proxyport=0): - BitcoinD.__init__(self, bitcoin_dir, rpcport=None) +class BitcoinRpcProxy(object): + def __init__(self, bitcoind, rpcport=0): self.app = Flask("BitcoindProxy") self.app.add_url_rule("/", "API entrypoint", self.proxy, methods=['POST']) - self.proxyport = proxyport + self.rpcport = rpcport self.mocks = {} + self.bitcoind = bitcoind + self.request_count = 0 def _handle_request(self, r): - conf_file = os.path.join(self.bitcoin_dir, 'bitcoin.conf') + conf_file = os.path.join(self.bitcoind.bitcoin_dir, 'bitcoin.conf') brpc = BitcoinProxy(btc_conf_file=conf_file) method = r['method'] @@ -55,6 +55,7 @@ def _handle_request(self, r): "error": e.error, "id": r['id'] } + self.request_count += 1 return reply def proxy(self): @@ -71,27 +72,23 @@ def proxy(self): def start(self): d = PathInfoDispatcher({'/': self.app}) - self.server = Server(('0.0.0.0', self.proxyport), d) + self.server = Server(('0.0.0.0', self.rpcport), d) self.proxy_thread = threading.Thread(target=self.server.start) self.proxy_thread.daemon = True self.proxy_thread.start() - BitcoinD.start(self) # Now that bitcoind is running on the real rpcport, let's tell all # future callers to talk to the proxyport. We use the bind_addr as a # signal that the port is bound and accepting connections. while self.server.bind_addr[1] == 0: pass - self.proxiedport = self.rpcport self.rpcport = self.server.bind_addr[1] - logging.debug("bitcoind reverse proxy listening on {}, forwarding to {}".format( - self.rpcport, self.proxiedport - )) + logging.debug("BitcoinRpcProxy proxying incoming port {} to {}".format(self.rpcport, self.bitcoind.rpcport)) def stop(self): - BitcoinD.stop(self) self.server.stop() self.proxy_thread.join() + logging.debug("BitcoinRpcProxy shut down after processing {} requests".format(self.request_count)) def mock_rpc(self, method, response=None): """Mock the response to a future RPC call of @method @@ -105,11 +102,3 @@ def mock_rpc(self, method, response=None): self.mocks[method] = response elif method in self.mocks: del self.mocks[method] - - -# The main entrypoint is mainly used to test the proxy. It is not used during -# lightningd testing. -if __name__ == "__main__": - p = ProxiedBitcoinD(bitcoin_dir='/tmp/bitcoind-test/', proxyport=5000) - p.start() - p.proxy_thread.join() diff --git a/tests/fixtures.py b/tests/fixtures.py index bd900da2ffe0..a097e249895d 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -1,6 +1,5 @@ from concurrent import futures -from btcproxy import ProxiedBitcoinD -from utils import NodeFactory +from utils import NodeFactory, BitcoinD import logging import os @@ -69,7 +68,7 @@ def test_name(request): @pytest.fixture def bitcoind(directory): - bitcoind = ProxiedBitcoinD(bitcoin_dir=directory) + bitcoind = BitcoinD(bitcoin_dir=directory) try: bitcoind.start() except Exception: diff --git a/tests/utils.py b/tests/utils.py index 8bc383f85686..d2742a5a430b 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -10,12 +10,12 @@ import threading import time +from btcproxy import BitcoinRpcProxy from bitcoin.rpc import RawProxy as BitcoinProxy from decimal import Decimal from ephemeral_port_reserve import reserve from lightning import LightningRpc - BITCOIND_CONFIG = { "regtest": 1, "rpcuser": "rpcuser", @@ -287,13 +287,15 @@ def generate_block(self, numblocks=1): class LightningD(TailableProc): - def __init__(self, lightning_dir, bitcoin_dir, port=9735, random_hsm=False, node_id=0, bitcoin_rpcport=18332): + def __init__(self, lightning_dir, bitcoind, port=9735, random_hsm=False, node_id=0): TailableProc.__init__(self, lightning_dir) self.lightning_dir = lightning_dir self.port = port self.cmd_prefix = [] self.disconnect_file = None + self.rpcproxy = BitcoinRpcProxy(bitcoind) + self.opts = LIGHTNINGD_CONFIG.copy() opts = { 'lightning-dir': lightning_dir, @@ -301,7 +303,6 @@ def __init__(self, lightning_dir, bitcoin_dir, port=9735, random_hsm=False, node 'allow-deprecated-apis': 'false', 'network': 'regtest', 'ignore-fee-limits': 'false', - 'bitcoin-rpcport': bitcoin_rpcport, 'bitcoin-rpcuser': BITCOIND_CONFIG['rpcuser'], 'bitcoin-rpcpassword': BITCOIND_CONFIG['rpcpassword'], } @@ -344,6 +345,9 @@ def cmd_line(self): return self.cmd_prefix + ['lightningd/lightningd'] + opts def start(self): + self.rpcproxy.start() + + self.opts['bitcoin-rpcport'] = self.rpcproxy.rpcport TailableProc.start(self) self.wait_for_log("Server started with public key") logging.info("LightningD started") @@ -355,6 +359,7 @@ def wait(self, timeout=10): not return before the timeout triggers. """ self.proc.wait(timeout) + self.rpcproxy.stop() return self.proc.returncode @@ -690,9 +695,8 @@ def get_node(self, disconnect=None, options=None, may_fail=False, may_reconnect= socket_path = os.path.join(lightning_dir, "lightning-rpc").format(node_id) daemon = LightningD( - lightning_dir, self.bitcoind.bitcoin_dir, - port=port, random_hsm=random_hsm, node_id=node_id, - bitcoin_rpcport=self.bitcoind.rpcport + lightning_dir, self.bitcoind, + port=port, random_hsm=random_hsm, node_id=node_id ) # If we have a disconnect string, dump it to a file for daemon. if disconnect: From 16869e3fe6522f83abfeaa477b1d08523dabda62 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 5 Sep 2018 01:01:50 +0200 Subject: [PATCH 26/36] pytest: Use the bitcoind proxy to mock feerates --- tests/btcproxy.py | 2 +- tests/test_connection.py | 7 +++++-- tests/test_misc.py | 5 +++-- tests/utils.py | 22 ++++++++++++++++++++-- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/tests/btcproxy.py b/tests/btcproxy.py index 30486a22e450..9f7d3d348d58 100644 --- a/tests/btcproxy.py +++ b/tests/btcproxy.py @@ -19,7 +19,7 @@ class DecimalEncoder(json.JSONEncoder): """ def default(self, o): if isinstance(o, decimal.Decimal): - return str(o) + return "{:.8f}".format(float(o)) return super(DecimalEncoder, self).default(o) diff --git a/tests/test_connection.py b/tests/test_connection.py index 5b555866ae9a..6486b72ba6ae 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1108,8 +1108,11 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs dev_fail") def test_no_fee_estimate(node_factory, bitcoind, executor): l1 = node_factory.get_node(start=False) - l1.bitcoind_cmd_override(cmd='estimatesmartfee', - failscript="""echo '{ "errors": [ "Insufficient data or no feerate found" ], "blocks": 0 }'; exit 0""") + + # Fail any fee estimation requests until we allow them further down + l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', { + 'error': {"errors": ["Insufficient data or no feerate found"], "blocks": 0} + }) l1.start() l2 = node_factory.get_node() diff --git a/tests/test_misc.py b/tests/test_misc.py index c067930e8fc2..282d4b522273 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -858,8 +858,9 @@ def test_ipv4_and_ipv6(node_factory): def test_feerates(node_factory): l1 = node_factory.get_node(options={'log-level': 'io'}, start=False) - l1.bitcoind_cmd_override(cmd='estimatesmartfee', - failscript="""echo '{ "errors": [ "Insufficient data or no feerate found" ], "blocks": 0 }'; exit 0""") + l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', { + 'error': {"errors": ["Insufficient data or no feerate found"], "blocks": 0} + }) l1.start() # Query feerates (shouldn't give any!) diff --git a/tests/utils.py b/tests/utils.py index d2742a5a430b..6a9d18a2b946 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -623,8 +623,26 @@ def bitcoind_cmd_remove_override(self, cmd=None): # it on a running daemon may not give expected result! def set_feerates(self, feerates, wait_for_effect=True): # (bitcoind returns bitcoin per kb, so these are * 4) - self.bitcoind_cmd_override("""case "$*" in *2\ CONSERVATIVE*) FEERATE={};; *4\ ECONOMICAL*) FEERATE={};; *100\ ECONOMICAL*) FEERATE={};; *) exit 98;; esac; echo '{{ "feerate": '$(printf 0.%08u $FEERATE)' }}'; exit 0""".format(feerates[0] * 4, feerates[1] * 4, feerates[2] * 4), - 'estimatesmartfee') + + def mock_estimatesmartfee(r): + params = r['params'] + if params == [2, 'CONSERVATIVE']: + feerate = feerates[0] * 4 + elif params == [4, 'ECONOMICAL']: + feerate = feerates[1] * 4 + elif params == [100, 'ECONOMICAL']: + feerate = feerates[2] * 4 + else: + raise ValueError() + return { + 'id': r['id'], + 'error': None, + 'result': { + 'feerate': Decimal(feerate) / 10**8 + }, + } + self.daemon.rpcproxy.mock_rpc('estimatesmartfee', mock_estimatesmartfee) + if wait_for_effect: self.daemon.wait_for_log('Feerate estimate for .* set to') From 9e5d7dacb0de8e8e125c761eeeee0768056a467d Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 5 Sep 2018 10:43:32 +0200 Subject: [PATCH 27/36] pytest: Use the mock bitcoind everywhere --- tests/test_connection.py | 12 ++++++++---- tests/test_misc.py | 13 +++++++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 6486b72ba6ae..4fa257f9ac04 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1088,19 +1088,23 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind): # Let blocks settle. time.sleep(1) - # Prevent funder from broadcasting funding tx. - l1.bitcoind_cmd_override('exit 1') + def mock_sendrawtransaction(r): + return {'error': 'sendrawtransaction disabled'} + + # Prevent funder from broadcasting funding tx (any tx really). + l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', mock_sendrawtransaction) + # Fund the channel. # The process will complete, but funder will be unable # to broadcast and confirm funding tx. l1.rpc.fundchannel(l2.info['id'], 10**6) - # Prevent l1 from timing out bitcoin-cli. - l1.bitcoind_cmd_remove_override() + # Generate blocks until unconfirmed. bitcoind.generate_block(blocks) # fundee will forget channel! l2.daemon.wait_for_log('Forgetting channel: It has been {} blocks'.format(blocks)) + # fundee will also forget and disconnect from peer. assert len(l2.rpc.listpeers(l1.info['id'])['peers']) == 0 diff --git a/tests/test_misc.py b/tests/test_misc.py index 282d4b522273..a52d0cf4e030 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -89,7 +89,12 @@ def test_bitcoin_failure(node_factory, bitcoind): # Make sure we're not failing it between getblockhash and getblock. sync_blockheight(bitcoind, [l1]) - l1.bitcoind_cmd_override('exit 1') + def crash_bitcoincli(r): + return {'error': 'go away'} + + # This is not a JSON-RPC response by purpose + l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', crash_bitcoincli) + l1.daemon.rpcproxy.mock_rpc('getblockhash', crash_bitcoincli) # This should cause both estimatefee and getblockhash fail l1.daemon.wait_for_logs(['estimatesmartfee .* exited with status 1', @@ -100,7 +105,9 @@ def test_bitcoin_failure(node_factory, bitcoind): 'getblockhash .* exited with status 1']) # Restore, then it should recover and get blockheight. - l1.bitcoind_cmd_remove_override() + l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', None) + l1.daemon.rpcproxy.mock_rpc('getblockhash', None) + bitcoind.generate_block(5) sync_blockheight(bitcoind, [l1]) @@ -921,6 +928,8 @@ def test_logging(node_factory): logpath = os.path.join(l1.daemon.lightning_dir, 'logfile') logpath_moved = os.path.join(l1.daemon.lightning_dir, 'logfile_moved') + l1.daemon.rpcproxy.start() + l1.daemon.opts['bitcoin-rpcport'] = l1.daemon.rpcproxy.rpcport TailableProc.start(l1.daemon) wait_for(lambda: os.path.exists(logpath)) From f29f92a5feeae7b81cf3487da2521ca22410716f Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 5 Sep 2018 10:47:39 +0200 Subject: [PATCH 28/36] pytest: Clean up bitcoind_cmd_override, it's no longer used --- tests/utils.py | 35 +++-------------------------------- 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 6a9d18a2b946..38c5493479b1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -4,7 +4,6 @@ import re import shutil import sqlite3 -import stat import string import subprocess import threading @@ -600,25 +599,6 @@ def wait_pay(): # wait for sendpay to comply self.rpc.waitsendpay(rhash) - def bitcoind_cmd_override(self, failscript='exit 1', cmd=None): - # Create and rename, for atomicity. - f = os.path.join(self.daemon.lightning_dir, "bitcoin-cli-fail.tmp") - with open(f, "w") as text_file: - text_file.write(failscript) - os.chmod(f, os.stat(f).st_mode | stat.S_IEXEC) - if cmd: - failfile = "bitcoin-cli-fail-{}".format(cmd) - else: - failfile = "bitcoin-cli-fail" - os.rename(f, os.path.join(self.daemon.lightning_dir, failfile)) - - def bitcoind_cmd_remove_override(self, cmd=None): - if cmd: - failfile = "bitcoin-cli-fail-{}".format(cmd) - else: - failfile = "bitcoin-cli-fail" - os.remove(os.path.join(self.daemon.lightning_dir, failfile)) - # Note: this feeds through the smoother in update_feerate, so changing # it on a running daemon may not give expected result! def set_feerates(self, feerates, wait_for_effect=True): @@ -699,7 +679,9 @@ def get_nodes(self, num_nodes, opts=None): return [j.result() for j in jobs] - def get_node(self, disconnect=None, options=None, may_fail=False, may_reconnect=False, random_hsm=False, feerates=(15000, 7500, 3750), start=True, fake_bitcoin_cli=False, log_all_io=False): + def get_node(self, disconnect=None, options=None, may_fail=False, + may_reconnect=False, random_hsm=False, + feerates=(15000, 7500, 3750), start=True, log_all_io=False): with self.lock: node_id = self.next_id self.next_id += 1 @@ -736,17 +718,6 @@ def get_node(self, disconnect=None, options=None, may_fail=False, may_reconnect= if not may_reconnect: daemon.opts["dev-no-reconnect"] = None - cli = os.path.join(lightning_dir, "fake-bitcoin-cli") - with open(cli, "w") as text_file: - text_file.write('#! /bin/sh\n' - '! [ -x bitcoin-cli-fail ] || exec ./bitcoin-cli-fail "$@"\n' - 'for a in "$@"; do\n' - '\t! [ -x bitcoin-cli-fail-"$a" ] || exec ./bitcoin-cli-fail-"$a" "$@"\n' - 'done\n' - 'exec bitcoin-cli "$@"\n') - os.chmod(cli, os.stat(cli).st_mode | stat.S_IEXEC) - daemon.opts['bitcoin-cli'] = cli - if options is not None: daemon.opts.update(options) From 36eab5de26e203311ceeb65c94ec5beb9c94ff5d Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 5 Sep 2018 10:52:49 +0200 Subject: [PATCH 29/36] pytest: Disable early abort if we run in parallel --- Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 48b900822d38..6eaa29719847 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ ifeq ($(COMPAT),1) COMPAT_CFLAGS=-DCOMPAT_V052=1 -DCOMPAT_V060=1 endif -PYTEST_OPTS := -v -x +PYTEST_OPTS := -v # This is where we add new features as bitcoin adds them. FEATURES := @@ -205,8 +205,14 @@ ifneq ($(TEST_GROUP_COUNT),) PYTEST_OPTS += --test-group=$(TEST_GROUP) --test-group-count=$(TEST_GROUP_COUNT) endif +# If we run the tests in parallel we can speed testing up by a lot, however we +# then don't exit on the first error, since that'd kill the other tester +# processes and result in loads in loads of output. So we only tell py.test to +# abort early if we aren't running in parallel. ifneq ($(PYTEST_PAR),) PYTEST_OPTS += -n=$(PYTEST_PAR) +else +PYTEST_OPTS += -x endif check: From 5f059ef3fea310e4f614274d47487a1818e16493 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Sep 2018 12:25:42 +0930 Subject: [PATCH 30/36] CHANGELOG.md: add Unreleased section at the top. Signed-off-by: Rusty Russell --- CHANGELOG.md | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e35234c1341..39b11ed808c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +### Changed + +### Deprecated + +Note: You should always set `allow-deprecated-apis=false` to test for +changes. + +### Removed + +### Fixed + +### Security + ## [0.6.1] - 2018-09-11: "Principled Opposition To Segwit" This release named by ZmnSCPxj. @@ -51,9 +68,6 @@ This release named by ZmnSCPxj. ### Deprecated -Note: You should always set `allow-deprecated-apis=false` to test for -changes. - ### Removed - JSON API: `listpeers` results no long have `alias` and `color` fields; From 7744c4152185ca2765bc45f008cd4bda929fcaab Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Sep 2018 12:26:54 +0930 Subject: [PATCH 31/36] listpeers: add 'scratch_txid' for the tx we would broadcast if necessary. Signed-off-by: Rusty Russell --- CHANGELOG.md | 2 ++ lightningd/peer_control.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39b11ed808c0..db1a501cbebc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added +- JSON API: `listpeers` has new field `scratch_txid`: the latest tx in channel. + ### Changed ### Deprecated diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index cba4f5130ee5..4dc9fa945b97 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -642,6 +642,12 @@ static void json_add_peer(struct lightningd *ld, json_object_start(response, NULL); json_add_string(response, "state", channel_state_name(channel)); + if (channel->last_tx) { + struct bitcoin_txid txid; + bitcoin_txid(channel->last_tx, &txid); + + json_add_txid(response, "scratch_txid", &txid); + } if (channel->owner) json_add_string(response, "owner", channel->owner->name); From 252bbe1d2d34e4e0f6c07d383b82f30178f21641 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Sep 2018 13:36:07 +0930 Subject: [PATCH 32/36] pytest: don't wait for sendrawtx, wait for expected tx. In particular, test_no_fee_estimate was flaky due to seeing the funding tx being sent. Signed-off-by: Rusty Russell --- tests/test_closing.py | 20 ++++++++++---------- tests/test_connection.py | 18 +++++++++--------- tests/test_misc.py | 2 +- tests/utils.py | 4 ++++ 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/tests/test_closing.py b/tests/test_closing.py index 9eacc727dc30..271b998f68da 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -475,7 +475,7 @@ def test_onchain_unwatch(node_factory, bitcoind): l1.rpc.dev_fail(l2.info['id']) l1.daemon.wait_for_log('Failing due to dev-fail command') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) l1.bitcoin.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') @@ -594,7 +594,7 @@ def test_onchain_dust_out(node_factory, bitcoind, executor): # l1 will drop to chain. l1.daemon.wait_for_log('permfail') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) l1.bitcoin.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') l2.daemon.wait_for_log(' to ONCHAIN') @@ -662,7 +662,7 @@ def test_onchain_timeout(node_factory, bitcoind, executor): # l1 will drop to chain. l1.daemon.wait_for_log('permfail') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) l1.bitcoin.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') l2.daemon.wait_for_log(' to ONCHAIN') @@ -814,7 +814,7 @@ def test_onchain_feechange(node_factory, bitcoind, executor): # l2 will drop to chain. l2.daemon.wait_for_log('permfail') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') l2.daemon.wait_for_log(' to ONCHAIN') @@ -890,7 +890,7 @@ def test_onchain_all_dust(node_factory, bitcoind, executor): # l2 will drop to chain. l2.daemon.wait_for_log('permfail') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) # Make l1's fees really high (and wait for it to exceed 50000) l1.set_feerates((100000, 100000, 100000)) @@ -944,7 +944,7 @@ def test_onchain_different_fees(node_factory, bitcoind, executor): # Drop to chain l1.rpc.dev_fail(l2.info['id']) - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') @@ -999,7 +999,7 @@ def test_permfail_new_commit(node_factory, bitcoind, executor): t = executor.submit(l1.pay, l2, 200000000) l2.daemon.wait_for_log('dev_disconnect permfail') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) bitcoind.generate_block(1) l1.daemon.wait_for_log('Their unilateral tx, new commit point') l1.daemon.wait_for_log(' to ONCHAIN') @@ -1037,7 +1037,7 @@ def test_permfail_htlc_in(node_factory, bitcoind, executor): t = executor.submit(l1.pay, l2, 200000000) l2.daemon.wait_for_log('dev_disconnect permfail') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) bitcoind.generate_block(1) l1.daemon.wait_for_log('Their unilateral tx, old commit point') l1.daemon.wait_for_log(' to ONCHAIN') @@ -1082,7 +1082,7 @@ def test_permfail_htlc_out(node_factory, bitcoind, executor): t = executor.submit(l2.pay, l1, 200000000) l2.daemon.wait_for_log('dev_disconnect permfail') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) bitcoind.generate_block(1) l1.daemon.wait_for_log('Their unilateral tx, old commit point') l1.daemon.wait_for_log(' to ONCHAIN') @@ -1144,7 +1144,7 @@ def test_permfail(node_factory, bitcoind): # We fail l2, so l1 will reconnect to it. l2.rpc.dev_fail(l1.info['id']) l2.daemon.wait_for_log('Failing due to dev-fail command') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) assert l1.bitcoin.rpc.getmempoolinfo()['size'] == 1 diff --git a/tests/test_connection.py b/tests/test_connection.py index 4fa257f9ac04..5c30f8b56c1b 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -783,7 +783,7 @@ def test_channel_persistence(node_factory, bitcoind, executor): # Now make sure l1 is watching for unilateral closes l2.rpc.dev_fail(l1.info['id']) l2.daemon.wait_for_log('Failing due to dev-fail command') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) bitcoind.generate_block(1) # L1 must notice. @@ -836,8 +836,8 @@ def test_update_fee(node_factory, bitcoind): l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') # And should put closing into mempool. - l1.daemon.wait_for_log('sendrawtx exit 0') - l2.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) + l2.wait_for_channel_onchain(l1.info['id']) bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') @@ -925,8 +925,8 @@ def test_update_fee_reconnect(node_factory, bitcoind): l1.rpc.close(chan) # And should put closing into mempool. - l1.daemon.wait_for_log('sendrawtx exit 0') - l2.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) + l2.wait_for_channel_onchain(l1.info['id']) bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') @@ -1150,7 +1150,7 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): l1.daemon.wait_for_log('sendrawtx exit 0') l1.rpc.dev_fail(l2.info['id']) l1.daemon.wait_for_log('Failing due to dev-fail command') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) bitcoind.generate_block(6) wait_for(lambda: only_one(l1.rpc.getpeer(l2.info['id'])['channels'])['state'] == 'ONCHAIN') wait_for(lambda: only_one(l2.rpc.getpeer(l1.info['id'])['channels'])['state'] == 'ONCHAIN') @@ -1172,9 +1172,9 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): l2.pay(l1, 10**9 // 2) l1.rpc.dev_fail(l2.info['id']) l1.daemon.wait_for_log('Failing due to dev-fail command') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) bitcoind.generate_block(5) - l1.daemon.wait_for_log('sendrawtx exit 0') + wait_for(lambda: len(bitcoind.rpc.getrawmempool()) > 0) bitcoind.generate_block(100) # Start estimatesmartfee. @@ -1280,7 +1280,7 @@ def test_dataloss_protection(node_factory, bitcoind): l2.daemon.wait_for_log("Peer permanent failure in CHANNELD_NORMAL: Awaiting unilateral close") # l1 should drop to chain. - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_channel_onchain(l2.info['id']) # l2 must NOT drop to chain. l2.daemon.wait_for_log("Cannot broadcast our commitment tx: they have a future one") diff --git a/tests/test_misc.py b/tests/test_misc.py index a52d0cf4e030..c57f6a44ed43 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -182,7 +182,7 @@ def test_htlc_sig_persistence(node_factory, executor): # This should reload the htlc_sig l2.rpc.dev_fail(l1.info['id']) # Make sure it broadcasts to chain. - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_channel_onchain(l1.info['id']) l2.stop() l1.bitcoin.rpc.generate(1) l1.start() diff --git a/tests/utils.py b/tests/utils.py index 38c5493479b1..565d306e6c72 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -560,6 +560,10 @@ def is_channel_active(self, chanid): active = [(c['short_channel_id'], c['flags']) for c in channels if c['active']] return (chanid, 0) in active and (chanid, 1) in active + def wait_for_channel_onchain(self, peerid): + txid = only_one(only_one(self.rpc.listpeers(peerid)['peers'])['channels'])['scratch_txid'] + wait_for(lambda: txid in self.bitcoin.rpc.getrawmempool()) + def wait_channel_active(self, chanid): wait_for(lambda: self.is_channel_active(chanid), interval=1) From 2cdc5fb96434fb29654d85a1bf4c914046e90022 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Sep 2018 12:00:36 +0930 Subject: [PATCH 33/36] lightningd: make some bitcoind requests high priority. fiatjaf has a cheap VPS, connecting remotely to his home bitcoind node. fiatjaf's latency on bitcoin-cli getblock is between 10 and 37 seconds. fiatjaf's c-lightning node is getting one block per hour. fiatjaf is sad. We single-file our bitcoind requests, because bitcoind has a limited thread pool and it *fails* rather than queueing if you upset it. We probably be fine using separate queues for each command type, but simply allowing some requests to cut in line should prove my theory that we're getting stuck behind gossip verification requests. fiatjaf now gets one block per 2 minutes. fiatjaf is less sad. Signed-off-by: Rusty Russell --- lightningd/bitcoind.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 744b579d92d2..30770392f22d 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -247,6 +247,7 @@ start_bitcoin_cli(struct bitcoind *bitcoind, const tal_t *ctx, bool (*process)(struct bitcoin_cli *), bool nonzero_exit_ok, + bool high_prio, void *cb, void *cb_arg, char *cmd, ...) { @@ -274,7 +275,10 @@ start_bitcoin_cli(struct bitcoind *bitcoind, bcli->args = gather_args(bitcoind, bcli, cmd, ap); va_end(ap); - list_add_tail(&bitcoind->pending, &bcli->list); + if (high_prio) + list_add(&bitcoind->pending, &bcli->list); + else + list_add_tail(&bitcoind->pending, &bcli->list); next_bcli(bitcoind); } @@ -352,7 +356,8 @@ static void do_one_estimatefee(struct bitcoind *bitcoind, char blockstr[STR_MAX_CHARS(u32)]; snprintf(blockstr, sizeof(blockstr), "%u", efee->blocks[efee->i]); - start_bitcoin_cli(bitcoind, NULL, process_estimatefee, false, NULL, efee, + start_bitcoin_cli(bitcoind, NULL, process_estimatefee, false, false, + NULL, efee, "estimatesmartfee", blockstr, efee->estmode[efee->i], NULL); } @@ -398,7 +403,8 @@ void bitcoind_sendrawtx_(struct bitcoind *bitcoind, void *arg) { log_debug(bitcoind->log, "sendrawtransaction: %s", hextx); - start_bitcoin_cli(bitcoind, NULL, process_sendrawtx, true, cb, arg, + start_bitcoin_cli(bitcoind, NULL, process_sendrawtx, true, true, + cb, arg, "sendrawtransaction", hextx, NULL); } @@ -429,7 +435,8 @@ void bitcoind_getrawblock_(struct bitcoind *bitcoind, char hex[hex_str_size(sizeof(*blockid))]; bitcoin_blkid_to_hex(blockid, hex, sizeof(hex)); - start_bitcoin_cli(bitcoind, NULL, process_rawblock, false, cb, arg, + start_bitcoin_cli(bitcoind, NULL, process_rawblock, false, true, + cb, arg, "getblock", hex, "false", NULL); } @@ -457,7 +464,8 @@ void bitcoind_getblockcount_(struct bitcoind *bitcoind, void *arg), void *arg) { - start_bitcoin_cli(bitcoind, NULL, process_getblockcount, false, cb, arg, + start_bitcoin_cli(bitcoind, NULL, process_getblockcount, false, true, + cb, arg, "getblockcount", NULL); } @@ -619,7 +627,8 @@ static bool process_getblockhash_for_txout(struct bitcoin_cli *bcli) /* Strip the newline at the end of the previous output */ blockhash = tal_strndup(NULL, bcli->output, bcli->output_bytes-1); - start_bitcoin_cli(bcli->bitcoind, NULL, process_getblock, false, cb, go, + start_bitcoin_cli(bcli->bitcoind, NULL, process_getblock, false, false, + cb, go, "getblock", take(blockhash), NULL); return true; } @@ -640,7 +649,7 @@ void bitcoind_getoutput_(struct bitcoind *bitcoind, /* We may not have topology ourselves that far back, so ask bitcoind */ start_bitcoin_cli(bitcoind, NULL, process_getblockhash_for_txout, - true, cb, go, + true, false, cb, go, "getblockhash", take(tal_fmt(NULL, "%u", blocknum)), NULL); @@ -685,7 +694,8 @@ void bitcoind_getblockhash_(struct bitcoind *bitcoind, char str[STR_MAX_CHARS(height)]; snprintf(str, sizeof(str), "%u", height); - start_bitcoin_cli(bitcoind, NULL, process_getblockhash, true, cb, arg, + start_bitcoin_cli(bitcoind, NULL, process_getblockhash, true, true, + cb, arg, "getblockhash", str, NULL); } @@ -697,7 +707,7 @@ void bitcoind_gettxout(struct bitcoind *bitcoind, void *arg) { start_bitcoin_cli(bitcoind, NULL, - process_gettxout, true, cb, arg, + process_gettxout, true, false, cb, arg, "gettxout", take(type_to_string(NULL, struct bitcoin_txid, txid)), take(tal_fmt(NULL, "%u", outnum)), From 9b8c8f652b82b12c8365b7784afdf36aca01ca6f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Sep 2018 12:01:36 +0930 Subject: [PATCH 34/36] lightningd: make bcli_args() helper take ctx. Otherwise we can get leak complaints: all callers now use tmpctx. Signed-off-by: Rusty Russell --- lightningd/bitcoind.c | 64 +++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 30770392f22d..6a6d86f3d42c 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -103,14 +103,14 @@ static struct io_plan *output_init(struct io_conn *conn, struct bitcoin_cli *bcl static void next_bcli(struct bitcoind *bitcoind); /* For printing: simple string of args. */ -static char *bcli_args(struct bitcoin_cli *bcli) +static char *bcli_args(const tal_t *ctx, struct bitcoin_cli *bcli) { size_t i; - char *ret = tal_strdup(bcli, bcli->args[0]); + char *ret = tal_strdup(ctx, bcli->args[0]); for (i = 1; bcli->args[i]; i++) { - ret = tal_strcat(bcli, take(ret), " "); - ret = tal_strcat(bcli, take(ret), bcli->args[i]); + ret = tal_strcat(ctx, take(ret), " "); + ret = tal_strcat(ctx, take(ret), bcli->args[i]); } return ret; } @@ -134,14 +134,15 @@ static void bcli_failure(struct bitcoind *bitcoind, t = timemono_between(time_mono(), bitcoind->first_error_time); if (time_greater(t, time_from_sec(60))) fatal("%s exited %u (after %u other errors) '%.*s'", - bcli_args(bcli), + bcli_args(tmpctx, bcli), exitstatus, bitcoind->error_count, (int)bcli->output_bytes, bcli->output); log_unusual(bitcoind->log, - "%s exited with status %u", bcli_args(bcli), exitstatus); + "%s exited with status %u", + bcli_args(tmpctx, bcli), exitstatus); bitcoind->error_count++; @@ -159,12 +160,12 @@ static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli) /* FIXME: If we waited for SIGCHILD, this could never hang! */ while ((ret = waitpid(bcli->pid, &status, 0)) < 0 && errno == EINTR); if (ret != bcli->pid) - fatal("%s %s", bcli_args(bcli), + fatal("%s %s", bcli_args(tmpctx, bcli), ret == 0 ? "not exited?" : strerror(errno)); if (!WIFEXITED(status)) fatal("%s died with signal %i", - bcli_args(bcli), + bcli_args(tmpctx, bcli), WTERMSIG(status)); if (!bcli->exitstatus) { @@ -292,13 +293,13 @@ static bool extract_feerate(struct bitcoin_cli *bcli, tokens = json_parse_input(output, output_bytes, &valid); if (!tokens) fatal("%s: %s response", - bcli_args(bcli), + bcli_args(tmpctx, bcli), valid ? "partial" : "invalid"); if (tokens[0].type != JSMN_OBJECT) { log_unusual(bcli->bitcoind->log, "%s: gave non-object (%.*s)?", - bcli_args(bcli), + bcli_args(tmpctx, bcli), (int)output_bytes, output); return false; } @@ -418,7 +419,7 @@ static bool process_rawblock(struct bitcoin_cli *bcli) blk = bitcoin_block_from_hex(bcli, bcli->output, bcli->output_bytes); if (!blk) fatal("%s: bad block '%.*s'?", - bcli_args(bcli), + bcli_args(tmpctx, bcli), (int)bcli->output_bytes, bcli->output); cb(bcli->bitcoind, blk, bcli->cb_arg); @@ -452,7 +453,7 @@ static bool process_getblockcount(struct bitcoin_cli *bcli) blockcount = strtol(p, &end, 10); if (end == p || *end != '\n') fatal("%s: gave non-numeric blockcount %s", - bcli_args(bcli), p); + bcli_args(tmpctx, bcli), p); cb(bcli->bitcoind, blockcount, bcli->cb_arg); return true; @@ -498,7 +499,7 @@ static bool process_gettxout(struct bitcoin_cli *bcli) string on a spent gettxout */ if (*bcli->exitstatus != 0 || bcli->output_bytes == 0) { log_debug(bcli->bitcoind->log, "%s: not unspent output?", - bcli_args(bcli)); + bcli_args(tmpctx, bcli)); cb(bcli->bitcoind, NULL, bcli->cb_arg); return true; } @@ -506,35 +507,41 @@ static bool process_gettxout(struct bitcoin_cli *bcli) tokens = json_parse_input(bcli->output, bcli->output_bytes, &valid); if (!tokens) fatal("%s: %s response", - bcli_args(bcli), valid ? "partial" : "invalid"); + bcli_args(tmpctx, bcli), valid ? "partial" : "invalid"); if (tokens[0].type != JSMN_OBJECT) fatal("%s: gave non-object (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); valuetok = json_get_member(bcli->output, tokens, "value"); if (!valuetok) fatal("%s: had no value member (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); if (!json_tok_bitcoin_amount(bcli->output, valuetok, &out.amount)) fatal("%s: had bad value (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); scriptpubkeytok = json_get_member(bcli->output, tokens, "scriptPubKey"); if (!scriptpubkeytok) fatal("%s: had no scriptPubKey member (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); hextok = json_get_member(bcli->output, scriptpubkeytok, "hex"); if (!hextok) fatal("%s: had no scriptPubKey->hex member (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); out.script = tal_hexdata(bcli, bcli->output + hextok->start, hextok->end - hextok->start); if (!out.script) fatal("%s: scriptPubKey->hex invalid hex (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); cb(bcli->bitcoind, &out, bcli->cb_arg); return true; @@ -563,7 +570,7 @@ static bool process_getblock(struct bitcoin_cli *bcli) * the callback with NULL to indicate failure */ log_debug(bcli->bitcoind->log, "%s: returned invalid block, is this a pruned node?", - bcli_args(bcli)); + bcli_args(tmpctx, bcli)); cb(bcli->bitcoind, NULL, cbarg); tal_free(go); return true; @@ -571,7 +578,8 @@ static bool process_getblock(struct bitcoin_cli *bcli) if (tokens[0].type != JSMN_OBJECT) fatal("%s: gave non-object (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); /* "tx": [ "1a7bb0f58a5d235d232deb61d9e2208dabe69848883677abe78e9291a00638e8", @@ -581,13 +589,14 @@ static bool process_getblock(struct bitcoin_cli *bcli) txstok = json_get_member(bcli->output, tokens, "tx"); if (!txstok) fatal("%s: had no tx member (%.*s)?", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); /* Now, this can certainly happen, if txnum too large. */ txidtok = json_get_arr(txstok, go->txnum); if (!txidtok) { log_debug(bcli->bitcoind->log, "%s: no txnum %u", - bcli_args(bcli), go->txnum); + bcli_args(tmpctx, bcli), go->txnum); cb(bcli->bitcoind, NULL, cbarg); tal_free(go); return true; @@ -597,7 +606,7 @@ static bool process_getblock(struct bitcoin_cli *bcli) txidtok->end - txidtok->start, &txid)) fatal("%s: had bad txid (%.*s)?", - bcli_args(bcli), + bcli_args(tmpctx, bcli), txidtok->end - txidtok->start, bcli->output + txidtok->start); @@ -618,7 +627,7 @@ static bool process_getblockhash_for_txout(struct bitcoin_cli *bcli) if (*bcli->exitstatus != 0) { void *cbarg = go->cbarg; log_debug(bcli->bitcoind->log, "%s: invalid blocknum?", - bcli_args(bcli)); + bcli_args(tmpctx, bcli)); tal_free(go); cb(bcli->bitcoind, NULL, cbarg); return true; @@ -677,7 +686,8 @@ static bool process_getblockhash(struct bitcoin_cli *bcli) || !bitcoin_blkid_from_hex(bcli->output, bcli->output_bytes-1, &blkid)) { fatal("%s: bad blockid '%.*s'", - bcli_args(bcli), (int)bcli->output_bytes, bcli->output); + bcli_args(tmpctx, bcli), + (int)bcli->output_bytes, bcli->output); } cb(bcli->bitcoind, &blkid, bcli->cb_arg); From e7a0ffca05b4a5d7a0eb770129527fe9aa770ced Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Sep 2018 12:02:36 +0930 Subject: [PATCH 35/36] lightningd: verbose debugging for bitcoind commands. Signed-off-by: Rusty Russell --- lightningd/bitcoind.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 6a6d86f3d42c..e4628e814a3a 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -74,6 +74,7 @@ struct bitcoin_cli { int *exitstatus; pid_t pid; const char **args; + struct timeabs start; char *output; size_t output_bytes; size_t new_output; @@ -157,6 +158,10 @@ static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli) struct bitcoind *bitcoind = bcli->bitcoind; bool ok; + log_debug(bitcoind->log, "bitcoin-cli: finished %s (%"PRIu64" ms)", + bcli_args(tmpctx, bcli), + time_to_msec(time_between(time_now(), bcli->start))); + /* FIXME: If we waited for SIGCHILD, this could never hang! */ while ((ret = waitpid(bcli->pid, &status, 0)) < 0 && errno == EINTR); if (ret != bcli->pid) @@ -211,11 +216,15 @@ static void next_bcli(struct bitcoind *bitcoind) if (!bcli) return; + log_debug(bitcoind->log, "bitcoin-cli: starting %s", + bcli_args(tmpctx, bcli)); bcli->pid = pipecmdarr(&bcli->fd, NULL, &bcli->fd, cast_const2(char **, bcli->args)); if (bcli->pid < 0) fatal("%s exec failed: %s", bcli->args[0], strerror(errno)); + bcli->start = time_now(); + bitcoind->current = bcli; /* This lifetime is attached to bitcoind command fd */ conn = notleak(io_new_conn(bitcoind, bcli->fd, output_init, bcli)); From f6fb120e4a5deb34ae5a31b5bbdca10569056990 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Sep 2018 12:15:25 +0930 Subject: [PATCH 36/36] lightningd: allow more than one bitcoind request at once, run multiple queues. With the previous patch, we could still get stuck behind a low-prio request. Generalize it into separate queues, and allow more than one request in parallel. Worth noting that the test time for `VALGRIND=0 pytest -vx tests/ -n 10` doesn't change measurably. Signed-off-by: Rusty Russell --- lightningd/bitcoind.c | 69 +++++++++++++++++++++++++++---------------- lightningd/bitcoind.h | 14 ++++++--- 2 files changed, 53 insertions(+), 30 deletions(-) diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index e4628e814a3a..20b434e24746 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -21,6 +21,13 @@ #include #include +/* Bitcoind's web server has a default of 4 threads, with queue depth 16. + * It will *fail* rather than queue beyond that, so we must not stress it! + * + * This is how many request for each priority level we have. + */ +#define BITCOIND_MAX_PARALLEL 4 + /* Add the n'th arg to *args, incrementing n and keeping args of size n+1 */ static void add_arg(const char ***args, const char *arg) { @@ -75,6 +82,7 @@ struct bitcoin_cli { pid_t pid; const char **args; struct timeabs start; + enum bitcoind_prio prio; char *output; size_t output_bytes; size_t new_output; @@ -101,7 +109,7 @@ static struct io_plan *output_init(struct io_conn *conn, struct bitcoin_cli *bcl return read_more(conn, bcli); } -static void next_bcli(struct bitcoind *bitcoind); +static void next_bcli(struct bitcoind *bitcoind, enum bitcoind_prio prio); /* For printing: simple string of args. */ static char *bcli_args(const tal_t *ctx, struct bitcoin_cli *bcli) @@ -118,8 +126,8 @@ static char *bcli_args(const tal_t *ctx, struct bitcoin_cli *bcli) static void retry_bcli(struct bitcoin_cli *bcli) { - list_add_tail(&bcli->bitcoind->pending, &bcli->list); - next_bcli(bcli->bitcoind); + list_add_tail(&bcli->bitcoind->pending[bcli->prio], &bcli->list); + next_bcli(bcli->bitcoind, bcli->prio); } /* We allow 60 seconds of spurious errors, eg. reorg. */ @@ -156,11 +164,13 @@ static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli) { int ret, status; struct bitcoind *bitcoind = bcli->bitcoind; + enum bitcoind_prio prio = bcli->prio; bool ok; log_debug(bitcoind->log, "bitcoin-cli: finished %s (%"PRIu64" ms)", bcli_args(tmpctx, bcli), time_to_msec(time_between(time_now(), bcli->start))); + assert(bitcoind->num_requests[prio] > 0); /* FIXME: If we waited for SIGCHILD, this could never hang! */ while ((ret = waitpid(bcli->pid, &status, 0)) < 0 && errno == EINTR); @@ -176,7 +186,7 @@ static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli) if (!bcli->exitstatus) { if (WEXITSTATUS(status) != 0) { bcli_failure(bitcoind, bcli, WEXITSTATUS(status)); - bitcoind->current = NULL; + bitcoind->num_requests[prio]--; goto done; } } else @@ -185,7 +195,7 @@ static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli) if (WEXITSTATUS(status) == 0) bitcoind->error_count = 0; - bitcoind->current = NULL; + bitcoind->num_requests[bcli->prio]--; /* Don't continue if were only here because we were freed for shutdown */ if (bitcoind->shutdown) @@ -201,18 +211,18 @@ static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli) tal_free(bcli); done: - next_bcli(bitcoind); + next_bcli(bitcoind, prio); } -static void next_bcli(struct bitcoind *bitcoind) +static void next_bcli(struct bitcoind *bitcoind, enum bitcoind_prio prio) { struct bitcoin_cli *bcli; struct io_conn *conn; - if (bitcoind->current) + if (bitcoind->num_requests[prio] >= BITCOIND_MAX_PARALLEL) return; - bcli = list_pop(&bitcoind->pending, struct bitcoin_cli, list); + bcli = list_pop(&bitcoind->pending[prio], struct bitcoin_cli, list); if (!bcli) return; @@ -225,7 +235,8 @@ static void next_bcli(struct bitcoind *bitcoind) bcli->start = time_now(); - bitcoind->current = bcli; + bitcoind->num_requests[prio]++; + /* This lifetime is attached to bitcoind command fd */ conn = notleak(io_new_conn(bitcoind, bcli->fd, output_init, bcli)); io_set_finish(conn, bcli_finished, bcli); @@ -257,7 +268,7 @@ start_bitcoin_cli(struct bitcoind *bitcoind, const tal_t *ctx, bool (*process)(struct bitcoin_cli *), bool nonzero_exit_ok, - bool high_prio, + enum bitcoind_prio prio, void *cb, void *cb_arg, char *cmd, ...) { @@ -266,6 +277,7 @@ start_bitcoin_cli(struct bitcoind *bitcoind, bcli->bitcoind = bitcoind; bcli->process = process; + bcli->prio = prio; bcli->cb = cb; bcli->cb_arg = cb_arg; if (ctx) { @@ -285,11 +297,8 @@ start_bitcoin_cli(struct bitcoind *bitcoind, bcli->args = gather_args(bitcoind, bcli, cmd, ap); va_end(ap); - if (high_prio) - list_add(&bitcoind->pending, &bcli->list); - else - list_add_tail(&bitcoind->pending, &bcli->list); - next_bcli(bitcoind); + list_add_tail(&bitcoind->pending[bcli->prio], &bcli->list); + next_bcli(bitcoind, bcli->prio); } static bool extract_feerate(struct bitcoin_cli *bcli, @@ -366,7 +375,8 @@ static void do_one_estimatefee(struct bitcoind *bitcoind, char blockstr[STR_MAX_CHARS(u32)]; snprintf(blockstr, sizeof(blockstr), "%u", efee->blocks[efee->i]); - start_bitcoin_cli(bitcoind, NULL, process_estimatefee, false, false, + start_bitcoin_cli(bitcoind, NULL, process_estimatefee, false, + BITCOIND_LOW_PRIO, NULL, efee, "estimatesmartfee", blockstr, efee->estmode[efee->i], NULL); @@ -413,7 +423,8 @@ void bitcoind_sendrawtx_(struct bitcoind *bitcoind, void *arg) { log_debug(bitcoind->log, "sendrawtransaction: %s", hextx); - start_bitcoin_cli(bitcoind, NULL, process_sendrawtx, true, true, + start_bitcoin_cli(bitcoind, NULL, process_sendrawtx, true, + BITCOIND_HIGH_PRIO, cb, arg, "sendrawtransaction", hextx, NULL); } @@ -445,7 +456,8 @@ void bitcoind_getrawblock_(struct bitcoind *bitcoind, char hex[hex_str_size(sizeof(*blockid))]; bitcoin_blkid_to_hex(blockid, hex, sizeof(hex)); - start_bitcoin_cli(bitcoind, NULL, process_rawblock, false, true, + start_bitcoin_cli(bitcoind, NULL, process_rawblock, false, + BITCOIND_HIGH_PRIO, cb, arg, "getblock", hex, "false", NULL); } @@ -474,7 +486,8 @@ void bitcoind_getblockcount_(struct bitcoind *bitcoind, void *arg), void *arg) { - start_bitcoin_cli(bitcoind, NULL, process_getblockcount, false, true, + start_bitcoin_cli(bitcoind, NULL, process_getblockcount, false, + BITCOIND_HIGH_PRIO, cb, arg, "getblockcount", NULL); } @@ -645,7 +658,8 @@ static bool process_getblockhash_for_txout(struct bitcoin_cli *bcli) /* Strip the newline at the end of the previous output */ blockhash = tal_strndup(NULL, bcli->output, bcli->output_bytes-1); - start_bitcoin_cli(bcli->bitcoind, NULL, process_getblock, false, false, + start_bitcoin_cli(bcli->bitcoind, NULL, process_getblock, false, + BITCOIND_LOW_PRIO, cb, go, "getblock", take(blockhash), NULL); return true; @@ -667,7 +681,7 @@ void bitcoind_getoutput_(struct bitcoind *bitcoind, /* We may not have topology ourselves that far back, so ask bitcoind */ start_bitcoin_cli(bitcoind, NULL, process_getblockhash_for_txout, - true, false, cb, go, + true, BITCOIND_LOW_PRIO, cb, go, "getblockhash", take(tal_fmt(NULL, "%u", blocknum)), NULL); @@ -713,7 +727,8 @@ void bitcoind_getblockhash_(struct bitcoind *bitcoind, char str[STR_MAX_CHARS(height)]; snprintf(str, sizeof(str), "%u", height); - start_bitcoin_cli(bitcoind, NULL, process_getblockhash, true, true, + start_bitcoin_cli(bitcoind, NULL, process_getblockhash, true, + BITCOIND_HIGH_PRIO, cb, arg, "getblockhash", str, NULL); } @@ -726,7 +741,7 @@ void bitcoind_gettxout(struct bitcoind *bitcoind, void *arg) { start_bitcoin_cli(bitcoind, NULL, - process_gettxout, true, false, cb, arg, + process_gettxout, true, BITCOIND_LOW_PRIO, cb, arg, "gettxout", take(type_to_string(NULL, struct bitcoin_txid, txid)), take(tal_fmt(NULL, "%u", outnum)), @@ -832,14 +847,16 @@ struct bitcoind *new_bitcoind(const tal_t *ctx, bitcoind->datadir = NULL; bitcoind->ld = ld; bitcoind->log = log; - bitcoind->current = NULL; + for (size_t i = 0; i < BITCOIND_NUM_PRIO; i++) { + bitcoind->num_requests[i] = 0; + list_head_init(&bitcoind->pending[i]); + } bitcoind->shutdown = false; bitcoind->error_count = 0; bitcoind->rpcuser = NULL; bitcoind->rpcpass = NULL; bitcoind->rpcconnect = NULL; bitcoind->rpcport = NULL; - list_head_init(&bitcoind->pending); tal_add_destructor(bitcoind, destroy_bitcoind); return bitcoind; diff --git a/lightningd/bitcoind.h b/lightningd/bitcoind.h index 99fa8a0cc34d..379a56730ff9 100644 --- a/lightningd/bitcoind.h +++ b/lightningd/bitcoind.h @@ -24,6 +24,12 @@ enum bitcoind_mode { BITCOIND_REGTEST }; +enum bitcoind_prio { + BITCOIND_LOW_PRIO, + BITCOIND_HIGH_PRIO +}; +#define BITCOIND_NUM_PRIO (BITCOIND_HIGH_PRIO+1) + struct bitcoind { /* eg. "bitcoin-cli" */ char *cli; @@ -37,11 +43,11 @@ struct bitcoind { /* Main lightningd structure */ struct lightningd *ld; - /* Are we currently running a bitcoind request (it's ratelimited) */ - struct bitcoin_cli *current; + /* How many high/low prio requests are we running (it's ratelimited) */ + size_t num_requests[BITCOIND_NUM_PRIO]; - /* Pending requests. */ - struct list_head pending; + /* Pending requests (high and low prio). */ + struct list_head pending[BITCOIND_NUM_PRIO]; /* What network are we on? */ const struct chainparams *chainparams;