diff --git a/daemon/channel.c b/daemon/channel.c index 9c2eb9b9d..8be5d9d65 100644 --- a/daemon/channel.c +++ b/daemon/channel.c @@ -299,19 +299,6 @@ void cstate_fulfill_htlc(struct channel_state *cstate, remove_htlc(cstate, side, !side, htlc); } -struct htlc *cstate_find_htlc(const struct channel_state *cstate, - const struct sha256 *rhash, - enum channel_side side) -{ - size_t i; - - for (i = 0; i < tal_count(cstate->side[side].htlcs); i++) { - if (structeq(&cstate->side[side].htlcs[i]->rhash, rhash)) - return cstate->side[side].htlcs[i]; - } - return NULL; -} - struct htlc *cstate_htlc_by_id(const struct channel_state *cstate, uint64_t id, enum channel_side side) diff --git a/daemon/channel.h b/daemon/channel.h index 74a96262a..d3f87c16b 100644 --- a/daemon/channel.h +++ b/daemon/channel.h @@ -111,18 +111,6 @@ void adjust_fee(struct channel_state *cstate, uint32_t fee_rate); */ bool force_fee(struct channel_state *cstate, uint64_t fee); -/** - * cstate_find_htlc: find an HTLC on this side of the channel. - * @cstate: The channel state - * @rhash: hash of redeem secret - * @side: OURS or THEIRS - * - * Returns the HTLC, or NULL on fail. - */ -struct htlc *cstate_find_htlc(const struct channel_state *cstate, - const struct sha256 *rhash, - enum channel_side side); - /** * cstate_htlc_by_id: find an HTLC on this side of the channel by ID. * @cstate: The channel state diff --git a/daemon/peer.c b/daemon/peer.c index 2adb04ca8..c3eb73ddd 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -3210,25 +3210,6 @@ 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, struct peer *peer, @@ -3406,38 +3387,22 @@ const struct json_command newhtlc_command = { "Returns { id: u64 } result on success" }; -/* Looks for their HTLC, but must be committed. */ -static struct htlc *find_their_committed_htlc(struct peer *peer, - const struct sha256 *rhash) -{ - struct htlc *htlc; - - /* Must be in last committed cstate. */ - 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); -} - static void json_fulfillhtlc(struct command *cmd, const char *buffer, const jsmntok_t *params) { struct peer *peer; - jsmntok_t *peeridtok, *rtok; - struct rval r; - struct sha256 rhash; + jsmntok_t *peeridtok, *idtok, *rtok; + u64 id; struct htlc *htlc; + struct sha256 rhash; + struct rval r; if (!json_get_params(buffer, params, "peerid", &peeridtok, + "id", &idtok, "r", &rtok, NULL)) { - command_fail(cmd, "Need peerid and r"); + command_fail(cmd, "Need peerid, id and r"); return; } @@ -3452,6 +3417,13 @@ static void json_fulfillhtlc(struct command *cmd, return; } + if (!json_tok_u64(buffer, idtok, &id)) { + command_fail(cmd, "'%.*s' is not a valid id", + (int)(idtok->end - idtok->start), + buffer + idtok->start); + return; + } + if (!hex_decode(buffer + rtok->start, rtok->end - rtok->start, &r, sizeof(r))) { @@ -3461,14 +3433,24 @@ static void json_fulfillhtlc(struct command *cmd, return; } - sha256(&rhash, &r, sizeof(r)); - - htlc = find_their_committed_htlc(peer, &rhash); + htlc = htlc_get(&peer->htlcs, id, REMOTE); if (!htlc) { command_fail(cmd, "preimage htlc not found"); return; } + if (htlc->state != RCVD_ADD_ACK_REVOCATION) { + command_fail(cmd, "htlc in state %s", + htlc_state_name(htlc->state)); + return; + } + + sha256(&rhash, &r, sizeof(r)); + if (!structeq(&htlc->rhash, &rhash)) { + command_fail(cmd, "preimage incorrect"); + return; + } + assert(!htlc->r); htlc->r = tal_dup(htlc, struct rval, &r); @@ -3483,7 +3465,7 @@ static void json_fulfillhtlc(struct command *cmd, const struct json_command fulfillhtlc_command = { "fulfillhtlc", json_fulfillhtlc, - "Redeem htlc proposed by {peerid} using {r}", + "Redeem htlc proposed by {peerid} of {id} using {r}", "Returns an empty result on success" }; @@ -3491,15 +3473,15 @@ static void json_failhtlc(struct command *cmd, const char *buffer, const jsmntok_t *params) { struct peer *peer; - jsmntok_t *peeridtok, *rhashtok; - struct sha256 rhash; + jsmntok_t *peeridtok, *idtok; + u64 id; struct htlc *htlc; if (!json_get_params(buffer, params, "peerid", &peeridtok, - "rhash", &rhashtok, + "id", &idtok, NULL)) { - command_fail(cmd, "Need peerid and rhash"); + command_fail(cmd, "Need peerid and id"); return; } @@ -3514,22 +3496,25 @@ static void json_failhtlc(struct command *cmd, return; } - if (!hex_decode(buffer + rhashtok->start, - rhashtok->end - rhashtok->start, - &rhash, sizeof(rhash))) { - command_fail(cmd, "'%.*s' is not a valid sha256 preimage", - (int)(rhashtok->end - rhashtok->start), - buffer + rhashtok->start); + if (!json_tok_u64(buffer, idtok, &id)) { + command_fail(cmd, "'%.*s' is not a valid id", + (int)(idtok->end - idtok->start), + buffer + idtok->start); return; } - /* Look in peer->remote.staging_cstate->a, as that's where we'll - * immediately remove it from: avoids double-handling. */ - htlc = find_their_committed_htlc(peer, &rhash); + htlc = htlc_get(&peer->htlcs, id, REMOTE); if (!htlc) { - command_fail(cmd, "htlc not found"); + command_fail(cmd, "preimage htlc not found"); return; } + + if (htlc->state != RCVD_ADD_ACK_REVOCATION) { + command_fail(cmd, "htlc in state %s", + htlc_state_name(htlc->state)); + return; + } + if (command_htlc_fail(peer, htlc)) command_success(cmd, null_response(cmd)); else @@ -3541,7 +3526,7 @@ static void json_failhtlc(struct command *cmd, const struct json_command failhtlc_command = { "failhtlc", json_failhtlc, - "Fail htlc proposed by {peerid} which has redeem hash {rhash}", + "Fail htlc proposed by {peerid} which has {id}", "Returns an empty result on success" }; diff --git a/daemon/test/test.sh b/daemon/test/test.sh index 59d5656da..d3f18a6fb 100755 --- a/daemon/test/test.sh +++ b/daemon/test/test.sh @@ -431,7 +431,7 @@ if [ -n "$DIFFERENT_FEES" ]; then [ ! -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 , state : RCVD_ADD_ACK_REVOCATION } " - lcli2 fulfillhtlc $ID1 $SECRET + lcli2 fulfillhtlc $ID1 $HTLCID $SECRET [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 @@ -557,7 +557,7 @@ if [ -n "$DUMP_ONCHAIN" ]; then all_ok fi -lcli2 fulfillhtlc $ID1 $SECRET +lcli2 fulfillhtlc $ID1 $HTLCID $SECRET [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 # Without manual commit, this check is racy. @@ -597,7 +597,7 @@ 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 , state : SENT_ADD_ACK_REVOCATION } " $B_AMOUNT $B_FEE "" -lcli2 failhtlc $ID1 $RHASH +lcli2 failhtlc $ID1 $HTLCID [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 @@ -667,7 +667,7 @@ HTLCID=`lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH | extract_id` 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 $B_FEE "" -lcli2 fulfillhtlc $ID1 $SECRET +lcli2 fulfillhtlc $ID1 $HTLCID $SECRET [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 @@ -702,12 +702,12 @@ if [ -n "$CLOSE_WITH_HTLCS" ]; then check_peerstate lcli2 STATE_CLEARING # Fail one, still waiting. - lcli2 failhtlc $ID1 $RHASH + lcli2 failhtlc $ID1 $HTLCID check_peerstate lcli1 STATE_CLEARING check_peerstate lcli2 STATE_CLEARING # Fulfill the other causes them to actually complete the close. - lcli1 fulfillhtlc $ID2 $SECRET2 + lcli1 fulfillhtlc $ID2 $HTLCID2 $SECRET2 check_peerstate lcli1 STATE_MUTUAL_CLOSING check_peerstate lcli2 STATE_MUTUAL_CLOSING @@ -728,8 +728,8 @@ if [ -n "$CLOSE_WITH_HTLCS" ]; then all_ok fi -lcli2 failhtlc $ID1 $RHASH -lcli1 fulfillhtlc $ID2 $SECRET2 +lcli2 failhtlc $ID1 $HTLCID +lcli1 fulfillhtlc $ID2 $HTLCID2 $SECRET2 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 @@ -776,8 +776,8 @@ lcli1 getpeers | tr -s '\012\011" ' ' ' | $FGREP "our_htlcs : [ { msatoshis : $H lcli2 getpeers | tr -s '\012\011" ' ' ' | $FGREP "our_htlcs : [ ], their_htlcs : [ { msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , state : RCVD_ADD_ACK_REVOCATION }, { msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH2 , state : RCVD_ADD_ACK_REVOCATION } ]" || lcli2 getpeers | tr -s '\012\011" ' ' ' | $FGREP "our_htlcs : [ ], their_htlcs : [ { msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH2 , state : RCVD_ADD_ACK_REVOCATION }, { msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , state : RCVD_ADD_ACK_REVOCATION } ]" # Node2 collects the HTLCs. -lcli2 fulfillhtlc $ID1 $SECRET -lcli2 fulfillhtlc $ID1 $SECRET2 +lcli2 fulfillhtlc $ID1 $HTLCID $SECRET +lcli2 fulfillhtlc $ID1 $HTLCID2 $SECRET2 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2