diff --git a/common/amount.c b/common/amount.c index beeaa14b4a65..1eae69cefc8c 100644 --- a/common/amount.c +++ b/common/amount.c @@ -532,6 +532,13 @@ struct amount_msat amount_msat_div(struct amount_msat msat, u64 div) return msat; } +struct amount_msat amount_msat_div_ceil(struct amount_msat msat, u64 div) +{ + u64 res = msat.millisatoshis / div; + msat.millisatoshis = res + (div * res == msat.millisatoshis ? 0 : 1); + return msat; +} + struct amount_sat amount_sat_div(struct amount_sat sat, u64 div) { sat.satoshis /= div; diff --git a/common/amount.h b/common/amount.h index dd6ad61bb262..b1cdbac1d570 100644 --- a/common/amount.h +++ b/common/amount.h @@ -104,7 +104,13 @@ WARN_UNUSED_RESULT bool amount_sat_add_sat_s64(struct amount_sat *val, WARN_UNUSED_RESULT bool amount_msat_accumulate(struct amount_msat *a, struct amount_msat b); +/* returns floor(msat/div) */ struct amount_msat amount_msat_div(struct amount_msat msat, u64 div); + +/* returns ceil(msat/div) */ +struct amount_msat amount_msat_div_ceil(struct amount_msat msat, u64 div); + +/* returns floor(sat/div) */ struct amount_sat amount_sat_div(struct amount_sat sat, u64 div); bool amount_sat_mul(struct amount_sat *res, struct amount_sat sat, u64 mul); diff --git a/common/test/run-amount.c b/common/test/run-amount.c index 5f8a96a0b0bc..0e9f295dc0c2 100644 --- a/common/test/run-amount.c +++ b/common/test/run-amount.c @@ -163,6 +163,75 @@ static void test_amount_with_fee(void) 2100000001234567890ULL); } +static void test_case_amount_div(u64 input, u64 div, u64 expected) +{ + struct amount_msat msat = amount_msat(input); + struct amount_msat expected_msat = amount_msat(expected); + struct amount_msat result_msat = amount_msat_div(msat, div); + assert(amount_msat_eq(result_msat, expected_msat)); +} + +static void test_case_amount_div_ceil(u64 input, u64 div, u64 expected) +{ + struct amount_msat msat = amount_msat(input); + struct amount_msat expected_msat = amount_msat(expected); + struct amount_msat result_msat = amount_msat_div_ceil(msat, div); + assert(amount_msat_eq(result_msat, expected_msat)); +} + +static void test_amount_div(void) +{ + test_case_amount_div(1, 1, 1); + test_case_amount_div(1, 2, 0); + test_case_amount_div(1, 3, 0); + + test_case_amount_div(2, 1, 2); + test_case_amount_div(2, 2, 1); + test_case_amount_div(2, 3, 0); + + test_case_amount_div(3, 1, 3); + test_case_amount_div(3, 2, 1); + test_case_amount_div(3, 3, 1); + test_case_amount_div(3, 4, 0); + + test_case_amount_div(10, 1, 10); + test_case_amount_div(10, 2, 5); + test_case_amount_div(10, 3, 3); + test_case_amount_div(10, 4, 2); + test_case_amount_div(10, 5, 2); + test_case_amount_div(10, 6, 1); + test_case_amount_div(10, 7, 1); + test_case_amount_div(10, 8, 1); + test_case_amount_div(10, 9, 1); + test_case_amount_div(10, 10, 1); + test_case_amount_div(10, 11, 0); + + test_case_amount_div_ceil(1, 1, 1); + test_case_amount_div_ceil(1, 2, 1); + test_case_amount_div_ceil(1, 3, 1); + + test_case_amount_div_ceil(2, 1, 2); + test_case_amount_div_ceil(2, 2, 1); + test_case_amount_div_ceil(2, 3, 1); + + test_case_amount_div_ceil(3, 1, 3); + test_case_amount_div_ceil(3, 2, 2); + test_case_amount_div_ceil(3, 3, 1); + test_case_amount_div_ceil(3, 4, 1); + + test_case_amount_div_ceil(10, 1, 10); + test_case_amount_div_ceil(10, 2, 5); + test_case_amount_div_ceil(10, 3, 4); + test_case_amount_div_ceil(10, 4, 3); + test_case_amount_div_ceil(10, 5, 2); + test_case_amount_div_ceil(10, 6, 2); + test_case_amount_div_ceil(10, 7, 2); + test_case_amount_div_ceil(10, 8, 2); + test_case_amount_div_ceil(10, 9, 2); + test_case_amount_div_ceil(10, 10, 1); + test_case_amount_div_ceil(10, 11, 1); +} + #define FAIL_MSAT(msatp, str) \ assert(!parse_amount_msat((msatp), (str), strlen(str))) #define PASS_MSAT(msatp, str, val) \ @@ -330,5 +399,6 @@ int main(int argc, char *argv[]) } test_amount_with_fee(); + test_amount_div(); common_shutdown(); } diff --git a/plugins/askrene/mcf.c b/plugins/askrene/mcf.c index 2c43ae4da78d..696b810c7574 100644 --- a/plugins/askrene/mcf.c +++ b/plugins/askrene/mcf.c @@ -360,9 +360,18 @@ static void linearize_channel(const struct pay_parameters *params, b = 1 + amount_msat_ratio_floor(maxcap, params->accuracy); /* An extra bound on capacity, here we use it to reduce the flow such - * that it does not exceed htlcmax. */ + * that it does not exceed htlcmax. + * The cap con capacity is not greater than the amount of payment units + * (msat/accuracy). The way a channel is decomposed into linear cost + * arcs (code below) in ascending cost order ensures that the only the + * necessary capacity to forward the payment is allocated in the lower + * cost arcs. This may lead to some arcs in the decomposition (at the + * high cost end) to have a capacity of 0, and we can prune them while + * keeping the solution optimal. */ u64 cap_on_capacity = - amount_msat_ratio_floor(gossmap_chan_htlc_max(c, dir), params->accuracy); + MIN(amount_msat_ratio_floor(gossmap_chan_htlc_max(c, dir), + params->accuracy), + amount_msat_ratio_ceil(params->amount, params->accuracy)); set_capacity(&capacity[0], a, &cap_on_capacity); cost[0]=0; @@ -598,8 +607,9 @@ static void init_linear_network(const tal_t *ctx, // when the `i` hits the `next` node. for(size_t k=0;ksource = source; params->target = target; params->amount = amount; - params->accuracy = AMOUNT_MSAT(1000); - /* FIXME: params->accuracy = amount_msat_max(amount_msat_div(amount, - * 1000), AMOUNT_MSAT(1)); + /* -> We reduce the granularity of the flow by limiting the subdivision + * of the payment amount into 1000 units of flow. That reduces the + * computational burden for algorithms that depend on it, eg. "capacity + * scaling" and "successive shortest path". + * -> Using Ceil operation instead of Floor so that + * accuracy x 1000 >= amount * */ + params->accuracy = + amount_msat_max(AMOUNT_MSAT(1), amount_msat_div_ceil(amount, 1000)); // template the channel partition into linear arcs params->cap_fraction[0]=0; diff --git a/tests/test_askrene.py b/tests/test_askrene.py index c566e6fa9de9..01a721e0df69 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -1204,10 +1204,10 @@ def test_real_data(node_factory, bitcoind): # CI, it's slow. if SLOW_MACHINE: limit = 25 - expected = (6, 25, 1544756, 142986, 91) + expected = (6, 25, 1568821, 143649, 91) else: limit = 100 - expected = (9, 95, 6347877, 566288, 92) + expected = (9, 96, 6565467, 630668, 91) fees = {} for n in range(0, limit): @@ -1321,10 +1321,10 @@ def test_real_biases(node_factory, bitcoind): # CI, it's slow. if SLOW_MACHINE: limit = 25 - expected = ({1: 5, 2: 7, 4: 7, 8: 11, 16: 14, 32: 19, 64: 25, 100: 25}, 0) + expected = ({1: 6, 2: 6, 4: 7, 8: 12, 16: 14, 32: 19, 64: 25, 100: 25}, 0) else: limit = 100 - expected = ({1: 23, 2: 31, 4: 40, 8: 53, 16: 70, 32: 82, 64: 96, 100: 96}, 0) + expected = ({1: 22, 2: 25, 4: 36, 8: 53, 16: 69, 32: 80, 64: 96, 100: 96}, 0) l1.rpc.askrene_create_layer('biases') num_changed = {} diff --git a/tests/test_xpay.py b/tests/test_xpay.py index ff7179c888dd..d895e51fd593 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -671,7 +671,7 @@ def test_xpay_bolt12_no_mpp(node_factory, chainparams, deprecations): # Amount needs to be enought that it bothers splitting, but not # so much that it can't pay without mpp. - AMOUNT = 500000000 + AMOUNT = 800000000 # l2 will advertize mpp, l3 won't. l2offer = l2.rpc.offer(AMOUNT, 'test_xpay_bolt12_no_mpp') @@ -686,10 +686,11 @@ def test_xpay_bolt12_no_mpp(node_factory, chainparams, deprecations): assert ret['failed_parts'] == 0 if deprecations: assert ret['successful_parts'] == 2 + assert ret['amount_sent_msat'] == AMOUNT + AMOUNT // 100000 + 2 else: assert ret['successful_parts'] == 1 + assert ret['amount_sent_msat'] == AMOUNT + AMOUNT // 100000 + 1 assert ret['amount_msat'] == AMOUNT - assert ret['amount_sent_msat'] == AMOUNT + AMOUNT // 100000 + 1 def test_xpay_slow_mode(node_factory, bitcoind): @@ -706,18 +707,18 @@ def test_xpay_slow_mode(node_factory, bitcoind): wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 10) # First try an MPP which fails - inv = l5.rpc.invoice(500000000, 'test_xpay_slow_mode_fail', 'test_xpay_slow_mode_fail', preimage='01' * 32)['bolt11'] + inv = l5.rpc.invoice(800000000, 'test_xpay_slow_mode_fail', 'test_xpay_slow_mode_fail', preimage='01' * 32)['bolt11'] l5.rpc.delinvoice('test_xpay_slow_mode_fail', status='unpaid') with pytest.raises(RpcError, match=r"Destination said it doesn't know invoice: incorrect_or_unknown_payment_details"): l1.rpc.xpay(inv) # Now a successful one - inv = l5.rpc.invoice(500000000, 'test_xpay_slow_mode', 'test_xpay_slow_mode', preimage='00' * 32)['bolt11'] + inv = l5.rpc.invoice(800000000, 'test_xpay_slow_mode', 'test_xpay_slow_mode', preimage='00' * 32)['bolt11'] assert l1.rpc.xpay(inv) == {'payment_preimage': '00' * 32, - 'amount_msat': 500000000, - 'amount_sent_msat': 500010002, + 'amount_msat': 800000000, + 'amount_sent_msat': 800016004, 'failed_parts': 0, 'successful_parts': 2} @@ -739,7 +740,7 @@ def test_fail_after_success(node_factory, bitcoind, executor, slow_mode): bitcoind.generate_block(5) wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 10) - inv = l5.rpc.invoice(500000000, 'test_xpay_slow_mode', 'test_xpay_slow_mode', preimage='00' * 32)['bolt11'] + inv = l5.rpc.invoice(800000000, 'test_xpay_slow_mode', 'test_xpay_slow_mode', preimage='00' * 32)['bolt11'] fut = executor.submit(l1.rpc.xpay, invstring=inv, retry_for=0) # Part via l3 is fine. Part via l4 is stuck, so we kill l4 and mine @@ -750,8 +751,8 @@ def test_fail_after_success(node_factory, bitcoind, executor, slow_mode): # Normally, we return as soon as first part succeeds. if slow_mode is False: assert fut.result(TIMEOUT) == {'payment_preimage': '00' * 32, - 'amount_msat': 500000000, - 'amount_sent_msat': 500010002, + 'amount_msat': 800000000, + 'amount_sent_msat': 800016004, 'failed_parts': 0, 'successful_parts': 2} @@ -763,15 +764,15 @@ def test_fail_after_success(node_factory, bitcoind, executor, slow_mode): l1.daemon.wait_for_log(r"UNUSUAL.*Destination accepted partial payment, failed a part \(Error permanent_channel_failure for path ->022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59->0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199->032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e, from 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59\)") # Could be either way around, check both line = l1.daemon.is_in_log(r"UNUSUAL.*Destination accepted partial payment, failed a part") - assert re.search(r'but accepted only 32000msat of 500000000msat\. Winning\?!', line) or re.search(r'but accepted only 499968000msat of 500000000msat\. Winning\?!', line) + assert re.search(r'but accepted only .* of 800000000msat\. Winning\?!', line) if slow_mode is True: # Now it succeeds, but notes that it only sent one part! res = fut.result(TIMEOUT) # Some variation due to floating point. - assert res['amount_sent_msat'] < 500000000 + assert res['amount_sent_msat'] < 800000000 assert res == {'payment_preimage': '00' * 32, - 'amount_msat': 500000000, + 'amount_msat': 800000000, 'amount_sent_msat': res['amount_sent_msat'], 'failed_parts': 1, 'successful_parts': 1}