Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peer channel split, so we can sanely allow a peer to reopen a channel while others are closed #975

Merged
merged 21 commits into from
Feb 14, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
102a591
gossipd/test: update mocks.
rustyrussell Feb 11, 2018
677d2a0
db: don't allow newer db versions.
rustyrussell Feb 11, 2018
f937b71
channeld: rename new_channel to new_full_channel.
rustyrussell Feb 11, 2018
5e0123c
wallet: add ld pointer.
rustyrussell Feb 12, 2018
b10cf34
wallet: delete channels in state OPENINGD.
rustyrussell Feb 12, 2018
2d959d2
wallet: delete peers with no channels.
rustyrussell Feb 12, 2018
8e25301
wallet: properly handle case where peer has no address when saving ch…
rustyrussell Feb 12, 2018
b41087d
lightningd: create new structure `channel` to hold per-channel info.
rustyrussell Feb 12, 2018
84becaf
lightningd: split struct peer into struct peer and struct channel.
rustyrussell Feb 12, 2018
018c93a
lightningd: channels own the peer.
rustyrussell Feb 12, 2018
fefb026
lightningd: rename peer_fail functions to channel_fail.
rustyrussell Feb 12, 2018
544f676
subd: keep pointer to channel, not peer.
rustyrussell Feb 12, 2018
6d63093
htlc: keep channel pointer, not peer pointer.
rustyrussell Feb 12, 2018
8704434
lightningd/peer_htlcs: remove remaining peer_ shims.
rustyrussell Feb 12, 2018
e0eb5f0
lightningd: bitcoind and topology routines take channel, not peer.
rustyrussell Feb 12, 2018
32293ed
lightningd: remove almost all other peer2channel / channel2peer shims.
rustyrussell Feb 12, 2018
9d16fa5
lightningd: remove peer->log in favor of channel->log.
rustyrussell Feb 12, 2018
ea40ebe
lightningd: allow a new channel open from peer if no *active* channels.
rustyrussell Feb 12, 2018
afa19c5
channel: rename free_channel to delete_channel.
rustyrussell Feb 13, 2018
c983225
wallet: don't implicitly remove peers, but do it explicitly.
rustyrussell Feb 14, 2018
383afe8
Rename (almost) all destructors to destroy_<type>.
rustyrussell Feb 14, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions channeld/channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -2514,15 +2514,15 @@ static void init_channel(struct peer *peer)
derive_basepoints(&seed, &funding_pubkey[LOCAL], &points[LOCAL],
&peer->our_secrets, &peer->shaseed);

peer->channel = new_channel(peer, &funding_txid, funding_txout,
funding_satoshi,
local_msatoshi,
feerate_per_kw,
&peer->conf[LOCAL], &peer->conf[REMOTE],
&points[LOCAL], &points[REMOTE],
&funding_pubkey[LOCAL],
&funding_pubkey[REMOTE],
funder);
peer->channel = new_full_channel(peer, &funding_txid, funding_txout,
funding_satoshi,
local_msatoshi,
feerate_per_kw,
&peer->conf[LOCAL], &peer->conf[REMOTE],
&points[LOCAL], &points[REMOTE],
&funding_pubkey[LOCAL],
&funding_pubkey[REMOTE],
funder);

if (!channel_force_htlcs(peer->channel, htlcs, hstates,
fulfilled, fulfilled_sides,
Expand Down
26 changes: 13 additions & 13 deletions channeld/full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,19 @@
#include <inttypes.h>
#include <string.h>

struct channel *new_channel(const tal_t *ctx,
const struct bitcoin_txid *funding_txid,
unsigned int funding_txout,
u64 funding_satoshis,
u64 local_msatoshi,
const u32 feerate_per_kw[NUM_SIDES],
const struct channel_config *local,
const struct channel_config *remote,
const struct basepoints *local_basepoints,
const struct basepoints *remote_basepoints,
const struct pubkey *local_funding_pubkey,
const struct pubkey *remote_funding_pubkey,
enum side funder)
struct channel *new_full_channel(const tal_t *ctx,
const struct bitcoin_txid *funding_txid,
unsigned int funding_txout,
u64 funding_satoshis,
u64 local_msatoshi,
const u32 feerate_per_kw[NUM_SIDES],
const struct channel_config *local,
const struct channel_config *remote,
const struct basepoints *local_basepoints,
const struct basepoints *remote_basepoints,
const struct pubkey *local_funding_pubkey,
const struct pubkey *remote_funding_pubkey,
enum side funder)
{
struct channel *channel = new_initial_channel(ctx, funding_txid,
funding_txout,
Expand Down
28 changes: 14 additions & 14 deletions channeld/full_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <common/sphinx.h>

/**
* new_channel: Given initial fees and funding, what is initial state?
* new_full_channel: Given initial fees and funding, what is initial state?
* @ctx: tal context to allocate return value from.
* @funding_txid: The commitment transaction id.
* @funding_txout: The commitment transaction output number.
Expand All @@ -25,19 +25,19 @@
*
* Returns state, or NULL if malformed.
*/
struct channel *new_channel(const tal_t *ctx,
const struct bitcoin_txid *funding_txid,
unsigned int funding_txout,
u64 funding_satoshis,
u64 local_msatoshi,
const u32 feerate_per_kw[NUM_SIDES],
const struct channel_config *local,
const struct channel_config *remote,
const struct basepoints *local_basepoints,
const struct basepoints *remote_basepoints,
const struct pubkey *local_funding_pubkey,
const struct pubkey *remote_funding_pubkey,
enum side funder);
struct channel *new_full_channel(const tal_t *ctx,
const struct bitcoin_txid *funding_txid,
unsigned int funding_txout,
u64 funding_satoshis,
u64 local_msatoshi,
const u32 feerate_per_kw[NUM_SIDES],
const struct channel_config *local,
const struct channel_config *remote,
const struct basepoints *local_basepoints,
const struct basepoints *remote_basepoints,
const struct pubkey *local_funding_pubkey,
const struct pubkey *remote_funding_pubkey,
enum side funder);

/**
* channel_txs: Get the current commitment and htlc txs for the channel.
Expand Down
34 changes: 18 additions & 16 deletions channeld/test/run-full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -443,22 +443,24 @@ int main(void)
to_local_msat = 7000000000;
to_remote_msat = 3000000000;
feerate_per_kw[LOCAL] = feerate_per_kw[REMOTE] = 15000;
lchannel = new_channel(tmpctx, &funding_txid, funding_output_index,
funding_amount_satoshi, to_local_msat,
feerate_per_kw,
local_config,
remote_config,
&localbase, &remotebase,
&local_funding_pubkey, &remote_funding_pubkey,
LOCAL);
rchannel = new_channel(tmpctx, &funding_txid, funding_output_index,
funding_amount_satoshi, to_remote_msat,
feerate_per_kw,
remote_config,
local_config,
&remotebase, &localbase,
&remote_funding_pubkey, &local_funding_pubkey,
REMOTE);
lchannel = new_full_channel(tmpctx, &funding_txid, funding_output_index,
funding_amount_satoshi, to_local_msat,
feerate_per_kw,
local_config,
remote_config,
&localbase, &remotebase,
&local_funding_pubkey,
&remote_funding_pubkey,
LOCAL);
rchannel = new_full_channel(tmpctx, &funding_txid, funding_output_index,
funding_amount_satoshi, to_remote_msat,
feerate_per_kw,
remote_config,
local_config,
&remotebase, &localbase,
&remote_funding_pubkey,
&local_funding_pubkey,
REMOTE);

/* BOLT #3:
*
Expand Down
6 changes: 0 additions & 6 deletions gossipd/test/run-bench-find_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,6 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for queue_broadcast */
bool queue_broadcast(struct broadcast_state *bstate UNNEEDED,
const int type UNNEEDED,
const u8 *tag UNNEEDED,
const u8 *payload UNNEEDED)
{ fprintf(stderr, "queue_broadcast called!\n"); abort(); }
/* Generated stub for replace_broadcast */
bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
u64 *index UNNEEDED,
Expand Down
6 changes: 0 additions & 6 deletions gossipd/test/run-find_route-specific.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for queue_broadcast */
bool queue_broadcast(struct broadcast_state *bstate UNNEEDED,
const int type UNNEEDED,
const u8 *tag UNNEEDED,
const u8 *payload UNNEEDED)
{ fprintf(stderr, "queue_broadcast called!\n"); abort(); }
/* Generated stub for replace_broadcast */
bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
u64 *index UNNEEDED,
Expand Down
6 changes: 0 additions & 6 deletions gossipd/test/run-find_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for queue_broadcast */
bool queue_broadcast(struct broadcast_state *bstate UNNEEDED,
const int type UNNEEDED,
const u8 *tag UNNEEDED,
const u8 *payload UNNEEDED)
{ fprintf(stderr, "queue_broadcast called!\n"); abort(); }
/* Generated stub for replace_broadcast */
bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
u64 *index UNNEEDED,
Expand Down
4 changes: 1 addition & 3 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ static void free_peer(struct peer *peer, const char *why)
command_fail(peer->opening_cmd, "%s", why);
peer->opening_cmd = NULL;
}
wallet_channel_delete(peer->ld->wallet, peer->channel->id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this also mean we delete all channels from the DB on a clean shutdown? That'd definitely be undesirable.

I prefer having an explicit call to wallet_channel_delete when we're sure we want to forget about the channel, rather than have this hidden DB change in a destructor anyway.

tal_free(peer);
}

Expand Down Expand Up @@ -1268,7 +1269,6 @@ static void handle_irrevocably_resolved(struct peer *peer, const u8 *msg)
free_htlcs(peer->ld, peer);

log_info(peer->log, "onchaind complete, forgetting peer");
wallet_channel_delete(peer->ld->wallet, peer->channel->id);

/* This will also free onchaind. */
free_peer(peer, "onchaind complete, forgetting peer");
Expand Down Expand Up @@ -2968,8 +2968,6 @@ static void process_dev_forget_channel(struct bitcoind *bitcoind UNUSED,
json_add_txid(response, "funding_txid", forget->peer->funding_txid);
json_object_end(response);

if (peer_persists(forget->peer))
wallet_channel_delete(forget->cmd->ld->wallet, forget->peer->channel->id);
free_peer(forget->peer, "dev-forget-channel called");

command_success(forget->cmd, response);
Expand Down
2 changes: 1 addition & 1 deletion lightningd/test/run-find_my_path.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ bool wallet_htlcs_reconnect(struct wallet *wallet UNNEEDED,
bool wallet_invoice_load(struct wallet *wallet UNNEEDED)
{ fprintf(stderr, "wallet_invoice_load called!\n"); abort(); }
/* Generated stub for wallet_new */
struct wallet *wallet_new(const tal_t *ctx UNNEEDED,
struct wallet *wallet_new(struct lightningd *ld UNNEEDED,
struct log *log UNNEEDED, struct timers *timers UNNEEDED)
{ fprintf(stderr, "wallet_new called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */
Expand Down
3 changes: 3 additions & 0 deletions wallet/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ static void db_migrate(struct db *db, struct log *log)

if (current == -1)
log_info(log, "Creating database");
else if (available < current)
fatal("Refusing to migrate down from version %u to %u",
current, available);
else if (current != available)
log_info(log, "Updating database from version %u to %u",
current, available);
Expand Down
14 changes: 11 additions & 3 deletions wallet/wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
#define DIRECTION_INCOMING 0
#define DIRECTION_OUTGOING 1

struct wallet *wallet_new(const tal_t *ctx,
struct wallet *wallet_new(struct lightningd *ld,
struct log *log, struct timers *timers)
{
struct wallet *wallet = tal(ctx, struct wallet);
struct wallet *wallet = tal(ld, struct wallet);
wallet->ld = ld;
wallet->db = db_setup(wallet, log);
wallet->log = log;
wallet->bip32_base = NULL;
Expand Down Expand Up @@ -625,9 +626,16 @@ static const char *channel_fields =
bool wallet_channels_load_active(const tal_t *ctx, struct wallet *w, struct list_head *peers)
{
bool ok = true;
sqlite3_stmt *stmt;

/* Get rid of OPENINGD entries; they don't last across reconnects */
stmt = db_prepare(w->db, "DELETE FROM channels WHERE state=?");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should really be a migration and OPENINGD channels should never be added again after the migration is applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's in a future patch. For the moment we still put opening channels in the DB, so this is needed.

sqlite3_bind_int64(stmt, 1, OPENINGD);
db_exec_prepared(w->db, stmt);

/* Channels are active if they have reached at least the
* opening state and they are not marked as complete */
sqlite3_stmt *stmt = db_query(
stmt = db_query(
__func__, w->db, "SELECT %s FROM channels WHERE state > %d AND state != %d;",
channel_fields, OPENINGD, CLOSINGD_COMPLETE);

Expand Down
3 changes: 2 additions & 1 deletion wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ struct pubkey;
struct timers;

struct wallet {
struct lightningd *ld;
struct db *db;
struct log *log;
struct ext_key *bip32_base;
Expand Down Expand Up @@ -111,7 +112,7 @@ struct wallet_payment {
* This is guaranteed to either return a valid wallet, or abort with
* `fatal` if it cannot be initialized.
*/
struct wallet *wallet_new(const tal_t *ctx,
struct wallet *wallet_new(struct lightningd *ld,
struct log *log, struct timers *timers);

/**
Expand Down