Browse Source

daemon: don't allow manual fulfill command until both sides committed.

We had an occasional race where we hadn't gotten the remote revocation
before submitting fulfill (spotted by the HTLC state transition code).

Disallow this, but also add to the json output so we can wait for
an HTLC to be irrevocably committed.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ppa-0.6.1
Rusty Russell 9 years ago
parent
commit
0e78ccca56
  1. 60
      daemon/peer.c
  2. 20
      daemon/test/test.sh

60
daemon/peer.c

@ -2874,20 +2874,55 @@ static void json_add_pubkey(struct json_result *response,
json_add_hex(response, id, der, sizeof(der));
}
/* Is this side fully committed to this HTLC? */
static bool htlc_fully_committed(const struct commit_info *ci,
const struct htlc *htlc)
{
/* Must have it in our commitment. */
if (!cstate_htlc_by_id(ci->cstate, htlc->id, htlc_channel_side(htlc)))
return false;
/* If it wasn't in previous commitment, that must be revoked. */
if (!ci->prev || ci->prev->revocation_preimage)
return true;
if (!cstate_htlc_by_id(ci->prev->cstate, htlc->id,
htlc_channel_side(htlc)))
return false;
return true;
}
static void json_add_htlcs(struct json_result *response,
const char *id,
const struct channel_oneside *side)
const struct peer *peer,
enum channel_side side)
{
size_t i;
struct htlc **htlcs = peer->local.commit->cstate->side[side].htlcs;
json_array_start(response, id);
for (i = 0; i < tal_count(side->htlcs); i++) {
for (i = 0; i < tal_count(htlcs); i++) {
const char *committed;
json_object_start(response, NULL);
json_add_u64(response, "msatoshis", side->htlcs[i]->msatoshis);
json_add_abstime(response, "expiry", &side->htlcs[i]->expiry);
json_add_u64(response, "msatoshis", htlcs[i]->msatoshis);
json_add_abstime(response, "expiry", &htlcs[i]->expiry);
json_add_hex(response, "rhash",
&side->htlcs[i]->rhash,
sizeof(side->htlcs[i]->rhash));
&htlcs[i]->rhash, sizeof(htlcs[i]->rhash));
if (htlc_fully_committed(peer->local.commit, htlcs[i])) {
if (htlc_fully_committed(peer->remote.commit, htlcs[i]))
committed = "both";
else
committed = "us";
} else {
if (htlc_fully_committed(peer->remote.commit, htlcs[i]))
committed = "them";
else
/* Weird, shouldn't happen. */
committed = "none";
}
json_add_string(response, "committed", committed);
json_object_end(response);
}
json_array_end(response);
@ -2929,8 +2964,8 @@ static void json_getpeers(struct command *cmd,
json_add_num(response, "our_fee", last->side[OURS].fee_msat);
json_add_num(response, "their_amount", last->side[THEIRS].pay_msat);
json_add_num(response, "their_fee", last->side[THEIRS].fee_msat);
json_add_htlcs(response, "our_htlcs", &last->side[OURS]);
json_add_htlcs(response, "their_htlcs", &last->side[THEIRS]);
json_add_htlcs(response, "our_htlcs", p, OURS);
json_add_htlcs(response, "their_htlcs", p, THEIRS);
/* Any changes since then? */
if (p->local.staging_cstate->changes != last->changes)
@ -3050,8 +3085,15 @@ const struct json_command newhtlc_command = {
static struct htlc *find_their_committed_htlc(struct peer *peer,
const struct sha256 *rhash)
{
struct htlc *htlc;
/* Must be in last committed cstate. */
if (!cstate_find_htlc(peer->remote.commit->cstate, rhash, THEIRS))
htlc = cstate_find_htlc(peer->remote.commit->cstate, rhash, THEIRS);
if (!htlc)
return NULL;
/* Dangerous to fulfill unless the remote side is obliged to honor it. */
if (!htlc_fully_committed(peer->remote.commit, htlc))
return NULL;
return cstate_find_htlc(peer->remote.staging_cstate, rhash, THEIRS);

20
daemon/test/test.sh

@ -426,7 +426,7 @@ if [ -n "$DIFFERENT_FEES" ]; then
lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
check_status_single lcli2 0 0 "" $(($AMOUNT - $HTLC_AMOUNT - $ONE_HTLCS_FEE2)) $(($ONE_HTLCS_FEE2)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } "
check_status_single lcli2 0 0 "" $(($AMOUNT - $HTLC_AMOUNT - $ONE_HTLCS_FEE2)) $(($ONE_HTLCS_FEE2)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both } "
lcli2 fulfillhtlc $ID1 $SECRET
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
@ -487,7 +487,7 @@ if [ -n "$MANUALCOMMIT" ]; then
A_FEE=$(($A_FEE + $EXTRA_FEE))
# Node 2 has it committed.
check_status_single lcli2 $B_AMOUNT $B_FEE "" $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } "
check_status_single lcli2 $B_AMOUNT $B_FEE "" $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : us } "
# There should be no "both committed" here yet
if lcli1 getlog debug | $FGREP "Both committed"; then
@ -511,7 +511,7 @@ else
fi
# Both should have committed tx.
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } " $B_AMOUNT $B_FEE ""
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both } " $B_AMOUNT $B_FEE ""
if [ -n "$STEAL" ]; then
STEAL_TX=`$LCLI1 dev-signcommit $ID2 | cut -d\" -f4`
@ -532,7 +532,7 @@ if [ -n "$DUMP_ONCHAIN" ]; then
check_peerstate lcli2 STATE_CLOSE_ONCHAIN_THEIR_UNILATERAL
# both still know about htlc
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } " $B_AMOUNT $B_FEE ""
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both } " $B_AMOUNT $B_FEE ""
# Generate 6 blocks so CSV timeout has expired.
$CLI generate 6
@ -596,7 +596,7 @@ lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
# Check channel status
A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
A_FEE=$(($A_FEE + $EXTRA_FEE))
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } " $B_AMOUNT $B_FEE ""
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both } " $B_AMOUNT $B_FEE ""
lcli2 failhtlc $ID1 $RHASH
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
@ -616,7 +616,7 @@ lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
# Check channel status
A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
A_FEE=$(($A_FEE + $EXTRA_FEE))
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } " $B_AMOUNT $B_FEE ""
check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both } " $B_AMOUNT $B_FEE ""
# Make sure node1 accepts the expiry packet.
while [ $(blockheight) != $EXPIRY ]; do
@ -667,7 +667,7 @@ lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
[ ! -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 } " $B_AMOUNT $B_FEE ""
check_status $(($A_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE)) $(($A_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both } " $B_AMOUNT $B_FEE ""
lcli2 fulfillhtlc $ID1 $SECRET
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
@ -693,7 +693,7 @@ lcli2 newhtlc $ID1 $HTLC_AMOUNT $EXPIRY $RHASH2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
check_status $(($A_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE)) $(($A_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } " $(($B_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE)) $(($B_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH2 } "
check_status $(($A_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE)) $(($A_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both } " $(($B_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE)) $(($B_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH2 , committed : both } "
if [ -n "$CLOSE_WITH_HTLCS" ]; then
# Now begin close
@ -748,7 +748,7 @@ lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
# Make sure node1 sends commit (in the background, since it will block!)
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 &
# node2 will consider this committed.
check_status_single lcli2 $(($B_AMOUNT - $EXTRA_FEE/2)) $(($B_FEE + $EXTRA_FEE/2)) "" $(($A_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE/2)) $(($A_FEE + $EXTRA_FEE/2)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH } "
check_status_single lcli2 $(($B_AMOUNT - $EXTRA_FEE/2)) $(($B_FEE + $EXTRA_FEE/2)) "" $(($A_AMOUNT - $HTLC_AMOUNT - $EXTRA_FEE/2)) $(($A_FEE + $EXTRA_FEE/2)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : us } "
# Now send another offer, and enable node2 output.
lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH2
@ -759,7 +759,7 @@ lcli2 dev-output $ID1 true
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
# Both sides should be committed to htlcs
check_status $(($A_AMOUNT - $HTLC_AMOUNT*2 - $EXTRA_FEE)) $(($A_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH }, { msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH2 } " $(($B_AMOUNT - $EXTRA_FEE)) $(($B_FEE + $EXTRA_FEE)) ""
check_status $(($A_AMOUNT - $HTLC_AMOUNT*2 - $EXTRA_FEE)) $(($A_FEE + $EXTRA_FEE)) "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , committed : both }, { msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH2 , committed : both } " $(($B_AMOUNT - $EXTRA_FEE)) $(($B_FEE + $EXTRA_FEE)) ""
# Node2 collects the HTLCs.
lcli2 fulfillhtlc $ID1 $SECRET

Loading…
Cancel
Save