itest: run lnd backend in remote-signing mode#1694
Conversation
Coverage Report for CI Build 26889171221Coverage decreased (-0.03%) to 34.83%Details
Uncovered Changes
Coverage Regressions37 previously-covered lines in 8 files lost coverage.
Coverage Stats
💛 - Coveralls |
e62df0f to
f200946
Compare
|
With |
f1b456f to
1e4f9ea
Compare
1e4f9ea to
1e2cb4a
Compare
1e2cb4a to
1da61fd
Compare
|
I've rebased this, as it seems easy enough to resurrect and review otherwise. Unsure we actually want to run w/remote signing mode in CI, though. |
e45ed66 to
0fec22a
Compare
|
lightninglabs-deploy mute |
|
@GeorgeTsagk: review reminder |
|
!lightninglabs-deploy mute |
This commit adds a new flag to the integration test suite that starts the single lnd node (alice) either in normal or in remote-signing mode. In remote-signing mode, alice is actually a watch-only node that is connected to a seconardy signer node over RPC. Co-authored-by: George Tsagkarelis <george.tsagkarelis@gmail.com>
0fec22a to
28b5766
Compare
To make sure `tapd` works when the connected `lnd` node is running in remote-signing mode, we add a new CI target that runs all integration tests in that mode. Co-authored-by: George Tsagkarelis <george.tsagkarelis@gmail.com>
Co-authored-by: George Tsagkarelis <george.tsagkarelis@gmail.com>
Co-authored-by: George Tsagkarelis <george.tsagkarelis@gmail.com>
The first iteration of this PR converted two FinalizePacket call sites
to a new FinalizeFullySigned helper to work around the fact that lnd's
FinalizePsbt RPC fails on p2tr inputs in watch-only mode ("is not a
p2wkh or np2wkh address"). Six more call sites following the same
signPacket -> FinalizePacket pattern have been added to main since
then and were broken under -lndremotesigner; convert them as well.
Two remaining FinalizePacket call sites (in MultiSigTest and in the
mint+fund+seal test) cannot use FinalizeFullySigned because they need
lnd to sign the BTC fee/change inputs that CommitVirtualPsbts added.
For those, FinalizePacket itself is rewritten to do SignPsbt + a local
MaybeFinalizeAll instead of calling FinalizePsbt RPC. SignPsbt handles
p2tr inputs correctly in both regular and watch-only modes, and the
local finalize sidesteps the FinalizePsbt bug entirely.
28b5766 to
5823c4c
Compare
|
Kept |
jtobin
left a comment
There was a problem hiding this comment.
LGTM, with only one substantial note: the CI workflow still uses the older style that we've since optimized. A few tweaks worth making:
diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 05bc726c..5cd5e16e 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -467,20 +467,30 @@ jobs:
integration-test-lnd-remote-signer:
name: run itests with lnd remote-signer
runs-on: ubuntu-latest
+ needs: build-itest
steps:
- - name: cleanup space
- run: rm -rf /opt/hostedtoolcache
-
- name: git checkout
uses: actions/checkout@v5
- - name: Setup go ${{ env.GO_VERSION }}
- uses: actions/setup-go@v5
+ - name: Download itest binaries
+ uses: actions/download-artifact@v8
with:
- go-version: '${{ env.GO_VERSION }}'
+ name: itest-binaries
+ path: itest
+
+ - name: Initialize environment
+ run: |
+ chmod +x itest/itest.test itest/btcd-itest itest/lnd-itest itest/tapd-itest itest/chantools/chantools
+ mkdir -p ~/.aperture
+
+ - name: CPU info
+ run: |
+ echo "nproc (available to process): $(nproc)"
+ echo "nproc --all (online): $(nproc --all)"
+ lscpu || true
- name: run itest
- run: make itest icase='(mint_assets|basic_send_unidirectional)' remotesigning=1
+ run: scripts/itest_part.sh 0 1 0 --verbose -lndremotesigner -test.run='TestTaprootAssetsDaemon/tranche.*/.*-of-.*/^(mint_assets|basic_send_unidirectional)$'
- name: Zip log files on failure
if: ${{ failure() }}
@@ -499,7 +509,7 @@ jobs:
if: ${{ success() }}
continue-on-error: true
with:
- file: itest/coverage.txt
+ files: itest/regtest/cover/coverage-tranche0.txt
flag-name: 'itest-remotesigner'
format: 'golang'
parallel: trueThis mostly just copies the stuff the other itests do, i.e.:
- ditches the old cleanup space step (no longer needed)
- removes the setup-go step (ditto)
- uses the itest binaries from the previous build job
- invokes scripts/itest_part.sh directly ('make itest' rebuilds the binaries, apparently)
- writes the coverage stuff to tranche0, which is apparently the right location
(One nit, too: the release note commit can probably be dropped.)
Depends on btcsuite/btcwallet#1022 and then lightningnetwork/lnd#10119 which will hopefully make it into
lnd v0.19.3-beta.Created this itest to reproduce lightninglabs/lightning-terminal#1123.
Further investigation then lead to the creation of this bug report issue: lightningnetwork/lnd#10120
So this test will not succeed until lightningnetwork/lnd#10120 is fixed. But putting up the PR as a draft as we should aim to fix that issue and then have these tests run in the CI by default.