diff --git a/daemon/peer.c b/daemon/peer.c index e949a0fcf..598e2e121 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -1837,42 +1837,6 @@ static void retransmit_updates(struct peer *peer) assert(!peer->feechanges[SENT_FEECHANGE]); } -/* FIXME: Maybe it would be neater to remember all pay commands, and simply - * re-run them after reconnect if they didn't get committed. */ -static void resend_local_requests(struct peer *peer) -{ - struct htlc_map_iter it; - struct htlc *h; - - for (h = htlc_map_first(&peer->htlcs, &it); - h; - h = htlc_map_next(&peer->htlcs, &it)) { - switch (h->state) { - case SENT_ADD_HTLC: - /* We removed everything which was routed. */ - assert(!h->src); - log_debug(peer->log, "Re-sending local add HTLC %"PRIu64, - h->id); - queue_pkt_htlc_add(peer, h); - remote_changes_pending(peer); - break; - case SENT_REMOVE_HTLC: - /* We removed everything which was routed. */ - assert(!h->src); - log_debug(peer->log, "Re-sending local %s HTLC %"PRIu64, - h->r ? "fulfill" : "fail", h->id); - if (h->r) - queue_pkt_htlc_fulfill(peer, h); - else - queue_pkt_htlc_fail(peer, h); - remote_changes_pending(peer); - break; - default: - break; - } - } -} - /* BOLT #2: * * On disconnection, a node MUST reverse any uncommitted changes sent by the @@ -1926,13 +1890,6 @@ again: h = htlc_map_next(&peer->htlcs, &it)) { switch (h->state) { case SENT_ADD_HTLC: - /* FIXME: re-submit these after connect, instead? */ - /* Keep local adds. */ - if (!h->src) { - if (!cstate_add_htlc(peer->remote.staging_cstate, h)) - fatal("Could not add HTLC?"); - break; - } /* Adjust counter to lowest HTLC removed */ if (peer->htlc_id_counter > h->id) { log_debug(peer->log, @@ -1955,17 +1912,6 @@ again: SENT_ADD_ACK_REVOCATION); break; case SENT_REMOVE_HTLC: - /* Keep local removes. */ - /* FIXME: re-submit these after connect, instead? */ - if (!h->src) { - if (h->r) { - cstate_fulfill_htlc(peer->remote.staging_cstate, - h); - } else { - cstate_fail_htlc(peer->remote.staging_cstate, h); - } - break; - } log_debug(peer->log, "Undoing %s %"PRIu64, htlc_state_name(h->state), h->id); htlc_undostate(h, SENT_REMOVE_HTLC, @@ -2061,8 +2007,6 @@ static void retransmit_pkts(struct peer *peer, s64 ack) ack++; } - resend_local_requests(peer); - /* We might need to re-propose HTLCs which were from other peers. */ retry_all_routing(peer); } @@ -4197,7 +4141,7 @@ static void json_getpeers(struct command *cmd, json_add_pubkey(response, cmd->dstate->secpctx, "peerid", p->id); - json_add_bool(response, "connected", p->conn && !p->fake_close); + json_add_bool(response, "connected", p->connected); /* FIXME: Report anchor. */ @@ -4352,6 +4296,11 @@ static void json_newhtlc(struct command *cmd, return; } + if (!peer->connected) { + command_fail(cmd, "peer not connected"); + return; + } + if (!json_tok_u64(buffer, msatoshistok, &msatoshis)) { command_fail(cmd, "'%.*s' is not a valid number", (int)(msatoshistok->end - msatoshistok->start), @@ -4427,6 +4376,11 @@ static void json_fulfillhtlc(struct command *cmd, return; } + if (!peer->connected) { + command_fail(cmd, "peer not connected"); + return; + } + if (!json_tok_u64(buffer, idtok, &id)) { command_fail(cmd, "'%.*s' is not a valid id", (int)(idtok->end - idtok->start), @@ -4508,6 +4462,11 @@ static void json_failhtlc(struct command *cmd, return; } + if (!peer->connected) { + command_fail(cmd, "peer not connected"); + return; + } + if (!json_tok_u64(buffer, idtok, &id)) { command_fail(cmd, "'%.*s' is not a valid id", (int)(idtok->end - idtok->start), @@ -4566,6 +4525,11 @@ static void json_commit(struct command *cmd, return; } + if (!peer->connected) { + command_fail(cmd, "peer not connected"); + return; + } + if (!state_can_commit(peer->state)) { command_fail(cmd, "peer in state %s", state_name(peer->state)); return; @@ -4677,6 +4641,7 @@ static void json_disconnect(struct command *cmd, * one side to freak out. We just ensure we ignore it. */ log_debug(peer->log, "Pretending connection is closed"); peer->fake_close = true; + peer->connected = false; set_peer_state(peer, STATE_ERR_BREAKDOWN, "json_disconnect", false); peer_breakdown(peer); diff --git a/daemon/test/test.sh b/daemon/test/test.sh index c81193fd9..9b412ba13 100755 --- a/daemon/test/test.sh +++ b/daemon/test/test.sh @@ -103,6 +103,19 @@ else SHOW="cat" fi +# Peer $1 -> $2's htlc $3 is in state $4 +htlc_is_state() +{ + if [ $# != 4 ]; then echo "htlc_is_state got $# ARGS: $@" >&2; exit 1; fi + $1 gethtlcs $2 true | tr -s '\012\011\" ' ' ' | $FGREP "id : $3, state : $4 ," >&2 +} + +# Peer $1 -> $2's htlc $3 exists +htlc_exists() +{ + $1 gethtlcs $2 true | tr -s '\012\011\" ' ' ' | $FGREP "id : $3," >&2 +} + lcli1() { if [ -n "$VERBOSE" ]; then @@ -130,31 +143,51 @@ lcli1() reconnect) [ -z "$VERBOSE" ] || echo RECONNECTING >&2 $LCLI1 dev-reconnect $ID2 >/dev/null - sleep 1 ;; restart) [ -z "$VERBOSE" ] || echo RESTARTING >&2 - # FIXME: Instead, check if command was committed, and - # if not, resubmit! - if [ "$1" = "newhtlc" ]; then - $LCLI1 commit $ID2 >/dev/null 2>&1 || true - fi $LCLI1 -- dev-restart $LIGHTNINGD1 >/dev/null 2>&1 || true if ! check "$LCLI1 getlog 2>/dev/null | fgrep -q Hello"; then echo "dev-restart failed!">&2 exit 1 fi - # These are safe to resubmit, will simply fail. - if [ "$1" = "fulfillhtlc" -o "$1" = "failhtlc" ]; then - if [ -z "$VERBOSE" ]; then - $LCLI1 "$@" >/dev/null 2>&1 || true - else - echo "Rerunning $LCLI1 $@" >&2 - $LCLI1 "$@" 2>&1 || true - fi - fi ;; esac + # Wait for reconnect; + if ! check "$LCLI1 getpeers | tr -s '\012\011\" ' ' ' | fgrep -q 'connected : true'"; then + echo "Failed to reconnect!">&2 + exit 1 + fi + + if [ "$1" = "newhtlc" ]; then + # It might have gotten committed, or might be forgotten. + ID=`echo "$OUT" | extract_id` + if ! htlc_exists "$LCLI1" $2 $ID; then + if [ -z "$VERBOSE" ]; then + $LCLI1 "$@" >/dev/null 2>&1 || true + else + echo "Rerunning $LCLI1 $@" >&2 + $LCLI1 "$@" >&2 || true + fi + fi + # Make sure it's confirmed before we run next command, + # in case *that* restarts (unless manual commit) + [ -n "$MANUALCOMMIT" ] || check ! htlc_is_state \'"$LCLI1"\' $2 $ID SENT_ADD_HTLC + # Removals may also be forgotten. + elif [ "$1" = "fulfillhtlc" -o "$1" = "failhtlc" ]; then + ID="$3" + if htlc_is_state "$LCLI1" $2 $ID RCVD_ADD_ACK_REVOCATION; then + if [ -z "$VERBOSE" ]; then + $LCLI1 "$@" >/dev/null 2>&1 || true + else + echo "Rerunning $LCLI1 $@" >&2 + $LCLI1 "$@" >&2 || true + fi + # Make sure it's confirmed before we run next command, + # in case *that* restarts. + [ -n "$MANUALCOMMIT" ] || check ! htlc_is_state \'"$LCLI1"\' $2 $ID SENT_REMOVE_HTLC + fi + fi ;; esac fi @@ -767,13 +800,13 @@ check_status $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE "" # Now, two HTLCs at once, one from each direction. # Both sides can afford this. HTLC_AMOUNT=1000000 -HTLCID=`lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH | extract_id` SECRET2=1de08917a61cb2b62ed5937d38577f6a7bfe59c176781c6d8128018e8b5ccdfe RHASH2=`lcli1 dev-rhash $SECRET2 | sed 's/.*"\([0-9a-f]*\)".*/\1/'` +HTLCID=`lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH | extract_id` HTLCID2=`lcli2 newhtlc $ID1 $HTLC_AMOUNT $EXPIRY $RHASH2 | extract_id` -[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 +[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 check_status $(($A_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE)) $(($A_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , state : SENT_ADD_ACK_REVOCATION } " $(($B_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE)) $(($B_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH2 , state : RCVD_ADD_ACK_REVOCATION } " @@ -812,8 +845,8 @@ if [ -n "$CLOSE_WITH_HTLCS" ]; then all_ok fi -lcli2 failhtlc $ID1 $HTLCID lcli1 fulfillhtlc $ID2 $HTLCID2 $SECRET2 +lcli2 failhtlc $ID1 $HTLCID [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 @@ -879,6 +912,11 @@ case "$RECONNECT" in ;; esac +if ! check "$LCLI2 getpeers | tr -s '\012\011\" ' ' ' | fgrep -q 'connected : true'"; then + echo "Failed to reconnect!">&2 + exit 1 +fi + # Node2 collects the HTLCs. lcli2 fulfillhtlc $ID1 $HTLCID $SECRET lcli2 fulfillhtlc $ID1 $HTLCID2 $SECRET2