| Age | Commit message (Collapse) | Author |
|---|
|
Bring main onto the line this project actually ships (incl. the 1.2.23 compactblock port)
Closes #6, #7, #8, #11, #35, #34, #29, #28, #27, and #16
See merge request !16
|
|
main had drifted to a history with no ancestor in common with the tree we
build and deploy. It sat at be63e83 (2026-08-09) with 28 commits; the shipped
line, deploy/soak-2026-08-21, had 383. The two shared nothing -- git
merge-base returns no result at all -- so anyone cloning this repository
landed on a default branch that was neither current nor related to the
running node.
The 28 commits on main were audited before this merge: every one is already
present in the tree by content, including the two that are hardest to see
because they were rewritten afterwards -- the payout reorg-safety work
(PayoutBlockStillOnChain, since evolved to re-check on every retry rather
than once at maturity) and the nostr relay certificate verification
(SSL_set1_host, now with a Windows root-store path). Nothing on main is lost
by this merge.
Recorded as a merge rather than a force-push: main only ever moves forward,
its old history stays reachable as the first parent, and this is revertible.
The tree is taken wholesale from the merged branch, so main's content is now
exactly what was built and tested.
What this brings onto main:
- compactblock: BIP152-style compact block relay ported from upstream 1.2.23,
primitives and unit test only, with the RAND_bytes failure path closed and
the reconstruction allocation bounded.
- upstream/main 17741d9: isolated testnet parameters and genesis, optional
2 MB large pages for RandomX. Mainnet genesis verified byte-identical
through upstream's BuildGenesisBlock() refactor.
- deploy: every image built on bookworm so binaries run on Debian 12, and the
container build now honours the RandomX and libsecp256k1 pins it had been
silently ignoring since before 1.2.15.
Gates at this tree: 15/15 selftests, compactblock's 17 checks, consensus
constants unchanged against e4f898c apart from the pre-existing
MAX_BLOCK_SIZE_GEN, mainnet genesis intact, money-log check ok.
|
|
Brings the nine commits that are actually the latest on our lineage:
isolated testnet parameters and genesis, optional 2 MB large pages for
RandomX, and five README changes. This is what "up to date" means here --
not the 1.2.2x tags, which carry higher version numbers but were cut
2026-07-30, three weeks before the line we ship, and which upstream/main
does not contain.
Three conflicts, all resolved toward upstream's structure while keeping
what this fork knows:
main.cpp -- upstream moved genesis construction into BuildGenesisBlock() so
mainnet and the new testnet can share it. Verified byte-for-byte before
taking it: MAINNET_GENESIS_TIMESTAMP, _TIME (1753315200), _NONCE (3141),
_HASH, _MERKLE, the scriptPubKey, nBits and nVersion all equal what this
tree built inline. The mainnet genesis hash is unchanged and the asserts
still guard it. Kept our warning about never editing those inputs, and kept
our note that 1753315200 is 2025-07-24, not 2026 -- upstream's comment on
that constant states the wrong year.
net.h -- DEFAULT_PORT = htons(8433) becomes MAINNET_PORT/TESTNET_PORT in
host order. Nothing is lost: net.cpp's GetDefaultPort() applies htons()
itself and is what every caller uses, so the value reaching a sockaddr is
unchanged and mainnet stays on 8433.
selftest.cpp -- the known-tests string had diverged in both directions.
Took the union: upstream's network-params alongside our payout-ledger,
rather than dropping either.
Gates: 15/15 selftests (our 14 plus upstream's network-params), consensus
constants unchanged against e4f898c except the pre-existing
MAX_BLOCK_SIZE_GEN, mainnet genesis intact, compactblock's 17 checks still
pass, money-log check ok, GNU_STACK RW.
|
|
Two fixes to the file imported in the previous commit. Kept separate so the
deviation from upstream's bytes is reviewable on its own.
BuildCompactBlock ignored the RAND_bytes return value. On failure nonceBytes
is left uninitialised and an indeterminate shortIdNonce is memcpy'd out and
sent on the wire -- and that nonce is exactly what the file relies on being
unpredictable so short IDs cannot be precomputed before a block exists or
ground across blocks. Every other RAND_bytes call site in this tree
(bip32.cpp:203, db.cpp:2462, db.cpp:2469) checks for != 1 and fails closed;
this one now does too. The caller falls back to full-block relay.
TryReconstructBlock sized a vector directly from the wire. nTotal is the sum
of two deserialised vector lengths, and serialize.h bounds a vector only by
MAX_SIZE (0x02000000), so once the P2P handlers are wired a single small
message could ask this node to default-construct ~33M CTransactions. The
smallest a transaction can serialise to is 10 bytes, so no block within
MAX_BLOCK_SIZE holds more than MAX_BLOCK_SIZE/10 of them; anything above
that is not a block this node could accept under any circumstance.
Rejecting falls back to full-block relay, which is always correct.
Neither is reachable today -- nothing feeds these functions from the network
yet -- but both are latent the moment the message handlers land, which is
the wrong time to be finding them.
test_compactblock (17 checks) and all 14 selftests pass with both in place.
|
|
Brings src/compactblock.{cpp,h} and test_compactblock.cpp across verbatim
from the upstream-v1.2.23 tag, and gives them a build.
1.2.23 is not a newer release of the line this fork ships. It is a
lightweight tag (7ab5a8c) cut 2026-07-30, on a lineage that upstream/main
(e0be70e, 2026-08-20) does not contain and no remote branch carries -- 272
commits that are not ours, against 376 of ours that are not theirs, from a
merge-base at 56df983. Merging that tag would have deleted 18 files from
src/, selftest.{cpp,h} and the whole 14-test suite among them, so the
content is ported file by file instead.
Compact block relay is the piece worth having: a peer that advertises
sendcmpct gets 48-bit keyed short IDs per transaction rather than the
transaction bytes, and reconstructs the block from its own mempool. It is
purely additive on the wire -- a peer that never sends sendcmpct keeps
receiving full block messages, so no coordinated upgrade is needed.
This commit is the primitives and their unit test only. Nothing calls
BuildCompactBlock or TryReconstructBlock yet: the P2P message handlers
(sendcmpct/cmpctblock/getblocktxn/blocktxn) live in 1.2.23's main.cpp and
are deliberately left for a follow-up, so that no attacker-controlled bytes
reach this code until that wiring is reviewed on its own.
The files needed no API adaptation -- mapTransactions, BEGIN/END and
IMPLEMENT_SERIALIZE are unchanged across the divergence, and it compiled
clean first time. Upstream never wired test_compactblock.cpp into its own
Makefile, so it had never been built there; it links standalone the way
test_bip32 does, supplying its own mapTransactions and GetAppDir to keep
main.cpp out of the link. TEST_LIBS is overridable for the same reason
NODE_LIBS is: the pinned dep tree ships libsodium as a static archive only.
All 17 checks in test_compactblock pass, all 14 selftests still pass, and
the consensus constants are unchanged against e4f898c.
|
|
deploy: land MRs 1-12 and upstream 1.2.20, as shipped in relay component
See merge request !13
|
|
# Conflicts:
# Makefile
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
scripts/check-money-log-lines.sh flattens each LogPrint call in
src/rpc.cpp and fails if a money-moving phrase appears in a plain
LogPrint call. A new line that reports money moving needs
LogPrintAlways and one more phrase in the list.
|
|
Integration review found two more payout lines still on plain LogPrint,
both fired per record inside the batch loop in PayoutThreadFn -- the
same one-window burst shape as the four already converted. The
"writing off" line in ForfeitPayoutRecipient and the "SKIPPED bad
address" line at the write-off in PayoutThreadFn both report
obligations being destroyed, which is the class of line one summary
must never stand in for (issue #3). Message text is unchanged.
|
|
2a804c2 resolves a job's prevhash against mapBlockIndex under cs_main so the
worker log names the chain the pool is building on (#15). It did that on every
mining.notify, unconditionally, before the LogPrint that consumes the answer --
and LogPrint's category gate and its 3-second dedup both run after the lookup
has already happened. The dedup bounds the log line. It never bounded the work.
That left an unbounded cs_main acquisition rate on the participant's
socket-reading thread, driven by the remote pool. Block validation can hold
cs_main for a long time, and this thread is the one that handles
mining.set_difficulty and share-submission replies, so a pool sending rapid
notifies could stall the responses a miner is paid for. Hashing was never at
risk -- jobShared is published before the lookup -- but the reader is not free
to block.
Most notifies in a round are a new job on the same tip, so the answer is now
cached and re-resolved only when it could come out differently: a prevhash we
have not resolved, or one we could not resolve last time while the chain has
since moved. A block's height is fixed once mapBlockIndex knows it, so a cached
"known" cannot go stale; an unknown is re-checked whenever nBestHeight changes,
which is what catches a pool we were behind once we have caught up. The worst
case -- a different prevhash every notify -- degrades to exactly the old cost,
never worse. Measured over a 10-tip round of 30 jobs each, 300 acquisitions
become 10; over 1000 notifies on a single fork we do not have, 1000 become 1.
The log line is unchanged and still prints in full for every job, cached or
not: prevhash plus height-or-"unknown to our chain". Nothing #15 asked for is
lost, and a fork we cannot resolve keeps saying so on every job.
The lookup is also wrapped in LogAcceptsCategory("worker") so the work follows
the gate deciding whether anyone reads it. That is a no-op today -- "worker" is
default-on in LogCategoryDefaultOn() -- and is there so the coupling survives
the category ever moving behind -debug. The caching, not the gate, is what
removes the acquisitions.
The cache is four locals in the session scope, not statics: participant mode
runs exactly one such thread (StartMinerThreads forces nThreads to 1 for
MINE_PARTICIPANT), and session scope additionally means a reconnect re-resolves
rather than trusting an answer from before a possible change of pool.
|
|
gui.cpp is not in NODE_OBJS, so bitflash-node never sees it -- but it is in
GUI_OBJS and `all: bitflash` is the default target, so this is what an
operator who runs the windowed build is running.
Two spawns there were still unchecked. Enabling pool mode from the options
dialog set gPoolServerRunning = true and then called _beginthread without
looking at the result, so a refused spawn left the operator with a pool
that reports itself running, no RPC server for miners to reach, and nothing
in the log. The flag latches, so the branch is never retried either. The
restore dialog called std::thread(RestoreThread).detach() bare, which on a
refused spawn is an uncaught std::system_error and takes the window down --
the same failure issue #18 was about, in the same tree, one target over.
The busy flag stays where it was, set before the spawn and rolled back in
the catch. It cannot move after the spawn: RestoreThread clears it itself
on the way out, and a restore that finishes quickly would have its `false`
overwritten by ours, leaving the dialog greyed out over a search that is
not running.
Verified by type-checking gui.cpp under the Makefile's exact flags against
a stub GLFW/GL header, since this host has neither. That is a syntax and
type check, not a link: gui.cpp reaches GLFW through 13 functions and three
GL calls, all declared with their real signatures, but the GUI target has
not been built here.
|
|
PumpIn's wrapper closed pumpEnd and the rendezvous socket the moment it
exited, without waiting for PumpOut, which then called shutdown() on both
at its own exit. Which of the two got there first decided whether that
shutdown addressed a live descriptor or a number the kernel had already
handed back out -- in which case it tears down an unrelated connection.
The inbound pump wins that race whenever the rendezvous socket dies before
the app socket, which is the ordinary shape of a relay restart or a far
side hanging up.
The failure path added in 0b566b4 has the same hole. Its catch shuts down
rv to wake an inbound pump it assumes is parked in RvReadN. PumpIn does
not park when the rendezvous connection is already dead: it returns on its
first length read and its wrapper closes both descriptors. A refused
pthread_create and a dead rv are not independent -- both are symptoms of a
loaded process -- so that is the likely case, not the exotic one.
Both descriptors now belong to a shared_ptr the two pumps hold for the
whole of their bodies, so the close happens after the last pump has
stopped touching them and no shutdown can precede it. appEnd is unchanged:
the caller gets it on success, StartTunnel closes it on every failure
path, no pump ever touches it.
0b566b4's message says of that catch that "closing those descriptors from
the failure path instead would race it". True as far as it goes, but it
reads as though what the code did instead was safe, and it was not -- the
shutdown raced the same close. There was no ordering there to prefer, only
a missing owner.
Measured against the real code with the relay in a separate process, every
close() and shutdown() traced and pthread_create refused on demand:
normal teardown, 25 tunnels 29 shutdowns on a closed fd -> 0
refused outbound spawn, 25 25 shutdowns on a closed fd -> 0
Close counts are identical before and after (8 and 5 per tunnel, every one
accounted for) and live descriptors return to their starting count, so the
defect was only ever the ordering, never a leak or a double close.
|
|
PoolParticipantMiner returned true from its normal cleanup path whether or
not the pool had ever sent work, so a pool that completed the tunnel and
then dropped was indistinguishable from a healthy one. The caller reset
nRetrySecs to 10 on every such "session", pinning the reconnect at a flat
10s and defeating the backoff in precisely the case it was added for. It
cleared nFailingSince as well, so the "failing for N minutes" line never
printed either -- the worker went back to idling silently.
Return true only once at least one well-formed mining.notify has arrived.
The existing early returns are unaffected: they already mean "no session",
which stays the right answer for any path that leaves the worker without
work, a failed hash-thread spawn included.
|
|
The retry sleep polled only fShutdown, while the loop around it polls
fShutdown, fGenerateBitcoins and nMineMode. That was survivable at a flat
10s. Raising the ceiling to 120s made it expensive: after an operator
cleared Generate or switched to Relay mode, the participant session --
and the vfThreadRunning[3] slot that gates restarting it -- stayed held
for up to two minutes.
Measured with the loop shape replicated and the stop condition flipped 1s
into a 120s sleep: 119.01s to notice before, 1.00s after, via either
fGenerateBitcoins or nMineMode. The fShutdown path is unchanged at 1.00s.
It also narrows the window on a pre-existing unsynchronised read of
vfThreadRunning[3] at gui.cpp:901, where a stop-then-start inside it
leaves mining silently not restarted.
|
|
34cb6e6 added `apt-get update;` before the install. The semicolon means
a failed update does not stop the install, which is the behaviour we
want -- one unreachable repository should not block an install the
lists already on disk can satisfy -- but it was punctuation rather than
a decision, and it was silent. The install then failed several seconds
later with "unable to locate package" and nothing tied the two
together.
Kept it non-fatal, made it explicit with `|| echo`, and wrote the
reasoning down next to it. The install itself is still the last command
in the branch, so a failed install is still the target's status.
Measured all four combinations with apt-get shimmed: a failed update
now warns and carries on, a failed install still aborts.
|
|
fd627b1's message said the rewrite hardened a pin check that used `;`
where it needed `&&`. It hardened one of four. Three recipes still
ended their chain with `echo "==> ... done"; ` after a semicolon, so a
mismatched pin failed the `test`, skipped every build step, and then
ran the echo anyway -- exit 0, and make announced a pinned dependency
that nothing had verified. deps-windows also had the same break after
mkdir and after `make randomx`.
The one the earlier message did not mention is deps-secp256k1, on the
Linux path: libsecp256k1 is signature verification, so that was the
worst of the three to leave reporting success on an unverified
checkout.
Joined each chain with && the whole way to the closing echo, in
deps-secp256k1, and in both halves of deps-windows.
Verified by extracting each recipe as make expands it, splitting it
into logical recipe lines and running each in its own shell the way
make does, with git reporting a sha that does not match the pin: all
three previously exited 0 and now exit 1, and each deps-windows half
fails independently rather than relying on the other. With a matching
sha the recipes still exit 0, still print "done", and still write the
marker. The Windows path was not executed -- there is no MSYS2 here --
so pacman was shimmed and only the shell logic below it was measured.
|
|
The marker guard added in fd627b1 used $(RANDOMX_DIR) unquoted, so an
operator-supplied value that expands to anything other than one word
walked straight past it. With a glob matching two directories,
`[ -d $(RANDOMX_DIR) ]` became a two-argument test, died with
"unexpected operator" rather than answering, and short-circuited the
&& before the marker was ever looked for -- leaving `rm -rf` to run
against every match. A value holding a space or a `&&` broke the outer
test the same way and reported "already built", and an apostrophe made
the recipe unparseable. So the commit's claim to refuse any directory
lacking the marker was falsifiable.
Quote every use, and reject up front the values that cannot name one
directory: empty and / now refuse deliberately rather than by way of
the one-argument `[ -d ]` quirk that happened to catch them, and
whitespace and glob characters are refused outright because
src/Makefile interpolates the same value into -I and -L unquoted,
where neither survives. A relative value is refused too: it named a
different directory here than in src/, and cd'ing into it made the
later mkdir -p resolve against the new cwd, so the build landed in
randomx-build/randomx-build/build and cmake found nothing.
Verified against the recipe text as make expands it, with rm replaced
by an echo, over empty, /, globs matching one and two directories,
spaces, tabs, relative paths, symlinks, apostrophes, semicolons,
backticks and newlines, under both dash and bash: every run either
refuses with a message or passes rm exactly one argument, identical to
the path the guard tested.
|
|
The latch added in 456b3b4 treats every rename() failure as permanent. One
is not: if an external rotator moves debug.log between the size check and
the rename, the rename fails ENOENT. Nothing is wrong -- the file has just
been rotated by someone else, and the reopen above has already recreated
it -- but latching there would switch the cap off for the rest of the run
on a race the operator cannot see or fix.
Treat ENOENT as "already rotated" and carry on. It cannot loop: the reopen
leaves an empty file, so the next line's size check does not fire.
|
|
2661de1 said all the "sending: " emission sites agree on the category.
There is a fourth, and it did not: EndMessage()'s -dropmessagestest path
hardcoded "net", so under plain -debug a dropped tx announced its own drop
while the "sending: tx" it interrupts was gated off under "relay". The
knob is test-only, so nothing operational rode on it, but the claim was
false as written.
Derive it from SendLogCategory() like the other three. nPushPos may be -1
at this point, and GetMessageCommand() returns "" there, so the category
is "net" exactly as before when there is no message in progress.
|
|
atoi64() is strtoll() with the endptr discarded, so it yields 0 for
anything it cannot read. -maxdebugfilesize=1G and -maxdebugfilesize=1024MB
both parsed as 0, which the roll reads as "never roll" -- indistinguishable
from requesting that deliberately, and silent. An operator who wrote a unit
suffix got no rotation and no warning. Only a negative value said anything.
The multiplication was unchecked as well: a large enough N made
nMB * 1024 * 1024 overflow to a negative cap, which disables the roll too.
Parse with strtoll() and its endptr, and reject trailing characters,
ERANGE, negatives, and any value that would overflow the byte cap --
each reported with the reason and the cap being kept. 0 still means
never roll and is still accepted without comment.
|
|
Neither remove() nor rename() was checked. When the rename fails -- a
read-only mount, permissions, EXDEV across a bind mount -- fopen() reopens
the same oversized file and writes the "grew past N MB" marker anyway. The
size never drops, so the next line takes the same branch, and every line
after that: the roll's own failure notice becomes unbounded growth, added
to the growth of issue #34 rather than replacing it.
Check rename(), and on failure latch the roll off for the rest of the run
after reporting it once -- on stderr as well as in the log, since whatever
blocked the rename may equally block the log write. An operator is told the
cap has stopped being enforced instead of learning it from the disk.
|
|
GetFilesize() returned int while ftell() returns long, so a debug.log of
2 GiB or more came back negative. Its only caller is the -maxdebugfilesize
roll added in ae1c6a7, which skips the roll on a negative size: the
rotation written for issue #34 would have switched itself off permanently
at exactly the size it exists to prevent. Production was at 1.95 GiB
growing 51-129 MB/day, about a day short of crossing 2 GiB.
The same truncation capped the comparison at INT_MAX (2047.99 MB), so
-maxdebugfilesize=2048 or higher could never be exceeded and disabled
rotation outright. Both failures were silent.
Widen the return to int64 and seek with ftello/fseeko (_ftelli64 and
_fseeki64 on Windows). The roll in util.h is the function's only caller
in the tree, so no other behaviour changes. A 32-bit off_t would
reintroduce the same truncation, so its width is now a compile-time
assertion rather than a silent runtime surprise.
|
|
The line was unconditional on the argument that failover firing is the
pathology of #33 and nobody could otherwise see it. That much is right.
Printing a peer address every time it fires is not: a peer decides how
often the line is written, by announcing hashes it will not serve, and
an address is the one thing the .btf transport otherwise takes trouble
not to spread around. Sustained, that was tens of lines a minute, each
naming a peer, driven entirely from outside.
Split it. The per-peer detail -- which block, how long, which peer is
being tried next -- moves behind the net category, where the rest of the
getdata logging already is. What stays unconditional is a count with no
addresses in it, over a window no peer can shorten: the first failover
after a quiet spell still prints immediately, because the change from
"blocks arrive" to "blocks are announced and not served" is the event
worth seeing, and everything after it is summarised once a minute.
Worst case is now one line per minute regardless of what a peer does,
instead of one per re-request.
|
|
nRequestTime lives in mapAlreadyAskedFor, which is global to the inv, so
the spacing AskFor applies is not per-peer pacing: it is what strings the
getdatas for one block across its announcers. Shortening it to 15s was
right at the tip and wrong everywhere else, and the eight-entry cap that
made the failover table safe for catch-up does not cover this at all --
it is a different mechanism.
At two minutes a second announcer's queued request almost always fired
after the block had arrived, so AlreadyHave dropped it. At fifteen
seconds, with a peer working through a 500-deep queue and block N
legitimately minutes away, the second announcer is asked at +15s, the
third at +30s, and the bulk download arrives once per announcer -- up to
eight times over, with nothing bounding it.
So the fast spacing is now conditional on tracking the tip, measured off
the age of our own best block. Not off a peer-reported height:
nStartingHeight is optional in the version message and every node
released so far sends the short form, so it is -1 for essentially every
peer, and where it does get filled in later it comes from the height of
a block that peer served us, which during catch-up tracks our own height
and would answer "at the tip" for the entire download. Block timestamps
are the one clock a peer cannot move, being bounded above by CheckBlock
and below by AcceptBlock's median-time rule.
Thirty block intervals is the threshold: far longer than any stall worth
failing over from, far shorter than catching up. Failover at the tip
does not rest on this in any case -- mapBlockFetch re-issues a stalled
getdata to the next peer on its own 15s timer, and that is what actually
answers #33.
|
|
Nothing on the path from inv to insertion can tell a real block hash
from thirty-two random bytes. AlreadyHave only asks whether we already
hold the block, and with no header there is no work to check. So one inv
carrying eight hashes that never existed filled all eight slots, and
because a full table refused new entries outright, no genuinely new
block was tracked again until the give-up timer expired ten minutes
later. One message per ten minutes from a single connection switched
failover off for the whole node -- not eight wasted slots, an off-switch.
A full table now evicts. Eviction on its own would just be the same
attack in a new shape, so the victim is chosen by occupancy: a peer may
only take a slot from a peer holding strictly more slots than itself,
and otherwise recycles one of its own oldest. A peer that has filled the
table is the heaviest holder by definition, so its own entries are the
only ones it can displace; a peer announcing its first block holds
nothing, so it always evicts from the heaviest. A genuine announcement
is therefore never refused a slot, whatever an attacker holds or across
however many connections it holds it.
Entries carry the announcing peer for this, set once on creation --
the failover sweep asks other peers later and none of them chose to
announce the hash, so none of them should be charged for it.
The two timestamps also get member initialisers. The insertion path uses
operator[] and tests nTimeFirstAsked == 0 to spot a fresh entry, which
worked on value-initialisation alone, but only until somebody gave the
struct a constructor -- and this commit gives it a member that wants one.
|
|
The mapBlockFetch entry was erased before ProcessBlock ran, on the
reasoning that the body had arrived so the fetch was over either way.
That reasoning does not survive the fact that GetHash() covers the
80-byte header and nothing else.
Any peer that saw X announced holds X's header. It can answer our
getdata with that header and a transaction list of its own invention;
the reply still hashes to X, so the entry was erased, and CheckBlock
then rejected the body on hashMerkleRoot and stored nothing. AlreadyHave
stayed false, the mapAskFor entry had already been consumed when the
getdata went out, and nothing recreated the tracking -- so X was never
requested from anybody again unless some peer happened to re-announce
it. That is the #33 stall, reachable on demand for one message.
The erase now sits inside the success branch, beside the
mapAlreadyAskedFor erase it belongs with. ProcessBlock returns true in
exactly two cases, block accepted and block parked as an orphan, and
both store it where AlreadyHave will find it, so one erase covers both
and neither needs different treatment. Of the false cases, the two
already-have ones are cleared by the sweep's own AlreadyHave check on
its next pass, and the rest are rejections that must keep the entry.
A mismatched merkle root is also now grounds to drop the peer. Nothing
honest produces it: peer traffic rides the .btf channel, which is
XChaCha20-Poly1305, so a body that arrives at all arrived intact, and
the mismatch is the peer's own construction. Checking it in the handler
also rejects a forged body before CheckBlock spends a RandomX hash on
it. The unconditional log line fires once per connection, because
ProcessMessages keeps draining the receive buffer after fDisconnect is
set and a peer that packs many forged bodies into one buffer must not
get a line for each.
|
|
2bca62a decayed m->hashRate from the event loop's miner sweep, which
turned a figure that is only ever displayed into an accumulator, and got
the curve wrong at both ends.
Each pass recomputed `expected` -- and so `grace` -- from the estimate it
had just decayed. Every decay raised `expected`, which raised the grace
that gates the next decay, so `grace` chased `silent` upward and the
reported rate settled into a 1/t crawl instead of the half-life the
commit's own comment documents: 291x too high at ten half-lives past the
grace, and still announcing 0.4% of baseline four hours into a silence.
#7 is about the pool over-reporting its hashrate, and this under-corrected
it for hours.
The far end was worse. Once `expected` reached HASHRATE_MAX_INTERVAL it
could not rise any further, the feedback that had been gating each pass
was gone, and the decay did begin to compound -- once a second, against
the total elapsed silence, because lastShareTime does not advance while a
miner is quiet and nothing recorded when the decay had last run. A miner
four hours silent fell from 0.4% of its baseline to 1e-11 of it over the
next hundred seconds, with nothing in the code or the announced number to
explain the cliff.
Neither behaviour is worth tuning, so drop the sweep. hashRate is read in
exactly one place -- RefreshStats, building the per-worker rows and the
total that the pool status and the nostr announcement publish -- and
vardiff retargets from share timing alone and never reads it. So leave
the stored value as the sample it always was and have RefreshStats ask
MinerReportedHashRate() what to publish: a pure function of that sample
and the absolute silence since the last share. With no stored decay state
there is nothing for a tick rate, a missed pass or a repeated pass to
perturb, and the answer is bit-identical from 1/60 Hz to 1000 Hz because
nothing accumulates, not because the arithmetic was written carefully.
It also keeps the fade out of the estimator: the EWMA on the share path
still blends against the value it last computed rather than a faded copy
of it, so a miner that resumes recovers exactly as it did before any of
this, and there is no decay timestamp for a newly accepted or newly
authorised connection to be caught holding unset.
RefreshStats takes one GetTime() for the whole sweep, so the rows and the
total they are summed into describe a single instant rather than a smear
across it. The clamp is written !(expected > MIN) so a NaN lands on the
floor instead of sailing through; a negative `silent` from a clock
stepping backwards reports the sample unchanged rather than scaling it
up; and past HASHRATE_DECAY_CUTOFF half-lives the answer is a flat zero
rather than pow() walking down into the denormals. This also drops the
std::max(m->hashRate, 1e-9) the old pass carried, which the
`hashRate <= 0.0` guard immediately above it had already made dead.
pool-stratum gains ten checks over the policy: the full estimate inside
the grace, a thousand samples at one instant returning one value and
leaving the estimate untouched, a few percent lost one second past the
grace rather than the whole estimate, exactly half at one expected
interval past it, zero after an hour of silence, no inflation from a
backwards clock, zero for a miner that has no rate sample yet, and
nothing non-finite anywhere across a long silence.
|
|
Issue #2 reported that /rvrelay does not govern registration. It does:
BtfAllRelays() returns vBtfMeetingRelays alone when fBtfRelayPinned, and
every relay-choosing site -- the pool's AcceptLoopFn, the node's
ThreadBtfAccept, and the dial path -- draws from it. vBtfMeetingRelays
has exactly one writer, the pin itself. A pinned pool therefore spawns
one accept thread, at the pin, and no discovered relay is registered at.
The registration the incident actually caught was not the pool's. The
"registered at" line it quotes exists only in ThreadBtfAccept, and
90.156.222.107:8434 is not a stranger's relay -- it is the third
hardcoded seed, one of the three ThreadBtfAccept picks between at random
when no pin is in force. That relay is only reachable with the pin
absent, so the binary that logged it did not have one; both the pin and
the operator-mode skip that keeps a pool out of that thread landed two
days before the observation.
What was genuinely wrong is that none of this could be read off a log.
The pool announced no successful registration at all, and retried
failures at one flat line, so "is this pool registered where its
descriptor says" had no answer. So:
- Advertise the relay after registering at it, not before. The
optimistic order published a meeting node we were not yet listed at,
and a miner resolving the descriptor inside that window spent its
dial on a relay holding no registration for us. ThreadBtfAccept has
always ordered it this way; the pool had not.
- Log the relay registration succeeds at, name the pin when one is in
force, and name each discovered relay the pin excludes.
- Escalate when a pinned relay keeps refusing: a pin has no fallback,
so an unregistered pin means the pool is unreachable, and that now
says so instead of scrolling past.
- Refuse to advertise a non-pinned relay in BtfSetActiveRelay. No
caller can reach it today, but the descriptor is the one thing a
miner obeys, so the invariant is enforced at the sink rather than
retrusted to every future caller.
Does not touch the registration-consumption race: the pool still gives
up its registration on each pairing and re-registers after wrap-and-push.
Moving the advertise call inside that loop does not widen the window, it
only stops the descriptor naming a relay we never reached.
|
|
The README offered XMRig and SRBMiner command lines pointed at the local
Stratum bridge. Neither can work. In RandomX mode both target the
Monero-style JSON-RPC protocol (login/job/submit) while the pool speaks
Bitcoin-style Stratum (mining.subscribe/notify/submit); both place the
nonce at byte 39 of the job blob, which in an 80-byte Bitflash header is
inside the merkle root, not the nonce at byte 76; and both require a
32-byte seed hash as 64 hex characters, where randomx_pow.cpp keys RandomX
from a fixed 36-byte string that is not a hash at all. None of it is
configurable.
-stratumbridge is a byte relay between a local socket and the .btf tunnel
(BridgePump, main.cpp) -- it fixes reachability, the one blocker an
operator could otherwise do something about, and translates nothing. Say
that where the commands used to be, so the question gets answered instead
of asked again.
Also correct the magic bytes in src/readme.txt, which documented
bf 1a 5c fd; net.h has bf 20 5c fd.
|
|
nTime is 1753315200, which is 2025-07-24T00:00:00Z. The comment beside it
claimed 2026-07-24, and the headline in pszTimestamp reads 24/Jul/2026, so
the constant looked like a typo somebody ought to correct.
Nothing is wrong with the data. nTime matters only as an input to the
genesis hash, the block hashes to hashGenesisBlock with it, the asserts
pass, and the whole network agrees. The hazard is entirely in acting on the
mismatch. Editing nTime alone trips the assert -- loud, and it reverts.
Editing it and regenerating GENESIS_NONCE and hashGenesisBlock to match is
the quiet one: that node starts without complaint, builds its own chain
from height 0, and rejects every real peer, and the re-mining branch a few
lines below prints exactly the constants needed to do it.
So: correct the comment rather than the constant, state the discrepancy as
known and uncorrectable, and have the re-mining printout say what those
constants are and are not for. No value changes -- pszTimestamp, the
scriptPubKey, nTime, nBits, nNonce, GENESIS_NONCE, hashGenesisBlock and
hashGenesisMerkleRoot are all byte-identical.
|
|
consensus-limits only ever asked what CheckBlock() refuses. Nothing asked what
the assemblers produce, which is why a miner able to overshoot the cap by an
entire mempool sat here unnoticed through both the deployed and candidate
builds.
Adds the other half: a template built from a 3 MB mempool stays inside the
budget and passes CheckSizeLimits(); a transaction too big for the budget is
skipped while the ones that fit are still taken; sigop-heavy transactions stop
selection at MAX_BLOCK_SIGOPS while the block is still small. Plus the rule's
edges, including that sigops exactly on the limit fit -- CheckBlock() fails on
`>`, so assembly must accept what validation accepts.
The test drives BlockTemplateFits the way the assemblers drive it rather than
calling them: BitcoinMiner needs a CTxDB, a wallet key and cs_main, and
RebuildJob is static in rpc.cpp. What that proves is the shared rule and the
property it exists to give, which is where the defect was -- the budget was
right and the enforcement was not.
Checked against the bug: with the fit test neutered, eight of these fail,
including both templates that CheckBlock() would have rejected. With the sigop
comparison stricter than CheckBlock's by one, the boundary case fails.
|
|
CheckBlock() caps a block at MAX_BLOCK_SIGOPS and neither assembler counted
against it. Bytes do not bound what a block costs to verify: every OP_CHECKSIG
is an elliptic-curve verification, so a template comfortably inside the size
budget can still be one the network refuses for demanding more work than a
block is allowed to.
Counted with CTransaction::GetSigOpCount -- the same function CheckBlock()
sums, not a second implementation of it -- and seeded from the coinbase,
because CheckBlock() sums over all of vtx. The comparison matches too:
CheckBlock() fails on `> MAX_BLOCK_SIGOPS`, so a block exactly on the limit is
valid and selection accepts it. Assembly and validation differing by one is
the shape of the bug this series started with.
Unlike the size budget this takes no margin. There is no use for headroom in a
sigop count, so assembly holds itself to the consensus number exactly.
Assembly only; MAX_BLOCK_SIGOPS and the CheckBlock() test are unchanged.
|
|
LogPrint()'s burst collapse is right for share and peer chatter and wrong
for anything that reports an irreversible change to money. A reorg voided
100 ledger entries totalling 4,950 BTF and the log carried one line, naming
one block and 49.49 BTF. The other 99 were counted and never printed, and
no "(N similar)" summary appeared either -- a summary was only ever flushed
when that same call site fired again, and after the reorg it never did. The
missing 4,950 BTF was found by counting entries in pending_payouts.json.
Add LogPrintAlways(), which skips the collapse entirely, and move the four
payout lines that report money to it: an obligation voided, an obligation
dropped, a payment sent, a payment that failed. All four are bursty by
nature -- one reorg voids every entry funded by the orphaned block, from a
single source line -- which is precisely why one line must not stand in for
all of them. Their volume is bounded by how much money moved, which is a
number worth the lines.
Also sweep expired windows for every call site rather than only the one
currently logging, so a burst that stops is still summarised, and report
the window's real elapsed time instead of a hardcoded 3s.
|
|
The node miner tested its size budget only on the outer while loop. The inner
for walked all of mapTransactions with no test, so a single pass could append
the entire mempool -- the overshoot was bounded by mempool size, not by one
transaction as the comment claimed.
The pool builder did test, but only after the push, which bounds the overshoot
at one transaction. Nothing caps how big one transaction may be: neither
CheckTransaction nor AcceptTransaction limits it, so the mempool will hold one
of any size the wire allows and "one transaction" is worth up to MAX_SIZE. A
lone 1.5 MB transaction offering a large fee is therefore enough to make every
template unmineable, and the fee is never paid because the block never
confirms.
Both now ask before appending and skip what does not fit, so the overshoot is
zero and an oversized transaction costs the template nothing rather than
invalidating it. The budget moves to MAX_BLOCK_SIZE_GEN so the two assemblers
cannot drift on it again -- drift is what produced this.
The test is ordered before ConnectInputs on purpose: ConnectInputs credits
nFees by reference on success, so declining a transaction after calling it
would leave the coinbase claiming a fee the block does not collect.
Assembly only. CheckBlock, CheckTransaction and AcceptTransaction are
untouched: this changes what we produce, not what the network accepts.
|
|
failing
|
|
failing
|
|
|
|
SetHex() parses whatever hex prefix it finds and stops, so a corrupt
blockHash loaded as a DIFFERENT hash: garbage with a hex-looking prefix
became a hash no block ever had, which the orphan check reads as an
orphaned block -- and the next flush voided the debt on the strength of
file corruption. Garbage with no hex prefix became 0 and skipped the
orphan check instead. A field that is not the 64-digit hex string this
tree writes now fails the load, and the file is quarantined like any
other parse error.
|
|
A pre-upgrade payout comment names only the height, so when a height is
won, orphaned, and won again, the old transaction matches the new round's
(height, address, amount) exactly and the miner is never paid. Round
proofs decide the ambiguity where it is decidable: a proof is written when
a round is created, and structured payout metadata (upstream 0b01d6c)
predates round proofs (upstream b1eb8b6), so a round with a proof was
created by a build whose every payment names its round -- a comment-only
match can only be another round's money. Skipping the fallback cannot
double-pay: any real payment for such a round is already found by the
structured probe. Proofless rounds keep the documented bias.
|
|
pool_payout_id keeps only the first sixteen hex digits of the block hash,
which is the end proof-of-work fills with zeros -- so twin rounds at one
height can carry the SAME id, and one twin's payment then settles the
other's debt through the structured probe: the same-height collision the
comment match had, one layer down. New sends record the full hash as
pool_payout_block and a structured match must agree with it. A transaction
without the key predates it and is matched by id as before; anything
stricter would make every payment already sent look unpaid and pay it
again.
|
|
A height the pool wins, loses to a reorg, and wins again yields two rounds
whose height-only comments are identical -- and identical amounts too when
the same miner held both rounds. New sends now write the funding block hash
into the comment. The legacy fallback's needle stays frozen at the
pre-upgrade format: it exists precisely for transactions that carry nothing
else, and a needle in any other format would make every one of those
payments look unpaid and pay it again.
|
|
The 15s retry spacing only helps when several peers announce the block:
the queued retries are created by inv handling, so a block announced by
a single peer that then goes quiet was re-requested from nobody, and the
tip stayed frozen until somebody happened to re-announce (#33).
Track every block getdata in mapBlockFetch, erase the entry when the
body arrives, and let SendMessages re-issue a request that has gone
unanswered for BLOCK_FETCH_RETRY_SECS to the next peer that has not had
a turn this round, restarting the round once every connected peer has.
Client peers are skipped -- they declared they cannot serve bodies.
Tracking is capped at 8 in-flight blocks so initial download, where
hundreds are legitimately outstanding to one peer, is not duplicated to
everybody else, and an entry is dropped after 10 minutes so a hash that
was announced and never existed cannot be chased forever.
The failover log line is deliberately not behind a debug category: it
firing means a peer announces blocks it does not serve, which is the
pathology being diagnosed.
|
|
AskFor keys its retry spacing off mapAlreadyAskedFor, which is global to
the inv, so the two-minute gap does more than pace re-requests to one
peer: it strings the getdatas for one block across its announcers two
minutes apart. When the first peer asked does not serve the body, the
second candidate is not asked for 120s and the ninth for 960s -- the
observed 16-minute tip stall of #33, during which the pool kept
broadcasting jobs against the stale tip and 5 of 12 found blocks
orphaned.
A block body is at most 1 MB and somebody demonstrably has it -- they
announced it -- so on this timescale "not here yet" means "not coming
from that peer". Blocks now retry on BLOCK_FETCH_RETRY_SECS (15s);
transactions keep the two-minute pacing, which for them is the point.
|
|
GetKeyFromPool() returns an empty vector when the wallet is encrypted
and locked, and when key generation itself fails. RebuildJob() fed
whatever came back straight into the coinbase, and CScript::operator<<
serialises an empty vector as a zero-length push, so the template's
coinbase script became `<empty> OP_CHECKSIG` -- satisfiable by nobody.
Every block the pool's miners found against such a job burned its whole
subsidy: the coins exist and can never move, while QueuePayouts records
the full obligation and the miners are then paid out of coin that block
never contributed. An operator who locks the wallet overnight while the
miners keep finding blocks pays twice for every one of them.
BitcoinMiner has exactly this guard ("no wallet key available for
coinbase; mining stopped"); this builder was missed. Mirror it: log the
reason and return false before any job state is touched. Every caller
already treats a false return as "no job this round" -- the submit
handler and the new-block path log a warning and leave gHaveJob false,
the no-job watchdog goes on retrying every few seconds, and the
periodic nTime refresh keeps broadcasting the old job, whose key was
drawn while the wallet could still produce one. Once the wallet can
hand out a key again the next retry issues a spendable job unprompted.
|
|
std::thread throws std::system_error when pthread_create is refused
(RLIMIT_NPROC, a container's pids cgroup, memory pressure), and nothing
caught it, so the node died by std::terminate. Threads are spawned per
peer connection, so the pressure that triggers this scales with peer
count, which is attacker-influenced. Observed killing a healthy
participant worker mid-mining.
Every spawn site in the node build, and what a failure now does:
- btftunnel.cpp: both tunnel pumps are guarded. The inbound pump starts
first because it owns cleanup of the pump end and rendezvous socket;
if the second spawn is refused, shutdown() wakes the first out of its
read and it cleans up itself -- closing those descriptors from the
failure path instead would race it onto reused descriptors. Both
paths return INVALID_SOCKET, which every caller already treats as
"this connection failed".
- btfrv.cpp: the relay's accept loop drops the connection it cannot
serve and stops accepting for a moment -- longer after consecutive
failures, capped at 5s -- instead of dying at an arbitrary one. The
per-pair second pump now runs on the handling thread itself, leaving
exactly one spawn to guard and nothing running yet to race.
- main.cpp: a participant that cannot spawn its hash workers stops the
ones that did start, drops the pool connection, and returns to the
reconnect loop, the same way an ordinary disconnect does.
- randomx_pow.cpp: dataset init joins the workers it got, releases the
dataset, and stays in light mode -- the same trade the allocation
failure path above it already makes.
- rpc.cpp, net.cpp: the remaining unchecked _beginthread calls (which
report failure by value, they cannot throw) now say precisely what
did not start. The pool's accept and payout threads failing silently
left the pool running deaf -- nobody accepts, nobody gets paid, and
nothing in the log says why.
Left alone deliberately: the per-relay accept spawns (rpc.cpp) already
check, log and retry on the next poll; startup-critical spawns in
net.cpp keep failing hard; the selftest-only std::thread uses may abort
a selftest run, which is acceptable where dying over one peer is not.
Verified: full node rebuilds clean and links; pool-stratum selftest
passes; a scratch harness under RLIMIT_NPROC=1 confirms _beginthread
reports -1 and std::thread throws system_error, the premises of every
path above.
|
|
pszFile/nLine were first written by CRITICAL_BLOCK on entry, so an
instance embedded in a dynamically allocated object (CNode::cs_vSend,
cs_vRecv, cs_inventory) held indeterminate values until its first
lock -- anything reading them for diagnostics dereferenced a wild
pointer.
|
|
|
|
RandomX links jit_compiler_x86_static.S, a hand-written assembly stub with
no .note.GNU-stack marker. binutils reads a missing marker as a request for
an executable stack, and older linkers grant it for the whole binary: NX off,
process-wide, for a node that parses attacker-supplied blocks and messages.
The same source therefore produced different security properties depending on
where it was built. Ubuntu 26.04 gave GNU_STACK RW; Debian 12 -- the build
target for the deploy image, because it is the oldest glibc we ship to and the
last Debian that still packages the Berkeley DB C++ binding -- gave RWE. The
only announcement was a linker warning in the middle of an otherwise clean
build.
-z noexecstack settles it in the tree instead of in the toolchain. RandomX's
JIT maps its own W^X pages and never executes from the stack, so nothing that
works today stops working. Both link lines take it, GUI and headless.
Verify: readelf -lW bitflash-node | grep -A1 GNU_STACK # expect RW
|
|
A Miner left gMiners in exactly one place: RecvLine reporting a clean
disconnect. Half-open TCP sessions send no FIN and raise no error, so
they lived forever -- on the live pool authorize outpaced disconnect
2:1, over half the table never submitted a share, and the leak grew
toward MAX_POOL_MINERS, past which the accept path refuses real miners.
Two changes. First, an idle reap in the event loop's miner sweep: any
miner whose last activity is older than POOL_IDLE_TIMEOUT_SECS (600s)
is closed and erased through exactly the bookkeeping the
clean-disconnect path uses -- log, sock_close, erase via std::find,
delete -- collected into a side vector first so the sweep's iteration
stays valid. Shares an idle miner already earned stay in
roundShareCount, just as they do when a miner cleanly disconnects
mid-round. To give the reap something honest to measure, lastSeen is
now stamped on every complete line received (authorize and valid shares
already stamped it; a miner that only subscribes, or sends
unrecognised methods, previously looked dead from birth), with
connectTime standing in for a connection that never completed a line.
Ten minutes is far beyond the 15s vardiff share target and the 45s
stall watchdog, so no miner that is actually hashing comes close.
Second, SO_KEEPALIVE on sockets accepted by the loopback stratum
listener, so a half-open session eventually errors out of recv and
lands in the reap path on its own. This is a backstop only: the default
cadence before the first probe is hours on most stacks, so the idle
timeout fires long before keepalive does. Miners arriving via a relay
ride a loopback socket pair whose real transport is the rendezvous
socket owned by the tunnel pump threads, which keepalive cannot reach
across -- for those the idle reap is also what finally tears the tunnel
down, since closing the app end unblocks the pumps.
Closes #6
|
|
m->hashRate is an EWMA re-sampled only on the valid-share path, with no
time-based term: a miner that stops hashing keeps its last estimate for
as long as the connection stays open, and RefreshStats sums every such
stale estimate into gStats.totalHashRate, which is what the pool status
and the nostr announcement publish. The announced pool hashrate ran
several times over ground truth because miners that had gone quiet were
still contributing their last sample.
Decay on time in the event loop's miner sweep. Per miner, compute the
expected share interval -- ExpectedHashesForDifficulty(m->difficulty)
divided by its current rate estimate -- and give it a grace of four such
intervals with no share (at the vardiff target of one share per 15s
that is about a minute of silence). Past the grace, decay the estimate
with a half-life of one more expected interval, computed from absolute
time so the curve does not depend on the loop's tick rate. The interval
is clamped to [1s, 1h] so a fast miner at the vardiff floor, whose
expected interval is milliseconds, is not decayed for ordinary
share-delivery jitter, and a very slow miner's grace cannot stretch
past the point where its estimate means anything. Per-share crediting
and ExpectedHashesForDifficulty are untouched: this changes only what
is reported about a miner that has stopped reporting itself.
Closes #7
|
|
mining.submit validated the nonce, rebuilt the block, re-hashed it with
RandomX -- and never asked whether it had seen this exact (job_id,
nonce) pair before. Nothing in that pipeline distinguishes a replay
from a first submission: a duplicated valid share passes every check
and lands in roundShareCount[address]++ a second time, and
roundShareCount is the divisor the payout split uses, so replaying one
valid share N times multiplies that address's cut of the round by N.
Keep a per-connection set of credited nonces on the Miner, reject a
repeat with a "Duplicate share" stratum error before the rebuild and
hash it would waste, and record the nonce at the moment the share is
credited. The set is keyed to the job id the miner is currently working
and cleared inside SendJob -- the single funnel every new-job path
(initial authorize, block found, new block, 60s refresh, no-job
watchdog, vardiff retargets) goes through -- so legitimate
resubmissions across jobs are never refused. The clear is keyed on the
id rather than unconditional because the vardiff retarget and stall
watchdog re-send the CURRENT job, and an unconditional clear there
would reopen the very replay window this closes. Job ids come from a
monotonic atomic counter in RebuildJob, so an id is never recycled
within a process.
Closes #8
|
|
ShareTargetFromDifficulty ended with
if (t < diff1 && difficulty < 1.0) t = ~uint256(0);
which can never fire: t = diff1/difficulty, so difficulty < 1.0 implies
t > diff1 and the two conditions exclude each other. The branch was
reaching for a real hazard, though, so say the hazard directly instead
of deleting the guard: diff1 is ~uint256(0) >> 12, a hair under 2^244,
so the true share target stops fitting in a uint256 once difficulty
reaches 2^-12 (1/4096), and the fixed-point divisor
(uint64)(difficulty * 65536.0) truncates to zero -- a division by zero
-- for any difficulty under 2^-16. Past the first threshold
getuint256() would return the low 256 bits of an oversized quotient, a
wrapped target that is arbitrarily HARDER than difficulty 1. Both
thresholds sit far below VARDIFF_MIN (0.001), so nothing live comes
near this; the guard exists for any caller that bypasses vardiff. At
those difficulties every hash is meant to be a share, so return
all-ones, the easiest representable target. Boundary verified
numerically against the exact fixed-point algorithm: at difficulty
exactly 1/4096 the quotient is 2^256 - 4096 (fits), one ulp below it
overflows, and below 2^-16 the divisor is zero.
Closes #11
|
|
A 13-hour production capture was ~71% "sending: tx", ~16% "Nostr:
discovered .btf peer", ~5% "Relaying wtx", and ~43% of raw lines were
CBlock/CTransaction dumps -- all already under -debug, which operators
run permanently, so -debug was useless as a filter.
The category mechanism gains a verbose tier: "relay" and
"nostrdiscovery" are accepted only with the new /debugrelay flag (which
implies /debug); everything else keeps its current gate. Demoted, with
message text unchanged for anyone grepping it:
net.h "sending: tx (N bytes)" (BeginMessage/EndMessage/
AbortMessage all pick the category from the command, so no
orphaned "(N bytes)" fragments leak into a plain -debug log)
main.cpp "Relaying wtx", "received block:" + CBlock::print, and the
BitcoinMiner block dump
nostr.cpp "Nostr: discovered .btf peer" -> "nostrdiscovery"
Pool/worker/payout, [payout], BLOCK ORPHANED/ACCEPTED and share flow
lines are untouched. Because LogPrint dedup keys on category, demoted
lines also keep their burst collapse under /debugrelay.
Closes #35
|
|
debug.log grew without bound (production: 1.95 GiB, 51-129 MB/day).
Nothing shrank it once the process was up -- there is no
ShrinkDebugFile in this tree, and upstream's only ran at startup.
OutputDebugStringF now rolls the file to debug.log.old when it
passes -maxdebugfilesize (MB, default 1024, 0 disables). The write
path already opens and closes the file per line under one lock, so
the rename-and-reopen is a few lines inside that lock and no fd can
be stranded mid-roll.
contrib/logrotate/bitflash-debug.logrotate is the longer-term policy
(size 1G, rotate 4, compress, missingok, notifempty, copytruncate);
copytruncate keeps the writer's append handle valid across rotation.
Closes #34
|
|
The appimage recipe piped the tool through grep ... || true, so a
failed run still ended with an unconditional "Built:" echo and make
reported success with no AppImage on disk. Run appimagetool bare so
its status propagates, then require the artefact to exist, be
non-empty, and be newer than src/bitflash before announcing it.
Closes #29
|
|
libboost-system-dev is gone from Ubuntu 26.04 -- Boost.System has been
header-only since 1.69, and this tree only includes two boost headers
and links no boost library. The recipe also installed without an
apt-get update, so a fresh image failed on stale or empty lists.
Closes #28
|
|
deps-randomx built into $(HOME)/RandomX and deleted that path whenever
the library was missing, taking out hand-managed checkouts the build
never owned. Default the build to the in-tree $(ROOT)/randomx-build,
like secp256k1-build, overridable via RANDOMX_DIR and exported so
src/Makefile's own fallback resolves to the same tree. As a backstop,
the recipe writes a .bitflash-built marker after a successful build and
refuses to delete any directory that lacks it, so a user-supplied
RANDOMX_DIR pointing at a pre-existing tree is never removed.
Closes #27
|
|
RandomX links jit_compiler_x86_static.S, a hand-written assembly stub with
no .note.GNU-stack marker. binutils reads a missing marker as a request for
an executable stack, and older linkers grant it for the whole binary: NX off,
process-wide, for a node that parses attacker-supplied blocks and messages.
The same source therefore produced different security properties depending on
where it was built. Ubuntu 26.04 gave GNU_STACK RW; Debian 12 -- the build
target for the deploy image, because it is the oldest glibc we ship to and the
last Debian that still packages the Berkeley DB C++ binding -- gave RWE. The
only announcement was a linker warning in the middle of an otherwise clean
build.
-z noexecstack settles it in the tree instead of in the toolchain. RandomX's
JIT maps its own W^X pages and never executes from the stack, so nothing that
works today stops working. Both link lines take it, GUI and headless.
Verify: readelf -lW bitflash-node | grep -A1 GNU_STACK # expect RW
|
|
|
|
v1.2.20 makes SQLite the default backend for a datadir that holds no
wallet. On a desktop that is a first run. On a headless node it is usually
a fault -- the datadir is not mounted, or wallet.dat was caught mid-rename
by a backup -- and the new default turns that fault into a silent loss of
custody rather than a loud failure.
The damage is not the empty wallet, it is the marker. Creating the wallet
also writes <datadir>/wallet-backend = sqlite, and every later start reads
that marker first: putting the real wallet.dat back no longer does
anything, because the node stays on the empty SQLite wallet it made.
1.2.19 had the same fail-open on a missing wallet, but it created a
wallet.dat, and restoring the backup over it recovered the funds. That
runbook stops working here.
So refuse when the startup is headless and no wallet of either kind is
present, and say which flag makes it deliberate: /createwallet to start a
new one, or /walletbackend=bdb for the Berkeley DB path. A node that meant
to create a wallet says so; a node that lost its datadir gets an error
instead of a fresh identity and a re-paid ledger.
Upstream keeps the old behaviour; this is a fork divergence, and the
three findings that would gate enabling the SQLite backend at all -- the
non-atomic backend marker, WriteHDMaster's connection-wide transaction,
and the encrypt-swap crash window -- remain open against upstream.
|
|
Merges upstream tag v1.2.20 (6aa3dade) into our 1.2.19+sec tree. The merge
base is bc3de30's descendant 9e39a50 -- upstream's own v1.2.19 -- so this is
an ordinary three-way merge, not a replay. Five conflict blocks in four
files, all resolved below.
No consensus change. CheckBlock, ConnectBlock, DisconnectBlock, ConnectInputs,
AcceptBlock, AcceptTransaction, GetNextWorkRequired and GetBlockValue hash
identical to v1.2.19; script.cpp and script.h are byte-identical on both
sides. main.cpp is +3 lines and main.h +2, all wallet-backend plumbing.
rpc.cpp is untouched, so the payout ledger arrives unchanged.
Makefile, VERSION. OURS, renumbered. 1.2.20+sec, for the same reason 1.2.19
carried +sec: this tree is not the upstream release of that number and the
string names every release artifact.
Makefile, AppImage recipe. OURS for the AppDir path, THEIRS for the bundled
library. Upstream's hunk reverts the icon paths to a hardcoded
/tmp/bitflash.AppDir, which is the local-user pre-placement hazard the fork
moved out of /tmp in the first place; taking their side wholesale would have
reintroduced it silently, inside a conflict about something else. The AppDir
stays $(APPDIR), and libsqlite3.so.0 joins the bundled list, which the new
backend genuinely needs.
src/Makefile, src/makefile.mingw, HEADERS. BOTH. Ours drops market.h,
theirs adds wallet_sqlite.h; the resolution does both. NODE_OBJS auto-merged
correctly in both files -- obj/wallet_sqlite.o present, obj/market.o still
gone -- and was verified rather than assumed, because that line has trapped
this fork before.
src/selftest.cpp, known-tests list. BOTH, union. Upstream adds eight
wallet-storage tests, we keep payout-ledger. The dispatch chain itself
auto-merged and still routes payout-ledger to RunPayoutLedgerSelfTest.
Hardening verified intact after merge: the TLS 1.2 floor in nostr.cpp, the
/rvrelay pin, opt-in managed Tor, strict sig3-on-onion descriptor
verification, and the marketplace removal. Upstream shipped no build logs or
__pycache__ this time; the three added files are docs/storage.md and
src/wallet_sqlite.{cpp,h}.
The SQLite backend this release introduces is NOT enabled here. An existing
wallet.dat resolves to Berkeley DB on every branch of the new selection
logic, and the conversion wizard is GUI-only and not compiled into the
headless node. The review findings that gate ever turning it on -- the
non-atomic backend marker, the connection-wide WriteHDMaster transaction,
and the encrypt-swap crash window -- are recorded on the tracker, not fixed
here.
|
|
The operator event loop still builds an fd_set by hand (#26), and FD_SET
on a descriptor the set cannot represent is undefined -- it writes past
the bitmap. MAX_POOL_MINERS caps the miner list at 1000, but that is a
count, and a count does not bound descriptor values: a Miner is removed
only on clean disconnect, so half-open sessions hold their fds while the
OS keeps issuing higher ones.
Refuse the connection instead, and refuse on the limit that actually
applies. On POSIX that is the descriptor value against FD_SETSIZE. On
Windows an fd_set is a counted array of SOCKET handles whose values are
arbitrary -- often large from the first accept -- so comparing a handle
against FD_SETSIZE there would reject nearly every miner; the bound is
how many sockets are held.
This is a behaviour change: a pool at the limit now turns miners away
with a log line rather than corrupting memory quietly. The real fix is
poll/epoll, which #26 tracks.
|
|
"broadcast to N miner(s)" counts the authorized sockets that were sent a
job. It says nothing about the rest of the connection table, so a pool
holding a pile of half-open sessions and one real miner reads the same as
a pool with N miners. #19 was opened on exactly that confusion: ~55
connections served per job while ~2 outside miners actually submitted
shares.
Log the breakdown every 60s instead: connected, subscribed (completed
mining.subscribe), authorized, active (authorized and seen within one
payout-rotate cycle), and never-auth-idle (past the grace period without
authorizing). The pool category is on by default, so this reaches a
production log without /debug, and the burst-collapse window is 3s, so a
60s cadence is never the duplicate this tree has swallowed before.
The census also reports how close the single select() loop is to running
out of room, which is not the same question on both platforms. On POSIX
FD_SETSIZE bounds the value of any descriptor handed to FD_SET, so the
highest fd we hold is what matters; on Windows an fd_set is a counted
array of SOCKET handles whose values are arbitrary, so the count is what
matters. FD_SETSIZE comes from the platform headers either way -- this
file must not define it, because redefining it after <sys/select.h> or
winsock2.h changes nothing at all.
|
|
Upstream 1.2.19 auto-starts a bundled tor whenever one sits next to the
executable: silent transport change, a spawned process, a new hidden service,
and descriptors going out with meeting_node="rendezvous-pending" -- on nodes
that never asked, against this fork's pinned-relay posture. The behavior moves
behind the explicit /automanagedtor; /managedtor stays as before.
Also ignore build-*.log, __pycache__/ and *.pyc so the five artifacts stripped
from the release merge cannot come back.
|
|
BtfStopManagedTor read, waited on and signalled g_managedTorPid without
holding cs_managedTor, while the watchdog concurrently reaps the same child
and resets it. If the watchdog won, the stop path spun ten seconds on ECHILD
and then sent SIGKILL to a stale, potentially recycled pid. Snapshot the pid
under the lock, treat ECHILD as already-exited, and only clear the global if
it still holds our pid.
|
|
Upstream 1.2.19 made the P2P listener wildcard-bind unconditionally because
managed Tor forwards hidden-service traffic to 127.0.0.1, which a LAN-only
bind cannot receive. That also widened every non-Tor node to listen on VPN
tunnels, docker bridges and secondary NICs. The onion endpoint is always
configured (or the managed-Tor hidden service established) before StartNode,
so the predicate is ready when the bind happens.
|
|
Upstream 1.2.19 signs the onion endpoint only under the optional sig3, and
VerifyDescriptor treated sig3 failure as 'drop the onion, accept the rest'.
Any relay could therefore strip an onion endpoint down to a rendezvous-only
descriptor (or, under /oniononly, cut the peer off) by deleting one field.
Now an onion field must arrive with a valid sig3 over the V3 digest or the
whole descriptor is rejected. Honest 1.2.19 writers always emit both fields,
so interop is preserved. The full two-field strip still verifies -- it is
indistinguishable from a peer that never had an onion -- and needs the onion
bound into the primary sig, which only upstream can do without breaking every
1.2.18/1.2.19 verifier; reported as an upstream protocol defect.
test_btfaddr flips the two upstream checks that enshrined the soft drop and
adds unsigned-onion and full-strip cases.
|
|
Merges upstream tag v1.2.19 (9e39a50) into merge/upstream-1.2.18 (991853f).
Unlike the 1.2.15->1.2.18 replay, bc3de30 is a true ancestor of our branch via
a9b4f89, so this is an ordinary three-way merge with merge-base bc3de30.
Delta: 36 files, +3265/-163, the Tor onion transport (managed Tor, signed
direct onion endpoints, onion-only dial mode) and the nHDNext restore fix.
src/main.cpp, main.h, script.cpp/h and db.cpp/h are byte-identical between the
two tags, and the poll() socket loop carries no diff hunks, so the merge is
consensus-neutral and the soak-tested socket paths are unchanged. relay/ is
untouched; the payout ledger (rpc.cpp) is untouched.
7 conflict blocks in 6 files, resolved as follows.
Makefile -- VERSION. Ours: stays 1.2.19+sec with the naming comment updated,
so artifacts from this tree cannot be mistaken for the upstream release.
src/Makefile, src/makefile.mingw -- HEADERS list. Ours (no market.h) plus
their tor.h. NODE_OBJS auto-merged correctly: no market.o, tor.o present.
src/selftest.cpp -- known-tests list. Union: payout-ledger (ours) and
managed-tor (theirs).
src/main_gui.cpp -- one block. Our /dumpaddresses kept; their
BtfStopManagedTor() on the StartNode failure path taken. The /rvrelay pin,
/noannounce and the marketplace removal auto-merged and were verified intact.
src/nostr.cpp -- two blocks in ResolveDescriptor. Signature takes both
out-params (our fExchangeOk, their rawOut). The body keeps our
drain-to-EOSE read -- returning early poisons the pooled socket for the next
caller -- and adds their rawOut capture. Our TLS 1.2 floor
(SSL_CTX_set_min_proto_version) is untouched by this merge; upstream 1.2.19
still has no floor of its own.
Upstream's release commit also added four build logs and a compiled Python
cache (build-1219.log, build-17.log, build-appimage.log, build-node.log,
scripts/__pycache__/bitflash_chain.cpython-313.pyc). Removed here; they are
build output, not source.
|
|
The comment justifying the absence of a version guard said RefreshStats
"runs only on the event loop". That was true when it was written and is
no longer: the payout-ledger self-test drives RefreshStats directly, to
check that no payout mutex is held across the write.
The conclusion still holds -- the test calls it from one thread at a
time with the pool not running, so pool_status.json still has a single
writer -- but the reason a reader would check is now stated, along with
what would invalidate it. A claim in a comment that has quietly stopped
being true is how 33f2e8a came to assert that no thread waits on I/O
holding gPayoutLedgerMutex on the commit that made it do so.
Property: comment only.
|
|
Mutation-tested, the concurrency test did not test concurrency. With the
per-file write mutex removed from PublishVersioned, `make tests` still
went green 2 runs out of 5, and the assertion named "four threads
publishing the same file never leave it unparseable" fired 0 times out
of 5. It was asserting something true for reasons unrelated to the code
it was pointed at.
Two causes.
The images were 64 arrays of 200 entries, identical in structure and
within bytes of each other in length, so an interleaving of any two of
them was still a valid JSON array. There was nothing structural for a
torn write to break. They now alternate between one entry and five
thousand: a write landing on top of another leaves either a truncated
array or a small image followed by the tail of a large one, and neither
parses.
Worse, almost nothing was being written. Each thread walked a
permutation of the images and PublishVersioned drops any blob not newer
than what is on disk, so only the record highs in each permutation ever
reached the disk -- about ten writes in total across four threads, which
is not enough contention to produce a race however the images are
shaped. Every publish now stamps its sequence number from a shared
counter at the moment it is made, so every publish is newer than the
last one issued and every publish writes: 400 of them, half of them
600 KB.
And the file is now watched while the writers run rather than inspected
once after they stop. "The published file is never observed
unparseable" is the real invariant -- rename(2) is atomic, so a reader
can only ever see a completely written image -- and sampling it only at
the end looks at the one moment a torn file is least likely to have
survived.
Measured on the build host, five runs with the file mutex removed: 5
detections out of 5, every one of them on the unparseable-file
assertion, which is the assertion that used to fire never. Five runs
with the mutex in place: 5 passes. The whole payout-ledger self-test
takes 1.4 s.
Property: removing the serialisation this test exists to check fails the
test, on every run.
|
|
The comment on FindRecordedPoolPayout justified the legacy fallback by
saying that where two pre-upgrade rounds share a height, the losing
one's ledger entry is voided by the orphan check before it can be paid.
That check is PayoutBlockStillOnChain(), which returns true for a zero
hash -- and every pre-upgrade entry has a zero hash, because they were
written before payouts recorded one. So it is a no-op for exactly the
population the fallback exists to serve. The stated reason for a
safety property was not the reason.
The property does hold, by a different mechanism. SamePayoutRound()
treats two zero-hash entries at the same height as the same round, so
the pair is indistinguishable to every consumer, not just to this one:
FlushMaturePayouts explodes one and skips the other as already queued,
PaymentInFlightLocked refuses a second record for that (round, address),
and RecordPayoutTxid settles against the first match. The pair presents
as one debt and can be paid at most once. The cost of that -- only one
of a genuine same-height pair would ever be paid -- is now written down
too, rather than left as an implication.
Both halves are asserted rather than reasoned about: the self-test
checks that the orphan check passes a zero hash, and that two identical
zero-hash entries at one height produce exactly one in-flight record.
Property: the comment describes the mechanism that is actually
load-bearing, and a test fails if either half of it stops being true.
|
|
ForfeitPayoutRecipient retired an entry on
`it->amounts.empty() || PayoutFullyPaid(*it)`. PayoutFullyPaid iterates
amounts looking for a recipient with no txid, so it returns true for an
empty map: the first test can only be true where the second already is.
Left in, it reads as though the two cases differ, which invites someone
to make them differ. The condition is unchanged; only the claim about it
is.
Property: unchanged behaviour, and the code now says only what it does.
|
|
LoadPaying appends to gPaying and increments gPayingNextId, and both are
annotated as guarded by gPayoutExecMutex. It took neither.
Nothing is racing it today: it is called from the pool startup path
before the payout thread is spawned, and the self-test calls it on one
thread. But that is a fact about its callers, not about the function,
and the annotation is what the next person to add a caller will read.
The lock is uncontended here by construction, so there is nothing to
weigh against taking it.
Property: every mutation of gPaying and gPayingNextId in this file
happens under the mutex they are documented to be guarded by.
|
|
PublishVersioned drops a default-constructed VersionedBlob only by
accident. Every serialiser stamps `++gSeq`, so it never produces 0, and
0 then loses the `seq <= publishedSeq` comparison because publishedSeq
starts at 0. Both halves of that are incidental. A serialiser written as
`seq = gSeq++` -- one character different, and the natural way to write
it -- publishes seq 0 for its first snapshot, which would sail past the
comparison on a fresh process and write an empty ledger over a real one.
There is also a live path that produces an unstamped blob today:
FlushMaturePayouts declares `VersionedBlob ledgerSnap` and only fills it
if something was voided, then publishes unconditionally. It is harmless
for exactly the reasons above, which is to say for no reason that is
written down anywhere.
One line, checked explicitly, so the guarantee does not depend on how a
future serialiser spells its increment.
Property: a blob that was never stamped with a version is never written
to disk.
|
|
All three loaders push entries into the live structure inside the try
block, so an exception part way down a file leaves a PARTIAL picture in
memory: the entries before the bad one, and nothing after it. Both
loaders then return normally, having logged one line under a debug
category, and the node carries on as though that were the whole ledger.
The first publish afterwards writes the partial state over the file.
ReconcilePayouts publishes unconditionally during startup, so that is
milliseconds later -- before an operator could see the warning, let
alone act on it. The only durable record of what the pool owed is then
gone, and it is gone in the direction that costs miners money: the
entries that failed to parse are the ones that vanish. 86b39b9's own
commit message names this failure mode and then fixes only the write
side.
On any parse exception the file is now renamed to
<path>.bad.<timestamp>, the path is added to a refusal set, and
PublishVersioned declines to write it for the rest of the process. A
node that has lost its ledger now comes up visibly missing it rather
than quietly appearing to owe nobody anything, and the bytes are still
there to reconcile against. Restarting into the same fault does not
consume the evidence, because the quarantine name carries the time.
Logged with printf rather than LogPrint: an operator who has just lost a
payout ledger has to be told at whatever settings they were already
running, and the name the file was moved to is the only route back to
the data.
Property: a payout file that did not parse completely is never
overwritten by what did parse.
|
|
FindRecordedPoolPayout's legacy branch matched any wallet transaction
carrying this height's payout comment and an output of exactly this
amount to this address, and never asked whether that transaction was in
the chain. A pre-upgrade payout that never confirmed is evidence of
NON-payment, and it was being read as payment: the recipient is dropped
from the in-flight list, a txid is recorded against the ledger entry,
and the debt retires having never been settled. The miner is not paid
and nothing on disk says they were owed.
That is not hypothetical here. This tree has had a 280-block reorg (see
the note in FlushMaturePayouts). A payout invalidated by one is still
sitting in mapWallet with its comment intact, and the first boot after
this upgrade -- the boot where the binary is swapped after an unclean
shutdown with payments in flight -- is precisely when the legacy probe
runs against it.
The structured branch is deliberately left alone. This process sent
those transactions moments earlier, and a send that failed leaves
nothing behind: SendMoney calls RollbackTransactionSpent, which calls
EraseFromWallet. Depth there would only reject payments that really were
made.
The check cannot be done inside the wallet scan.
GetDepthInMainChain() reads mapBlockIndex and pindexBest, which belong
to cs_main, and cs_main is taken before cs_mapWallet everywhere else in
this tree -- CreateTransaction, ConnectBlock -- so testing it under
cs_mapWallet would invert the lock order in the payout thread. Comment
matches are collected in the scan and adjudicated afterwards under
cs_main then cs_mapWallet, in that order. That also fixes a second
thing: the old code stopped at the first comment match, so one
unconfirmed transaction could hide a confirmed one behind it.
A rejection logs the txid, height, amount and depth, and says the
recipient will be paid, because this is the one decision where being
wrong is unrecoverable in the direction that costs a miner money.
Property: a payment is only ever inferred from a pre-upgrade comment
when the transaction carrying it is in the main chain.
|
|
A payment that the pool could not fund was being recorded as a fault
against the payment, and 1440 faults erased the debt with the coin still
in the operator's wallet.
The classifier read GetBalance() after the failure and asked whether it
covered `amount + nTransactionFee + CENT`. Every term of that was wrong.
nTransactionFee is 0 on this pool -- it is only ever assigned from the
GUI preferences dialog, which a headless node does not have -- so the
entire reserve was one CENT. The fee it is meant to reserve for is
GetMinFee(fDiscount=true), which is a step function of the serialised
size: 0 below 10,000 bytes and (1 + bytes/1000) * CENT at or above it.
There is no required-fee value between 1 and 11 CENT, so a one-CENT
reserve is never the right number -- below the step nothing needs
reserving, at the step it is short by a factor of eleven.
Worse, the trigger correlates with the failure. A pool wallet close to
broke selects many small inputs BECAUSE it is close to broke, and about
56 signed P2PKH inputs is 10 KB, which a pool that has been paying out
accumulates as change. So: balance of amount + 0.05, the pre-send gate
passes, CreateTransaction builds a transaction over 10 KB, the fee
becomes 0.11, SelectCoins(amount + 0.11) fails, SendMoney returns false
-- and (amount + 0.05) < (amount + 0 + 0.01) is false, so "funded", so a
fault. The condition is stable, so it recurred every 60 s until the
budget ran out and ForfeitPayoutRecipient erased the miner's money.
Three changes.
The wallet now says what it did. CreateTransaction takes an optional
WalletSendResult out-parameter and sets WALLET_SEND_INSUFFICIENT_FUNDS
on the one return that means a funding shortfall -- SelectCoins failing
to cover nValue plus whatever fee that pass is working with, which on
the second and later passes is GetMinFee() of the transaction the first
pass actually built. SendMoney gains a four-argument overload returning
that plus the fee that was required. The three-argument SendMoney stays,
as a separate declaration rather than a default argument, because
gui.cpp declares it itself instead of including main.h; walletcmd.cpp
and the GUI are unchanged. These are wallet functions, not consensus.
ClassifyPayoutSendFailure() then classifies the answer rather than
guessing: locked or short of coin defers, anything else is a fault.
There is no reserve left to get wrong, and the size-dependent fee is
accounted for exactly, because the wallet computed it.
And the give-up branch is gone. 1440 failures is not a decidable state,
it is a count of how long we have been asking, and this pool reached it
by misclassifying a permanent funding shortfall. A payment that keeps
failing now stays in flight, keeps its ledger entry, keeps being
reported as owed in pool_status.json, and keeps retrying once a minute.
A debt that is never written off cannot be written off wrongly. The
remaining call to ForfeitPayoutRecipient is the one decidable case --
the ledger names a string that is not a valid address, so no transaction
can pay it -- which previously only dropped the in-flight record and
left the ledger and the round proof claiming the debt forever.
Tested against a real wallet and the real CreateTransaction, not against
injected values, because the honest question is whether a wallet holding
many small coins reports the state this code assumes. 60 coins of 1 BTF,
balance 60.00, amount 59.95: CreateTransaction returns false with
WALLET_SEND_INSUFFICIENT_FUNDS and a required fee of 0.11 -- and since
GetMinFee returns 0 below 10,000 bytes, a required fee of 11 CENT is
itself the measurement that the transaction crossed the step. The same
test asserts that the old reserve judged that state "funded", and that
the same wallet pays a 3-input transaction with a required fee of 0.00,
which is why no fixed reserve is right for both.
Property: a send that failed because the wallet could not fund it is
deferred, whatever the transaction's size, and no number of failures
erases a debt.
|
|
RefreshStats runs on the event loop -- the one thread that reads and
writes every miner's socket -- and took gStatsMutex and then
gPayoutLedgerMutex at function scope, then called
WritePoolStatusJsonLocked() with both still held. Since 86b39b9 that
write ends in fsync() on the file and fsync() on its directory. So every
ten seconds the event loop performed two synchronous disk flushes
holding the mutex the payout thread must take to record that a miner has
been paid, and the payout thread waited behind them. Measured here on
ext4/LVM/SSD: 3.78 ms mean per publish. On the pool's shared virtual
disk it is higher and it has no upper bound under load.
That is worse than the code the merge replaced, which wrote the same
file under the same locks but buffered and without fsync. It also made
33f2e8a's headline claim -- "no thread waits on I/O holding
gPayoutLedgerMutex, gPayoutExecMutex or gPoolRoundsMutex" -- false on
the commit that asserted it.
Split the same way as the three ledgers: SerializePoolStatusLocked()
returns the image, PublishPoolStatus() writes it, and RefreshStats calls
the second one after leaving the scope that holds the first. The ledger
lock is also scoped to the loop that actually needs it, which is the
gPayoutViews rebuild; the serialiser never needed it, since those rows
were already copied.
No version guard is added and none is needed: RefreshStats is still the
only caller of either half, it still runs only on the event loop, and it
now calls them in sequence on that one thread, so pool_status.json still
has a single writer by construction. The comment saying so has been
rechecked against the new shape rather than carried over.
The claim is now tested rather than asserted. The self-test points
strPoolStatusFile at a path whose ".tmp" is a FIFO, so the publish parks
in open(2) until the test chooses to read, and every mutex the write
must not be holding is required to be acquirable while it is parked.
Against the previous code that test cannot pass: the writer sits inside
fopen() holding gStatsMutex and gPayoutLedgerMutex, and nothing releases
them until the write is allowed to finish. Verified by reintroducing the
defect -- two assertions fail, deterministically.
Property: pool_status.json is serialised under the locks and written
with all four released, so no payout mutex is ever held across I/O.
|
|
Every failure path in WriteFileAtomic returned without removing
"<path>.tmp". A disk that filled up once, or a single EIO, left a short
file called pending_payouts.json.tmp sitting next to a good ledger for
as long as the node ran. That file is worse than no file: it carries the
name of a half-finished save, and the operator cleaning up after the
incident is precisely the person who will move it into place and publish
a ledger missing whatever had not been written yet.
The failures also logged under LogPrint("payout", ...). pool_status.json
is written through the same function but everything else about it is
logged under "pool", so an operator running -debug=pool was told nothing
when the status file could not be replaced -- the message existed and
was discarded for having the wrong category. They now print
unconditionally: a write failure is not a trace of normal operation, it
is the file that is about to be read not being there, and gating it on a
category chosen before the failure happened is how it went missing. The
brief's alternative, a category argument, would still have required the
right guess in advance.
Properties: no failure of WriteFileAtomic leaves a .tmp behind, and no
failure of it is invisible at default log settings.
|
|
No behaviour change. Moving the file writes out from under the data mutexes
put a new scope around the locked sections of ReconcilePayouts and
FlushMaturePayouts, and their bodies were left at the old indentation, which
reads as if the lock is not held. Re-indented, with the comments and the
concatenated log strings inside them re-wrapped to fit -- `git show -w` on
this commit is comments and string line breaks only, and the strings
concatenate to the same text.
|
|
LogPrint binds debug.log to whatever GetAppDir() resolves to the first time it
fires, which for this test is its own scratch data directory -- so removing
the files the test knew it had written was never going to empty the directory,
and `make tests` left one behind in src/ on every run.
Sweep the directory instead of naming its contents, and take the process id
the same way the other self-tests do rather than assuming getpid() exists on
every target.
|
|
A new `payout-ledger` self-test, 40 assertions, wired into RunSelfTest and
into `make -C src tests`. It runs against a scratch data directory and an
in-memory wallet map: it never opens wallet.dat, never touches a real datadir,
and never sends anything.
What it asserts, defect by defect:
- a restart does not duplicate an in-flight record. A matured round is
exploded, the process state is thrown away and reloaded from the files
twice, and the in-flight list must still hold exactly one record per
recipient. Both guards are checked separately -- once with the round-level
set restored, and once with it deliberately emptied so only the
per-recipient check against gPaying can save it. A paying file already
containing the duplicate the old build wrote must collapse on load, with
the attempt counts merged upwards rather than reset.
- state updates address a slot, not a value. Two records equal in every
field -- exactly what the old code produced -- and rescheduling or
forgetting one must land on the one it was given.
- the ledger file cannot be published from a stale snapshot. An older
snapshot published after a newer one must not appear on disk, and four
threads publishing 64 large images of the same file in different orders
must leave it parseable and equal to the highest version, whatever order
they finish in.
- a payment with only a legacy comment is recognised as already paid, and
the fallback's limits hold: it requires the right recipient, the exact
amount and the right height; one legacy transaction cannot settle a second
debt; a transaction carrying another round's structured metadata is never
accepted as legacy evidence; and the structured match still wins where it
applies.
- grandfathered zero-hash rounds do not answer for each other, both as a
direct test of the round comparison and end to end -- recording a txid
against one zero-hash round must leave the other still owed.
- the retry clock that is actually used survives a restart.
- a forfeit is published rather than silently dropped: the ledger loses that
recipient and only that recipient, and the round proof records it with a
reason instead of going on advertising the address as owed.
- on-disk compatibility, checked against the real shapes: the live
production pending_payouts.json (amounts / blockHash / blockHeight /
matureAtHeight and nothing else), the merged build's shape including the
retryAfter this build no longer writes, and a round trip through this
build.
Each assertion was verified to fail when its defect is put back: seven
mutations were applied one at a time to a scratch copy and every one was
caught. Notably, re-introducing either half of the duplicate-record fix alone
is caught by its own assertion while the other half still prevents the
duplicate -- which is the defence in depth this series was asked for, checked
rather than asserted.
|
|
WritePendingPayoutsFile, SavePayingLocked and SavePoolRoundsLocked all wrote a
temp file and renamed it, with no fsync anywhere. That is atomic against the
process dying -- which is what the crash test exercised -- and not against the
machine dying. A power cut or a hard VM reset can land the rename on the
platter ahead of the data it points at, and what comes back is a short or
empty file.
For pending_payouts.json that is not a corrupt ledger, it is an absent one:
LoadPendingPayouts catches the parse error, logs one warning, and the node
starts up owing nobody anything. Total silent loss of the unpaid ledger, from
a clean-looking boot.
WriteFileAtomic now fflush()es and fsync()s the temp file before renaming it,
and fsync()s the containing directory afterwards so the new directory entry is
durable as well. If the data cannot be made durable the rename does not happen
at all: the previous file is still intact and still correct, and the next
write will try again. Ordering the sync before the rename is the point --
publishing a name that points at nothing is the exact failure being prevented.
The directory sync is best effort; a filesystem that will not let the
directory be opened for it is not a reason to refuse to write the ledger.
This is only affordable because the preceding commit moved all three writes
out from under gPayoutLedgerMutex / gPayoutExecMutex / gPoolRoundsMutex. An
fsync under any of those would park the pool event loop on the payout thread's
disk, which is the contention the merge was right to remove.
Applies to upstream too and is being reported there.
No on-disk format change.
|
|
There were two, and each was broken in the opposite direction.
PendingPayout::retryAfter was declared, serialised into pending_payouts.json,
loaded back, and tested at the flush gate -- and the only assignment to it
anywhere in the tree was `= 0`. The gate could never fire. PendingPayout's
`retries` and `deferrals` were the same: carried, never written. Dead fields
next to live ones in a structure this one is easy to reach for by mistake, and
a gate that looks like backoff but is not is worse than no gate at all.
PayingRecord::retryAfter is the clock the payout thread actually uses -- and
SavePayingLocked did not write it. Every restart therefore reset every
backing-off payment to "ready now" and put the entire in-flight set into the
very next 500 ms batch instead of once a minute. On a node restarting in a
loop that is a continuous retry storm against a wallet that has already said
no, each pass taking cs_main.
The dead copy is deleted, along with the flush gate that read it and the one
`retryAfter = 0` in RecordPayoutTxid. The live one is now persisted and
restored. Retry state stays where the failure happens -- per (round,
recipient), which is the granularity SendMoney works at.
On disk, both directions still read:
- pending_payouts.json no longer has "retryAfter" written into it, and no
longer reads one. Files that carry it (everything the merged build wrote,
always as 0) load unchanged, the key simply ignored. A build that expects
it defaults an absent key to 0, which is the only value it has ever held.
- paying_payouts.json gains "retryAfter". Older builds ignore unknown keys;
this build defaults an absent one to 0, which is exactly the behaviour a
file written without it used to get.
|
|
ForfeitPayoutRecipient permanently erases a miner's money: the amount leaves
the ledger, the operator keeps the coin, and nothing on disk records that it
was ever owed. It was reachable from two entirely ordinary conditions.
The deferral gate compared GetBalance() against r.amount, ignoring the fee.
A balance anywhere between amount and amount+fee therefore passed the gate,
failed inside SendMoney, and was counted as a send FAILURE -- a retry. At one
a minute, 1440 of those is 24 hours, and then the recipient is gone. A wallet
left locked overnight did exactly the same thing: every attempt fails because
nothing can be signed, and each failure spent budget.
Three changes.
The gate now requires amount + nTransactionFee, which is the part of the fee
knowable before the transaction exists. It deliberately does not try to
reserve GetMinFee(), which depends on the serialised size and so on how many
inputs SelectCoins happens to pick: refusing to attempt a payment the wallet
could actually have made is its own way of not paying a miner. A locked wallet
is now a deferral at the same gate, for the same reason maturity is.
Failures are classified after the fact instead of all counting as faults. The
balance and lock state are re-read after SendMoney returns -- it holds cs_main
and can block for a long time, so both may have moved -- and if the wallet is
locked, or the balance no longer covers amount + fee + one fee tier of
headroom, the attempt is recorded as a deferral. That headroom is where the
size-dependent part of the fee lands, so a fee shortfall is a wait rather than
a fault, which is what it is.
The retry budget therefore now only counts failures that happened while the
payment was funded, the wallet was unlocked, and the funding block was on the
main chain. Reaching 1440 of those means the wallet refuses a payment it has
every means to make, which is the only state in which the payment can be
called impossible rather than merely blocked. The give-up log says which
conditions held, rather than implying it.
Finally, the forfeit is reconciled with the public record. Dropping the amount
from the ledger while pool_rounds.json goes on advertising the address as owed
with "txid": null left the pool's two accounts of the same round permanently
disagreeing, with the operator holding the difference. PoolRoundProof gains a
per-address forfeited/forfeitReason, written by MarkPoolRoundForfeited when
the ledger writes the debt off, and pool_status.json counts those separately
from genuinely unpaid ones instead of reporting them as still pending.
On-disk: pool_rounds.json gains optional "forfeited" and "forfeitReason" keys
inside a payout object, and pool_status.json gains "forfeitedPayouts" per
round. Both are emitted only when a forfeit has actually happened, both are
read back with defaults, and files written by any earlier build load unchanged.
|
|
FindRecordedPoolPayout matches on pool_payout_id / pool_payout_address /
pool_payout_amount. That metadata is written only by this build. Every payout
the pool has actually sent, in the wallet right now, carries nothing but a
"comment".
So the first boot after this upgrade is the dangerous one: swap the binary
after an unclean shutdown with payments in flight, and the recovery probe
finds no evidence for any of them and pays every one of those recipients a
second time. The deleted WalletAlreadyPaid() matched on what the payment WAS
-- the comment plus an output paying exactly that amount to exactly that
script -- and that evidence is still sitting in the wallet. It just stopped
being read.
The probe now answers with which kind of evidence it found, and the legacy
form is restored as a fallback under three constraints that keep it from
answering for a round that is not the one being asked about:
1. it is consulted only when the structured lookup found nothing, so the
exact path stays primary and strictly better going forward;
2. any transaction carrying pool_payout_id is excluded from it outright.
That key means this build wrote the transaction, and this build's
transactions are adjudicated exactly -- so a newer round's payment can
never be offered as proof for an older one. Only pre-upgrade
transactions, which is the entire population the fallback exists for,
are eligible;
3. a txid already credited to a DIFFERENT (round, address) is excluded --
from the ledger's paidTxids, which is durable across restarts, and from
what this process has already claimed. One legacy transaction therefore
settles at most one debt, so N transactions can never discharge more than
N obligations. Two debts with one transaction pays the second, which is
the correct reading of the evidence.
What cannot be resolved from a comment alone is two PRE-UPGRADE rounds at the
same height paying the same address the same amount, because the comment
carries only the height. That requires a reorg, and the losing round's ledger
entry is voided by the orphan check before it can be paid, so the two cannot
both be live. Where the evidence genuinely runs out the fallback does not
match and the payment is made: paying a debt that cannot be proven settled is
the correct default for a pool, and it is the direction that does not keep a
miner's money.
Recovery on legacy evidence logs loudly and says exactly what it matched on,
because it is a judgement made about a binary that is no longer running.
The two lookups share one pass over mapWallet. The structured path used to
scan alone; adding a second scan would have doubled a linear walk taken under
cs_mapWallet, which the whole node contends on.
No on-disk format change.
|
|
A restart duplicated every in-flight payment, deterministically, and the
duplicates were load-bearing.
gQueuedPayouts is memory-only. ReconcilePayouts restored gPaying from disk but
left that set empty, and FlushMaturePayouts deduped only against it and
against the ledger's paidTxids -- never against gPaying itself. So a round
that had been exploded but not yet paid got a SECOND PayingRecord for every
unpaid recipient on the first flush after every restart. Another restart added
a third. The crash test saw exactly that: two byte-identical records for one
(block, address).
ForgetPayment and ReschedulePayment then found their record with SamePayment(),
a field-by-field comparison, which stops being an identity the moment two
records are equal. Every update landed in the first slot and the second was
never touched:
- the duplicate's retryAfter stayed 0, so it ran in every 500 ms batch
instead of once a minute;
- each pass wrote retries = 0+1 into the FIRST slot, resetting its count, so
the 1440-retry budget never completed and forfeit was unreachable;
- the duplicate's fProbeWallet was never cleared, so FindRecordedPoolPayout
-- a full mapWallet scan under cs_mapWallet -- ran twice a second per stuck
recipient, forever, along with GetBalance() and, whenever funds allowed,
SendMoney/CreateTransaction under cs_main.
That is a permanent hot loop holding cs_mapWallet and cs_main on a pool node,
starving RebuildJob() so job refresh and block submission stall. It is reached
by the ordinary state of a payment waiting for coin or for an unlocked wallet.
It did not double-pay only because a slot can reach SendMoney unprobed only
when fProbeWallet is already false, and any successful payment calls
ForgetPayment, which erases exactly the one slot that could be in that state.
That invariant is emergent, undocumented, and dies the moment anyone erases by
index or gives the record a stable identity -- which is precisely what this
commit does, so it is replaced by a structural one.
Both halves are fixed.
Not creating duplicates:
- ReconcilePayouts rebuilds gQueuedPayouts from the restored gPaying, so the
first flush after a restart no longer re-explodes rounds already in flight;
- FlushMaturePayouts additionally refuses to append a record for a
(round, address) that gPaying already holds. That check is the invariant
proper -- the round-level set is an optimisation that can be stale, gPaying
is the list being protected;
- LoadPaying collapses duplicates already on disk as it reads them, merging
the retry and deferral counts upwards, so a file written by the broken
build does not inherit the pathology into the fixed one.
Real identity: PayingRecord gains an id, assigned under gPayoutExecMutex, and
ForgetPayment/ReschedulePayment match on it instead of on values. SamePayment
is deleted. The id is process-local and deliberately not persisted -- gPaying
is rebuilt from disk at every start, so it only has to name a slot within one
run, and reading ids back would mean trusting a file to contain no duplicates.
The exactly-once property is now the conjunction of four checks that each hold
on their own, rather than one accident:
1. at most one gPaying record per (round, address), enforced at insert;
2. the ledger's paidTxids, re-checked at the top of every attempt;
3. the wallet probe, once per record, before its first send;
4. state updates addressed by record id, so no slot can be skipped.
No on-disk format change: the id is not written, and duplicate records in an
existing file are read and collapsed.
|
|
Every ledger lookup identified an entry by its block hash alone --
ReconcilePayouts, the payout thread's already-paid check, RecordPayoutTxid,
ForfeitPayoutRecipient, ForgetPayment's still-in-flight test, and SamePayment.
Entries restored from a file written before payouts were keyed by hash all
carry hash 0. Two of them therefore compare equal, and the first one found
answers for the second:
- RecordPayoutTxid writes a txid against the wrong round. That round can
then satisfy PayoutFullyPaid and be erased from the ledger while the debt
it recorded is still owed, and nothing on disk remembers it.
- ReconcilePayouts drops a live in-flight record on startup because a
different zero-hash round had already paid that same address.
SamePayoutRound() decides it properly: a non-zero hash is unique and settles
the question on its own, a zero hash falls back to the block height (which
LoadPendingPayouts supplies for any entry able to name its own maturity), and
a zero hash never matches a real one. Refusing to match is the safe direction
at all six sites, because each treats a match as "this debt is already
handled".
The orphan-void scan is deliberately left alone: PayoutBlockStillOnChain
returns true for a zero hash, so a grandfathered entry can never enter the
orphaned list and there is nothing there to cross-match.
For the residual case where an entry has neither hash nor height -- which no
file this tree has written can produce -- FlushMaturePayouts now refuses to
explode it and logs why, rather than paying against an identity that could
belong to another round. Not paying is recoverable; recording a payment
against the wrong debt is not.
PayoutIdForBlock's grandfathered "block-<height>" fallback is unchanged and
still safe: that id is only ever matched against pool_payout_id, which no
pre-upgrade transaction carries.
No on-disk format change.
|