diff --git a/channeld/channel.c b/channeld/channel.c index 463aeeddb..bd711d631 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -118,7 +118,7 @@ struct peer { u16 channel_direction; /* CLTV delta to announce to peers */ - u16 cltv_delta; + u16 their_cltv_delta; u32 fee_base; u32 fee_per_satoshi; @@ -247,7 +247,7 @@ static void send_channel_update(struct peer *peer, bool disabled) cupdate = towire_channel_update( tmpctx, sig, &peer->chain_hash, &peer->short_channel_ids[LOCAL], timestamp, flags, - peer->cltv_delta, 1, peer->fee_base, peer->fee_per_satoshi); + peer->their_cltv_delta, 1, peer->fee_base, peer->fee_per_satoshi); msg = towire_hsm_cupdate_sig_req(tmpctx, cupdate); @@ -1733,7 +1733,8 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg) e = channel_add_htlc(peer->channel, LOCAL, peer->htlc_id, amount_msat, cltv_expiry, &payment_hash, onion_routing_packet); - status_trace("Adding HTLC %"PRIu64" gave %i", peer->htlc_id, e); + status_trace("Adding HTLC %"PRIu64" msat=%"PRIu64" cltv=%u gave %i", + peer->htlc_id, amount_msat, cltv_expiry, e); switch (e) { case CHANNEL_ERR_ADD_OK: @@ -2022,7 +2023,7 @@ static void init_channel(struct peer *peer) &peer->node_ids[LOCAL], &peer->node_ids[REMOTE], &peer->commit_msec, - &peer->cltv_delta, + &peer->their_cltv_delta, &peer->last_was_revoke, &peer->last_sent_commit, &peer->next_index[LOCAL], diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 5ad595fde..a973e990e 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -29,7 +29,7 @@ channel_init,,seed,struct privkey channel_init,,local_node_id,struct pubkey channel_init,,remote_node_id,struct pubkey channel_init,,commit_msec,u32 -channel_init,,cltv_delta,u16 +channel_init,,their_cltv_delta,u16 channel_init,,last_was_revoke,bool channel_init,,num_last_sent_commit,u16 channel_init,,last_sent_commit,num_last_sent_commit*struct changed_htlc diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 8445192fd..ac39147b5 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2198,7 +2198,7 @@ static bool peer_start_channeld(struct peer *peer, &peer->ld->id, &peer->id, time_to_msec(cfg->commit_time), - cfg->min_htlc_expiry, + peer->channel_info->their_cltv_expiry_delta, peer->last_was_revoke, peer->last_sent_commit, peer->next_index[LOCAL], diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index f7c455620..db57f4551 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -386,6 +386,8 @@ static void handle_localpay(struct htlc_in *hin, log_info(ld->log, "Resolving invoice '%s' with HTLC %"PRIu64, invoice->label, hin->key.id); + log_debug(ld->log, "%s: Actual amount %"PRIu64"msat, HTLC expiry %u", + invoice->label, hin->msatoshi, cltv_expiry); fulfill_htlc(hin, &invoice->r); resolve_invoice(ld, invoice); return; diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 546cce71f..6af8a6ac2 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -1137,6 +1137,131 @@ class LightningDTests(BaseLightningDTests): route = copy.deepcopy(baseroute) l1.rpc.sendpay(to_json(route), rhash) + def test_forward_different_fees_and_cltv(self): + # FIXME: Check BOLT quotes here too + # BOLT #7: + # ``` + # B + # / \ + # / \ + # A C + # \ / + # \ / + # D + # ``` + # + # Each node asks for the following `cltv_expiry_delta` on every channel open: + # + # 1. A: 10 blocks + # 2. B: 20 blocks + # 3. C: 30 blocks + # 4. D: 40 blocks + # + # Also, each node has the same fee scheme which it uses for each of its + # channels: + # + # 1. A: 100 base + 1000 millionths + # 1. B: 200 base + 2000 millionths + # 1. C: 300 base + 3000 millionths + # 1. D: 400 base + 4000 millionths + + # We don't do D yet. + l1 = self.node_factory.get_node(options=['--min-htlc-expiry=10', '--fee-base=100', '--fee-per-satoshi=1000']) + l2 = self.node_factory.get_node(options=['--min-htlc-expiry=20', '--fee-base=200', '--fee-per-satoshi=2000']) + l3 = self.node_factory.get_node(options=['--min-htlc-expiry=30', '--fee-base=300', '--fee-per-satoshi=3000']) + + ret = l1.rpc.connect('localhost', l2.info['port'], l2.info['id']) + assert ret['id'] == l2.info['id'] + + l1.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER') + l2.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER') + + ret = l2.rpc.connect('localhost', l3.info['port'], l3.info['id']) + assert ret['id'] == l3.info['id'] + + l2.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER') + l3.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER') + + self.fund_channel(l1, l2, 10**6) + c1='{}:2:1'.format(bitcoind.rpc.getblockcount()) + self.fund_channel(l2, l3, 10**6) + c2='{}:2:1'.format(bitcoind.rpc.getblockcount()) + + # Allow announce messages. + l1.bitcoin.rpc.generate(5) + + # Make sure l1 has seen announce for all channels. + l1.daemon.wait_for_logs([ + 'Received channel_update for channel {}\\(0\\)'.format(c1), + 'Received channel_update for channel {}\\(1\\)'.format(c1), + 'Received channel_update for channel {}\\(0\\)'.format(c2), + 'Received channel_update for channel {}\\(1\\)'.format(c2)]) + + # BOLT #7: + # + # If B were to send 4,999,999 millisatoshi to C, it wouldn't charge + # itself a fee, but it would need to consider the `cltv_expiry_delta` in + # the B->C `channel_update` (ie. the `cltv_expiry_delta` C told B in + # `open_channel` or `accept_channel`). We also assume it adds a "shadow + # route" to give an extra CLTV of 42. + + # FIXME: Add shadow route + shadow_route=0 + route = l2.rpc.getroute(l3.info['id'], 4999999, 1)["route"] + assert len(route) == 1 + + # BOLT #7: + # + # * `amount_msat`: 4999999 + # * `cltv_expiry`: current-block-height + 30 + 42 + # * `onion_routing_packet`: + # * `amt_to_forward` = 4999999 + # * `outgoing_cltv_value` = current-block-height + 30 + 42 + assert route[0]['msatoshi'] == 4999999 + assert route[0]['delay'] == 30 + shadow_route + + # BOLT #7: + # If A were to send an 4,999,999 millisatoshi to C via B, it needs to + # pay B the fee it specified in the B->C `channel_update`, calculated as + # per [HTLC Fees](#htlc_fees): + # + # 200 + 4999999 * 2000 / 1000000 = 10199 + # + # Assume a "shadow route" CLTV of 42 again, it would set `cltv_expiry` + # to the current-block-height plus the `channel_update` amounts A->B + # (20) and B->C (30), plus 42: + # + # Thus the `update_add_htlc` message from A to B would be: + # + # * `amount_msat`: 5010198 + # * `cltv_expiry`: current-block-height + 50 + 42 + # * `onion_routing_packet`: + # * `amt_to_forward` = 4999999 + # * `outgoing_cltv_value` = current-block-height + 30 + 42 + route = l1.rpc.getroute(l3.info['id'], 4999999, 1)["route"] + assert len(route) == 2 + + assert route[0]['msatoshi'] == 5010198 + assert route[0]['delay'] == 50 + shadow_route + assert route[1]['msatoshi'] == 4999999 + assert route[1]['delay'] == 30 + shadow_route + + rhash = l3.rpc.invoice(4999999, 'test_forward_different_fees_and_cltv')['rhash'] + assert l3.rpc.listinvoice('test_forward_different_fees_and_cltv')[0]['complete'] == False + + # This should work. + l1.rpc.sendpay(to_json(route), rhash) + + # We add one to the blockcount for a bit of fuzz (FIXME: Shadowroute would fix this!) + shadow_route = 1 + l1.daemon.wait_for_log("Adding HTLC 0 msat=5010198 cltv={} gave 0" + .format(bitcoind.rpc.getblockcount() + 50 + shadow_route)) + l2.daemon.wait_for_log("Adding HTLC 0 msat=4999999 cltv={} gave 0" + .format(bitcoind.rpc.getblockcount() + 30 + shadow_route)) + l3.daemon.wait_for_log("test_forward_different_fees_and_cltv: Actual amount 4999999msat, HTLC expiry {}" + .format(bitcoind.rpc.getblockcount() + 30 + shadow_route)) + assert l3.rpc.listinvoice('test_forward_different_fees_and_cltv')[0]['complete'] == True + def test_disconnect(self): # These should all make us fail. disconnects = ['-WIRE_INIT', diff --git a/wallet/db.c b/wallet/db.c index efb850704..ea4d83a4c 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -64,6 +64,7 @@ char *dbmigrations[] = { " per_commit_remote BLOB," " old_per_commit_remote BLOB," " feerate_per_kw INTEGER," + " their_cltv_expiry_delta INTEGER," /* END channel_info */ " shachain_remote_id INTEGER," " shutdown_scriptpubkey_remote BLOB," diff --git a/wallet/wallet.c b/wallet/wallet.c index df1ab4c6f..52e3ce708 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -506,10 +506,11 @@ static bool wallet_stmt2channel(struct wallet *w, sqlite3_stmt *stmt, ok &= sqlite3_column_pubkey(stmt, col++, &channel_info->remote_per_commit); ok &= sqlite3_column_pubkey(stmt, col++, &channel_info->old_remote_per_commit); channel_info->feerate_per_kw = sqlite3_column_int64(stmt, col++); + channel_info->their_cltv_expiry_delta = sqlite3_column_int64(stmt, col++); wallet_channel_config_load(w, remote_config_id, &chan->peer->channel_info->their_config); } else { /* No channel_info, skip positions in the result */ - col += 7; + col += 8; } /* Load shachain */ @@ -549,7 +550,7 @@ static bool wallet_stmt2channel(struct wallet *w, sqlite3_stmt *stmt, col += 2; } - assert(col == 33); + assert(col == 34); chan->peer->channel = chan; @@ -568,7 +569,7 @@ const char *channel_fields = "fundingkey_remote, revocation_basepoint_remote, " "payment_basepoint_remote, " "delayed_payment_basepoint_remote, per_commit_remote, " - "old_per_commit_remote, feerate_per_kw, shachain_remote_id, " + "old_per_commit_remote, feerate_per_kw, their_cltv_expiry_delta, shachain_remote_id, " "shutdown_scriptpubkey_remote, shutdown_keyidx_local, " "last_sent_commit_state, last_sent_commit_id, " "last_tx, last_sig"; @@ -778,6 +779,7 @@ bool wallet_channel_save(struct wallet *w, struct wallet_channel *chan){ " per_commit_remote='%s'," " old_per_commit_remote='%s'," " feerate_per_kw=%d," + " their_cltv_expiry_delta=%d," " channel_config_remote=%"PRIu64 " WHERE id=%"PRIu64, db_serialize_pubkey(tmpctx, &p->channel_info->remote_fundingkey), @@ -787,6 +789,7 @@ bool wallet_channel_save(struct wallet *w, struct wallet_channel *chan){ db_serialize_pubkey(tmpctx, &p->channel_info->remote_per_commit), db_serialize_pubkey(tmpctx, &p->channel_info->old_remote_per_commit), p->channel_info->feerate_per_kw, + p->channel_info->their_cltv_expiry_delta, p->channel_info->their_config.id, chan->id); }