Update Bisq to 1.10.4 and migrate desktop to Selkies - #8
Conversation
|
tested the PR bisq is unable to connect to bitcoin core after update |
|
It just used P2P nodes before so it wasn't a part of this PR but it makes sense to connect locally so I'll add it. |
|
Tested, working as expected. Thanks @BeeJoe |
helix-nine
left a comment
There was a problem hiding this comment.
Thanks for this — the 1.10.4 bump is a security release we want, and the peer-local wiring in the follow-up commit is textbook. A few things to resolve before merge, one of them mechanical.
1. Why does Bisq need hardwareAcceleration and nvidiaContainer?
I traced both flags through start-core:
hardwareAcceleration: true(startos/manifest/index.ts:21) makes StartOSmknodeverything matching["/dev/dri", "/dev/nvidia*", "/dev/kfd"]into the service's LXC —lxc/mod.rs:42and:304.nvidiaContainer: true(:18) overlays the host's NVIDIA userspace driver stack onto the subcontainer rootfs —service/effects/subcontainer/mod.rs:132-142.
The Dockerfile undercuts the justification: Dockerfile:104 pins SELKIES_ENCODER="x264enc,jpeg", and both of those are software GStreamer encoders. The stream is CPU-encoded either way, so the GPU buys nothing on the encode path. The only residual benefit is hardware GL for the desktop via DISABLE_ZINK=false / DISABLE_DRI3=false — and what's being rendered is a JavaFX trading UI that ran fine on KasmVNC's software pipeline for the last four revisions.
The cost isn't theoretical. On any box with an iGPU — most of them — /dev/dri exists, so this passes the host GPU render node into a hot-wallet container, and main.ts:33-37 then widens it with chmod o+rw so the unprivileged webtop user can reach it. Across both registries these flags appear only on ollama, llama-cpp, vllm, and immich — packages that actually compute on a GPU. Bisq is the only webtop in the fleet, so there's no precedent being followed here; it reads like the LinuxServer base image's optional capability carried in wholesale rather than deliberately chosen.
Please drop both unless there's a measured benefit I'm missing. Doing so also removes the churn they caused: the SubContainer.eager switch, the chmod exec, and the AGENTS.md edit documenting the rename all exist only to serve the DRI passthrough.
If they come out — does Selkies fall back to llvmpipe cleanly with DISABLE_ZINK=false / DISABLE_DRI3=false, or should those flip to true? And was your test box GPU-equipped, i.e. was the passthrough path ever actually exercised?
2. Blocker: the head commit was never built by CI
abbbc93 (head) → build | completed | skipped
5c8005a → build / DiscoverMatrix | completed | success
build / BuildMatrix (x86) | completed | success
The PR was converted to draft at 05:03:44, abbbc93 was pushed at 05:13:30 (job skipped by if: github.event.pull_request.draft == false), then marked ready at 05:16:04. build.yml's on: pull_request doesn't list ready_for_review in its activity types, so nothing re-fired. Note that gh pr view's status rollup reports a green build / Build for this PR — that's stale, carried over from commit 1; the check-runs API for the head SHA is authoritative.
That matters because commit 2 is exactly the part that needs validating: a new npm dependency, a new cross-package TypeScript import, and a lockfile change. A workflow_dispatch run or an empty commit will do it.
FWIW I ran npm ci and npm run check locally against the head commit and both are clean, and I confirmed the Prettier warning on startos/i18n/dictionaries/translations.ts is pre-existing on master — so your validation notes check out. CI green is still the merge gate.
3. Likely functional gap: peerbloomfilters
Bisq is a BitcoinJ SPV client, so it needs its peer to serve BIP37 bloom filters. Three facts from bitcoin-core-startos itself:
peerbloomfiltersis aValue.triStatewithdefault: nullandfootnote: Default: false— off unless the user turns it on.- Its own description reads "It is highly recommended AGAINST using for anything except Bisq integration", and its warning reads "This is ONLY for use with Bisq integration." Bisq is the reason that toggle exists.
whitebindis set bare (0.0.0.0:58334,fileModels/bitcoin.conf.ts:651), so the listener grants only Core's default whitebind permissions. Your own validation notes confirm what that resolves to: "noban, relay, mempool, and download." Nobloomfilter.
Before this PR btcNodes= was empty and Bisq discovered public peers, so the toggle never mattered. Now btcNodes pins Bisq to that one node with no fallback — so a user who hasn't enabled bloom filters gets no Bitcoin connectivity at all, which is a regression from "works out of the box." @martinbarilik, you're an existing Bisq user, so it's quite possible you already had this set; could you confirm whether Serve Bloom Filters to Peers is on in your Bitcoin config?
The fix is the documented mechanism, and electrs-startos is the reference implementation for this exact host:
await sdk.action.createTask(effects, 'bitcoind', autoconfig, 'critical', {
input: {
kind: 'partial',
accept: [{ peerbloomfilters: true }],
set: { peerbloomfilters: true },
},
when: { condition: 'input-not-matches', once: false },
reason: i18n('Bisq connects to your Bitcoin node as an SPV client and needs it to serve bloom filters.'),
})I haven't proven Bisq hard-fails without it — worth testing against a node with the toggle explicitly off.
4. Release notes name a specific Bitcoin flavor
All five locales in startos/versions/current.ts say "Bitcoin Core" (de_DE: "von Bitcoin Core", fr_FR: "de Bitcoin Core", etc.). The packaging guide requires user-facing text — release notes explicitly included — to call the dependency Bitcoin, never Bitcoin Core or Bitcoin Knots.
This isn't just style here: Bitcoin Knots #knots:29.3.1:16 carries .satisfies('28.4:17') and exposes the same peer-local host, so it satisfies your versionRange and Knots users will read release notes telling them Bisq now uses "Bitcoin Core." README.md is developer-facing and can keep the precise name.
5. Smaller items
SubContainer.eager→SubContainer.of. The SDK docs say prefer the lazy factory unless you need synchronousrootfs/guid/subpath()access before calling any method. This only calls.exec(), which materializes a lazy handle internally. Moot if the DRI passthrough goes.- The chmod belongs in an
.addOneshot().recipe-oneshot.mdnames "fix file ownership before the main daemon starts" as the canonical oneshot case. A bareexecinsetupMaindoes run first, but it's off-pattern. Also,ls /dev/dri/* | xargs -r chmodparsesls;chmod o+rw /dev/dri/* 2>/dev/null || trueis the same thing. Again, moot if the flags go. Dockerfile:109.RUN chmod 755 …adds a layer to the stage whose comment says "Flatten into a single layer from scratch."COPY --chmod=755 root/ /does it in one.
What's right
Most of this is careful work and I want to be specific about it:
- The
peer-localwiring inutils.tsmatchesservice-to-service.mdexactly —sdk.host.getBridgeAddresskeyed on the dependency's internal port (notassignedPort),.const()on the minimal mapped value, and absent-means-absent: the env var is omitted andstartwm.shleavesbtcNodesout entirely rather than fabricating an address. versionRange: '>=28.4:17'matcheselectrs-startosand is satisfied by both Core and Knots, so neither flavor is stranded.- The
overridesentry is present and the lockfile correctly collapsestor-startos's transitive@start9labs/start-sdk@2.0.7onto your 2.0.9 pin. - Base image pinned by OCI index digest, with the re-pin procedure documented in
UPDATING.md. ThePATHfix in the flattened stage is a genuine correctness improvement over/lsiopy/bin:$PATH. - Docs updated in the same change rather than deferred.
- Dropping the openbox
rc.xml<maximized>sed is safe — thewmctrlloop instartwm.shstill covers it.
Happy to look again once CI is green.
|
@helix-nine confirming, without bloom filter turned on, there is no connection to a local instance to bitcoin node, which is the state before the abbbc93 commit Other thing, can any of this changes cause DAO being not synced and needed resyncing and getting it even after resync? I had this before here and there, but struggling with it ever since this update constantly. There is also a new JAVA error i haven't seen before, i'll post here later when it pops again if needed. edit: Cannot invoke "javafx.scene.Node.getScene()" is the message of JAVA error mentioned
above are two errors i get constantly now with current state no matter how many times i do resync ( tho this is bisq problem itself, not the update matter ) |
|
Implemented the requested review changes in
Validation is green: @helix-nine ready for re-review. |
|
Thanks @martinbarilik — that settles the bloom-filter question. It's a confirmed blocker rather than a suspicion now, so @BeeJoe the On the DAO / Java errorsI looked at both screenshots, and I think they're one problem rather than two — and it isn't the JavaFX bug the That fits your description precisely. An OOM during DAO block parsing leaves the persisted DAO state inconsistent with the chain; Bisq detects the hash mismatch and prompts for a resync; the resync then re-parses the entire BSQ chain, which is the most memory-intensive thing Bisq does — and OOMs again. Hence "needed resyncing and getting it even after resync." The So I'd hold off on calling it purely an upstream Bisq issue. There's a plausible packaging-side mechanism here. Nothing in this PR touches the JVM heap ceiling — I diffed the env blocks and neither I want to be clear that's a hypothesis, not a diagnosis. Three things would settle it, and they're one command each: For (3), the ~50 lines before each hit are the useful part. And if you can say how much RAM the box itself has, that's the top-line number that makes the rest interpretable. If it does turn out to be memory pressure, it feeds back into the One thing I'd keep separate: DAO state in lite-node mode comes from Bisq's seed nodes over its own P2P network, not from your Bitcoin node, so pruning isn't implicated in the DAO symptom. It would matter for wallet rescan, though — @martinbarilik, is your Bitcoin node pruned? Worth knowing either way, since this PR now pins Bisq to it as its only Bitcoin peer. |
helix-nine
left a comment
There was a problem hiding this comment.
Approving. First, a correction: my comment of 2026-07-29 17:51 UTC was written against the pre-791cb46 tree and posted without reading your 11:34 summary. Two things in it are wrong — the peerbloomfilters task had already landed rather than still needing to, and the GPU passthrough had already been removed rather than still being paired with the software encoder. Sorry for the wasted round trip; that's what prompted the second re-review request, and it shouldn't have.
Review items — all resolved
| Item | Resolution |
|---|---|
Drop hardwareAcceleration / nvidiaContainer |
Gone, along with NVIDIA_DRIVER_CAPABILITIES, DISABLE_ZINK, DISABLE_DRI3, and the chmod o+rw exec |
| CI skipped the head commit | types: [opened, synchronize, reopened, ready_for_review] — and both new commits are green |
peerbloomfilters not required |
Recurring critical autoconfig task, gated on local-only mode |
| Release notes named a specific Bitcoin flavor | Generic wording across all five locales |
SubContainer.eager |
Back to lazy SubContainer.of |
chmod as a bare exec in setupMain |
Now a real bitcoin-node-ready oneshot with requires on the daemon |
RUN chmod in the flatten stage |
COPY --chmod=755 root/ / |
Verified on df6e229: npm ci and tsc --noEmit clean, build / DiscoverMatrix and build / BuildMatrix (x86) green on both 791cb46 and df6e229.
I also checked the one thing that looked like it might be a silent no-op — sdk.action.clearTask(effects, 'bitcoind:autoconfig') passes a replay ID that's never explicitly set on the createTask side. It's correct: createTask defaults replayId to `${packageId}:${actionId}`, so the fallback-mode switch really does clear the task.
On the fail-closed design
I want to retract something I was going to raise. Throwing from setupMain when local-only mode can't resolve the peer is the sanctioned pattern, not a deviation — dependencies.md says a service that genuinely cannot operate without a dependency should "handle that at runtime in setupMain — poll the dependency, retry, or surface your own error. Don't rely on the dependency declaration to block startup for you." Failing closed rather than silently falling back to public peers is also the right default for a trading app. Good call, and it's well documented in instructions.md.
The only thing left is a labeling nit, non-blocking: optional: true on bitcoind reads oddly when the default mode hard-requires it. With a real fallback mode behind it that's defensible either way — just flagging it in case you'd rather the manifest match the default path.
Follow-up for beta, not a merge gate
@martinbarilik's report is still open: the second screenshot is Java heap space, an OutOfMemoryError, and I think the DAO-resync warning and the Node.getScene() NPE are both downstream of it — an OOM during DAO parsing leaves DAO state inconsistent, the resync re-parses the whole BSQ chain, and OOMs again, which is why resyncing never sticks.
That's unreproduced and may well be upstream in 1.10.4, as you both suspected. I don't think it should hold this PR: 1.10.4 is a required security update, and merging here deploys to community-beta, not community-prod — promotion needs a separate go-ahead. Beta is the right place to get more testers on it.
If anyone hits it there, these three are what would settle whether it's packaging or upstream:
start-cli package attach bisq -n bisq-sub -- ps -eo pid,rss,args --sort=-rss
start-cli package attach bisq -n bisq-sub -- free -m
start-cli package attach bisq -n bisq-sub -- grep -n -i "OutOfMemory\|heap space" /config/.local/share/Bisq/bisq.log
The hypothesis worth ruling out first is that SELKIES_ENCODER="x264enc,jpeg" (software libx264) has a larger resident footprint than the KasmVNC pipeline it replaced, squeezing JVM headroom on a box where it was already marginal. Nothing in this PR changes the heap ceiling directly — neither branch sets -Xmx — so the mechanism would be reduced headroom rather than a smaller ceiling. Plus the box's total RAM, which makes the rest interpretable.
Nice work on this one, and thanks for the thorough validation notes.
|
Merged — thanks @BeeJoe. Tag and Release is green on One thing worth knowing for next time: this needed an admin override to merge, and it wasn't the review — it was commit signing. Nothing wrong with the work; GitHub just won't let unsigned commits through that rule on its own. Overriding is fine as a one-off, but it isn't something to lean on, so it's worth setting signing up once and forgetting about it. SSH signing is the least friction — it reuses the key you already push with: git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub # your public key
git config --global commit.gpgsign trueThe step people miss is the second half: the same key has to be registered on GitHub a second time, as a Signing Key. Settings → SSH and GPG keys → New SSH key → set Key type: Signing Key. An authentication key alone will sign locally but still show as Unverified here. If you ever need to fix commits you've already pushed: git rebase --exec 'git commit --amend --no-edit -S' origin/master
git push --force-with-leaseHeads up that Also still open, and now something to watch in beta rather than a blocker: @martinbarilik's |
Sorry for late reply, i was testing the commits of this PR, but i couldnt run bisq from it ( when there was 3rd commit in place ). Every time after entering username, and pass i got black screen and nothing was working. Some tryies i got java and dao error straight right away. i had to downgrade to my custom build which changed just security update to 1.10.4 i am gonna build this PR and run it later today, and paste the above commands for you. i have full node do we create new bug / issue or we keep working here? |


Summary
sha256:a5f7b38bb806c913bdabbe5667aa462d97d2c5ab3710498fe5aeee97c17287f8FROM scratchstagepeer-localinterface instead of public peer discovery, using the interface metadata exported bybitcoin-core-startos>=28.4:17, resolve its live bridge address reactively, and writebtcNodesonly while that binding is availableUpstream release: https://github.com/bisq-network/bisq/releases/tag/v1.10.4
The downloaded Debian package and detached signature were verified during the image build with pinned signer fingerprint
B493319106CC3D1F252E19CBF806F422E222AA02.Validation
Local
npm cinpm run checknpm run buildbash -n root/defaults/startwm.shgit diff --checkmake x861.10.4:0, SDK2.0.9, x86_64 packed image, hardware acceleration enabled, and NVIDIA container metadata enabledjavascript.squashfs: directory andindex.jsare both mode0755bisq_x86_64.s9pk(892 MB), SHA-25628956520e0633fc7089c522b7fe81ba7d49042c8a785b0d55cf89bc7793317acA local container runtime smoke test verified:
wmctrlreports a visibleBisqwindow/manifest.jsonreports Bisq/fullscreenbisq.propertiesuses the supplied peer-local bridge when present and omitsbtcNodeswhen absentStartOS
The original Bisq 1.10.4/Selkies work was tested on two authorized StartOS hosts:
1.10.4:0installed successfully; StartOS correctly blocks service start on its unresolved critical setup/dependency task, and this was not bypassed with--force1.10.3:4to1.10.4:0; after a normal restart, nginx, Selkies, Openbox, and Bisq were live, a visible Bisq window was present, the persistent log contained both the prior 1.10.3 startup and the new 1.10.4 startup, Tor restarted, unauthenticated/authenticated UI checks returned 401/200, and the host had no new notificationsThe peer-local follow-up was exercised with the new x86_64 artifact on an authorized StartOS host:
1.10.4:0bisq.propertiesselected Bitcoin Core's bridge-only port58334noban,relay,mempool, anddownloadpermissionsA full data-safety cycle also passed: backup, hard uninstall, fresh reinstall, and restore. After restore, the StartOS credential-store SHA-256 matched the pre-uninstall value exactly, all 909 durable Bisq files were present, and wallet, user, trade, offer, and dispute state loaded successfully. Bisq then created fresh internal wallet/key backups during startup; those expected runtime writes changed the post-start aggregate data hash without indicating data loss.
The first diagnostic sideload exposed restrictive local umask permissions (
javascript/index.jspacked as mode 0600), which StartOS surfaced as/usr/lib/startos/package/index.js not found. The build now normalizes the generated directory and entrypoint to mode 0755; the corrected package then installed on both hosts.The repository-wide Prettier check still warns on the unchanged
startos/i18n/dictionaries/translations.ts; this PR does not include that unrelated rewrite.