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

VLS Modifications to CLN v24.02 #106

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Conversation

ksedgwic
Copy link
Collaborator

Don't merge this, it's for tracking ...

daywalker90 and others added 5 commits February 27, 2024 13:29
Increasing the min version of the hsmd due that we
added new code that required the hsmd to sign an announcements.

One of the solution is to increase the min version in this way
a signer like VLS fails directly during the init phase.

Link: ElementsProject#7074
Changelog-None: hsmd: increase the min version
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
This workflow has never had a particularly good signal-to-noise ratio,
and disabling it saves us a couple of GH Actions cycles, with only
minor reduction in test reach. We should rather rethink this and use
the installation instructions to write Dockerfiles for each described
architecture, and then have the CI test-build those Dockerfiles. That
would also cover the docs during testing, rather than having yet another
place where the instructions can bitrot away.

Changelog-None No user-visible change.
Thanks to amazing debugging assistance from grubles, we figured out
that indeed, my memory was correct: write and mmap are not consistent
on all platforms.  The easiest fix is to disable mmap on OpenBSD for now:
the better fix is to do in-place updates using the mmap, and only rely
on write() for append (which always causes a remap anyway before it's accessed).

Fixes: ElementsProject#7109
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
cdecker and others added 21 commits February 28, 2024 14:38
We have installation instructions that tell the user to use `poetry`
and then we ourselves think we're clever and install only a known
subset? It was only a matter of time until we broke this.

Changelog-None
Signed-off-by: Lakshya Singh <lakshay.singh1108@gmail.com>
Added explicit preapproveinvoice/preapprovekeysend:
- tests/test_pay.py::test_setchannel_routing
- tests/test_pay.py::test_forward_pad_fees_and_cltv
- tests/test_pay.py::test_forward_different_fees_and_cltv
- tests/test_renepay.py::test_pay
- tests/test_renepay.py::test_htlc_max
- tests/test_renepay.py::test_fee_allocation
- tests/test_renepay.py::test_fees
- tests/test_renepay.py::test_previous_sendpays

SKIPPED:
- tests/test_db.py::test_db_forward_migrate : no such channel
- tests/test_invoices.py::test_invoices_wait_db_migration : no such channel
- tests/test_reckless.py::test_poetry_install : no canned github server in gitlab CI
- tests/test_reckless.py::test_install : no canned github server in gitlab CI
- tests/test_reckless.py::test_tag_install : no canned github server in gitlab CI

VLS_SKIP_SPLICE_TESTS:
- tests/test_splicing_insane.py::test_splice_insane
- tests/test_bookkeeper.py::test_bookkeeping_inspect_mfc_dual_funded

VLS_PERMISSIVE:
- tests/test_pay.py::test_pay_disconnect : feerate above maximum: 880782 > 151000
The problem is that the test first tries w/ a zero amount invoice and
then tries to succeed w/ an explicit amount.  This only works if the
amount is reduced to sub-htlc levels and thus avoids the VLS balance
check.
Copy link
Collaborator

@devrandom devrandom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for going over the tests in detail.

looks good except for below. however, I didn't evaluate about every single skip.

@@ -960,6 +960,7 @@ def test_expiry_startup_crash(node_factory, bitcoind):
l1.start()


@unittest.skipIf(os.getenv('SUBDAEMON').startswith('hsmd:remote_hsmd'), "no such channel")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm... does CLN change the dbid? that will break things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants