From 477832057dbaba8dd25642ff723b156bfe091358 Mon Sep 17 00:00:00 2001 From: niftynei Date: Wed, 5 May 2021 18:03:03 -0500 Subject: [PATCH] funder: print reason that we don't contribute funds If we don't put funds into a channel, say why in the logs. Should make it a bit easier to figure out what's going on. --- plugins/funder.c | 17 +++--- plugins/funder_policy.c | 97 +++++++++++++++++++++-------- plugins/funder_policy.h | 5 +- plugins/test/run-funder_policy.c | 102 +++++++++++++++++++++++++------ 4 files changed, 170 insertions(+), 51 deletions(-) diff --git a/plugins/funder.c b/plugins/funder.c index 081194a8a..1731f7fbd 100644 --- a/plugins/funder.c +++ b/plugins/funder.c @@ -354,6 +354,7 @@ listfunds_success(struct command *cmd, const jsmntok_t *outputs_tok, *tok; struct out_req *req; size_t i; + const char *funding_err; outputs_tok = json_get_member(buf, result, "outputs"); if (!outputs_tok) @@ -410,16 +411,18 @@ listfunds_success(struct command *cmd, "`listfunds` overflowed output values"); } - info->our_funding = calculate_our_funding(current_policy, - info->id, - info->their_funding, - available_funds, - info->channel_max); + funding_err = calculate_our_funding(current_policy, + info->id, + info->their_funding, + available_funds, + info->channel_max, + &info->our_funding); plugin_log(cmd->plugin, LOG_DBG, - "Policy %s returned funding amount of %s", + "Policy %s returned funding amount of %s. %s", funder_policy_desc(tmpctx, current_policy), type_to_string(tmpctx, struct amount_sat, - &info->our_funding)); + &info->our_funding), + funding_err ? funding_err : ""); if (amount_sat_eq(info->our_funding, AMOUNT_SAT(0))) return command_hook_success(cmd); diff --git a/plugins/funder_policy.c b/plugins/funder_policy.c index baf9bfa2c..956fb1542 100644 --- a/plugins/funder_policy.c +++ b/plugins/funder_policy.c @@ -166,62 +166,109 @@ apply_policy(struct funder_policy policy, abort(); } -struct amount_sat +const char * calculate_our_funding(struct funder_policy policy, struct node_id id, struct amount_sat their_funding, struct amount_sat available_funds, - struct amount_sat channel_max) + struct amount_sat channel_max, + struct amount_sat *our_funding) { - struct amount_sat our_funding, avail_channel_space, - net_available_funds; + struct amount_sat avail_channel_space, net_available_funds; /* Are we skipping this one? */ - if (pseudorand(100) >= policy.fund_probability) - return AMOUNT_SAT(0); + if (pseudorand(100) >= policy.fund_probability) { + *our_funding = AMOUNT_SAT(0); + return tal_fmt(tmpctx, + "Skipping, failed fund_probability test"); + } /* Figure out amount of actual headroom we have */ - if (!amount_sat_sub(&avail_channel_space, channel_max, their_funding)) - return AMOUNT_SAT(0); + if (!amount_sat_sub(&avail_channel_space, channel_max, their_funding) + || amount_sat_eq(avail_channel_space, AMOUNT_SAT(0))) { + *our_funding = AMOUNT_SAT(0); + return tal_fmt(tmpctx, "No space available in channel." + " channel_max %s, their_funding %s", + type_to_string(tmpctx, struct amount_sat, + &channel_max), + type_to_string(tmpctx, struct amount_sat, + &their_funding)); + } /* Figure out actual available funds, given our requested * 'reserve_tank' */ if (!amount_sat_sub(&net_available_funds, available_funds, - policy.reserve_tank)) - return AMOUNT_SAT(0); + policy.reserve_tank) + || amount_sat_eq(net_available_funds, AMOUNT_SAT(0))) { + *our_funding = AMOUNT_SAT(0); + return tal_fmt(tmpctx, "Reserve tank too low." + " available_funds %s, reserve_tank requires %s", + type_to_string(tmpctx, struct amount_sat, + &available_funds), + type_to_string(tmpctx, struct amount_sat, + &policy.reserve_tank)); + } /* Are they funding enough ? */ - if (amount_sat_less(their_funding, policy.min_their_funding)) - return AMOUNT_SAT(0); + if (amount_sat_less(their_funding, policy.min_their_funding)) { + *our_funding = AMOUNT_SAT(0); + return tal_fmt(tmpctx, "Peer's funding too little." + " their_funding %s," + " min_their_funding requires %s", + type_to_string(tmpctx, struct amount_sat, + &their_funding), + type_to_string(tmpctx, struct amount_sat, + &policy.min_their_funding)); + } /* Are they funding too much ? */ - if (amount_sat_greater(their_funding, policy.max_their_funding)) - return AMOUNT_SAT(0); + if (amount_sat_greater(their_funding, policy.max_their_funding)) { + *our_funding = AMOUNT_SAT(0); + return tal_fmt(tmpctx, "Peer's funding too much." + " their_funding %s," + " max_their_funding requires %s", + type_to_string(tmpctx, struct amount_sat, + &their_funding), + type_to_string(tmpctx, struct amount_sat, + &policy.max_their_funding)); + } /* What's our amount, given our policy */ - our_funding = apply_policy(policy, their_funding, available_funds); + *our_funding = apply_policy(policy, their_funding, available_funds); + + /* Don't return an 'error' if we're already at 0 */ + if (amount_sat_eq(*our_funding, AMOUNT_SAT(0))) + return NULL; /* our_funding is probably sane, so let's fuzz this amount a bit */ - our_funding = apply_fuzz(policy.fuzz_factor, our_funding); + *our_funding = apply_fuzz(policy.fuzz_factor, *our_funding); /* Is our_funding more than we can fit? if so set to avail space */ - if (amount_sat_greater(our_funding, avail_channel_space)) - our_funding = avail_channel_space; + if (amount_sat_greater(*our_funding, avail_channel_space)) + *our_funding = avail_channel_space; /* Is our_funding more than we want to fund in a channel? * if so set at our desired per-channel max */ - if (amount_sat_greater(our_funding, policy.per_channel_max)) - our_funding = policy.per_channel_max; + if (amount_sat_greater(*our_funding, policy.per_channel_max)) + *our_funding = policy.per_channel_max; /* Is our_funding more than we have available? if so * set to max available */ - if (amount_sat_greater(our_funding, net_available_funds)) - our_funding = net_available_funds; + if (amount_sat_greater(*our_funding, net_available_funds)) + *our_funding = net_available_funds; /* Is our_funding less than our per-channel minimum? * if so, don't fund */ - if (amount_sat_less(our_funding, policy.per_channel_min)) - return AMOUNT_SAT(0); + if (amount_sat_less(*our_funding, policy.per_channel_min)) { + *our_funding = AMOUNT_SAT(0); + return tal_fmt(tmpctx, "Can't meet our min channel requirement." + " our_funding %s," + " per_channel_min requires %s", + type_to_string(tmpctx, struct amount_sat, + our_funding), + type_to_string(tmpctx, struct amount_sat, + &policy.per_channel_min)); + } - return our_funding; + return NULL; } diff --git a/plugins/funder_policy.h b/plugins/funder_policy.h index 51e1cf172..9c7bd9ef6 100644 --- a/plugins/funder_policy.h +++ b/plugins/funder_policy.h @@ -73,12 +73,13 @@ default_funder_policy(enum funder_opt policy, /* Given the policy and this request's details, figure * out how much we should contribute to this channel */ -struct amount_sat +const char * calculate_our_funding(struct funder_policy policy, struct node_id id, struct amount_sat their_funding, struct amount_sat available_funds, - struct amount_sat channel_max); + struct amount_sat channel_max, + struct amount_sat *our_funding); /* Get the name of this policy option */ const char *funder_opt_name(enum funder_opt opt); diff --git a/plugins/test/run-funder_policy.c b/plugins/test/run-funder_policy.c index d10513117..b397f70a1 100644 --- a/plugins/test/run-funder_policy.c +++ b/plugins/test/run-funder_policy.c @@ -37,6 +37,7 @@ struct test_case { struct funder_policy policy; struct amount_sat exp_our_funds; + bool expect_err; }; struct test_case cases[] = { @@ -58,6 +59,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(1111), + .expect_err = false, }, /* Match 0 */ { @@ -77,6 +79,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(0), + .expect_err = false, }, /* Match 100 */ { @@ -96,6 +99,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(5000), + .expect_err = false, }, /* Match 200 */ { @@ -115,6 +119,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(5000), + .expect_err = false, }, /* Available 0 */ { @@ -134,6 +139,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(0), + .expect_err = false, }, /* Available 50 */ { @@ -153,6 +159,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(1500), + .expect_err = false, }, /* Available 100+ */ { @@ -172,6 +179,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(5000), + .expect_err = false, }, /* Fixed above per-channel max*/ { @@ -193,6 +201,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(900), + .expect_err = false, }, /* Fixed less than available space */ { @@ -212,6 +221,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(500), + .expect_err = false, }, /* Fixed less than available funds */ { @@ -231,6 +241,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(500), + .expect_err = false, }, /* Peer is under 'min_their_funding' */ { @@ -250,6 +261,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(0), + .expect_err = true, }, /* Peer exceeds 'max_their_funding' */ { @@ -269,6 +281,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(0), + .expect_err = true, }, /* Fixed less than available funds less reserve tank */ { @@ -288,6 +301,47 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(900), + .expect_err = false, + }, + /* Fixed no funds available after reserve */ + { + .their_funds = AMOUNT_SAT(5000), + .available_funds = AMOUNT_SAT(999), + .channel_max = AMOUNT_SAT(10000), + .policy = { + .opt = FIXED, + .mod = 996, + .min_their_funding = AMOUNT_SAT(0), + .max_their_funding = AMOUNT_SAT(10000), + .per_channel_max = AMOUNT_SAT(10000), + .per_channel_min = AMOUNT_SAT(0), + .fuzz_factor = 0, + .reserve_tank = AMOUNT_SAT(1000), + .fund_probability = 100, + }, + + .exp_our_funds = AMOUNT_SAT(0), + .expect_err = true, + }, + /* Fixed no funds in channel */ + { + .their_funds = AMOUNT_SAT(5000), + .available_funds = AMOUNT_SAT(5000), + .channel_max = AMOUNT_SAT(5000), + .policy = { + .opt = FIXED, + .mod = 995, + .min_their_funding = AMOUNT_SAT(0), + .max_their_funding = AMOUNT_SAT(10000), + .per_channel_max = AMOUNT_SAT(10000), + .per_channel_min = AMOUNT_SAT(0), + .fuzz_factor = 0, + .reserve_tank = AMOUNT_SAT(1000), + .fund_probability = 100, + }, + + .exp_our_funds = AMOUNT_SAT(0), + .expect_err = true, }, /* Fixed below per-channel min */ { @@ -307,6 +361,7 @@ struct test_case cases[] = { }, .exp_our_funds = AMOUNT_SAT(0), + .expect_err = true, }, }; @@ -321,10 +376,11 @@ static void check_fuzzing(struct test_case fuzzcase) memset(&id, 2, sizeof(struct node_id)); for (size_t i = 0; i < 100; i++) { - our_funds = calculate_our_funding(fuzzcase.policy, id, - fuzzcase.their_funds, - fuzzcase.available_funds, - fuzzcase.channel_max); + calculate_our_funding(fuzzcase.policy, id, + fuzzcase.their_funds, + fuzzcase.available_funds, + fuzzcase.channel_max, + &our_funds); if (amount_sat_greater(our_funds, fuzz_max)) fuzz_max = our_funds; if (amount_sat_less(our_funds, fuzz_min)) @@ -344,6 +400,7 @@ int main(int argc, const char *argv[]) size_t i = 0, flips = 0; struct test_case flipcase, fuzzcase; size_t flipcount = 0; + const char *err; common_setup(argv[0]); memset(&id, 2, sizeof(struct node_id)); @@ -351,18 +408,20 @@ int main(int argc, const char *argv[]) /* Check the default funder policy, at fixed (0msat) */ policy = default_funder_policy(FIXED, 0); - /* Use the first test case inputs? */ - our_funds = calculate_our_funding(policy, id, - cases[i].their_funds, - cases[i].available_funds, - cases[i].channel_max); + err = calculate_our_funding(policy, id, + AMOUNT_SAT(50000), + AMOUNT_SAT(50000), + AMOUNT_SAT(100000), + &our_funds); assert(amount_sat_eq(empty, our_funds)); + assert(!err); for (i = 0; i < ARRAY_SIZE(cases); i++) { - our_funds = calculate_our_funding(cases[i].policy, id, - cases[i].their_funds, - cases[i].available_funds, - cases[i].channel_max); + err = calculate_our_funding(cases[i].policy, id, + cases[i].their_funds, + cases[i].available_funds, + cases[i].channel_max, + &our_funds); if (!amount_sat_eq(cases[i].exp_our_funds, our_funds)) { fprintf(stderr, "FAIL policy: %s. expected %s, got %s\n", funder_policy_desc(NULL, cases[i].policy), @@ -372,6 +431,14 @@ int main(int argc, const char *argv[]) &our_funds)); ok = false; } + if (cases[i].expect_err != (err != NULL)) { + fprintf(stderr, "FAIL policy: %s. expected %serr," + " got %s\n", + funder_policy_desc(NULL, cases[i].policy), + cases[i].expect_err ? "" : "no ", + err ? err : "no err"); + ok = false; + } } if (!ok) exit(1); @@ -383,10 +450,11 @@ int main(int argc, const char *argv[]) flipcase.policy.fund_probability = flips; for (i = 0; i < 100 * flips; i++) { - our_funds = calculate_our_funding(flipcase.policy, id, - flipcase.their_funds, - flipcase.available_funds, - flipcase.channel_max); + calculate_our_funding(flipcase.policy, id, + flipcase.their_funds, + flipcase.available_funds, + flipcase.channel_max, + &our_funds); if (!amount_sat_eq(our_funds, AMOUNT_SAT(0))) flipcount++; }