Pool fixed

[Vic-Nas]

The pool system is fragile.
I made a few fixes but there r still some connexions problems.

I merged your security fixes

[Bitflash-sh]

Thanks for this, and sorry for the slow review — there is a lot of real work in
here. I went through it properly rather than skimming.

The headline number is misleading: 84 files and +73k lines, but most of that is
vendored ImGui, bitmaps and content already on main. The actual source diff is
24 files, about +2000/-1000, and that part is good work. The compact block
subsystem with its own test, the Stratum share fixes, the connectivity changes —
those are worth having.

Two things block it as it stands. Neither is a criticism of the code you wrote.

1. It would revert the 1.2.2 hardening

You mentioned merging the security fixes, and the 1.2.1 ones are all here — I
checked by content, not by history, since you reapplied them under your own
commits. OP_RETURN, the OP_CAT group, the duplicate-transaction check, the
halving-by-height fix: all present and correct.

The 1.2.2 set is not, and the timing explains why — you opened this at 18:09 and
that batch merged at 18:25, so you could not have had it:

	present
#16 orphan cap	no
#17 Nostr frame bounds	no
#18 string unserialize	no — still str.resize(nSize)
#19 Stratum bounds	no
#20 sigops cap (consensus)	no

The last one is the problem. MAX_BLOCK_SIGOPS and GetSigOpCount do not exist
in this branch, and CBlock::CheckBlock differs from main by exactly that. A
node built from here accepts blocks that 1.2.2 rejects, which means a chain
split. Everything else in consensus — ConnectBlock, AcceptBlock,
ConnectInputs, GetBlockValue — is byte-identical, and compactblock does not
touch validation at all. So a rebase on current main should fix this cleanly.

2. update.cpp cannot go in without signature verification

The self-updater downloads a release asset and replaces the running executable.
There is no signature check and no checksum check — the header says it plainly:

Downloads info.asset_url to a temp file, verifies it's non-empty, then
atomically swaps it in for the running executable and relaunches

Non-empty is the only validation before swapping the binary that holds users'
private keys. Anyone who compromises the release, or the account that publishes
it, gets code execution on every node that updates.

I want to be clear that I am not reading anything into this. It is not wired up —
nothing calls it outside the file itself — and it reads as a convenience feature.
But it does compile into the binary, so it is one line away from being live, and
UPDATE_REPO is a build flag whose documented example points at a different repo.

If you want to pursue it: sign the release artifacts and verify the signature
against a key baked into the binary before the swap. Then it becomes a genuinely
useful feature. Until then it cannot merge.

What I would suggest

Split it up, and I will review each part quickly:

1. Pool and Stratum fixes — rebased on current main. This is the one I would
   take first; the share-rejection fixes in particular sound like they matter.
2. compactblock — self-contained, has a test, easy to review on its own.
3. The updater — separately, and only with signature verification.

Happy to help with the rebase if the conflicts are annoying — main has moved
quite a bit (1.2.1, 1.2.2, and three changes today).

One unrelated question while I have you: 62b41c8e stopped persisting the mining
mode across restarts and b00bba93 made Relay the startup default. That is on
main now and it caught me out twice today — a node restarts and quietly stops
mining. What was the reasoning? If it was working around something specific I
would rather fix that than revert your change.

[Bitflash-sh]

That was fast, thank you. I reviewed the code rather than the commit messages,
and both blockers are properly closed.

Consensus: MAX_BLOCK_SIGOPS is back and CBlock::CheckBlock matches main
again, so the chain-split risk is gone. #16, #18 and #19 are in as well.

The updater: this is a real fix, not a gesture. Compiled-in 32-byte Ed25519
key, .sig companion fetched separately, EVP_DigestVerify before anything is
swapped, and it fails closed — your own comment says missing signature, network
error and mismatch are all treated the same, which is exactly right. I confirmed
VerifyDownloadedAsset is called on both the Windows and the Linux path before
the swap, and that the CI step fails the whole release when the signing secret is
absent, so an unsigned release can't be published by accident. Good work.

Three things left.

1. The signing key has to be this project's

kUpdatePubKey is your key, and the private half is a secret in your CI. If this
merges as-is, whoever holds that key can sign an update that every node installs.

Nothing about that is aimed at you — it is simply the wrong place for the key to
live, in both directions. It makes you a single point of compromise for a project
you contribute to rather than run, and it is a burden you did not sign up for.

So: I will generate a keypair for the project, hold the private half as a secret
here, and send you the 32 bytes to put in the constant. Nothing else in your code
changes.

2. When it gets wired up, it must ask every time

The updater is not called from anywhere yet, so how it surfaces is still open.
Your header already describes the right flow — show version and notes, then apply
only if the user accepts — and I would like that to stay a hard rule rather than a
default.

A signed auto-updater on a consensus system is a channel for changing the rules on
every node at once. Signature verification makes that channel authentic; it does
not make it reviewed. Explicit consent per update is what keeps a release an offer
instead of an instruction. Check and apply being separate functions already gets
this right — I just want it stated so nobody wires them together later.

3. Two smaller things

• nostr: bound what a relay can make the node allocate #17 did not come along. The Nostr relay bounds are still missing on this
  branch: a 64-bit WebSocket frame length going straight into a resize, no ceiling
  on reassembly, and six get<string>() calls without a type check — a relay
  answering [1] takes the thread down. Same rebase should pick it up.

• CURLOPT_CAINFO, "cacert.pem" is a relative path, at three call sites. It
  resolves against the working directory, so a node launched from anywhere other
  than its own folder can't find the CA bundle, TLS fails, and the updater quietly
  never works. It fails closed, so it is not dangerous — just invisible. Worth
  resolving against the executable's directory.

The offer to split this into separate PRs still stands and I still think it would
be easier on both of us, but if you would rather keep it as one I will review it
as one. Either way, tell me when you want the public key.

[Bitflash-sh]

Here is the project's update signing key. Replacing the constant in update.cpp
with these bytes is the last thing blocking the updater from my side:

// Ed25519 public key that release signatures are checked against. The
// matching private key never touches this repo; it lives only as a CI
// secret and is used to sign each release asset (see release.yml).
const unsigned char kUpdatePubKey[32] = {
    0x73, 0x59, 0x32, 0xc7, 0x06, 0x69, 0x3b, 0xb4, 0x0f, 0x3b, 0x3c, 0x3a,
    0xd7, 0x41, 0x3e, 0x3f, 0x69, 0x73, 0xb8, 0x6d, 0x6a, 0xc5, 0x63, 0xd1,
    0x60, 0xff, 0x6a, 0x6d, 0x43, 0x28, 0xb2, 0xb2
};

The private half is an UPDATE_SIGNING_KEY secret on this repository and is not
held by either of us personally in any form that could sign a release from a
laptop. Your release.yml already fails the build when that secret is missing,
so this repo cannot publish an unsigned release by accident — that check is doing
real work now rather than being theoretical.

Your comment above it can stay exactly as written; it describes the arrangement
correctly.

Still open from my last review, all minor next to what you have already done:

• nostr: bound what a relay can make the node allocate #17 — the Nostr relay bounds did not come across with Cap orphan transactions held in memory #16/serialize: grow strings as data arrives, not to the declared length #18/rpc: bound and guard the Stratum surface #19/Cap signature operations per block #20.
• CURLOPT_CAINFO, "cacert.pem" — relative path at three call sites, so it
  resolves against the working directory rather than the executable's.

Two things landed on main meanwhile that will touch your rebase, both small:

• /port now exists and is parsed (main_gui: implement /port, which net.cpp already documented #28). net.cpp had documented it for a long
  time without anything reading it, so two nodes could never share a machine.
• OutputDebugStringF was dropping log lines under concurrency (util: stop debug.log dropping lines under concurrency #29). The path
  was resolved lazily without a lock, so fopen could get a torn path and the
  line vanished silently. Unconditional startup lines were reaching the file in
  as few as 2 runs out of 6. If you have ever chased something in this codebase
  that "never ran" because it was absent from debug.log, that may be why — it
  cost me an afternoon.

[Bitflash-sh]

Correction to my comment above — you fixed the cacert.pem path while I was writing it. CaInfoPath() at all three call sites, resolved against the executable's own directory. Disregard that bullet; #17 is the only one of the three still outstanding.

[Bitflash-sh]

Saw you closed this — no problem, and no need to explain.

Just so it is said plainly: the invitation stands, with no deadline and no
particular shape. If you ever want to bring any of it over, split or whole, I
will review it. If you would rather keep building on your own fork, that is
entirely legitimate and I hope it goes well.

Either way, today was good work. You closed a consensus revert and built proper
signed-update verification in about two hours, from a review that landed on you
without warning. That is not a small thing.