Skip to content

Commit

Permalink
lightningd: do RBF again for all the txs.
Browse files Browse the repository at this point in the history 
Now we've set everything up, the replacement code is quite simple.

Some tests now have to deal with RBF though, and our rbf tests need work
since they look for the old onchaind messages.

In particular, when we can't afford the fee we want, we back off to
the next blockcount estimate, rather than spending all on fees
(necessarily).  So test_penalty_rbf_burn no longer applies.

Changelog-Changed: Protocol: spending unilateral close transactions now use dynamic fees based on deadlines (and RBF), instead of fixed fees.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
  • Loading branch information
@rustyrussell
rustyrussell committed Mar 24, 2023
1 parent 1a0834e commit ec1f116
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 153 deletions.
14 changes: 14 additions & 0 deletions contrib/pyln-testing/pyln/testing/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,20 @@ def wait_for_onchaind_txs(self, *args):
def wait_for_onchaind_tx(self, name, resolve):
return self.wait_for_onchaind_txs((name, resolve))[0]

def wait_for_txid_or_rbf(self, txid):
"""Wait for a txid to be broadcast, or an rbf. Returns the actual txid"""
# Hack so we can mutate the txid: pass it in a list
def rbf_or_txid_broadcast(maybe_rbf):
# RBF onchain txid d4b597505b543a4b8b42ab4d481fd7a533febb7e7df150ca70689e6d046612f7 (fee 6564sat) with txid 979878b8f855d3895d1cd29bd75a60b21492c4842e38099186a8e649bee02c7c (fee 8205sat)
line = self.daemon.is_in_log("RBF onchain txid {}".format(maybe_rbf[0]))
if line is not None:
maybe_rbf[0] = re.search(r'with txid ([0-9a-fA-F]*)', line).group(1)
return maybe_rbf[0] in self.bitcoin.rpc.getrawmempool()

maybe_rbf = [txid]
wait_for(lambda: rbf_or_txid_broadcast(maybe_rbf))
return maybe_rbf[0]

def wait_for_onchaind_broadcast(self, name, resolve=None):
"""Wait for onchaind to drop tx name to resolve (if any)"""
if resolve:
Expand Down
67 changes: 65 additions & 2 deletions lightningd/onchain_control.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,68 @@ static bool consider_onchain_rebroadcast(struct channel *channel,
const struct bitcoin_tx **tx,
struct onchain_signing_info *info)
{
/* FIXME: Implement rbf! */
struct bitcoin_tx *newtx;
struct amount_sat newfee, rbf_thresh;
struct bitcoin_txid oldtxid, newtxid;
u8 **witness;

newtx = onchaind_tx_unsigned(tmpctx, channel, info, &newfee, NULL);
if (!newtx)
return true;

/* BIP-125:
*
* One or more transactions currently in the mempool (original
* transactions) will be replaced by a new transaction (replacement
* transaction) that spends one or more of the same inputs if,
*...
*
* 4. The replacement transaction must also pay for its own bandwidth
* at or above the rate set by the node's minimum relay fee
* setting. For example, if the minimum relay fee is 1 satoshi/byte
* and the replacement transaction is 500 bytes total, then the
* replacement must pay a fee at least 500 satoshis higher than the
* sum of the originals.
*/
if (!amount_sat_add(&rbf_thresh,
amount_sat(bitcoin_tx_weight(*tx)), info->fee)) {
log_broken(channel->log, "Can't add weight %zu to fee %s!",
bitcoin_tx_weight(*tx),
fmt_amount_sat(tmpctx, info->fee));
return true;
}

bitcoin_txid(*tx, &oldtxid);
if (amount_sat_less(newfee, rbf_thresh)) {
log_debug(channel->log,
"Not going to RBF onchain %s (fee %s vs %s)",
type_to_string(tmpctx, struct bitcoin_txid, &oldtxid),
fmt_amount_sat(tmpctx, info->fee),
fmt_amount_sat(tmpctx, newfee));
return true;
}

/* OK! RBF time! */
witness = sign_and_get_witness(NULL, channel, newtx, info);
bitcoin_tx_input_set_witness(newtx, 0, take(witness));

bitcoin_txid(newtx, &newtxid);
log_info(channel->log,
"RBF onchain txid %s (fee %s) with txid %s (fee %s)",
type_to_string(tmpctx, struct bitcoin_txid, &oldtxid),
fmt_amount_sat(tmpctx, info->fee),
type_to_string(tmpctx, struct bitcoin_txid, &newtxid),
fmt_amount_sat(tmpctx, newfee));
log_debug(channel->log,
"RBF %s->%s",
type_to_string(tmpctx, struct bitcoin_tx, *tx),
type_to_string(tmpctx, struct bitcoin_tx, newtx));

/* FIXME: This is ugly, but we want the same parent as old tx. */
tal_steal(tal_parent(*tx), newtx);
tal_free(*tx);
*tx = newtx;
info->fee = newfee;
return true;
}

Expand Down Expand Up @@ -917,8 +978,10 @@ static void create_onchain_tx(struct channel *channel,
type_to_string(tmpctx, struct bitcoin_tx, tx),
worthwhile ? "" : "(NOT WORTHWHILE, LOWBALL FEE!)");

/* We allow "excessive" fees, as we may be fighting with censors and
* we'd rather spend fees than have our adversary win. */
broadcast_tx(ld->topology,
channel, take(tx), NULL, false, info->minblock,
channel, take(tx), NULL, true, info->minblock,
NULL, consider_onchain_rebroadcast, take(info));

subd_send_msg(channel->owner,
Expand Down
177 changes: 26 additions & 151 deletions tests/test_closing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,6 @@ def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams):
assert acc['resolved_at_block'] > 0


@pytest.mark.xfail(strict=True)
@pytest.mark.developer("uses dev_sign_last_tx")
def test_penalty_rbf_normal(node_factory, bitcoind, executor, chainparams):
'''
Expand Down Expand Up @@ -1628,40 +1627,39 @@ def censoring_sendrawtx(r):
# l2 notices.
l2.daemon.wait_for_log(' to ONCHAIN')

def get_rbf_tx(self, depth, name, resolve):
r = self.daemon.wait_for_log('Broadcasting RBF {} .* to resolve {} depth={}'
.format(name, resolve, depth))
return re.search(r'.* \(([0-9a-fA-F]*)\)', r).group(1)
((_, txid1, blocks1), (_, txid2, blocks2)) = \
l2.wait_for_onchaind_txs(('OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/THEIR_HTLC'),
('OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM'))
assert blocks1 == 0
assert blocks2 == 0

def get_rbf_txid(node, txid):
line = node.daemon.wait_for_log("RBF onchain .*{}".format(txid))
if "Not going to RBF" in line:
return txid
return re.search(r'with txid ([0-9a-fA-F]*)', line).group(1)

rbf_txes = []
# Now the censoring miners generate some blocks.
for depth in range(2, 8):
for depth in range(2, 10):
bitcoind.generate_block(1)
sync_blockheight(bitcoind, [l2])
# l2 should RBF, twice even, one for the l1 main output,
# one for the l1 HTLC output.
rbf_txes.append(get_rbf_tx(l2, depth,
'OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/THEIR_HTLC'))
rbf_txes.append(get_rbf_tx(l2, depth,
'OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM'))
# Don't assume a specific order!
start = l2.daemon.logsearch_start
txid1 = get_rbf_txid(l2, txid1)
l2.daemon.logsearch_start = start
txid2 = get_rbf_txid(l2, txid2)

# Now that the transactions have high fees, independent miners
# realize they can earn potentially more money by grabbing the
# high-fee censored transactions, and fresh, non-censoring
# hashpower arises, evicting the censor.
l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', None)

# Check that the order in which l2 generated RBF transactions
# would be acceptable to Bitcoin.
for tx in rbf_txes:
# Use the bcli interface as well, so that we also check the
# bcli interface.
l2.rpc.call('sendrawtransaction', [tx, True])

# Now the non-censoring miners overpower the censoring miners.
bitcoind.generate_block(1)
bitcoind.generate_block(1, wait_for_mempool=[txid1, txid2])
sync_blockheight(bitcoind, [l2])

# And l2 should consider it resolved now.
Expand All @@ -1687,134 +1685,6 @@ def get_rbf_tx(self, depth, name, resolve):
check_utxos_channel(l2, [channel_id], expected_2)


@pytest.mark.xfail(strict=True)
@pytest.mark.developer("uses dev_sign_last_tx")
def test_penalty_rbf_burn(node_factory, bitcoind, executor, chainparams):
'''
Test that penalty transactions are RBFed and we are willing to burn
it all up to spite the thief.
'''
# We track channel balances, to verify that accounting is ok.
coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py')
to_self_delay = 10
# l1 is the thief, which causes our honest upstanding lightningd
# code to break, so l1 can fail.
# Initially, disconnect before the HTLC can be resolved.
l1 = node_factory.get_node(options={'dev-disable-commit-after': 1},
may_fail=True, allow_broken_log=True)
l2 = node_factory.get_node(options={'dev-disable-commit-after': 1,
'watchtime-blocks': to_self_delay,
'plugin': coin_mvt_plugin},
# Exporbitant feerates mean we don't have cap on RBF!
feerates=(15000000, 11000, 7500, 3750))

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
l1.fundchannel(l2, 10**7)
channel_id = first_channel_id(l1, l2)

# Trigger an HTLC being added.
t = executor.submit(l1.pay, l2, 1000000 * 1000)

# Make sure the channel is still alive.
assert len(l1.getactivechannels()) == 2
assert len(l2.getactivechannels()) == 2

# Wait for the disconnection.
l1.daemon.wait_for_log('dev-disable-commit-after: disabling')
l2.daemon.wait_for_log('dev-disable-commit-after: disabling')
# Make sure l1 gets the new HTLC.
l1.daemon.wait_for_log('got commitsig')

# l1 prepares a theft commitment transaction
theft_tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx']

# Now continue processing until fulfilment.
l1.rpc.dev_reenable_commit(l2.info['id'])
l2.rpc.dev_reenable_commit(l1.info['id'])

# Wait for the fulfilment.
l1.daemon.wait_for_log('peer_in WIRE_UPDATE_FULFILL_HTLC')
l1.daemon.wait_for_log('peer_out WIRE_REVOKE_AND_ACK')
l2.daemon.wait_for_log('peer_out WIRE_UPDATE_FULFILL_HTLC')
l1.daemon.wait_for_log('peer_in WIRE_REVOKE_AND_ACK')

# Now payment should complete.
t.result(timeout=10)

# l1 goes offline and bribes the miners to censor transactions from l2.
l1.rpc.stop()

def censoring_sendrawtx(r):
return {'id': r['id'], 'result': {}}

l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', censoring_sendrawtx)

# l1 now performs the theft attack!
bitcoind.rpc.sendrawtransaction(theft_tx)
bitcoind.generate_block(1)

# l2 notices.
l2.daemon.wait_for_log(' to ONCHAIN')

def get_rbf_tx(self, depth, name, resolve):
r = self.daemon.wait_for_log('Broadcasting RBF {} .* to resolve {} depth={}'
.format(name, resolve, depth))
return re.search(r'.* \(([0-9a-fA-F]*)\)', r).group(1)

rbf_txes = []
# Now the censoring miners generate some blocks.
for depth in range(2, 10):
bitcoind.generate_block(1)
sync_blockheight(bitcoind, [l2])
# l2 should RBF, twice even, one for the l1 main output,
# one for the l1 HTLC output.
rbf_txes.append(get_rbf_tx(l2, depth,
'OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/THEIR_HTLC'))
rbf_txes.append(get_rbf_tx(l2, depth,
'OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM'))

# Now that the transactions have high fees, independent miners
# realize they can earn potentially more money by grabbing the
# high-fee censored transactions, and fresh, non-censoring
# hashpower arises, evicting the censor.
l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', None)

# Check that the last two txes can be broadcast.
# These should donate the total amount to miners.
rbf_txes = rbf_txes[-2:]
for tx in rbf_txes:
l2.rpc.call('sendrawtransaction', [tx, True])

# Now the non-censoring miners overpower the censoring miners.
bitcoind.generate_block(1)
sync_blockheight(bitcoind, [l2])

# And l2 should consider it resolved now.
l2.daemon.wait_for_log('Resolved THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM by our proposal OUR_PENALTY_TX')
l2.daemon.wait_for_log('Resolved THEIR_REVOKED_UNILATERAL/THEIR_HTLC by our proposal OUR_PENALTY_TX')

# l2 donated it to the miners, so it owns nothing
assert(len(l2.rpc.listfunds()['outputs']) == 0)
assert account_balance(l2, channel_id) == 0

expected_2 = {
'A': [('cid1', ['channel_open'], ['channel_close'], 'B')],
'B': [('cid1', ['penalty'], ['to_miner'], 'C'), ('cid1', ['penalty'], ['to_miner'], 'D')],
}

if anchor_expected():
expected_2['B'].append(('external', ['anchor'], None, None))
expected_2['B'].append(('wallet', ['anchor', 'ignored'], None, None))

check_utxos_channel(l2, [channel_id], expected_2)

# Make sure that l2's account is considered closed (has a fee output)
fees = [e for e in l2.rpc.bkpr_listincome()['income_events'] if e['tag'] == 'onchain_fee']
assert len(fees) == 1


@pytest.mark.developer("needs DEVELOPER=1")
def test_onchain_first_commit(node_factory, bitcoind):
"""Onchain handling where opener immediately drops to chain"""
Expand Down Expand Up @@ -1964,6 +1834,8 @@ def test_onchaind_replay(node_factory, bitcoind):
'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US')
assert blocks == 200
bitcoind.generate_block(200)
# Could be RBF!
txid = l1.wait_for_txid_or_rbf(txid)
bitcoind.generate_block(1, wait_for_mempool=txid)


Expand Down Expand Up @@ -2457,6 +2329,8 @@ def try_pay():
assert not t.is_alive()

# 100 blocks after last spend, l1+l2 should be done.
# Could be RBF!
txid = l1.wait_for_txid_or_rbf(txid)
l2.bitcoin.generate_block(100, wait_for_mempool=txid)
l1.daemon.wait_for_log('onchaind complete, forgetting peer')
l2.daemon.wait_for_log('onchaind complete, forgetting peer')
Expand Down Expand Up @@ -2577,7 +2451,8 @@ def test_onchain_feechange(node_factory, bitcoind, executor):
assert blocks == 5
bitcoind.generate_block(5)

# Make sure that gets included.
# Could be RBF!
txid = l1.wait_for_txid_or_rbf(txid)
bitcoind.generate_block(1, wait_for_mempool=txid)
# Now we restart with different feerates.
l1.stop()
Expand Down
2 changes: 2 additions & 0 deletions tests/test_pay.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1626,6 +1626,8 @@ def test_forward_local_failed_stats(node_factory, bitcoind, executor):
assert blocks == 5
bitcoind.generate_block(5)

# Could be RBF!
txid = l2.wait_for_txid_or_rbf(txid)
bitcoind.generate_block(1, wait_for_mempool=txid)
l2.daemon.wait_for_log('Resolved THEIR_UNILATERAL/OUR_HTLC by our proposal OUR_HTLC_TIMEOUT_TO_US')
l4.daemon.wait_for_log('Ignoring output.*: OUR_UNILATERAL/THEIR_HTLC')
Expand Down
2 changes: 2 additions & 0 deletions tests/test_plugin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,8 @@ def test_forward_event_notification(node_factory, bitcoind, executor):
assert blocks == 5
bitcoind.generate_block(5)

# Could be RBF!
txid = l2.wait_for_txid_or_rbf(txid)
bitcoind.generate_block(1, wait_for_mempool=txid)
l2.daemon.wait_for_log('Resolved THEIR_UNILATERAL/OUR_HTLC by our proposal OUR_HTLC_TIMEOUT_TO_US')
l5.daemon.wait_for_log('Ignoring output.*: OUR_UNILATERAL/THEIR_HTLC')
Expand Down

0 comments on commit ec1f116

Please sign in to comment.