lukechilds
/
lightning
mirror of https://github.com/lukechilds/lightning.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

daemon: don't restart newhtlc/failhtlc/fulfill htlc commands on reconnect, 

These low level commands we restarted on reconnect for ease of
testing.  Don't do that, and check that we're connected when those
commands occur.

This introduces subtle issues with --manual-commit --reconnect: restarting
node1 also forgets uncommitted things from node2, requiring reordering for
some tests.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ppa-0.6.1
Rusty Russell 9 years ago
parent
92187ae5e7
commit
d964ad2d94
2 changed files with 78 additions and 75 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 79
      daemon/peer.c
  2. 74
      daemon/test/test.sh

79
daemon/peer.c
View File

@ -1837,42 +1837,6 @@ static void retransmit_updates(struct peer *peer)
assert(!peer->feechanges[SENT_FEECHANGE]); 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: /* BOLT #2:
* *
* On disconnection, a node MUST reverse any uncommitted changes sent by the * On disconnection, a node MUST reverse any uncommitted changes sent by the
@ -1926,13 +1890,6 @@ again:
h = htlc_map_next(&peer->htlcs, &it)) { h = htlc_map_next(&peer->htlcs, &it)) {
switch (h->state) { switch (h->state) {
case SENT_ADD_HTLC: 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 */ /* Adjust counter to lowest HTLC removed */
if (peer->htlc_id_counter > h->id) { if (peer->htlc_id_counter > h->id) {
log_debug(peer->log, log_debug(peer->log,
@ -1955,17 +1912,6 @@ again:
SENT_ADD_ACK_REVOCATION); SENT_ADD_ACK_REVOCATION);
break; break;
case SENT_REMOVE_HTLC: 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, log_debug(peer->log, "Undoing %s %"PRIu64,
htlc_state_name(h->state), h->id); htlc_state_name(h->state), h->id);
htlc_undostate(h, SENT_REMOVE_HTLC, htlc_undostate(h, SENT_REMOVE_HTLC,
@ -2061,8 +2007,6 @@ static void retransmit_pkts(struct peer *peer, s64 ack)
ack++; ack++;
} }
resend_local_requests(peer);
/* We might need to re-propose HTLCs which were from other peers. */ /* We might need to re-propose HTLCs which were from other peers. */
retry_all_routing(peer); retry_all_routing(peer);
} }
@ -4197,7 +4141,7 @@ static void json_getpeers(struct command *cmd,
json_add_pubkey(response, cmd->dstate->secpctx, json_add_pubkey(response, cmd->dstate->secpctx,
"peerid", p->id); "peerid", p->id);
json_add_bool(response, "connected", p->conn && !p->fake_close); json_add_bool(response, "connected", p->connected);
/* FIXME: Report anchor. */ /* FIXME: Report anchor. */
@ -4352,6 +4296,11 @@ static void json_newhtlc(struct command *cmd,
return; return;
} }
if (!peer->connected) {
command_fail(cmd, "peer not connected");
return;
}
if (!json_tok_u64(buffer, msatoshistok, &msatoshis)) { if (!json_tok_u64(buffer, msatoshistok, &msatoshis)) {
command_fail(cmd, "'%.*s' is not a valid number", command_fail(cmd, "'%.*s' is not a valid number",
(int)(msatoshistok->end - msatoshistok->start), (int)(msatoshistok->end - msatoshistok->start),
@ -4427,6 +4376,11 @@ static void json_fulfillhtlc(struct command *cmd,
return; return;
} }
if (!peer->connected) {
command_fail(cmd, "peer not connected");
return;
}
if (!json_tok_u64(buffer, idtok, &id)) { if (!json_tok_u64(buffer, idtok, &id)) {
command_fail(cmd, "'%.*s' is not a valid id", command_fail(cmd, "'%.*s' is not a valid id",
(int)(idtok->end - idtok->start), (int)(idtok->end - idtok->start),
@ -4508,6 +4462,11 @@ static void json_failhtlc(struct command *cmd,
return; return;
} }
if (!peer->connected) {
command_fail(cmd, "peer not connected");
return;
}
if (!json_tok_u64(buffer, idtok, &id)) { if (!json_tok_u64(buffer, idtok, &id)) {
command_fail(cmd, "'%.*s' is not a valid id", command_fail(cmd, "'%.*s' is not a valid id",
(int)(idtok->end - idtok->start), (int)(idtok->end - idtok->start),
@ -4566,6 +4525,11 @@ static void json_commit(struct command *cmd,
return; return;
} }
if (!peer->connected) {
command_fail(cmd, "peer not connected");
return;
}
if (!state_can_commit(peer->state)) { if (!state_can_commit(peer->state)) {
command_fail(cmd, "peer in state %s", state_name(peer->state)); command_fail(cmd, "peer in state %s", state_name(peer->state));
return; return;
@ -4677,6 +4641,7 @@ static void json_disconnect(struct command *cmd,
* one side to freak out. We just ensure we ignore it. */ * one side to freak out. We just ensure we ignore it. */
log_debug(peer->log, "Pretending connection is closed"); log_debug(peer->log, "Pretending connection is closed");
peer->fake_close = true; peer->fake_close = true;
peer->connected = false;
set_peer_state(peer, STATE_ERR_BREAKDOWN, "json_disconnect", false); set_peer_state(peer, STATE_ERR_BREAKDOWN, "json_disconnect", false);
peer_breakdown(peer); peer_breakdown(peer);

74
daemon/test/test.sh
View File

@ -103,6 +103,19 @@ else
SHOW="cat" SHOW="cat"
fi 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() lcli1()
{ {
if [ -n "$VERBOSE" ]; then if [ -n "$VERBOSE" ]; then
@ -130,31 +143,51 @@ lcli1()
reconnect) reconnect)
[ -z "$VERBOSE" ] || echo RECONNECTING >&2 [ -z "$VERBOSE" ] || echo RECONNECTING >&2
$LCLI1 dev-reconnect $ID2 >/dev/null $LCLI1 dev-reconnect $ID2 >/dev/null
sleep 1
;; ;;
restart) restart)
[ -z "$VERBOSE" ] || echo RESTARTING >&2 [ -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 $LCLI1 -- dev-restart $LIGHTNINGD1 >/dev/null 2>&1 || true
if ! check "$LCLI1 getlog 2>/dev/null | fgrep -q Hello"; then if ! check "$LCLI1 getlog 2>/dev/null | fgrep -q Hello"; then
echo "dev-restart failed!">&2 echo "dev-restart failed!">&2
exit 1 exit 1
fi 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 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 esac
fi fi
@ -767,13 +800,13 @@ check_status $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE ""
# Now, two HTLCs at once, one from each direction. # Now, two HTLCs at once, one from each direction.
# Both sides can afford this. # Both sides can afford this.
HTLC_AMOUNT=1000000 HTLC_AMOUNT=1000000
HTLCID=`lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH | extract_id`
SECRET2=1de08917a61cb2b62ed5937d38577f6a7bfe59c176781c6d8128018e8b5ccdfe SECRET2=1de08917a61cb2b62ed5937d38577f6a7bfe59c176781c6d8128018e8b5ccdfe
RHASH2=`lcli1 dev-rhash $SECRET2 | sed 's/.*"\([0-9a-f]*\)".*/\1/'` 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` HTLCID2=`lcli2 newhtlc $ID1 $HTLC_AMOUNT $EXPIRY $RHASH2 | extract_id`
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 [ ! -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 } " 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 all_ok
fi fi
lcli2 failhtlc $ID1 $HTLCID
lcli1 fulfillhtlc $ID2 $HTLCID2 $SECRET2 lcli1 fulfillhtlc $ID2 $HTLCID2 $SECRET2
lcli2 failhtlc $ID1 $HTLCID
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
@ -879,6 +912,11 @@ case "$RECONNECT" in
;; ;;
esac 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. # Node2 collects the HTLCs.
lcli2 fulfillhtlc $ID1 $HTLCID $SECRET lcli2 fulfillhtlc $ID1 $HTLCID $SECRET
lcli2 fulfillhtlc $ID1 $HTLCID2 $SECRET2 lcli2 fulfillhtlc $ID1 $HTLCID2 $SECRET2

Write Preview
Loading…
Cancel
Save