Integrating VLS option on tests #8917
Open
enaples wants to merge 1 commit intoElementsProject:masterfrom
Open
Conversation
Collaborator
Author
|
On because some error on vls-side. I report logs for convenience: |
Collaborator
Author
|
WHAT SHOULD BE TAKEN INTO ACCOUNTHaving |
Member
Good catch, that we can only run a single node using the vls signer. However if we do not set the envvar, and set it in the proc.env field we can give each node their own signer. So don't set the envvar, set the proc.env as you spawn the signer in start() and it should wire things up correctly. For reference: |
cdecker
reviewed
Apr 1, 2026
Member
cdecker
left a comment
cdecker
left a comment
There was a problem hiding this comment.
And it looks like I forgot to Submit my review once again. Apologies for the delay 🙏
| class ValidatingLightningSignerD(TailableProc): | ||
| def __init__(self, vlsd_dir, vlsd_port, vlsd_rpc_port, node_id, network): | ||
| TailableProc.__init__(self, vlsd_dir, verbose=True) | ||
| self.executable = env("REMOTE_SIGNER_CMD", 'vlsd2') |
Member
There was a problem hiding this comment.
These are commonly called something_PATH cmd implies a full command including arguments.
|
|
||
| @pytest.mark.openchannel('v1') | ||
| @pytest.mark.openchannel('v2') | ||
| def test_vls_simple(node_factory): |
Member
There was a problem hiding this comment.
We probably want a marker for opt-in here: @pytest.mark.vls
This allows us to pick only allowlisted tests in the VLS test, and gives us time to go through the tests and see if they are supposed to work with VLS.
| }[self.vls_mode] | ||
|
|
||
| self.use_vlsd = subdaemon is not None | ||
| self.use_vls = use_vls is not None |
Member
There was a problem hiding this comment.
Technically this maps use_vls=False to self.use_vls=True
| self.daemon.env["VLS_LSS"] = os.environ.get("LSS_URI", "") | ||
| self.daemon.env["VLS_NETWORK"] = self.network | ||
| self.daemon.env["BITCOIND_RPC_URL"] = env("BITCOIND_RPC_URL", "http://rpcuser:[email protected]:{}".format(self.bitcoin.rpcport)) | ||
| self.daemon.env["VLS_CLN_VERSION"] = env("VLS_CLN_VERSION", "v25.12-391-gc1dc506-modded") |
Member
There was a problem hiding this comment.
Don't set a default value, error out if it is required and not set.
Member
There was a problem hiding this comment.
The default value will always be wrong, except for the one day the snapshot was taken.
| self.daemon.env["VLS_CLN_VERSION"] = env("VLS_CLN_VERSION", "v25.12-391-gc1dc506-modded") | ||
| self.daemon.env["BITCOIND_RPC_URL"] = env("BITCOIND_RPC_URL", f"http://{BITCOIND_CONFIG['rpcuser']}:{BITCOIND_CONFIG['rpcpassword']}@127.0.0.1:{self.bitcoin.rpcport}") | ||
| cln_version_str = subprocess.check_output([self.daemon.executable, "--version"]).decode('ascii').strip() | ||
| self.daemon.env["VLS_CLN_VERSION"] = env("VLS_CLN_VERSION", cln_version_str) |
| self.daemon.env["VLS_LSS"] = os.environ.get("LSS_URI", "") | ||
| self.daemon.env["VLS_NETWORK"] = self.network | ||
| self.daemon.env["VLS_LSS"] = env("LSS_URI", "") | ||
| self.daemon.env["VLS_NETWORK"] = env("VLS_NETWORK", self.network) |
Member
There was a problem hiding this comment.
I think these get inherited automatically, but good habit to explicitly pass them in.
enaples
force-pushed
the
vls-hsmd-test
branch
2 times, most recently
from
9e087a6 to
b1ccf87
Compare
Add a `use_vls` per-node option to `LightningNode` that spawns a `vlsd` signer process per node and routes hsmd through `remote_hsmd_socket`. Each node gets its own datadir, ports, and signer env so multiple signers can run in parallel. - New `ValidatingLightningSignerD` (tests/vls.py) wraps vlsd as a TailableProc; resolves the binary from `REMOTE_SIGNER_PATH` or builds it via `VLS_AUTO_BUILD=1`. - `NodeFactory` is extended so `use_vls` reaches the node ctor as a kwarg instead of being forwarded as a lightningd CLI flag. - `dev-force-features=-63` is set on VLS-backed nodes to strip the optional splice bit, since VLS does not implement WIRE_HSMD_SIGN_SPLICE_TX and lightningd would otherwise fatal() during hsm_init. - `VLS_AUTOAPPROVE=1` is set on the vlsd process env so invoice preapproval doesn't decline payments in tests. - Three smoke tests added: send/receive/route through a VLS-backed node.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
26.04 FREEZE March 11th: Non-bugfix PRs not ready by this date will wait for 26.06.
RC1 is scheduled on March 23rd
The final release is scheduled for April 15th.
Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:
tools/lightning-downgradeChangelog-None
Refactoring fixtures to test VLS implementation
The goal is to integrate VLS testing into the CLN test suite in a way that is self-contained — no changes to the
pyln-testinglibrary — and flexible enough to test different VLS implementations at different stages (different repos or commits) against the current CLN version, taking inspiration fromtools/reckless.The core idea is that
ValidatingLightningSignerDshould be generic: driven by the repos variable and optionally a target commit, it clones, builds and manages any conforming signer implementation. The fixture layer then decides which nodes use it and which implementation to test (so far only this is supported).