← Watch feed
pytest: remove channel upgrade tests.
Public commit record
70/100 · Adequate
What the developer wrote
Authored by Rusty Russell
pytest: remove channel upgrade tests.
We removed the functionality, but only disabled the tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
✓ Descriptive subject✓ Names a concrete action or component✓ Provides an explanatory body✓ Mentions testing or verification
What changed, and why it matters
This commit simply deletes three test cases from the test suite. The underlying feature (experimental channel upgrade protocol) had already been removed from the actual software; this change only cleans up the now-unused tests. There is no code change to the running program, so it does not introduce or fix a security vulnerability on its own.
Recommended action
No security action required; this is a test-suite cleanup. Reviewers may want to confirm that the production feature removal was completed in a prior commit and that no dead code remains.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff removes 272 lines from tests/test_connection.py, specifically three pytest functions related to the ‘experimental-upgrade-protocol’ feature: test_upgrade_statickey, test_upgrade_statickey_onchaind, and test_upgrade_statickey_fail. The commit message states the functionality was already removed and only the tests remained disabled. No production code is modified.
Changed components
tests/test_connection.pyInspect captured patch +0 / −272
diff --git a/tests/test_connection.py b/tests/test_connection.py
index 92a2a12f..bd887416 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -3729,278 +3729,6 @@ def test_openchannel_init_alternate(node_factory, executor):
print("nothing to do")
-@unittest.skip("experimental-upgrade-protocol TLV fields conflict with splicing TLV fields")
-def test_upgrade_statickey(node_factory, executor):
- """l1 doesn't have option_static_remotekey, l2 offers it."""
- l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True,
- 'experimental-upgrade-protocol': None,
- # This forces us to allow sending non-static-remotekey!
- 'dev-any-channel-type': None},
- {'may_reconnect': True,
- # This forces us to accept non-static-remotekey!
- 'dev-any-channel-type': None,
- 'experimental-upgrade-protocol': None}])
-
- l1.fundwallet(2000000)
- l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
- l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
-
- # Now reconnect.
- l1.rpc.disconnect(l2.info['id'], force=True)
- l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-
- l1.daemon.wait_for_logs([r"They sent current_channel_type \[\]",
- r"They offered upgrade to \[12\]"])
- l2.daemon.wait_for_log(r"They sent desired_channel_type \[12\]")
-
- l1.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
- l2.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
-
- # Make sure it's committed to db!
- wait_for(lambda: l1.db_query('SELECT local_static_remotekey_start, remote_static_remotekey_start FROM channels;') == [{'local_static_remotekey_start': 1, 'remote_static_remotekey_start': 1}])
-
- # They will consider themselves upgraded.
- l1.rpc.disconnect(l2.info['id'], force=True)
- # They won't offer upgrade!
- assert not l1.daemon.is_in_log("They offered upgrade",
- start=l1.daemon.logsearch_start)
- l1.daemon.wait_for_log(r"They sent current_channel_type \[12\]")
- l2.daemon.wait_for_log(r"They sent desired_channel_type \[12\]")
-
-
-@unittest.skip("experimental-upgrade-protocol TLV fields conflict with splicing TLV fields")
-def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind):
- """We test penalty before/after, and unilateral before/after"""
- l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True,
- 'experimental-upgrade-protocol': None,
- # This forces us to allow sending non-static-remotekey!
- 'dev-any-channel-type': None,
- # We try to cheat!
- 'broken_log': r"onchaind-chan#[0-9]*: Could not find resolution for output .*: did \*we\* cheat\?"},
- {'may_reconnect': True,
- # This forces us to allow non-static-remotekey!
- 'dev-any-channel-type': None,
- 'experimental-upgrade-protocol': None}])
-
- l1.fundwallet(FUNDAMOUNT + 1000)
- l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
- l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
- bitcoind.generate_block(1, wait_for_mempool=1)
- wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')
-
- # TEST 1: Cheat from pre-upgrade.
- tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx']
-
- l1.rpc.disconnect(l2.info['id'], force=True)
- l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
- l1.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
-
- # Make sure another commitment happens, sending failed payment.
- routestep = {
- 'amount_msat': 1,
- 'id': l2.info['id'],
- 'delay': 5,
- 'channel': first_scid(l1, l2)
- }
- l1.rpc.sendpay([routestep], '00' * 32, payment_secret='00' * 32)
- with pytest.raises(RpcError, match=r'WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS'):
- l1.rpc.waitsendpay('00' * 32)
-
- # Make sure l2 gets REVOKE_AND_ACK from previous.
- l2.daemon.wait_for_log('peer_in WIRE_UPDATE_ADD_HTLC')
- l2.daemon.wait_for_log('peer_out WIRE_REVOKE_AND_ACK')
- l2.daemon.wait_for_log('peer_in WIRE_REVOKE_AND_ACK')
-
- # Pre-statickey penalty works.
- bitcoind.rpc.sendrawtransaction(tx)
- bitcoind.generate_block(1)
-
- _, txid, blocks = l2.wait_for_onchaind_tx('OUR_PENALTY_TX',
- 'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM')
- assert blocks == 0
-
- bitcoind.generate_block(100, wait_for_mempool=txid)
- # This works even if they disconnect and listpeerchannels() is empty:
- wait_for(lambda: l1.rpc.listpeerchannels()['channels'] == [])
- wait_for(lambda: l2.rpc.listpeerchannels()['channels'] == [])
-
- # TEST 2: Cheat from post-upgrade.
- l1.fundwallet(FUNDAMOUNT + 1000)
- l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
- l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
-
- l1.rpc.disconnect(l2.info['id'], force=True)
- l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-
- l1.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
- l2.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
- bitcoind.generate_block(1, wait_for_mempool=1)
- wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')
-
- l1.pay(l2, 1000000)
-
- # We will try to cheat later.
- tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx']
-
- l1.pay(l2, 1000000)
-
- # Pre-statickey penalty works.
- bitcoind.rpc.sendrawtransaction(tx)
- bitcoind.generate_block(1)
-
- _, txid, blocks = l2.wait_for_onchaind_tx('OUR_PENALTY_TX',
- 'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM')
- assert blocks == 0
-
- bitcoind.generate_block(100, wait_for_mempool=txid)
- # This works even if they disconnect and listpeers() is empty:
- wait_for(lambda: len(l1.rpc.listpeerchannels()['channels']) == 0)
- wait_for(lambda: len(l2.rpc.listpeerchannels()['channels']) == 0)
-
- # TEST 3: Unilateral close from pre-upgrade
- l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
- l1.fundwallet(FUNDAMOUNT + 1000)
- l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
- bitcoind.generate_block(1, wait_for_mempool=1)
- wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')
-
- # Give them both something for onchain close.
- l1.pay(l2, 1000000)
-
- # Make sure it's completely quiescent.
- l1.daemon.wait_for_log("chan#3: Removing out HTLC 0 state RCVD_REMOVE_ACK_REVOCATION FULFILLED")
-
- l1.rpc.disconnect(l2.info['id'], force=True)
- l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
- l1.daemon.wait_for_log('option_static_remotekey enabled at 3/3')
-
- # But this is the *pre*-update commit tx!
- l2.stop()
- l1.rpc.close(l2.info['id'], unilateraltimeout=1)
- bitcoind.generate_block(1, wait_for_mempool=1)
- l2.start()
-
- # They should both handle it fine.
- _, txid, blocks = l1.wait_for_onchaind_tx('OUR_DELAYED_RETURN_TO_WALLET',
- 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US')
- assert blocks == 4
- l2.daemon.wait_for_logs(['Ignoring output .*: THEIR_UNILATERAL/OUTPUT_TO_US',
- 'Ignoring output .*: THEIR_UNILATERAL/DELAYED_OUTPUT_TO_THEM'])
- bitcoind.generate_block(4)
- bitcoind.generate_block(100, wait_for_mempool=txid)
-
- # This works even if they disconnect and listpeerchannels() is empty:
- wait_for(lambda: len(l1.rpc.listpeerchannels()['channels']) == 0)
- wait_for(lambda: len(l2.rpc.listpeerchannels()['channels']) == 0)
-
- # TEST 4: Unilateral close from post-upgrade
- l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
- l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
-
- l1.rpc.disconnect(l2.info['id'], force=True)
- l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
- l1.daemon.wait_for_log('option_static_remotekey enabled at 1/1')
-
- bitcoind.generate_block(1, wait_for_mempool=1)
- wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')
-
- # Move to static_remotekey.
- l1.pay(l2, 1000000)
-
- l2.stop()
- l1.rpc.close(l2.info['id'], unilateraltimeout=1)
- bitcoind.generate_block(1, wait_for_mempool=1)
- l2.start()
-
- # They should both handle it fine.
- _, txid, blocks = l1.wait_for_onchaind_tx('OUR_DELAYED_RETURN_TO_WALLET',
- 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US')
- assert blocks == 4
- l2.daemon.wait_for_logs(['Ignoring output .*: THEIR_UNILATERAL/OUTPUT_TO_US',
- 'Ignoring output .*: THEIR_UNILATERAL/DELAYED_OUTPUT_TO_THEM'])
-
- bitcoind.generate_block(4)
- bitcoind.generate_block(100, wait_for_mempool=txid)
-
- # This works even if they disconnect and listpeerchannels() is empty:
- wait_for(lambda: len(l2.rpc.listpeerchannels()['channels']) == 0)
-
-
-@unittest.skip("experimental-upgrade-protocol TLV fields conflict with splicing TLV fields")
-def test_upgrade_statickey_fail(node_factory, executor, bitcoind):
- """We reconnect at all points during retransmit, and we won't upgrade."""
- l1_disconnects = ['-WIRE_COMMITMENT_SIGNED',
- '-WIRE_REVOKE_AND_ACK']
- l2_disconnects = ['-WIRE_REVOKE_AND_ACK',
- '-WIRE_COMMITMENT_SIGNED']
-
- l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True,
- 'dev-no-reconnect': None,
- 'disconnect': l1_disconnects,
- # This allows us to send non-static-remotekey!
- 'dev-any-channel-type': None,
- 'experimental-upgrade-protocol': None,
- # Don't have feerate changes!
- 'feerates': (7500, 7500, 7500, 7500)},
- {'may_reconnect': True,
- 'dev-no-reconnect': None,
- 'experimental-upgrade-protocol': None,
- # This forces us to accept non-static-remotekey!
- 'dev-any-channel-type': None,
- 'disconnect': l2_disconnects,
- 'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_htlcs.py'),
- 'hold-time': 10000,
- 'hold-result': 'fail'}])
- l1.fundwallet(FUNDAMOUNT + 1000)
- l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
- l1.rpc.fundchannel(l2.info['id'], 'all', channel_type=[])
- bitcoind.generate_block(1, wait_for_mempool=1)
- wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'CHANNELD_NORMAL')
-
- # This HTLC will fail
- l1.rpc.sendpay([{'amount_msat': 1000, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}], '00' * 32, payment_secret='00' * 32)
-
- # Each one should cause one disconnection, no upgrade.
- for d in l1_disconnects + l2_disconnects:
- l1.daemon.wait_for_log('Peer connection lost')
- l2.daemon.wait_for_log('Peer connection lost')
- assert not l1.daemon.is_in_log('option_static_remotekey enabled')
- assert not l2.daemon.is_in_log('option_static_remotekey enabled')
- l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
- line1 = l1.daemon.wait_for_log('No upgrade')
- line2 = l2.daemon.wait_for_log('No upgrade')
-
- # On the last reconnect, it retransmitted revoke_and_ack.
- assert re.search('No upgrade: we retransmitted', line1)
- assert re.search('No upgrade: pending changes', line2)
-
- # Make sure we already skip the first of these.
- l1.daemon.wait_for_log('billboard perm: Reconnected, and reestablished.')
- assert 'option_static_remotekey' not in only_one(l1.rpc.listpeerchannels()['channels'])['features']
- assert 'option_static_remotekey' not in only_one(l2.rpc.listpeerchannels()['channels'])['features']
-
- sleeptime = 1
- while True:
- # Now when we reconnect, despite having an HTLC, we're quiescent.
- l1.rpc.disconnect(l2.info['id'], force=True)
- l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-
- oldstart = l1.daemon.logsearch_start
- l1.daemon.wait_for_log('billboard perm: Reconnected, and reestablished.')
- if not l1.daemon.is_in_log('No upgrade:', start=oldstart):
- break
-
- # Give it some processing time before reconnect...
- time.sleep(sleeptime)
- sleeptime += 1
-
- l1.daemon.logsearch_start = oldstart
- assert l1.daemon.wait_for_log('option_static_remotekey enabled at 2/2')
- assert l2.daemon.wait_for_log('option_static_remotekey enabled at 2/2')
- assert 'option_static_remotekey' in only_one(l1.rpc.listpeerchannels()['channels'])['features']
- assert 'option_static_remotekey' in only_one(l2.rpc.listpeerchannels()['channels'])['features']
-
-
def test_quiescence(node_factory, executor):
l1, l2 = node_factory.line_graph(2)
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.