closing: update to match proposal to restart negotiation on reconnect.

This simplifies significantly, as we don't need to remember what we offered.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/closing/closing.c

@@ -254,17 +254,6 @@ int main(int argc, char *argv[])
 			      &funding_pubkey[LOCAL],
 			      &sig);
 
-		/* Tell master we're making an offer, wait for db commit. */
-		msg = towire_closing_offered_signature(tmpctx, sent_fee, &sig);
-		if (!wire_sync_write(REQ_FD, msg))
-			status_failed(WIRE_CLOSING_INTERNAL_ERROR,
-				      "Writing offer to master failed: %s",
-				      strerror(errno));
-		msg = wire_sync_read(tmpctx, REQ_FD);
-		if (!fromwire_closing_offered_signature_reply(msg, NULL))
-			status_failed(WIRE_CLOSING_INTERNAL_ERROR,
-				      "Reading offer reply from master failed");
-
 		status_trace("sending fee offer %"PRIu64, sent_fee);
 
 		/* Now send closing offer */
@@ -370,13 +359,21 @@ int main(int argc, char *argv[])
 
 		status_trace("Received fee offer %"PRIu64, received_fee);
 
+		/* BOLT #2:
+		 *
+		 * Otherwise, the recipient MUST fail the connection if
+		 * `fee_satoshis` is greater than the base fee of the final
+		 * commitment transaction as calculated in [BOLT #3] */
+		if (received_fee > maxfee)
+			status_failed(WIRE_CLOSING_PEER_BAD_MESSAGE,
+				      "Bad closing_signed fee %"PRIu64
+				      " > %"PRIu64,
+				      received_fee, maxfee);
+
 		/* Is fee reasonable?  Tell master. */
 		if (received_fee < minfee) {
 			status_trace("Fee too low, below %"PRIu64, minfee);
 			limit_fee = minfee;
-		} else if (received_fee > maxfee) {
-			status_trace("Fee too high, above %"PRIu64, maxfee);
-			limit_fee = maxfee;
 		} else {
 			status_trace("Fee accepted.");
 			msg = towire_closing_received_signature(tmpctx,
@@ -402,8 +399,12 @@ int main(int argc, char *argv[])
 		if (received_fee == sent_fee)
 			break;
 
-		/* Check that they moved in right direction.  Not really
-		 * a requirement that we check, but good to catch their bugs. */
+		/* BOLT #2:
+		 *
+		 * the recipient SHOULD fail the connection if `fee_satoshis`
+		 * is not strictly between its last-sent `fee_satoshis` and
+		 * its previously-received `fee_satoshis`, unless it has
+		 * reconnected since then. */
 		if (last_received_fee != -1) {
 			bool previous_dir = sent_fee < last_received_fee;
 			bool dir = received_fee < last_received_fee;

lightningd/closing/closing_wire.csv

@@ -47,12 +47,5 @@ closing_received_signature,,signature,secp256k1_ecdsa_signature
 
 closing_received_signature_reply,102
 
-# We sent an offer, save it in case we crash.
-closing_offered_signature,3
-closing_offered_signature,,fee_satoshi,u64
-closing_offered_signature,,signature,secp256k1_ecdsa_signature
-
-closing_offered_signature_reply,103
-
 # Negotiations complete, we're exiting.
 closing_complete,4

lightningd/peer_control.c

@@ -479,7 +479,7 @@ void add_peer(struct lightningd *ld, u64 unique_id,
 		= peer->num_revocations_received = 0;
 	peer->next_htlc_id = 0;
 	shachain_init(&peer->their_shachain);
-	peer->closing_sig_sent = peer->closing_sig_received = NULL;
+	peer->closing_sig_received = NULL;
 
 	idname = type_to_string(peer, struct pubkey, id);
 
@@ -1185,6 +1185,32 @@ static int closingd_got_negotiation_error(struct peer *peer, const u8 *msg)
 	return -1;
 }
 
+static bool better_closing_fee(struct peer *peer, u64 fee_satoshi)
+{
+	/* FIXME: Use estimatefee 24 or something? */
+	u64 min_feerate = get_feerate(peer->ld->topology) / 2;
+	/* FIXME: Real fee, using real tx, and estimatefee 6 */
+	u64 ideal_fee = commit_tx_base_fee(get_feerate(peer->ld->topology), 0);
+	s64 old_diff, new_diff;
+
+	/* FIXME: Use real tx here! (+ 74 for sig). */
+	if (fee_satoshi < commit_tx_base_fee(min_feerate, 0))
+		return false;
+
+	/* FIXME: Derive old fee from last tx, which would work even
+	 * for the case where we're using the final commitment tx. */
+	if (!peer->closing_sig_received)
+		return true;
+
+	/* FIXME: Real fee, using real tx, and estimatefee 6 */
+
+	/* We prefer fee which is closest to our ideal. */
+	old_diff = imaxabs((s64)ideal_fee - (s64)peer->closing_fee_received);
+	new_diff = imaxabs((s64)ideal_fee - (s64)fee_satoshi);
+
+	return (new_diff < old_diff);
+}
+
 static int peer_received_closing_signature(struct peer *peer, const u8 *msg)
 {
 	u64 fee_satoshi;
@@ -1197,26 +1223,16 @@ static int peer_received_closing_signature(struct peer *peer, const u8 *msg)
 		return -1;
 	}
 
-	/* If we were only doing this to retransmit, we should only send one. */
-	if (peer->state == CLOSINGD_COMPLETE) {
-		if (fee_satoshi != peer->closing_fee_sent) {
-			peer_internal_error(peer,
-					    "CLOSINGD_COMPLETE:"
-					    " Bad offer %"PRIu64
-					    " not %"PRIu64,
-					    fee_satoshi, peer->closing_fee_sent);
-			return -1;
-		}
-	}
-
-	/* FIXME: Make sure offer is in useful range! */
 	/* FIXME: Make sure signature is correct! */
-	/* FIXME: save to db. */
 
-	peer->closing_fee_received = fee_satoshi;
-	tal_free(peer->closing_sig_received);
-	peer->closing_sig_received
-		= tal_dup(peer, secp256k1_ecdsa_signature, &sig);
+	if (better_closing_fee(peer, fee_satoshi)) {
+		/* FIXME: save to db. */
+
+		peer->closing_fee_received = fee_satoshi;
+		tal_free(peer->closing_sig_received);
+		peer->closing_sig_received
+			= tal_dup(peer, secp256k1_ecdsa_signature, &sig);
+	}
 
 	/* OK, you can continue now. */
 	subd_send_msg(peer->owner,
@@ -1224,45 +1240,14 @@ static int peer_received_closing_signature(struct peer *peer, const u8 *msg)
 	return 0;
 }
 
-static int peer_offered_closing_signature(struct peer *peer, const u8 *msg)
-{
-	u64 fee_satoshi;
-	secp256k1_ecdsa_signature sig;
-
-	if (!fromwire_closing_offered_signature(msg, NULL, &fee_satoshi, &sig)) {
-		peer_internal_error(peer, "Bad closing_offered_signature %s",
-				    tal_hex(peer, msg));
-		return -1;
-	}
-
-	/* If we were only doing this to retransmit, we ignore its offer. */
-	if (peer->state == CLOSINGD_COMPLETE) {
-		log_debug(peer->log,
-			  "CLOSINGD_COMPLETE: Ignoring their offer %"PRIu64,
-			  fee_satoshi);
-	} else {
-		/* FIXME: Make sure offer is in useful range! */
-		/* FIXME: Make sure signature is correct! */
-		/* FIXME: save to db. */
-
-		peer->closing_fee_sent = fee_satoshi;
-		tal_free(peer->closing_sig_sent);
-		peer->closing_sig_sent
-			= tal_dup(peer, secp256k1_ecdsa_signature, &sig);
-	}
-
-	/* OK, you can continue now. */
-	subd_send_msg(peer->owner,
-		      take(towire_closing_offered_signature_reply(peer)));
-	return 0;
-}
-
 static int peer_closing_complete(struct peer *peer, const u8 *msg)
 {
 	struct bitcoin_tx *tx;
-	u8 *local_scriptpubkey;
+	u8 *local_scriptpubkey, *funding_wscript;
 	u64 out_amounts[NUM_SIDES];
 	struct pubkey local_funding_pubkey;
+	struct secrets secrets;
+	secp256k1_ecdsa_signature sig;
 
 	if (!fromwire_closing_complete(msg, NULL)) {
 		peer_internal_error(peer, "Bad closing_complete %s",
@@ -1270,12 +1255,6 @@ static int peer_closing_complete(struct peer *peer, const u8 *msg)
 		return -1;
 	}
 
-	if (!peer->closing_sig_sent) {
-		peer_internal_error(peer,
-				    "closing_complete without receiving sig!");
-		return -1;
-	}
-
 	/* Retransmission only, ignore closing. */
 	if (peer->state == CLOSINGD_COMPLETE)
 		return -1;
@@ -1303,7 +1282,8 @@ static int peer_closing_complete(struct peer *peer, const u8 *msg)
 		- (*peer->our_msatoshi / 1000);
 	out_amounts[peer->funder] -= peer->closing_fee_received;
 
-	derive_basepoints(peer->seed, &local_funding_pubkey, NULL, NULL, NULL);
+	derive_basepoints(peer->seed, &local_funding_pubkey, NULL, &secrets,
+			  NULL);
 
 	tx = create_close_tx(msg, local_scriptpubkey,
 			     peer->remote_shutdown_scriptpubkey,
@@ -1314,10 +1294,18 @@ static int peer_closing_complete(struct peer *peer, const u8 *msg)
 			     out_amounts[REMOTE],
 			     peer->our_config.dust_limit_satoshis);
 
+	funding_wscript = bitcoin_redeem_2of2(msg,
+					      &local_funding_pubkey,
+					      &peer->channel_info->remote_fundingkey);
+	sign_tx_input(tx, 0, NULL, funding_wscript,
+		      &secrets.funding_privkey,
+		      &local_funding_pubkey,
+		      &sig);
+
 	tx->input[0].witness
 		= bitcoin_witness_2of2(tx->input,
 				       peer->closing_sig_received,
-				       peer->closing_sig_sent,
+				       &sig,
 				       &peer->channel_info->remote_fundingkey,
 				       &local_funding_pubkey);
 
@@ -1351,16 +1339,12 @@ static int closing_msg(struct subd *sd, const u8 *msg, const int *fds)
 	case WIRE_CLOSING_RECEIVED_SIGNATURE:
 		return peer_received_closing_signature(sd->peer, msg);
 
-	case WIRE_CLOSING_OFFERED_SIGNATURE:
-		return peer_offered_closing_signature(sd->peer, msg);
-
 	case WIRE_CLOSING_COMPLETE:
 		return peer_closing_complete(sd->peer, msg);
 
 	/* We send these, not receive them */
 	case WIRE_CLOSING_INIT:
 	case WIRE_CLOSING_RECEIVED_SIGNATURE_REPLY:
-	case WIRE_CLOSING_OFFERED_SIGNATURE_REPLY:
 		break;
 	}
 
@@ -1412,13 +1396,10 @@ static void peer_start_closingd(struct peer *peer,
 		return;
 	}
 
-	/* FIXME: Real fees! */
 	maxfee = commit_tx_base_fee(get_feerate(peer->ld->topology), 0);
+	/* FIXME: Real fees! */
 	minfee = maxfee / 2;
-	if (peer->closing_sig_sent)
-		startfee = peer->closing_fee_sent;
-	else
-		startfee = (maxfee + minfee)/2;
+	startfee = (maxfee + minfee)/2;
 
 	/* BOLT #3:
 	 *

lightningd/peer_control.h

@@ -87,8 +87,8 @@ struct peer {
 	s64 local_shutdown_idx;
 
 	/* Closing stuff. */
-	u64 closing_fee_received, closing_fee_sent;
-	secp256k1_ecdsa_signature *closing_sig_sent, *closing_sig_received;
+	u64 closing_fee_received;
+	secp256k1_ecdsa_signature *closing_sig_received;
 
 	/* Reestablishment stuff: last sent commit and revocation details. */
 	bool last_was_revoke;