Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgraded greenlight core lightning version to 24.02.1. #408

Merged
merged 16 commits into from
Jun 6, 2024

Conversation

Randy808
Copy link
Collaborator

@Randy808 Randy808 commented Apr 15, 2024

Rebased and made this branch into a draft pull request so it's ready to merge when we've upgraded the cln version on our servers

  • Requires core lightning v24.02.1 to be pushed to cloud storage as a greenlight artifact. Otherwise check-self will fail with make: *** [Makefile:148: cln-versions/lightningd-v24.02.1gl1.tar.bz2] Error 8

@nepet
Copy link
Collaborator

nepet commented Apr 15, 2024

Also, an update of the signer version is needed to upgrade the node to v24.02. Can be found in signer/mod.rs : VERSION. This is called on maybe_upgrade.

@Randy808 Randy808 force-pushed the cln-upgrade-24.02.1 branch 3 times, most recently from 917021c to 2cc11b7 Compare April 18, 2024 19:17
@cdecker
Copy link
Collaborator

cdecker commented May 17, 2024

Rebased on top of master

@cdecker cdecker force-pushed the cln-upgrade-24.02.1 branch 2 times, most recently from 28c3511 to 90108a0 Compare May 24, 2024 16:22
@nepet
Copy link
Collaborator

nepet commented May 28, 2024

libs/gl-testing/node.py class NodeProcess is missing the --developer flag on versions >= 24.02. Adding the following snippets fixes this for the time being until pyln-testing supports multiple versions.

#libs/gl-testing/gltesting/utils.py

def version_to_int(nv: NodeVersion):
        # Remove the initial 'v'
        version_string = nv.name.lstrip('v')

        # Split the version string into parts by '.' and strip any non-numeric suffix
        parts = version_string.split('.')
        numeric_parts = []
        
        for part in parts:
            numeric_part = ''.join(filter(str.isdigit, part))
            numeric_parts.append(numeric_part.zfill(2))  # Zero-fill to ensure 2 digits per part
        
        # Ensure there are exactly 3 parts, padding with '00' if necessary
        while len(numeric_parts) < 3:
            numeric_parts.append('00')
        
        # Join the parts into a single string
        parsed_version = ''.join(numeric_parts[:3])

        return int(parsed_version)
#libs/gl-testing/gltesting/node.py

class NodeVersion(TailableProcess):
...

   self.cmd_line = [
            str(self.executable),
            f'--lightning-dir={self.directory}',
            f'--network={network}',
            '--log-level=debug',
            '--bitcoin-rpcuser=rpcuser',
            '--bitcoin-rpcpassword=rpcpass',
            f'--bitcoin-rpcconnect=127.0.0.1:{self.bitcoind.rpcport}',
            "--disable-plugin=commando",
            '--rescan=1',
            "--log-timestamps=false",
            "--cltv-final=6",
            f"--addr=127.0.0.1:{self.p2p_port}",
            # Stock `cln-grpc` disabled in favor of `gl-plugin`
            '--disable-plugin=cln-grpc',
            f'--subdaemon=hsmd:{signerproxy_path}',
            f'--important-plugin={plugin_path}',
            '--dev-bitcoind-poll=5',
            '--dev-fast-gossip',
            '--offline',
            '--experimental-anchors',
        ]

        if version_to_int(self.version) >= 240200:
            self.cmd_line.append('--developer');

...

This reduces the checks that fail to

sendcustommsg changed on pyln-client (no keyword args):

  • libs/gl-testing/tests/test_node.py::test_custommsg
  • libs/gl-testing/tests/test_lsps.py::test_lsps_list_protocol

some signer errors: - ValueError: Some signer reported an error: [msg: "resolver error:

  • libs/gl-testing/tests/test_gl_node.py::test_configure_close_to_addr
  • libs/gl-testing/tests/test_node.py::test_node_invoice_amountless
  • libs/gl-testing/tests/test_node.py::test_lsp_jit_fee
  • libs/gl-testing/tests/test_node.py::test_vls_crash_repro
  • libs/gl-testing/tests/test_node.py::test_sendpay_signer
  • libs/gl-testing/tests/test_node.py::test_node_listpays_preimage

Other tests are good!

@cdecker
Copy link
Collaborator

cdecker commented May 31, 2024

Ok, I'm going to try and fix this up asap, so we have the new version unlocked 👍

@cdecker cdecker marked this pull request as ready for review June 4, 2024 13:17
Randy808 and others added 12 commits June 5, 2024 16:59
The hook's amount has been changed from string to u64, so we need to
support either.
Dropping the hotfix suffix since that is subsumed into the gl versions.
I was wondering why my signer was not reconnecting to the node, and
found out that I was on the v24.02 branch, which isn't deployed on the
servers yet. The problem was that we were calling the naked, fallible,
future in `task::spawn()` and therefore dropping an eventual
error. Maybe we should also move the upgrade out of the loop in the
first place 🤔
Just patching this one over for now until we find the root cause.
We had missed this one place where the unit was not being parsed
correctly from an amount with a unit attached at the end, resulting in
incomplete parses. This fixes that.
@cdecker cdecker enabled auto-merge (rebase) June 6, 2024 12:54
@cdecker cdecker merged commit 76297c5 into main Jun 6, 2024
13 checks passed
@cdecker cdecker deleted the cln-upgrade-24.02.1 branch June 6, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants