Skip to content

Commit

Permalink
onchaind: fix too-eager OUR_HTLC_TIMEOUT_TX.
Browse files Browse the repository at this point in the history 
With the following patch applied, we could clearly see onchaind try to
broadcast the timeout tx one block too early:

	sendrawtx exit 26, gave error code: -26?error message:?non-final (code 64)?

This is because of an out-by-one error in calculating the relative
depth required, since the out->tx_blockheight is already 1 before the
current block.

Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
@rustyrussell
rustyrussell committed Mar 6, 2018
1 parent c3621d5 commit 9c460a1
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 19 deletions.
4 changes: 2 additions & 2 deletions onchaind/onchain.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,8 @@ static void propose_resolution_at_block(struct tracked_output *out,
/* Expiry could be in the past! */
if (block_required < out->tx_blockheight)
depth = 0;
else
depth = block_required - out->tx_blockheight;
else /* Note that out->tx_blockheight is already at depth 1 */
depth = block_required - out->tx_blockheight + 1;
propose_resolution(out, tx, depth, tx_type);
}

Expand Down
32 changes: 16 additions & 16 deletions tests/test_lightningd.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1695,8 +1695,8 @@ def test_onchain_feechange(self):
l2.daemon.wait_for_log(' to ONCHAIN')

# Wait for timeout.
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US .* in 5 blocks')
bitcoind.generate_block(5)
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US .* in 6 blocks')
bitcoind.generate_block(6)

l1.daemon.wait_for_log('sendrawtx exit 0')

Expand All @@ -1720,15 +1720,15 @@ def test_onchain_feechange(self):
# and due to the l1 restart, there is none here.
l1.daemon.wait_for_log('WIRE_PERMANENT_CHANNEL_FAILURE')

# 91 later, l2 is done
bitcoind.generate_block(90)
# 90 later, l2 is done
bitcoind.generate_block(89)
sync_blockheight([l2])
assert not l2.daemon.is_in_log('onchaind complete, forgetting peer')
bitcoind.generate_block(1)
l2.daemon.wait_for_log('onchaind complete, forgetting peer')

# Now, 6 blocks and l1 should be done.
bitcoind.generate_block(5)
# Now, 7 blocks and l1 should be done.
bitcoind.generate_block(6)
sync_blockheight([l1])
assert not l1.daemon.is_in_log('onchaind complete, forgetting peer')
bitcoind.generate_block(1)
Expand Down Expand Up @@ -1773,16 +1773,16 @@ def test_onchain_all_dust(self):
l2.daemon.wait_for_log(' to ONCHAIN')

# Wait for timeout.
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by DONATING_TO_MINERS .* in 5 blocks')
bitcoind.generate_block(5)
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by DONATING_TO_MINERS .* in 6 blocks')
bitcoind.generate_block(6)

l1.daemon.wait_for_log('sendrawtx exit 0')
bitcoind.generate_block(1)

l1.daemon.wait_for_log('Resolved THEIR_UNILATERAL/OUR_HTLC by our proposal DONATING_TO_MINERS')

# 100 deep and l2 forgets.
bitcoind.generate_block(92)
bitcoind.generate_block(91)
sync_blockheight([l2])
assert not l2.daemon.is_in_log('onchaind complete, forgetting peer')
bitcoind.generate_block(1)
Expand Down Expand Up @@ -1814,8 +1814,8 @@ def test_permfail_new_commit(self):
l1.daemon.wait_for_log('Their unilateral tx, new commit point')
l1.daemon.wait_for_log(' to ONCHAIN')
l2.daemon.wait_for_log(' to ONCHAIN')
l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) in 5 blocks')
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US (.*) in 5 blocks')
l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) in 6 blocks')
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US (.*) in 6 blocks')

# OK, time out HTLC.
bitcoind.generate_block(5)
Expand Down Expand Up @@ -1850,8 +1850,8 @@ def test_permfail_htlc_in(self):
l1.daemon.wait_for_log('Their unilateral tx, old commit point')
l1.daemon.wait_for_log(' to ONCHAIN')
l2.daemon.wait_for_log(' to ONCHAIN')
l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) in 5 blocks')
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US (.*) in 5 blocks')
l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) in 6 blocks')
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US (.*) in 6 blocks')
# l2 then gets preimage, uses it instead of ignoring
l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by OUR_HTLC_SUCCESS_TX .* in 0 blocks')
l2.daemon.wait_for_log('sendrawtx exit 0')
Expand All @@ -1860,7 +1860,7 @@ def test_permfail_htlc_in(self):
# OK, l1 sees l2 fulfill htlc.
l1.daemon.wait_for_log('THEIR_UNILATERAL/OUR_HTLC gave us preimage')
l2.daemon.wait_for_log('Propose handling OUR_HTLC_SUCCESS_TX/DELAYED_OUTPUT_TO_US by OUR_DELAYED_RETURN_TO_WALLET .* in 5 blocks')
bitcoind.generate_block(5)
bitcoind.generate_block(6)

l2.daemon.wait_for_log('sendrawtx exit 0')

Expand Down Expand Up @@ -1892,10 +1892,10 @@ def test_permfail_htlc_out(self):
l1.daemon.wait_for_log('Their unilateral tx, old commit point')
l1.daemon.wait_for_log(' to ONCHAIN')
l2.daemon.wait_for_log(' to ONCHAIN')
l2.daemon.wait_for_logs(['Propose handling OUR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TX \\(.*\\) in 5 blocks',
l2.daemon.wait_for_logs(['Propose handling OUR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TX \\(.*\\) in 6 blocks',
'Propose handling OUR_UNILATERAL/DELAYED_OUTPUT_TO_US by OUR_DELAYED_RETURN_TO_WALLET .* in 5 blocks'])

l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) in 5 blocks')
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) in 6 blocks')
# l1 then gets preimage, uses it instead of ignoring
l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_FULFILL_TO_US .* in 0 blocks')
l1.daemon.wait_for_log('sendrawtx exit 0')
Expand Down
2 changes: 1 addition & 1 deletion tests/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def wait_for_logs(self, regexs, timeout=60):
return self.logs[pos]
pos += 1

def wait_for_log(self, regex, timeout=60):
def wait_for_log(self, regex, timeout=10):
"""Look for `regex` in the logs.
Convenience wrapper for the common case of only seeking a single entry.
Expand Down

0 comments on commit 9c460a1

Please sign in to comment.