pyln-client: reimplement NodeVersion, simply.
This broke my build machine, because lightningd --version was malformed (I had no tags somehow in that branch). I dived into the code to figure out what was wrong, and I was horrified. 1. STOP. Never write this much code. 2. You just need a NodeVersion class. That's it. No others. 3. Don't throw away the entire first part if it starts with 'v'. Just remove the v. 4. Handle untagged versions cleanly. 5. Always fail on invalid strings in the constructor, NOT on the first time you use it. I have rewritten it. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
committed by
ShahanaFarooqui
ShahanaFarooqui
parent
40215917ab
commit
1dee02c5e3
@@ -2,7 +2,7 @@ from .lightning import LightningRpc, RpcError, Millisatoshi
|
||||
from .plugin import Plugin, monkey_patch, RpcException
|
||||
from .gossmap import Gossmap, GossmapNode, GossmapChannel, GossmapHalfchannel, GossmapNodeId, LnFeatureBits
|
||||
from .gossmapstats import GossmapStats
|
||||
from .version import NodeVersion, VersionSpec
|
||||
from .version import NodeVersion
|
||||
|
||||
__version__ = "25.02"
|
||||
|
||||
@@ -22,5 +22,4 @@ __all__ = [
|
||||
"LnFeatureBits",
|
||||
"GossmapStats",
|
||||
"NodeVersion",
|
||||
"VersionSpec",
|
||||
]
|
||||
|
||||
@@ -1,16 +1,10 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
from functools import total_ordering
|
||||
import re
|
||||
from typing import List, Optional, Protocol, runtime_checkable, Union
|
||||
|
||||
|
||||
_MODDED_PATTERN = "[0-9a-f]+-modded"
|
||||
from typing import Union
|
||||
|
||||
|
||||
@total_ordering
|
||||
@dataclass
|
||||
class NodeVersion:
|
||||
"""NodeVersion
|
||||
|
||||
@@ -29,25 +23,28 @@ class NodeVersion:
|
||||
See `strict_equal` if an exact match is required
|
||||
- `v23.11` < `v24.02`
|
||||
The oldest version is the smallest
|
||||
- `vd6fa78c`
|
||||
This is an untagged version, such as in CI. This is assumed to be the latest, greater than
|
||||
any test.
|
||||
"""
|
||||
def __init__(self, version: str):
|
||||
# e.g. v24.11-225-gda793e66b9
|
||||
if version.startswith('v'):
|
||||
version = version[1:]
|
||||
version = version.split('-')[0]
|
||||
parts = version.split('.')
|
||||
# rc is considered "close enough"
|
||||
if 'rc' in parts[-1]:
|
||||
parts[-1] = parts[-1].split('rc')[0]
|
||||
|
||||
version: str
|
||||
self.parts: int = []
|
||||
|
||||
def to_parts(self) -> List[_NodeVersionPart]:
|
||||
parts = self.version[1:].split(".")
|
||||
# If the first part contains a v we will ignore it
|
||||
if not parts[0][0].isdigit():
|
||||
parts[0] = parts[1:]
|
||||
|
||||
return [_NodeVersionPart.parse(p) for p in parts]
|
||||
|
||||
def strict_equal(self, other: NodeVersion) -> bool:
|
||||
if not isinstance(other, NodeVersion):
|
||||
raise TypeError(
|
||||
"`other` is expected to be of type `NodeVersion` but is `{type(other)}`"
|
||||
)
|
||||
# Single part? It's a git version, so treat it as the future.
|
||||
if len(parts) == 1:
|
||||
self.parts.append(100)
|
||||
else:
|
||||
return self.version == other.version
|
||||
for p in parts:
|
||||
self.parts.append(int(p))
|
||||
|
||||
def __eq__(self, other: Union[NodeVersion, str]) -> bool:
|
||||
if isinstance(other, str):
|
||||
@@ -55,21 +52,12 @@ class NodeVersion:
|
||||
if not isinstance(other, NodeVersion):
|
||||
return False
|
||||
|
||||
if self.strict_equal(other):
|
||||
return True
|
||||
elif re.match(_MODDED_PATTERN, self.version):
|
||||
if len(self.parts) != len(other.parts):
|
||||
return False
|
||||
else:
|
||||
self_parts = [p.num for p in self.to_parts()]
|
||||
other_parts = [p.num for p in other.to_parts()]
|
||||
|
||||
if len(self_parts) != len(other_parts):
|
||||
for a, b in zip(self.parts, other.parts):
|
||||
if a != b:
|
||||
return False
|
||||
|
||||
for ps, po in zip(self_parts, other_parts):
|
||||
if ps != po:
|
||||
return False
|
||||
return True
|
||||
return True
|
||||
|
||||
def __lt__(self, other: Union[NodeVersion, str]) -> bool:
|
||||
if isinstance(other, str):
|
||||
@@ -77,148 +65,14 @@ class NodeVersion:
|
||||
if not isinstance(other, NodeVersion):
|
||||
return NotImplemented
|
||||
|
||||
# If we are in CI the version will by a hex ending on modded
|
||||
# We will assume it is the latest version
|
||||
if re.match(_MODDED_PATTERN, self.version):
|
||||
return False
|
||||
elif re.match(_MODDED_PATTERN, other.version):
|
||||
return True
|
||||
else:
|
||||
self_parts = [p.num for p in self.to_parts()]
|
||||
other_parts = [p.num for p in other.to_parts()]
|
||||
|
||||
# zip truncates to shortes length
|
||||
for sp, op in zip(self_parts, other_parts):
|
||||
if sp < op:
|
||||
return True
|
||||
if sp > op:
|
||||
return False
|
||||
|
||||
# If the initial parts are all equal the longest version is the biggest
|
||||
#
|
||||
# self = 'v24.02'
|
||||
# other = 'v24.02.1'
|
||||
return len(self_parts) < len(other_parts)
|
||||
|
||||
def matches(self, version_spec: VersionSpecLike) -> bool:
|
||||
"""Returns True if the version matches the spec
|
||||
|
||||
The `version_spec` can be represented as a string and has 8 operators
|
||||
which are `=`, `===`, `!=`, `!===`, `<`, `<=`, `>`, `>=`.
|
||||
|
||||
The `=` is the equality operator. The verson_spec `=v24.02` matches
|
||||
all versions that equal `v24.02` including release candidates such as `v24.02rc1`.
|
||||
You can use the strict-equality operator `===` if strict equality is required.
|
||||
|
||||
Specifiers can be combined by separating the with a comma ','. The `version_spec`
|
||||
`>=v23.11, <v24.02" includes any version which is greater than or equal to `v23.11`
|
||||
and smaller than `v24.02`.
|
||||
"""
|
||||
spec = VersionSpec.parse(version_spec)
|
||||
return spec.matches(self)
|
||||
|
||||
|
||||
@dataclass
|
||||
class _NodeVersionPart:
|
||||
num: int
|
||||
text: Optional[str] = None
|
||||
|
||||
@classmethod
|
||||
def parse(cls, part: str) -> _NodeVersionPart:
|
||||
# We assume all parts start with a number and are followed by a text
|
||||
# E.g: v24.01rc2 has two parts
|
||||
# - "24" -> num = 24, text = None
|
||||
# - "01rc" -> num = 01, text = "rc"
|
||||
|
||||
number = re.search(r"\d+", part).group()
|
||||
text = part[len(number):]
|
||||
text_opt = text if text != "" else None
|
||||
return _NodeVersionPart(int(number), text_opt)
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class VersionSpec(Protocol):
|
||||
def matches(self, other: NodeVersionLike) -> bool:
|
||||
...
|
||||
|
||||
@classmethod
|
||||
def parse(cls, spec: VersionSpecLike) -> VersionSpec:
|
||||
if isinstance(spec, VersionSpec):
|
||||
return spec
|
||||
else:
|
||||
parts = [p.strip() for p in spec.split(",")]
|
||||
subspecs = [_CompareSpec.parse(p) for p in parts]
|
||||
return _AndVersionSpecifier(subspecs)
|
||||
|
||||
|
||||
@dataclass
|
||||
class _AndVersionSpecifier(VersionSpec):
|
||||
specs: List[VersionSpec]
|
||||
|
||||
def matches(self, other: NodeVersionLike) -> bool:
|
||||
for spec in self.specs:
|
||||
if not spec.matches(other):
|
||||
# We want a zero-padded zip. Pad both to make one.
|
||||
totlen = max(len(self.parts), len(other.parts))
|
||||
for a, b in zip(self.parts + [0] * totlen, other.parts + [0] * totlen):
|
||||
if a < b:
|
||||
return True
|
||||
if a > b:
|
||||
return False
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
_OPERATORS = [
|
||||
"===", # Strictly equal
|
||||
"!===", # not strictly equal
|
||||
"=", # Equal
|
||||
">=", # Greater or equal
|
||||
"<=", # Less or equal
|
||||
"<", # less
|
||||
">", # greater than
|
||||
"!=", # not equal
|
||||
]
|
||||
|
||||
|
||||
@dataclass
|
||||
class _CompareSpec(VersionSpec):
|
||||
operator: str
|
||||
version: NodeVersion
|
||||
|
||||
def __post_init__(self):
|
||||
if self.operator not in _OPERATORS:
|
||||
raise ValueError(f"Invalid operator '{self.operator}'")
|
||||
|
||||
def matches(self, other: NodeVersionLike):
|
||||
if isinstance(other, str):
|
||||
other = NodeVersion(other)
|
||||
if self.operator == "===":
|
||||
return other.strict_equal(self.version)
|
||||
if self.operator == "!===":
|
||||
return not other.strict_equal(self.version)
|
||||
if self.operator == "=":
|
||||
return other == self.version
|
||||
if self.operator == ">=":
|
||||
return other >= self.version
|
||||
if self.operator == "<=":
|
||||
return other <= self.version
|
||||
if self.operator == "<":
|
||||
return other < self.version
|
||||
if self.operator == ">":
|
||||
return other > self.version
|
||||
if self.operator == "!=":
|
||||
return other != self.version
|
||||
else:
|
||||
ValueError("Unknown operator")
|
||||
|
||||
@classmethod
|
||||
def parse(cls, spec_string: str) -> _CompareSpec:
|
||||
spec_string = spec_string.strip()
|
||||
|
||||
for op in _OPERATORS:
|
||||
if spec_string.startswith(op):
|
||||
version = spec_string[len(op):]
|
||||
version = version.strip()
|
||||
return _CompareSpec(op, NodeVersion(version))
|
||||
|
||||
raise ValueError(f"Failed to parse '{spec_string}'")
|
||||
|
||||
|
||||
NodeVersionLike = Union[NodeVersion, str]
|
||||
VersionSpecLike = Union[VersionSpec, str]
|
||||
|
||||
__all__ = [NodeVersion, NodeVersionLike, VersionSpec, VersionSpecLike]
|
||||
__all__ = [NodeVersion]
|
||||
|
||||
@@ -1,42 +1,22 @@
|
||||
from pyln.client.version import NodeVersion, VersionSpec, _NodeVersionPart, _CompareSpec
|
||||
from pyln.client.version import NodeVersion
|
||||
|
||||
|
||||
def test_create_version():
|
||||
# These are the strings returned by `lightningd --version`
|
||||
_ = NodeVersion("v24.02")
|
||||
_ = NodeVersion("23.08.1")
|
||||
|
||||
|
||||
def test_parse_parts():
|
||||
assert _NodeVersionPart.parse("2rc2") == _NodeVersionPart(2, "rc2")
|
||||
assert _NodeVersionPart.parse("0rc1") == _NodeVersionPart(0, "rc1")
|
||||
assert _NodeVersionPart.parse("2") == _NodeVersionPart(2, None)
|
||||
assert _NodeVersionPart.parse("2").text is None
|
||||
|
||||
|
||||
def test_version_to_parts():
|
||||
|
||||
assert NodeVersion("v24.02rc1").to_parts() == [
|
||||
_NodeVersionPart(24),
|
||||
_NodeVersionPart(2, "rc1"),
|
||||
]
|
||||
|
||||
assert NodeVersion("v24.02.1").to_parts() == [
|
||||
_NodeVersionPart(24),
|
||||
_NodeVersionPart(2),
|
||||
_NodeVersionPart(1),
|
||||
]
|
||||
_ = NodeVersion("v24.11-232-g5a76c7a")
|
||||
_ = NodeVersion("v24.11-225-gda793e6-modded")
|
||||
_ = NodeVersion("v24.11")
|
||||
_ = NodeVersion("vd6fa78c")
|
||||
|
||||
|
||||
def test_equality_classes_in_node_versions():
|
||||
assert NodeVersion("v24.02") == NodeVersion("v24.02")
|
||||
assert NodeVersion("v24.02") == NodeVersion("v24.02rc1")
|
||||
assert NodeVersion("v24.02rc1") == NodeVersion("v24.02")
|
||||
assert NodeVersion("v24.11-217-g77989b1-modded") == NodeVersion("v24.11")
|
||||
assert NodeVersion("v24.02") == NodeVersion("24.02")
|
||||
assert NodeVersion("v24.02-225") == NodeVersion("v24.02")
|
||||
|
||||
assert NodeVersion("v24.02") != NodeVersion("v24.02.1")
|
||||
assert NodeVersion("v24.02rc1") != NodeVersion("v24.02.1")
|
||||
assert NodeVersion("v23.10") != NodeVersion("v23.02")
|
||||
assert NodeVersion("v24.02") == NodeVersion("v24.02rc1")
|
||||
assert NodeVersion("v24.11-217-g77989b1-modded") == NodeVersion("v24.11")
|
||||
assert NodeVersion("vd6fa78c") == NodeVersion("vabcdefg")
|
||||
|
||||
|
||||
def test_inequality_of_node_versions():
|
||||
@@ -44,49 +24,26 @@ def test_inequality_of_node_versions():
|
||||
assert NodeVersion("v24.02.1") > NodeVersion("v24.02")
|
||||
assert NodeVersion("v24.02.1") > NodeVersion("v24.02rc1")
|
||||
assert NodeVersion("v24.02.1") > NodeVersion("v23.05")
|
||||
assert NodeVersion("v24.05") > NodeVersion("v24.02")
|
||||
assert NodeVersion("vd6fa78c") > NodeVersion("v26.02")
|
||||
|
||||
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02.1")
|
||||
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02")
|
||||
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02rc1")
|
||||
assert NodeVersion("v24.02.1") >= NodeVersion("v23.05")
|
||||
assert NodeVersion("v24.05") >= NodeVersion("v24.02")
|
||||
assert NodeVersion("vd6fa78c") >= NodeVersion("v26.02")
|
||||
|
||||
assert NodeVersion("v24.02.1") <= NodeVersion("v24.02.1")
|
||||
assert not NodeVersion("v24.02.1") <= NodeVersion("v24.02")
|
||||
assert not NodeVersion("v24.02.1") <= NodeVersion("v24.02rc1")
|
||||
assert not NodeVersion("v24.02.1") <= NodeVersion("v23.05")
|
||||
assert not NodeVersion("v24.05") <= NodeVersion("v24.02")
|
||||
assert not NodeVersion("vd6fa78c") <= NodeVersion("v26.02")
|
||||
|
||||
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02.1")
|
||||
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02")
|
||||
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02rc1")
|
||||
assert not NodeVersion("v24.02.1") < NodeVersion("v23.05")
|
||||
|
||||
|
||||
def test_comparision_parse():
|
||||
assert _CompareSpec.parse("===v24.02").operator == "==="
|
||||
assert _CompareSpec.parse("=v24.02").operator == "="
|
||||
assert _CompareSpec.parse("!===v24.02").operator == "!==="
|
||||
assert _CompareSpec.parse("!=v24.02").operator == "!="
|
||||
assert _CompareSpec.parse(">v24.02").operator == ">"
|
||||
assert _CompareSpec.parse("<v24.02").operator == "<"
|
||||
assert _CompareSpec.parse(">=v24.02").operator == ">="
|
||||
assert _CompareSpec.parse("<=v24.02").operator == "<="
|
||||
|
||||
|
||||
def test_compare_spec_from_string():
|
||||
assert VersionSpec.parse("=v24.02").matches("v24.02rc1")
|
||||
assert VersionSpec.parse("=v24.02").matches("v24.02")
|
||||
assert not VersionSpec.parse("=v24.02").matches("v24.02.1")
|
||||
|
||||
# Yes, I use weird spaces here as a part of the test
|
||||
list_spec = VersionSpec.parse(">= v24.02, !=== v24.02rc1")
|
||||
assert list_spec.matches("v24.02")
|
||||
assert list_spec.matches("v24.02.1")
|
||||
|
||||
assert not list_spec.matches("v24.02rc1")
|
||||
assert not list_spec.matches("v23.11")
|
||||
|
||||
|
||||
def test_ci_modded_version_is_always_latest():
|
||||
v1 = NodeVersion("1a86e50-modded")
|
||||
|
||||
assert v1 > NodeVersion("v24.02")
|
||||
assert not NodeVersion("v24.05") < NodeVersion("v24.02")
|
||||
assert not NodeVersion("vd6fa78c") < NodeVersion("v26.02")
|
||||
|
||||
@@ -10,7 +10,7 @@ from collections import OrderedDict
|
||||
from decimal import Decimal
|
||||
from pyln.client import LightningRpc
|
||||
from pyln.client import Millisatoshi
|
||||
from pyln.client import NodeVersion, VersionSpec
|
||||
from pyln.client import NodeVersion
|
||||
|
||||
import ephemeral_port_reserve # type: ignore
|
||||
import json
|
||||
@@ -629,11 +629,8 @@ class LightningD(TailableProc):
|
||||
cln_version_proc = subprocess.check_output([self.executable, "--version"])
|
||||
self.cln_version = NodeVersion(cln_version_proc.decode('ascii').strip())
|
||||
|
||||
try:
|
||||
if VersionSpec.parse("<=v24.02.2").matches(self.cln_version):
|
||||
self.opts['log-level'] = "debug"
|
||||
except Exception:
|
||||
raise ValueError(f"Invalid version {type(self.cln_version)} - {self.cln_version}")
|
||||
if self.cln_version <= "v24.02.2":
|
||||
self.opts['log-level'] = "debug"
|
||||
|
||||
opts = {
|
||||
'lightning-dir': lightning_dir,
|
||||
@@ -671,11 +668,9 @@ class LightningD(TailableProc):
|
||||
# In case you want specific ordering!
|
||||
self.early_opts = []
|
||||
|
||||
try:
|
||||
if VersionSpec.parse(">=v23.11").matches(self.cln_version):
|
||||
self.early_opts.append('--developer')
|
||||
except Exception:
|
||||
raise ValueError(f"Invalid version {type(self.cln_version)} - {self.cln_version}")
|
||||
# Before this we had a developer build option.
|
||||
if self.cln_version >= "v23.11":
|
||||
self.early_opts.append('--developer')
|
||||
|
||||
def cleanup(self):
|
||||
# To force blackhole to exit, disconnect file must be truncated!
|
||||
@@ -855,11 +850,8 @@ class LightningNode(object):
|
||||
if EXPERIMENTAL_SPLICING:
|
||||
self.daemon.opts["experimental-splicing"] = None
|
||||
# Avoid test flakes cause by this option unless explicitly set.
|
||||
try:
|
||||
if VersionSpec.parse(">=v24.11").matches(self.cln_version):
|
||||
self.daemon.opts.update({"autoconnect-seeker-peers": 0})
|
||||
except Exception:
|
||||
raise ValueError(f"Invalid version {type(self.cln_version)} - {self.cln_version}")
|
||||
if self.cln_version >= "v24.11":
|
||||
self.daemon.opts.update({"autoconnect-seeker-peers": 0})
|
||||
|
||||
if options is not None:
|
||||
self.daemon.opts.update(options)
|
||||
@@ -1204,7 +1196,7 @@ class LightningNode(object):
|
||||
#
|
||||
# The field `updates`-field didn't exist prio to v24.02 and will be
|
||||
# ignored for older versions of cln
|
||||
if VersionSpec.parse(">=v24.02").matches(self.cln_version):
|
||||
if self.cln_version >= "v24.02":
|
||||
if "remote" not in channel.get("updates", {}):
|
||||
return False
|
||||
|
||||
|
||||
Reference in New Issue
Block a user