tlightning channels reserves: use pretty balance in Qt, fix bugs, add tests - electrum - Electrum Bitcoin wallet
HTML git clone https://git.parazyd.org/electrum
DIR Log
DIR Files
DIR Refs
DIR Submodules
---
DIR commit a5a7c1406e02f899a62a544eaef8e771d2aa472a
DIR parent eb4e6b2e54a091a729f5cdef6e7116de4ea1500b
HTML Author: Janus <ysangkok@gmail.com>
Date: Wed, 24 Oct 2018 20:39:07 +0200
lightning channels reserves: use pretty balance in Qt, fix bugs, add tests
Diffstat:
M electrum/gui/qt/channels_list.py | 10 +++++-----
M electrum/lnbase.py | 18 ++++++++++++++----
M electrum/lnchan.py | 34 ++++++++++++++++++++++++++-----
M electrum/tests/test_lnchan.py | 126 ++++++++++++++++++++++++++++---
4 files changed, 162 insertions(+), 26 deletions(-)
---
DIR diff --git a/electrum/gui/qt/channels_list.py b/electrum/gui/qt/channels_list.py
t@@ -25,12 +25,12 @@ class ChannelsList(MyTreeWidget):
def format_fields(self, chan):
labels = {}
for subject in (REMOTE, LOCAL):
- available = chan.available_to_spend(subject)//1000
- label = self.parent.format_amount(available)
+ bal_minus_htlcs = chan.balance_minus_outgoing_htlcs(subject)//1000
+ label = self.parent.format_amount(bal_minus_htlcs)
bal_other = chan.balance(-subject)//1000
- available_other = chan.available_to_spend(-subject)//1000
- if bal_other != available_other:
- label += ' (+' + self.parent.format_amount(bal_other - available_other) + ')'
+ bal_minus_htlcs_other = chan.balance_minus_outgoing_htlcs(-subject)//1000
+ if bal_other != bal_minus_htlcs_other:
+ label += ' (+' + self.parent.format_amount(bal_other - bal_minus_htlcs_other) + ')'
labels[subject] = label
return [
bh2u(chan.node_id),
DIR diff --git a/electrum/lnbase.py b/electrum/lnbase.py
t@@ -427,7 +427,7 @@ class Peer(PrintError):
to_self_delay=local_config.to_self_delay,
max_htlc_value_in_flight_msat=local_config.max_htlc_value_in_flight_msat,
channel_flags=0x00, # not willing to announce channel
- channel_reserve_satoshis=546
+ channel_reserve_satoshis=local_config.reserve_sat,
)
payload = await self.channel_accepted[temp_channel_id].get()
if payload.get('error'):
t@@ -440,6 +440,7 @@ class Peer(PrintError):
remote_max = int.from_bytes(payload['max_htlc_value_in_flight_msat'], 'big')
assert remote_max >= 198 * 1000 * 1000, remote_max
their_revocation_store = RevocationStore()
+ remote_reserve_sat = self.validate_remote_reserve(payload["channel_reserve_satoshis"], remote_dust_limit_sat, funding_sat)
remote_config = RemoteConfig(
payment_basepoint=OnlyPubkeyKeypair(payload['payment_basepoint']),
multisig_key=OnlyPubkeyKeypair(payload["funding_pubkey"]),
t@@ -454,7 +455,7 @@ class Peer(PrintError):
ctn = -1,
amount_msat=push_msat,
next_htlc_id = 0,
- reserve_sat = int.from_bytes(payload["channel_reserve_satoshis"], 'big'),
+ reserve_sat = remote_reserve_sat,
next_per_commitment_point=remote_per_commitment_point,
current_per_commitment_point=None,
t@@ -528,7 +529,7 @@ class Peer(PrintError):
temporary_channel_id=temp_chan_id,
dust_limit_satoshis=local_config.dust_limit_sat,
max_htlc_value_in_flight_msat=local_config.max_htlc_value_in_flight_msat,
- channel_reserve_satoshis=546,
+ channel_reserve_satoshis=local_config.reserve_sat,
htlc_minimum_msat=1000,
minimum_depth=min_depth,
to_self_delay=local_config.to_self_delay,
t@@ -546,6 +547,7 @@ class Peer(PrintError):
channel_id, funding_txid_bytes = channel_id_from_funding_tx(funding_txid, funding_idx)
their_revocation_store = RevocationStore()
remote_balance_sat = funding_sat * 1000 - push_msat
+ remote_reserve_sat = self.validate_remote_reserve(payload['channel_reserve_satoshis'], remote_dust_limit_sat, funding_sat)
chan = {
"node_id": self.peer_addr.pubkey,
"channel_id": channel_id,
t@@ -565,7 +567,7 @@ class Peer(PrintError):
ctn = -1,
amount_msat=remote_balance_sat,
next_htlc_id = 0,
- reserve_sat = int.from_bytes(payload['channel_reserve_satoshis'], 'big'),
+ reserve_sat = remote_reserve_sat,
next_per_commitment_point=payload['first_per_commitment_point'],
current_per_commitment_point=None,
t@@ -614,6 +616,14 @@ class Peer(PrintError):
m.set_state('DISCONNECTED')
raise Exception('funding outpoint mismatch')
+ def validate_remote_reserve(self, payload_field, dust_limit, funding_sat):
+ remote_reserve_sat = int.from_bytes(payload_field, 'big')
+ if remote_reserve_sat < dust_limit:
+ raise Exception('protocol violation: reserve < dust_limit')
+ if remote_reserve_sat > funding_sat/100:
+ raise Exception(f'reserve too high: {remote_reserve_sat}, funding_sat: {funding_sat}')
+ return remote_reserve_sat
+
@log_exceptions
async def reestablish_channel(self, chan):
await self.initialized
DIR diff --git a/electrum/lnchan.py b/electrum/lnchan.py
t@@ -164,7 +164,7 @@ class Channel(PrintError):
if self.get_state() != 'OPEN':
raise PaymentFailure('Channel not open')
if self.available_to_spend(LOCAL) < amount_msat:
- raise PaymentFailure('Not enough local balance')
+ raise PaymentFailure(f'Not enough local balance. Have: {self.available_to_spend(LOCAL)}, Need: {amount_msat}')
if len(self.htlcs(LOCAL, only_pending=True)) + 1 > self.config[REMOTE].max_accepted_htlcs:
raise PaymentFailure('Too many HTLCs already in channel')
current_htlc_sum = htlcsum(self.htlcs(LOCAL, only_pending=True))
t@@ -213,7 +213,9 @@ class Channel(PrintError):
assert type(htlc) is dict
htlc = UpdateAddHtlc(**htlc, htlc_id = self.config[REMOTE].next_htlc_id)
if self.available_to_spend(REMOTE) < htlc.amount_msat:
- raise RemoteMisbehaving('Remote dipped below channel reserve')
+ raise RemoteMisbehaving('Remote dipped below channel reserve.' +\
+ f' Available at remote: {self.available_to_spend(REMOTE)},' +\
+ f' HTLC amount: {htlc.amount_msat}')
adds = self.log[REMOTE]['adds']
adds[htlc.htlc_id] = htlc
self.print_error("receive_htlc")
t@@ -481,6 +483,16 @@ class Channel(PrintError):
return received_this_batch, sent_this_batch
def balance(self, subject):
+ """
+ This balance in mSAT is not including reserve and fees.
+ So a node cannot actually use it's whole balance.
+ But this number is simple, since it is derived simply
+ from the initial balance, and the value of settled HTLCs.
+ Note that it does not decrease once an HTLC is added,
+ failed or fulfilled, since the balance change is only
+ commited to later when the respective commitment
+ transaction as been revoked.
+ """
initial = self.config[subject].initial_msat
initial -= sum(self.settled[subject])
t@@ -489,15 +501,27 @@ class Channel(PrintError):
assert initial == self.config[subject].amount_msat
return initial
- def available_to_spend(self, subject):
+ def balance_minus_outgoing_htlcs(self, subject):
+ """
+ This balance in mSAT, which includes the value of
+ pending outgoing HTLCs, is used in the UI.
+ """
return self.balance(subject)\
+ - htlcsum(self.log[subject]['adds'].values())
+
+ def available_to_spend(self, subject):
+ """
+ This balance in mSAT, while technically correct, can
+ not be used in the UI cause it fluctuates (commit fee)
+ """
+ return self.balance_minus_outgoing_htlcs(subject)\
- htlcsum(self.log[subject]['adds'].values())\
- - self.config[subject].reserve_sat * 1000\
+ - self.config[-subject].reserve_sat * 1000\
- calc_onchain_fees(
# TODO should we include a potential new htlc, when we are called from receive_htlc?
len(list(self.included_htlcs(subject, LOCAL)) + list(self.included_htlcs(subject, REMOTE))),
self.pending_feerate(subject),
- subject == LOCAL,
+ True, # for_us
self.constraints.is_initiator,
)[subject]
DIR diff --git a/electrum/tests/test_lnchan.py b/electrum/tests/test_lnchan.py
t@@ -151,7 +151,19 @@ class TestChannel(unittest.TestCase):
# this htlc to his remote state update log.
self.aliceHtlcIndex = self.alice_channel.add_htlc(self.htlc_dict)
+ before = self.bob_channel.balance_minus_outgoing_htlcs(REMOTE)
+ beforeLocal = self.bob_channel.balance_minus_outgoing_htlcs(LOCAL)
+
self.bobHtlcIndex = self.bob_channel.receive_htlc(self.htlc_dict)
+
+ after = self.bob_channel.balance_minus_outgoing_htlcs(REMOTE)
+ afterLocal = self.bob_channel.balance_minus_outgoing_htlcs(LOCAL)
+
+ self.assertEqual(before - after, self.htlc_dict['amount_msat'])
+ self.assertEqual(beforeLocal, afterLocal)
+
+ self.bob_pending_remote_balance = after
+
self.htlc = self.bob_channel.log[lnutil.REMOTE]['adds'][0]
def test_SimpleAddSettleWorkflow(self):
t@@ -259,11 +271,28 @@ class TestChannel(unittest.TestCase):
# revocation.
#self.assertEqual(alice_channel.local_update_log, [], "alice's local not updated, should be empty, has %s entries instead"% len(alice_channel.local_update_log))
#self.assertEqual(alice_channel.remote_update_log, [], "alice's remote not updated, should be empty, has %s entries instead"% len(alice_channel.remote_update_log))
+ self.assertEqual(self.bob_pending_remote_balance, self.alice_channel.balance(LOCAL))
+
alice_channel.update_fee(100000)
+ bob_channel.receive_update_fee(100000)
+ force_state_transition(alice_channel, bob_channel)
+
+ self.htlc_dict['amount_msat'] *= 5
+ bob_index = bob_channel.add_htlc(self.htlc_dict)
+ alice_index = alice_channel.receive_htlc(self.htlc_dict)
+ force_state_transition(alice_channel, bob_channel)
+ alice_channel.settle_htlc(self.paymentPreimage, alice_index)
+ bob_channel.receive_htlc_settle(self.paymentPreimage, bob_index)
+ force_state_transition(alice_channel, bob_channel)
+ self.assertEqual(alice_channel.total_msat(SENT), one_bitcoin_in_msat, "alice satoshis sent incorrect")
+ self.assertEqual(alice_channel.total_msat(RECEIVED), 5 * one_bitcoin_in_msat, "alice satoshis received incorrect")
+ self.assertEqual(bob_channel.total_msat(RECEIVED), one_bitcoin_in_msat, "bob satoshis received incorrect")
+ self.assertEqual(bob_channel.total_msat(SENT), 5 * one_bitcoin_in_msat, "bob satoshis sent incorrect")
+
alice_channel.serialize()
- def alice_to_bob_fee_update(self):
- fee = 111
+
+ def alice_to_bob_fee_update(self, fee=111):
self.alice_channel.update_fee(fee)
self.bob_channel.receive_update_fee(fee)
return fee
t@@ -325,6 +354,13 @@ class TestChannel(unittest.TestCase):
self.assertEqual(fee, bob_channel.constraints.feerate)
def test_AddHTLCNegativeBalance(self):
+ # the test in lnd doesn't set the fee to zero.
+ # probably lnd subtracts commitment fee after deciding weather
+ # an htlc can be added. so we set the fee to zero so that
+ # the test can work.
+ self.alice_to_bob_fee_update(0)
+ force_state_transition(self.alice_channel, self.bob_channel)
+
self.htlc_dict['payment_hash'] = bitcoin.sha256(32 * b'\x02')
self.alice_channel.add_htlc(self.htlc_dict)
self.htlc_dict['payment_hash'] = bitcoin.sha256(32 * b'\x03')
t@@ -339,7 +375,7 @@ class TestChannel(unittest.TestCase):
new['payment_hash'] = bitcoin.sha256(32 * b'\x04')
with self.assertRaises(lnutil.PaymentFailure) as cm:
self.alice_channel.add_htlc(new)
- self.assertEqual(('Not enough local balance',), cm.exception.args)
+ self.assertIn('Not enough local balance', cm.exception.args[0])
class TestAvailableToSpend(unittest.TestCase):
def test_DesyncHTLCs(self):
t@@ -381,22 +417,23 @@ class TestChanReserve(unittest.TestCase):
def setUp(self):
alice_channel, bob_channel = create_test_channels()
alice_min_reserve = int(.5 * one_bitcoin_in_msat // 1000)
- alice_channel.config[LOCAL] =\
- alice_channel.config[LOCAL]._replace(reserve_sat=alice_min_reserve)
- bob_channel.config[REMOTE] =\
- bob_channel.config[REMOTE]._replace(reserve_sat=alice_min_reserve)
# We set Bob's channel reserve to a value that is larger than
# his current balance in the channel. This will ensure that
# after a channel is first opened, Bob can still receive HTLCs
# even though his balance is less than his channel reserve.
bob_min_reserve = 6 * one_bitcoin_in_msat // 1000
- bob_channel.config[LOCAL] =\
- bob_channel.config[LOCAL]._replace(reserve_sat=bob_min_reserve)
+ # bob min reserve was decided by alice, but applies to bob
+
+ alice_channel.config[LOCAL] =\
+ alice_channel.config[LOCAL]._replace(reserve_sat=bob_min_reserve)
alice_channel.config[REMOTE] =\
- alice_channel.config[REMOTE]._replace(reserve_sat=bob_min_reserve)
+ alice_channel.config[REMOTE]._replace(reserve_sat=alice_min_reserve)
+
+ bob_channel.config[LOCAL] =\
+ bob_channel.config[LOCAL]._replace(reserve_sat=alice_min_reserve)
+ bob_channel.config[REMOTE] =\
+ bob_channel.config[REMOTE]._replace(reserve_sat=bob_min_reserve)
- self.bob_min = bob_min_reserve
- self.alice_min = bob_min_reserve
self.alice_channel = alice_channel
self.bob_channel = bob_channel
t@@ -439,6 +476,71 @@ class TestChanReserve(unittest.TestCase):
with self.assertRaises(lnutil.RemoteMisbehaving):
self.alice_channel.receive_htlc(htlc_dict)
+ def part2(self):
+ paymentPreimage = b"\x01" * 32
+ paymentHash = bitcoin.sha256(paymentPreimage)
+ # Now we'll add HTLC of 3.5 BTC to Alice's commitment, this should put
+ # Alice's balance at 1.5 BTC.
+ #
+ # Resulting balances:
+ # Alice: 1.5
+ # Bob: 9.5
+ htlc_dict = {
+ 'payment_hash' : paymentHash,
+ 'amount_msat' : int(3.5 * one_bitcoin_in_msat),
+ 'cltv_expiry' : 5,
+ }
+ self.alice_channel.add_htlc(htlc_dict)
+ self.bob_channel.receive_htlc(htlc_dict)
+ # Add a second HTLC of 1 BTC. This should fail because it will take
+ # Alice's balance all the way down to her channel reserve, but since
+ # she is the initiator the additional transaction fee makes her
+ # balance dip below.
+ htlc_dict['amount_msat'] = one_bitcoin_in_msat
+ with self.assertRaises(lnutil.PaymentFailure):
+ self.alice_channel.add_htlc(htlc_dict)
+ with self.assertRaises(lnutil.RemoteMisbehaving):
+ self.bob_channel.receive_htlc(htlc_dict)
+
+ def part3(self):
+ # Add a HTLC of 2 BTC to Alice, and the settle it.
+ # Resulting balances:
+ # Alice: 3.0
+ # Bob: 7.0
+ htlc_dict = {
+ 'payment_hash' : paymentHash,
+ 'amount_msat' : int(2 * one_bitcoin_in_msat),
+ 'cltv_expiry' : 5,
+ }
+ alice_idx = self.alice_channel.add_htlc(htlc_dict)
+ bob_idx = self.bob_channel.receive_htlc(htlc_dict)
+ force_state_transition(self.alice_channel, self.bob_channel)
+ self.check_bals(one_bitcoin_in_msat*3\
+ - self.alice_channel.pending_local_fee,
+ one_bitocin_in_msat*5)
+ self.bob_channel.settle_htlc(paymentPreimage, bob_idx)
+ self.alice_channel.receive_htlc_settle(paymentPreimage, alice_idx)
+ force_state_transition(self.alice_channel, self.bob_channel)
+ self.check_bals(one_bitcoin_in_msat*3\
+ - self.alice_channel.pending_local_fee,
+ one_bitocin_in_msat*7)
+ # And now let Bob add an HTLC of 1 BTC. This will take Bob's balance
+ # all the way down to his channel reserve, but since he is not paying
+ # the fee this is okay.
+ htlc_dict['amount_msat'] = one_bitcoin_in_msat
+ self.bob_channel.add_htlc(htlc_dict)
+ self.alice_channel.receive_htlc(htlc_dict)
+ force_state_transition(self.alice_channel, self.bob_channel)
+ self.check_bals(one_bitcoin_in_msat*3\
+ - self.alice_channel.pending_local_fee,
+ one_bitocin_in_msat*6)
+
+ def check_bals(self, amt1, amt2):
+ self.assertEqual(self.alice_channel.available_to_spend(LOCAL), amt1)
+ self.assertEqual(self.bob_channel.available_to_spend(REMOTE), amt1)
+ self.assertEqual(self.alice_channel.available_to_spend(REMOTE), amt2)
+ self.assertEqual(self.bob_channel.available_to_spend(LOCAL), amt2)
+
class TestDust(unittest.TestCase):
def test_DustLimit(self):
alice_channel, bob_channel = create_test_channels()