Skip to content

lightningd: count the witness stack element when sizing onchain spends #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davelund110 wants to merge 1 commit into
base: blake2b-unified
Choose a base branch
from
Open

lightningd: count the witness stack element when sizing onchain spends #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lightningd/onchain_control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,8 +977,11 @@ static struct bitcoin_tx *onchaind_tx_unsigned(const tal_t *ctx,
channel_internal_error(channel, "Could not add keypath?");
return tal_free(tx);
}
/* Worst-case sig is 73 bytes */
weight = bitcoin_tx_weight(tx) + 1 + 3 + 73 + 0 + tal_count(info->wscript);
/* Worst-case sig is 73 bytes. The stack element (a revocation
* pubkey or a preimage, if any) goes in the witness too: without it,
* at the minimum feerate the fee falls below the minimum relay fee. */
weight = bitcoin_tx_weight(tx) + 1 + 3 + 73
+ tal_bytelen(info->stack_elem) + tal_count(info->wscript);
weight += elements_tx_overhead(chainparams, 1, 1);

/* BOLT #5:
Expand Down
51 changes: 51 additions & 0 deletions tests/test_closing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,57 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams, anchors):
check_utxos_channel(l2, [channel_id], expected_2, tags)


@pytest.mark.parametrize("anchors", [False, True])
def test_penalty_htlc_min_feerate(node_factory, bitcoind, executor, chainparams, anchors):
"""As test_penalty_inhtlc, but with every feerate estimate at the floor:
the penalty for the HTLC must still pay the minimum relay fee, so its
weight has to include the revocation pubkey in its witness."""

if chainparams['elements'] and anchors:
pytest.skip('elementsd anchors unsupported')

opts = {'dev-disable-commit-after': 1,
'feerates': (253, 253, 253, 253)}
if anchors is False:
opts['dev-force-features'] = "-23"

l1, l2 = node_factory.line_graph(2, opts=[{**opts, 'may_fail': True,
# We try to cheat!
'broken_log': r"onchaind-chan#[0-9]*: Could not find resolution for output .*: did \*we\* cheat\?"},
opts])
scid = first_scid(l1, l2)

# This gets stuck once the HTLC is in l1's commitment.
t = executor.submit(l1.pay, l2, 100000000)
l1.daemon.wait_for_log('dev-disable-commit-after: disabling')
l2.daemon.wait_for_log('dev-disable-commit-after: disabling')
l1.daemon.wait_for_log('got commitsig')

tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx']

l1.rpc.dev_reenable_commit(l2.info['id'])
l2.rpc.dev_reenable_commit(l1.info['id'])
t.result(timeout=TIMEOUT)
wait_for(lambda: all([only_one(n.rpc.listpeerchannels()['channels'])['htlcs'] == [] for n in (l1, l2)]))

bitcoind.rpc.sendrawtransaction(tx)
bitcoind.generate_block(1)
l2.daemon.wait_for_log(' to ONCHAIN')
wait_for(lambda: l2.is_local_channel_active(scid) is False)

((_, txid1, _), (_, txid2, _)) = \
l2.wait_for_onchaind_txs(('OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM'),
('OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/THEIR_HTLC'))
assert not l2.daemon.is_in_log('min relay fee not met')

# Both get into the mempool, and are mined.
bitcoind.generate_block(1, wait_for_mempool=[txid1, txid2])
l2.daemon.wait_for_logs([r'Resolved THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM by our proposal OUR_PENALTY_TX',
r'Resolved THEIR_REVOKED_UNILATERAL/THEIR_HTLC by our proposal OUR_PENALTY_TX'])


@pytest.mark.parametrize("anchors", [False, True])
def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams, anchors):
"""Test penalty transaction with an outgoing HTLC"""
Expand Down
12 changes: 6 additions & 6 deletions tests/test_coinmoves.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1398,10 +1398,10 @@ def test_coinmoves_unilateral_htlc_fulfill(node_factory, bitcoind):
# Note: the invoice is fulfilled in the *chain* moves, not *channel*.
expected_chain2 += [{'account_id': 'wallet',
'blockheight': 105,
'credit_msat': 94534000,
'credit_msat': 94238000,
'debit_msat': 0,
'extra_tags': [],
'output_msat': 94534000,
'output_msat': 94238000,
'primary_tag': 'deposit',
'utxo': f"{htlc_success_txid}:0"},
{'account_id': fundchannel['channel_id'],
Expand Down Expand Up @@ -1829,18 +1829,18 @@ def test_coinmoves_unilateral_htlc_penalty(node_factory, bitcoind):

expected_chain2 += [{'account_id': 'wallet',
'blockheight': 105,
'credit_msat': 49858187000,
'credit_msat': 49858175000,
'debit_msat': 0,
'extra_tags': [],
'output_msat': 49858187000,
'output_msat': 49858175000,
'primary_tag': 'deposit',
'utxo': f"{to_l1_penalty}:0"},
{'account_id': 'wallet',
'blockheight': 105,
'credit_msat': 92908000,
'credit_msat': 92512000,
'debit_msat': 0,
'extra_tags': [],
'output_msat': 92908000,
'output_msat': 92512000,
'primary_tag': 'deposit',
'utxo': f"{htlc_penalty}:0"},
{'account_id': fundchannel['channel_id'],
Expand Down