lnnet - Some code improvements and fixes from @bigspider's review

Author: sr-gi

common/net/bigsize.py

@@ -10,7 +10,7 @@ def encode(value):
 
     Raises:
         :obj:`TypeError`: If the provided value is not an integer.
-        :obj:`ValueError`: If the provided value is negative or bigger than ``pow(2, 64)``.
+        :obj:`ValueError`: If the provided value is negative or bigger than ``pow(2, 64) - 1``.
     """
 
     if not isinstance(value, int):
@@ -19,13 +19,13 @@ def encode(value):
     if value < 0:
         raise ValueError(f"value must be a positive integer, {value} received")
 
-    if value < pow(2, 8) - 3:
+    if value < 253:
         return value.to_bytes(1, "big")
     elif value < pow(2, 16):
         return b"\xfd" + value.to_bytes(2, "big")
     elif value < pow(2, 32):
         return b"\xfe" + value.to_bytes(4, "big")
-    elif value <= pow(2, 64):
+    elif value < pow(2, 64):
         return b"\xff" + value.to_bytes(8, "big")
     else:
         raise ValueError("BigSize can only encode up to 8-byte values")
@@ -49,26 +49,37 @@ def decode(value):
     if not isinstance(value, bytes):
         raise TypeError(f"value must be bytes, {type(value)} received")
 
+    if len(value) == 0:
+        raise ValueError("Unexpected EOF while decoding BigSize")
+
     if len(value) > 9:
         raise ValueError(f"value must be, at most, 9-bytes long, {len(value)} received")
 
-    if len(value) > 1:
-        prefix = value[0]
-        decoded_value = int.from_bytes(value[1:], "big")
-    else:
-        prefix = None
-        decoded_value = int.from_bytes(value, "big")
+    if len(value) == 1 and value[0] < 253:
+        return value[0]
 
-    if not prefix and len(value) == 1 and decoded_value < pow(2, 8) - 3:
-        return decoded_value
-    elif prefix == 253 and len(value) == 3 and pow(2, 8) - 3 <= decoded_value < pow(2, 16):
-        return decoded_value
-    elif prefix == 254 and len(value) == 5 and pow(2, 16) <= decoded_value < pow(2, 32):
-        return decoded_value
-    elif prefix == 255 and len(value) == 9 and pow(2, 32) <= decoded_value:
-        return decoded_value
+    prefix = value[0]
+    decoded_value = int.from_bytes(value[1:], "big")
+
+    if prefix == 253:
+        length = 3
+        min_v = 253
+        max_v = pow(2, 16)
+    elif prefix == 254:
+        length = 5
+        min_v = pow(2, 16)
+        max_v = pow(2, 32)
     else:
-        raise ValueError("value is not properly encoded")
+        length = 9
+        min_v = pow(2, 32)
+        max_v = pow(2, 64)
+
+    if not len(value) == length:
+        raise ValueError("Unexpected EOF while decoding BigSize")
+    elif not min_v <= decoded_value < max_v:
+        raise ValueError("Encoded BigSize is non-canonical")
+    else:
+        return decoded_value
 
 
 def parse(value):
@@ -93,6 +104,7 @@ def parse(value):
 
     prefix = value[0]
 
+    # message length is not explicitly checked here, but wrong length will fail at decode.
     if prefix < 253:
         # prefix is actually the value to be parsed
         return decode(value[0:1]), 1

common/net/bolt1.py

@@ -207,7 +207,7 @@ class PingMessage(Message):
 
     def __init__(self, num_pong_bytes, ignored_bytes=None):
         if not 0 <= num_pong_bytes < pow(2, 16):
-            raise ValueError(f"num_pong_bytes must be between 0 and {pow(2, 16)}")
+            raise ValueError(f"num_pong_bytes must be between 0 and {pow(2, 16) - 1}")
 
         payload = num_pong_bytes.to_bytes(2, "big")
 

common/net/bolt9.py

@@ -109,13 +109,16 @@ def from_bytes(cls, features):
 
         int_features = int.from_bytes(features, "big")
         padding = max(2 * len(known_features), int_features.bit_length())
-        padding = padding + 1 if padding % 2 else padding
+        padding += padding % 2  # round up to the nearest even number
 
         bit_features = f"{int_features:b}".zfill(padding)
         bit_pairs = [bit_features[i : i + 2] for i in range(0, len(bit_features), 2)]
         features_dict = {}
 
         for i, pair in enumerate(reversed(bit_pairs)):
+            if pair == "11":
+                raise ValueError("Both odd and even bits cannot be set in a pair")
+
             # Known features are stored no matter if they are set or not
             odd_bit = 2 * i
             feature_name = known_odd_bits.get(odd_bit)
@@ -126,8 +129,7 @@ def from_bytes(cls, features):
                     features_dict[feature_name] = Feature(odd_bit, is_set=True)
                 elif pair == "10":
                     features_dict[feature_name] = Feature(odd_bit + 1, is_set=True)
-                else:
-                    raise ValueError("Both odd and even bits cannot be set in a pair")
+
             # For unknown features, we only store the ones that are set
             else:
                 feature_name = f"unknown_{odd_bit}"

common/net/tlv.py

@@ -39,7 +39,7 @@ def __len__(self):
         return len(self.serialize())
 
     def __eq__(self, other):
-        return isinstance(other, TLVRecord) and self.value == other.value
+        return isinstance(other, TLVRecord) and self.serialize() == other.serialize()
 
     @classmethod
     def from_bytes(cls, message):

test/common/unit/net/test_bigsize.py

@@ -60,11 +60,11 @@ def test_decode_wrong():
 
     # Wrong encoding
     for v in non_canonical:
-        with pytest.raises(ValueError, match="value is not properly encoded"):
+        with pytest.raises(ValueError, match="Encoded BigSize is non-canonical"):
             bigsize.decode(v)
 
     for v in unexpected_eof:
-        with pytest.raises(ValueError, match="value is not properly encoded"):
+        with pytest.raises(ValueError, match="Unexpected EOF while decoding BigSize"):
             bigsize.decode(v)
 
 
@@ -76,7 +76,7 @@ def test_parse():
 
     # Wrong encoding (behaves exactly like decode_wrong)
     for v in non_canonical:
-        with pytest.raises(ValueError, match="value is not properly encoded"):
+        with pytest.raises(ValueError, match="Encoded BigSize is non-canonical"):
             bigsize.parse(v)
 
 

test/common/unit/net/test_bolt1.py

@@ -268,9 +268,9 @@ def test_ping_message():
 
 def test_ping_message_wrong():
     # num_pong_bytes must be an integer between 0 and pow(2, 16)
-    with pytest.raises(ValueError, match=f"num_pong_bytes must be between 0 and {pow(2, 16)}"):
+    with pytest.raises(ValueError, match=f"num_pong_bytes must be between 0 and {pow(2, 16) -1}"):
         PingMessage(-1)
-    with pytest.raises(ValueError, match=f"num_pong_bytes must be between 0 and {pow(2, 16)}"):
+    with pytest.raises(ValueError, match=f"num_pong_bytes must be between 0 and {pow(2, 16) - 1}"):
         PingMessage(pow(2, 16))
 
     # ignore_bytes must be bytes if set