diff --git a/daemon/commit_tx.c b/daemon/commit_tx.c index 76d6c591f..1816f0783 100644 --- a/daemon/commit_tx.c +++ b/daemon/commit_tx.c @@ -111,29 +111,32 @@ u8 *commit_output_to_them(const tal_t *ctx, } } -static void add_output(struct bitcoin_tx *tx, u8 *script, u64 amount, +static bool add_output(struct bitcoin_tx *tx, u8 *script, u64 amount, u64 *total) { assert(tx->output_count < tal_count(tx->output)); if (is_dust(amount)) - return; + return false; tx->output[tx->output_count].script = script; tx->output[tx->output_count].script_length = tal_count(script); tx->output[tx->output_count].amount = amount; tx->output_count++; (*total) += amount; + return true; } struct bitcoin_tx *create_commit_tx(const tal_t *ctx, struct peer *peer, const struct sha256 *rhash, const struct channel_state *cstate, - enum htlc_side side) + enum htlc_side side, + bool *otherside_only) { struct bitcoin_tx *tx; uint64_t total = 0; struct htlc_map_iter it; struct htlc *h; + bool pays_to[2]; int committed_flag = HTLC_FLAG(side,HTLC_F_COMMITTED); /* Now create commitment tx: one input, two outputs (plus htlcs) */ @@ -145,21 +148,31 @@ struct bitcoin_tx *create_commit_tx(const tal_t *ctx, tx->input[0].amount = tal_dup(tx->input, u64, &peer->anchor.satoshis); tx->output_count = 0; - add_output(tx, commit_output_to_us(tx, peer, rhash, side, NULL), - cstate->side[OURS].pay_msat / 1000, &total); - add_output(tx, commit_output_to_them(tx, peer, rhash, side, NULL), - cstate->side[THEIRS].pay_msat / 1000, &total); + pays_to[LOCAL] = add_output(tx, commit_output_to_us(tx, peer, rhash, + side, NULL), + cstate->side[OURS].pay_msat / 1000, + &total); + pays_to[REMOTE] = add_output(tx, commit_output_to_them(tx, peer, rhash, + side, NULL), + cstate->side[THEIRS].pay_msat / 1000, + &total); + + /* If their tx doesn't pay to them, or our tx doesn't pay to us... */ + *otherside_only = !pays_to[side]; /* First two outputs done, now for the HTLCs. */ for (h = htlc_map_first(&peer->htlcs, &it); h; h = htlc_map_next(&peer->htlcs, &it)) { + const u8 *wscript; + if (!htlc_has(h, committed_flag)) continue; - add_output(tx, scriptpubkey_p2wsh(tx, - wscript_for_htlc(tx, peer, h, - rhash, side)), - h->msatoshis / 1000, &total); + wscript = wscript_for_htlc(tx, peer, h, rhash, side); + /* If we pay any HTLC, it's txout is not just to other side. */ + if (add_output(tx, scriptpubkey_p2wsh(tx, wscript), + h->msatoshis / 1000, &total)) + *otherside_only = false; } assert(total <= peer->anchor.satoshis); diff --git a/daemon/commit_tx.h b/daemon/commit_tx.h index 3101ce968..dd0cdb1f2 100644 --- a/daemon/commit_tx.h +++ b/daemon/commit_tx.h @@ -34,5 +34,6 @@ struct bitcoin_tx *create_commit_tx(const tal_t *ctx, struct peer *peer, const struct sha256 *rhash, const struct channel_state *cstate, - enum htlc_side side); + enum htlc_side side, + bool *otherside_only); #endif diff --git a/daemon/packets.c b/daemon/packets.c index 3bcea2822..a6194142f 100644 --- a/daemon/packets.c +++ b/daemon/packets.c @@ -185,7 +185,11 @@ void queue_pkt_commit(struct peer *peer, const struct bitcoin_signature *sig) /* Now send message */ update_commit__init(u); - u->sig = signature_to_proto(u, peer->dstate->secpctx, &sig->sig); + if (sig) + u->sig = signature_to_proto(u, peer->dstate->secpctx, + &sig->sig); + else + u->sig = NULL; queue_pkt(peer, PKT__PKT_UPDATE_COMMIT, u); } @@ -479,6 +483,15 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt, { const UpdateCommit *c = pkt->update_commit; + if (!c->sig && sig) + return pkt_err(peer, "Expected signature"); + + if (!sig && c->sig) + return pkt_err(peer, "Unexpected signature"); + + if (!sig && !c->sig) + return NULL; + sig->stype = SIGHASH_ALL; if (!proto_to_signature(peer->dstate->secpctx, c->sig, &sig->sig)) return pkt_err(peer, "Malformed signature"); diff --git a/daemon/peer.c b/daemon/peer.c index b6a553070..6d33c7d02 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -290,7 +290,7 @@ static void peer_breakdown(struct peer *peer) log_unusual(peer->log, "Peer breakdown: sending close tx"); broadcast_tx(peer, bitcoin_close(peer)); /* If we have a signed commit tx (maybe not if we just offered - * anchor, or they supplied anchor). */ + * anchor, or they supplied anchor, or no outputs to us). */ } else if (peer->local.commit->sig) { log_unusual(peer->log, "Peer breakdown: sending commit tx"); broadcast_tx(peer, bitcoin_commit(peer)); @@ -769,6 +769,7 @@ static Pkt *handle_pkt_commit(struct peer *peer, const Pkt *pkt) Pkt *err; struct sha256 preimage; struct commit_info *ci; + bool to_them_only; /* FIXME: We can actually merge these two... */ static const struct state_table commit_changes[] = { { RCVD_ADD_REVOCATION, RCVD_ADD_ACK_COMMIT }, @@ -784,10 +785,6 @@ static Pkt *handle_pkt_commit(struct peer *peer, const Pkt *pkt) }; ci = new_commit_info(peer, peer->local.commit->commit_num + 1); - ci->sig = tal(ci, struct bitcoin_signature); - err = accept_pkt_commit(peer, pkt, ci->sig); - if (err) - return err; /* BOLT #2: * @@ -808,21 +805,35 @@ static Pkt *handle_pkt_commit(struct peer *peer, const Pkt *pkt) /* (We already applied them to staging_cstate as we went) */ ci->cstate = copy_cstate(ci, peer->local.staging_cstate); ci->tx = create_commit_tx(ci, peer, &ci->revocation_hash, - ci->cstate, LOCAL); + ci->cstate, LOCAL, &to_them_only); bitcoin_txid(ci->tx, &ci->txid); + /* BOLT #2: + * + * If the commitment transaction has only a single output which pays + * to the other node, `sig` MUST be unset. Otherwise, a sending node + * MUST apply all remote acked and unacked changes except unacked fee + * changes to the remote commitment before generating `sig`. + */ + if (!to_them_only) + ci->sig = tal(ci, struct bitcoin_signature); + + err = accept_pkt_commit(peer, pkt, ci->sig); + if (err) + return err; + /* BOLT #2: * * A receiving node MUST apply all local acked and unacked changes * except unacked fee changes to the local commitment, then it MUST * check `sig` is valid for that transaction. */ - if (!check_tx_sig(peer->dstate->secpctx, - ci->tx, 0, - NULL, 0, - peer->anchor.witnessscript, - &peer->remote.commitkey, - ci->sig)) + if (ci->sig && !check_tx_sig(peer->dstate->secpctx, + ci->tx, 0, + NULL, 0, + peer->anchor.witnessscript, + &peer->remote.commitkey, + ci->sig)) return pkt_err(peer, "Bad signature"); /* Switch to the new commitment. */ @@ -835,7 +846,7 @@ static Pkt *handle_pkt_commit(struct peer *peer, const Pkt *pkt) /* Now, send the revocation. */ /* We have their signature on the current one, right? */ - assert(peer->local.commit->sig); + assert(to_them_only || peer->local.commit->sig); assert(peer->local.commit->commit_num > 0); if (!htlcs_changestate(peer, revocation_changes, @@ -1666,13 +1677,6 @@ static void retransmit_pkts(struct peer *peer, s64 ack) */ while (ack < peer->order_counter) { if (peer->remote.commit && ack == peer->remote.commit->order) { - if (!peer->remote.commit->sig) { - log_broken(peer->log, "No sig for commit order %" - PRIu64, ack); - peer_comms_err(peer, - pkt_err(peer, "invalid ack")); - return; - } /* BOLT #2: * * Before retransmitting `update_commit`, the node @@ -1783,6 +1787,7 @@ static void do_commit(struct peer *peer, struct command *jsoncmd) { SENT_ADD_REVOCATION, SENT_ADD_ACK_COMMIT}, { SENT_REMOVE_HTLC, SENT_REMOVE_COMMIT} }; + bool to_us_only; /* We can have changes we suggested, or changes they suggested. */ if (!peer_uncommitted_changes(peer)) { @@ -1818,25 +1823,27 @@ static void do_commit(struct peer *peer, struct command *jsoncmd) ci->revocation_hash = peer->remote.next_revocation_hash; /* BOLT #2: * - * A sending node MUST apply all remote acked and unacked + * ...a sending node MUST apply all remote acked and unacked * changes except unacked fee changes to the remote commitment * before generating `sig`. */ ci->cstate = copy_cstate(ci, peer->remote.staging_cstate); ci->tx = create_commit_tx(ci, peer, &ci->revocation_hash, - ci->cstate, REMOTE); + ci->cstate, REMOTE, &to_us_only); bitcoin_txid(ci->tx, &ci->txid); - log_debug(peer->log, "Signing tx for %u/%u msatoshis, %u/%u htlcs (%u non-dust)", - ci->cstate->side[OURS].pay_msat, - ci->cstate->side[THEIRS].pay_msat, - ci->cstate->side[OURS].num_htlcs, - ci->cstate->side[THEIRS].num_htlcs, - ci->cstate->num_nondust); - log_add_struct(peer->log, " (txid %s)", struct sha256_double, &ci->txid); + if (!to_us_only) { + log_debug(peer->log, "Signing tx for %u/%u msatoshis, %u/%u htlcs (%u non-dust)", + ci->cstate->side[OURS].pay_msat, + ci->cstate->side[THEIRS].pay_msat, + ci->cstate->side[OURS].num_htlcs, + ci->cstate->side[THEIRS].num_htlcs, + ci->cstate->num_nondust); + log_add_struct(peer->log, " (txid %s)", struct sha256_double, &ci->txid); - ci->sig = tal(ci, struct bitcoin_signature); - ci->sig->stype = SIGHASH_ALL; - peer_sign_theircommit(peer, ci->tx, &ci->sig->sig); + ci->sig = tal(ci, struct bitcoin_signature); + ci->sig->stype = SIGHASH_ALL; + peer_sign_theircommit(peer, ci->tx, &ci->sig->sig); + } /* Switch to the new commitment. */ tal_free(peer->remote.commit); @@ -3523,6 +3530,8 @@ const struct bitcoin_tx *bitcoin_anchor(struct peer *peer) * insufficient funds. */ bool setup_first_commit(struct peer *peer) { + bool to_them_only, to_us_only; + assert(!peer->local.commit->tx); assert(!peer->remote.commit->tx); @@ -3549,14 +3558,18 @@ bool setup_first_commit(struct peer *peer) peer, &peer->local.commit->revocation_hash, peer->local.commit->cstate, - LOCAL); + LOCAL, &to_them_only); bitcoin_txid(peer->local.commit->tx, &peer->local.commit->txid); peer->remote.commit->tx = create_commit_tx(peer->remote.commit, peer, &peer->remote.commit->revocation_hash, peer->remote.commit->cstate, - REMOTE); + REMOTE, &to_us_only); + assert(to_them_only != to_us_only); + + /* If we offer anchor, their commit is to-us only. */ + assert(to_us_only == (peer->local.offer_anchor == CMD_OPEN_WITH_ANCHOR)); bitcoin_txid(peer->remote.commit->tx, &peer->remote.commit->txid); peer->local.staging_cstate = copy_cstate(peer, peer->local.commit->cstate); diff --git a/lightning.pb-c.c b/lightning.pb-c.c index 29ffedd53..a30658e87 100644 --- a/lightning.pb-c.c +++ b/lightning.pb-c.c @@ -2270,7 +2270,7 @@ static const ProtobufCFieldDescriptor update_commit__field_descriptors[1] = { "sig", 1, - PROTOBUF_C_LABEL_REQUIRED, + PROTOBUF_C_LABEL_OPTIONAL, PROTOBUF_C_TYPE_MESSAGE, 0, /* quantifier_offset */ offsetof(UpdateCommit, sig), diff --git a/lightning.pb-c.h b/lightning.pb-c.h index dd617a8af..06732c67a 100644 --- a/lightning.pb-c.h +++ b/lightning.pb-c.h @@ -439,7 +439,7 @@ struct _UpdateCommit { ProtobufCMessage base; /* - * Signature for your new commitment tx. + * Signature for your new commitment tx (if any outputs are HTLCs or to you) */ Signature *sig; }; diff --git a/lightning.proto b/lightning.proto index 9373dd2a6..4e8d2909c 100644 --- a/lightning.proto +++ b/lightning.proto @@ -180,8 +180,8 @@ message update_fail_htlc { // Commit all the staged changes. message update_commit { - // Signature for your new commitment tx. - required signature sig = 1; + // Signature for your new commitment tx (if any outputs are HTLCs or to you) + optional signature sig = 1; } // Complete the update. diff --git a/test/test_protocol.c b/test/test_protocol.c index 247de4500..c41cbd62f 100644 --- a/test/test_protocol.c +++ b/test/test_protocol.c @@ -648,7 +648,7 @@ static void send_commit(struct peer *peer) /* BOLT #2: * - * A sending node MUST apply all remote acked and unacked + * ...a sending node MUST apply all remote acked and unacked * changes except unacked fee changes to the remote commitment * before generating `sig`. */