| Age | Commit message (Collapse) | Author |
|---|
|
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.
|
|
# Conflicts:
# Makefile
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
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.
|
|
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
|
|
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.
|
|
Splitting SavePendingPayoutsLocked() into a serialise step that needs
gPayoutLedgerMutex and a write step that does not left the write step with
no serialisation at all. The event loop wrote pending_payouts.json from
QueuePayouts and the orphan-void path with the ledger mutex HELD; the payout
thread wrote it from RecordPayoutTxid and ForfeitPayoutRecipient with the
mutex RELEASED. Both fopen()ed the same "<path>.tmp" with independent handles
and both rename()d it.
Two ways that ends, both live money:
- the renamed file is a mixture of two images. LoadPendingPayouts throws
inside its parse loop, logs one warning, and the node comes up with a
nearly empty ledger -- every obligation not yet exploded into an in-flight
record is simply gone, with nothing on disk saying anyone was owed it.
- a lost update: the payout thread's older snapshot lands after the event
loop's newer one and silently erases a whole round that had just been
queued.
Neither parent had this. Upstream writes under the mutex everywhere; the
pre-merge tree had one writer thread and no ledger mutex. The merge invented
the window while fixing a real problem -- the event loop should not block in
FlushMaturePayouts behind a per-recipient fsync on the payout thread -- so
putting the write back under the ledger mutex is not the fix.
Instead: a per-file write mutex that is never held together with a data
mutex, plus a version on every snapshot.
- serialising happens under the data mutex and stamps a monotonically
increasing sequence number (gPayoutLedgerSeq).
- publishing happens with the data mutex released, under the file mutex,
and drops any blob whose sequence number is not strictly newer than the
one already on disk.
Because each blob is a whole-state dump of a single in-memory structure and
the sequence number is assigned under the mutex guarding that structure, a
higher sequence number is by construction a later state -- so discarding an
older blob loses nothing, and the file can only move forwards. Writers may
queue and complete in any order and the published file is still the newest
state anyone has serialised. A failed write deliberately does not advance the
published sequence, so the next write still tries.
The same treatment is applied to paying_payouts.json and pool_rounds.json.
Neither had the interleaving race -- gPayoutExecMutex and gPoolRoundsMutex
already serialise their writers -- but both did their I/O under a mutex the
event loop takes (FlushMaturePayouts, RecordPoolRound), so both were a place
for the event loop to wait on the payout thread's disk. They are now written
outside those mutexes on the same versioned terms, which is also what makes
adding fsync to these paths safe.
pool_status.json keeps a plain atomic write with no version guard: RefreshStats
is its only caller and only ever runs on the event loop, so it has a single
writer by construction. It shares the same WriteFileAtomic helper so the
durability rules live in one place.
No on-disk format change.
|
|
Merges upstream tag v1.2.18 (bc3de30) into our 24 commits. The merge base is
a259215, upstream's own commit, whose src/ tree is byte-identical to 68de575 --
our commits were replayed onto a259215 first so that git had a real base to work
from. That produced 28 conflict hunks in 9 files instead of the 21 conflict
events / 51 hunks a direct cherry-pick onto the tag would have forced, most of
which were artifacts of lifting our commits away from their own predecessors.
No consensus change. main.cpp's GetBlockValue, GetNextWorkRequired,
ConnectInputs, ConnectBlock, CheckBlock, AcceptBlock and AcceptTransaction are
untouched by this merge; script.cpp, serialize.h and randomx_pow.h are
identical on both sides. The frozen-chain oracle reproduces the pinned UTXO root
116a2b6b... byte for byte, and the merged binary loads the same 12,586-block
chain to the same tip aac622f0a3b2f8... with all blocks re-verified and none
rejected.
Which side won, per file and per hunk.
src/rpc.cpp -- 15 hunks.
1. RebuildJob transaction selection. THEIRS for the multi-pass loop and the
nFees fix, OURS for the cap. Their vfAlreadyAdded/fFoundSomething loop lands
dependent transactions that our single pass dropped deterministically, and
their nFees is accumulated across the scan instead of being redeclared inside
it -- ours passed a hardcoded 0 to GetBlockValue, so every pool block built
here discarded its fees. But their bound is MAX_SIZE/2 = 16 MB, and
MAX_BLOCK_SIZE is 1 MB, so their template can exceed what their own
CheckBlock accepts. Cap kept at MAX_BLOCK_SIZE/2, with our inner break so a
single pass cannot overshoot by more than one transaction. Our RebuildJob
sync gating and deaf-node detection are kept; upstream has none.
2. struct PendingPayout. OURS for the key, THEIRS for paidTxids. Their id is
the string "block-<height>", which is height keying -- the exact bug 44849e6
fixed after a 280-block reorg left ~14.4k BTF owed against blocks that no
longer existed. Height keying also lets the crash-recovery probe match the
previous round's transaction after a reorg and silently skip a miner, so
their id is dropped entirely and blockHash stays the key. paidTxids is taken
because it is what makes an entry survivable. Their nextAttempt and our
retryAfter are the same concept under two names; ours is kept and theirs
dropped. matureAtHeight gained an explicit = 0, which neither side had in
our style of initialiser.
3./4. Ledger writer and loader. Mechanical follow-ons of 2. The loader keeps
ours and takes their tolerance for an entry with no usable height, guarded so
it cannot fire on a well-formed entry. One redundancy was removed rather than
merged: their unguarded `if (obj.contains("blockHeight")) get<int>()` sat two
lines above our is_number-guarded version, and an unguarded get<int>() on a
malformed value throws out of the whole loop, losing every entry in the file.
Verified against the live production pending_payouts.json (2 entries, keys
amounts/blockHash/blockHeight/matureAtHeight): loads both. A file in the new
shape, with retryAfter and paidTxids, also loads.
5. Insertion point after the loader. BOTH. Our PayingRecord/gPaying in-flight
ledger and their PoolRoundProof/PoolRoundToJson are disjoint features that
landed at the same offset. Both kept, in full.
6. Writer tail. THEIRS, copied into our writer. Checking fclose() matters here
because fclose is where the buffered write actually reaches the OS: renaming
a short .tmp over a good file is precisely the truncation the atomic write
exists to prevent. The Windows remove()-before-rename() is required because
rename() there will not replace an existing file.
7. Loader bodies. BOTH -- LoadPaying and LoadPoolRounds are different functions
that happened to share a common tail.
8. Function block, 111 vs 36 lines. BOTH, with one deliberate deletion.
PayoutBlockStillOnChain, PayoutComment, RecordPoolRound, MarkPoolRoundPaid
all kept. WalletAlreadyPaid is DELETED and ReconcilePayouts rewritten,
because our reconciliation and their FindRecordedPoolPayout are two answers
to the same crash-recovery question and running both would give the same debt
two authorities. Their wallet probe wins: it matches structured metadata
rather than a comment string, so it can tell two rounds paying the same
address the same amount apart. ReconcilePayouts keeps only the job that is
still real -- dropping in-flight records the ledger already shows paid, and
arming each restored record for its one wallet probe. It no longer touches
gPendingPayouts at all; under the new model dropping ledger entries there
would reintroduce the loss this merge adopts their model to prevent. Their
gPayoutExecMutex/gPayoutThreadRunning re-declarations were dropped as
duplicates of ours, and gPayoutExecQueue was not taken: gPaying is the exec
queue in this tree, at finer granularity.
9. QueuePayouts signature. THEIRS for the round-proof arguments, OURS for the
type of blockHash. It stays a uint256 and is converted to hex only where the
round proof needs text -- a ledger key that has been through ToString() is a
key that can be compared wrong.
10. Maturity policy. OURS. matureAtHeight = blockHeight +
COINBASE_SPENDABLE_DEPTH (120), not + COINBASE_MATURITY (100). At depth 100
the block's own coinbase is still 20 blocks short of spendable, so the
payment must come from unrelated mature coin and the pool runs a permanent
float. Our own history contradicts itself here -- d6972a0 argued for
COINBASE_MATURITY citing "96% of 934 payments", 44849e6 reversed it on the
grounds that the measurement was taken while the node was alone on a private
fork paying itself. HEAD's position is kept. This is a considered
disagreement with upstream, not an oversight. The live production ledger
confirms it: both entries have matureAtHeight - blockHeight = 120.
11. Comment above FlushMaturePayouts. Rewritten to describe the merged design.
12. FlushMaturePayouts, the central difference. THEIRS for the model. The ledger
entry now survives on disk until every recipient has a txid, instead of
being erased as soon as its payments were handed off -- the old ordering
left a window where the debt was described by nothing at all, and a
container restart on 2026-07-30 silently dropped ~1,156 BTF through it. Our
gPaying is kept on top as the in-flight working set, because it carries the
per-recipient retry and deferral state a per-round nextAttempt cannot
express. It is explicitly a cache derived from the ledger, not a second
record of debt: if it were lost the next flush would rebuild it.
13. PayoutThreadFn, 79 vs 70 lines. OURS for the loop and the reorg re-check,
THEIRS for the per-recipient dedupe. The re-check on every attempt is
load-bearing and upstream has no equivalent -- it decides once, at maturity,
and a payment can sit through hours of retries during which the reorg that
kills its funding block can land. Their paidTxids check is taken, against
the ledger rather than against the batch. Our deferral policy is kept:
waiting for coin is not a failure and must not consume the retry budget.
Two fixes upstream does not have are folded in here rather than deferred to
a follow-up commit:
- FindRecordedPoolPayout runs once per record, on its first attempt, not on
every attempt. It is a linear scan of mapWallet under cs_mapWallet, per
recipient; upstream repeats it for every retry, which with 2000 rotating
payout addresses and a budget of 1440 retries is a full wallet scan per
address per minute for a day. The question it answers can only change
across a restart, and every restart re-arms the flag.
- The ledger file write no longer happens under gPayoutLedgerMutex in the
per-recipient loop. SavePendingPayoutsLocked was split into a serialise
step that needs the lock and a write step that does not, so the mutex is
held only long enough to read the structure. The orphan scan in
FlushMaturePayouts was likewise restructured so the ledger mutex is not
held across PayoutBlockStillOnChain's cs_main acquisition.
One case the merged model created and neither side handled: under
survives-until-fully-paid, our GIVING UP path dropped the in-flight record
but left the recipient owed in the ledger forever, so every later flush
would explode it again -- a forfeit that silently becomes an infinite retry.
ForfeitPayoutRecipient writes the amount off so the entry can retire.
14. QueuePayouts call site. Follow-on of 9.
15. ThreadRPCServer startup. BOTH. LoadPoolRounds added; LoadPaying and
ReconcilePayouts stay, and stay before the payout thread starts.
Crash safety, stated as the property that must hold: after kill -9 at any
point, restart pays each recipient exactly once. Before SendMoney, the
ledger has no txid and the payment is retried. Between SendMoney and the
ledger write, the wallet holds the transaction and the first-attempt probe
finds it. After the ledger write, paidTxids suppresses it. If the in-flight
file is lost entirely, the ledger rebuilds it and the probe still covers the
middle window.
src/nostr.cpp -- 3 hunks. THEIRS as the base, with two things restored from
ours. Theirs loads the Windows ROOT certificate store, which ours lacks and
genuinely needs, since OpenSSL's default verify paths are usually empty there
and our version would fail closed. But the plan's claim that theirs is a
superset of our 06bceb4 is FALSE: upstream's nostr.cpp contains no
min_proto_version, no TLS1_2, no SSL_OP_NO_*, and no set_options anywhere, so
taking it wholesale would have removed the TLS-version floor outright. Restored.
Second, their SSL_get_verify_result check sits after a successful handshake,
where with SSL_VERIFY_PEER set it is nearly dead code -- a bad certificate
aborts inside SSL_connect. The reporting belongs in the failure branch, which is
where the merge put it; their post-handshake duplicate was dropped. Our
relay-list and socket-pooling hunks auto-merged and were verified intact.
src/net.cpp -- 1 hunk. THEIRS. Their guard, hSocket == INVALID_SOCKET ||
pnode->hSocket != hSocket, subsumes ours and additionally catches descriptor
recycling, which ours did not; our version also referenced a pnode that only
existed in the select() loop. Our other db34011 hunk, nulling pnode->hSocket in
the poll-failure branch, auto-merged and was NOT re-applied -- plan section 7
Batch D says to re-apply it, and that instruction is stale. Two pieces of
residue the merge left behind were cleaned up: a comment still describing
select() semantics, and a pnode->hSocket != INVALID_SOCKET guard made redundant
by a continue two lines above it.
src/main.cpp -- 2 hunks. h1 BOTH, pure add/add adjacency between our
ParticipantHashThreadCount and their SecureClear forward declarations. h2 OURS,
and their handler discarded: 44004e3 no-ops checkorder/submitorder but keeps
market.cpp and market.h, whereas 11766be deletes the marketplace outright
including CRequestTracker, subscribe/publish and mapProducts. Ours is strictly
more.
src/main_gui.cpp -- 2 hunks. BOTH, additive on both sides. Our /payoutaddr,
/payoutfile and /hashthreads alongside their /stratumbridge and
/stratumbridgeport.
src/btfrv.cpp -- 1 hunk, not predicted by the plan. False adjacency: our
INVALID_SOCKET check and their #endif landed at the same offset. Ordering is
load-bearing -- our check goes inside the #else arm, before the #endif, or the
non-proxy build silently loses it. The BITFLASH_NODE_BUILD arm already returns
early, so both paths now have the check.
Makefile -- 2 hunks plus a hazard with no conflict marker. THEIRS for both
dependency-fetch recipes: checkout --detach FETCH_HEAD followed by test
"$(git rev-parse HEAD)" = "$(..._COMMIT)" asserts the pin actually landed, where
ours only checked it out. Everything of ours that they lack is kept: the
appimagetool 1.9.1 release pin with a sha256 check, where upstream fetches the
mutable "continuous" tag and executes it undigested, and the move of both the
tool and the AppDir out of /tmp, where any local user could pre-place an
executable and have it run as whoever runs make.
The hazard: both sides added a pin block in a different place, so git merged
both, outside any conflict region. SECP256K1_COMMIT and RANDOMX_COMMIT ended up
assigned twice, near L16 and near L44, and GNU Make takes the last := -- a naive
resolution would have silently built against our old secp256k1 pin while
appearing to adopt upstream's. The duplicate block was deleted by hand. Each of
the four variables is now assigned exactly once, and make -n resolves
SECP256K1_COMMIT to 7fecac74 with 6e2c8bc4 appearing nowhere.
Also applied here, from plan phase 4: SECP256K1_COMMIT moved to upstream's
7fecac74aed8e1fd9078380d67dd04663705c989 so signature validation matches the
binaries the rest of the network runs. secp256k1 was rebuilt from scratch
against the new pin on the build host and the checked-out dependency commit
verified. RANDOMX_COMMIT already matched at 1e9d4b2d and is unchanged.
src/Makefile, src/makefile.mingw -- 1 hunk each, both the HEADERS list. Ours
drops market.h, theirs adds proxy.h; the resolution does both. Taking theirs
wholesale would have left the Makefiles referencing a header git had already
deleted in our favour, silently, without a conflict.
Files the plan predicted would conflict and did not: net.h, main.h and db.cpp
auto-merged. Verified rather than assumed. net.h carries both our
BTF_MSG_WAIT_MAX_SECS work, including the constructor fix from 2d84982 that
starts nMsgWaitSince and nMsgWaitSize at 0, and their comment rewrite with
extern int nPeersWatched. The dead members that fix removed are gone from every
file.
Deviations from MERGE-1.2.18-PLAN.md.
- Section 6's mechanism was not used. Replayed onto the real merge base and
merged, rather than cherry-picking onto the tag. Same result, 28 hunks
instead of 51.
- Section 2 calls 09ff575 a superset of our 06bceb4. It is not; upstream has no
TLS-version floor at all. Ours restored on top of theirs.
- Section 7 Batch D says to re-apply our db34011 line into their poll-failure
branch. It auto-merged; re-applying would have duplicated it.
- Section 2 says do not take the stratum bridge. It is taken, and left off.
fStratumBridge defaults false in main.cpp, the thread is only spawned when
/stratumbridge is passed, and the listener binds 127.0.0.1 only. Excising it
would cost real divergence and future conflict for no gain while it is
disabled. Its defects -- select()/FD_SET on a node that can hold more than
1024 descriptors, and an uncapped thread per connection -- are real only when
it is enabled and are being reported upstream separately.
- Section 7's staged batches were not run as separate gated commits. The
conflict surface is interleaved within single functions, chiefly the payout
ledger, and splitting it would have meant committing states that do not
build. The merge is one commit, as briefed.
- Batch F's two fixes are folded into hunk 13 rather than deferred, because
they change the same lines the resolution rewrites.
- The plan did not anticipate the btfrv.cpp conflict, the duplicate pin block,
the market.h trap in the two Makefiles, or the give-up/infinite-retry
interaction created by adopting upstream's ledger model.
|
|
CNode's constructor never wrote nMsgWaitSince or nMsgWaitSize, so every peer
began life with indeterminate values in the pair ProcessMessages uses to
decide whether that peer has stopped sending, and the first partial message
it read was measured against them.
The guard restarts the clock only when nMsgWaitSince is 0 or the buffer has
grown since the last look. Garbage in nMsgWaitSince is neither, so the very
first look fell through to the timeout branch. A peer that merely split a
message across two TCP segments -- header in one, body in the next, which is
ordinary on any real link -- could be dropped as unreadable on first contact,
before it had sent anything wrong and often before its version message. It
was never reproducible either: what happened depended on what had last used
that heap. The log reported the stall as roughly the age of the epoch:
peer 127.0.0.1:8433 announced a 100 byte 'block' and has sent nothing more
for 1786593923 s (0 of 100 payload bytes) -- dropping, this peer is
unreadable
Every node is running this.
Introduced in 7f6e5ad, which added the pair to the class and to
ProcessMessages but not to the constructor; the constructor went on
initialising nIncompleteMessageStart, nIncompleteMessageSize and
strIncompleteMessageCommand, the upstream members our implementation
supersedes. 5490d66 removed their last reader from ProcessMessages, and
nothing has read or written them since, so they go, with
BTF_INCOMPLETE_MESSAGE_TIMEOUT_SECS -- decoy state in a networking struct is
how this happened, and the merge ahead of us would carry it further. The
initialiser list now follows declaration order, so a member missing from it
is visible.
The net-message selftest was asserting on those dead members too, which is
why it could not have caught this: it read state no code path writes, and its
stale-message case passed only when the garbage in nMsgWaitSince happened to
look old. Rewired onto nMsgWaitSince / nMsgWaitSize / BTF_MSG_WAIT_MAX_SECS,
and given the two cases it never covered -- a new peer's clock must start
stopped, and a peer that is still delivering payload must not be dropped
however old its clock is.
Measured on the build host, 20 runs each: before, net-message failed 20/20
("a fresh incomplete message does not disconnect immediately" failed 17/20 by
itself). After, 20/20 pass. Rewired selftest against a reverted constructor
fix: fails 20/20, deterministically, where the old suite caught it by luck or
not at all.
|
|
StartMinerThreads() sizes its pool with MinerThreadCount(), which is right for
solo mining -- each thread hashes a template of its own -- and wrong for
participant mode, where every thread entering BitcoinMiner() is a whole miner:
its own Stratum session, its own payout address off the rotation, its own pool
of /hashthreads workers. The two counts multiplied instead of dividing.
Measured on the pool box with /hashthreads=2: a container with no cpuset logged
"Mining with 11 thread(s) of 12 core(s)" -- 11 sessions, 22 hash threads on 12
cores, 1058% CPU, load 22.68 -- while the same binary under --cpuset-cpus 10,11
logged one session and 199%. The second miner (8 cores, /hashthreads=1) ran 7
sessions and went 74C to 89C. No code differed between those runs. Only the
cpuset did, which means the cpuset was holding the thread count down by
accident and every deployment without one was oversubscribed.
Participant mode now starts exactly one session, so /hashthreads means what it
says instead of being multiplied by the core count. The worker count moves to
ParticipantHashThreadCount(), next to MinerThreadCount() where the two are
easy to confuse, so what is printed at startup and what actually hashes cannot
drift apart -- and it IS printed now. It was only ever visible under -debug,
which is how eleven sessions ran unnoticed.
Same host, same cpuset (0-3), same arguments, measured as a delta over 20 s
more than four minutes after start:
before 399% CPU, 6-7 threads hot, 3 sessions
after 201% CPU, exactly 2 threads at 99.9%, 1 session
Solo mining is untouched: /gen without /participant still starts
MinerThreadCount() threads.
(cherry picked from commit 45bc9ed5bc132712092f3e7902fef746c6cae898)
|
|
Four inherited handlers took attacker-controlled input and wrote to disk
with nothing authorising it. None of them had a caller left anywhere in
the tree, so all four were reachable only from the outside.
checkorder called GenerateNewKey() -- AddKey(), a wallet.dat write -- for
any peer that asked, and handed back the pubkey. Its comment claims the
cache keeps one key per ip, and on a 2009 network of routable addresses
it did. Not here: inbound peers arrive through a rendezvous tunnel and
are tagged by BtfMarkerAddr() over three random bytes, so addr.ip is
fresh every connection and the cache never hits. Reconnect in a loop and
the node mints a key per connection, none of them in any backup taken
beforehand, which for a pre-BIP32 wallet is how coins go missing. The map
holding them was static and never trimmed.
submitorder AddToWallet()'d a CWalletTx from any peer that sent one. It
had to be a valid transaction; it did not have to be ours.
review checked a signature the sender made with its own key, which proves
possession and authorises nothing, then appended to reviews.dat by
reading the whole vector, pushing one entry and writing it all back --
quadratic disk on an unbounded map<string,string> off the socket -- then
walked a link graph the same peer supplied and relayed the message to
every other node.
reply dispatched into CRequestTracker. Nothing ever called PushRequest,
so the table was permanently empty and the handler was a parser and
nothing else.
Removed with the subsystem behind them: market.cpp, CProduct, CReview,
CUser, CReviewDB, the advert publish templates and the subscribe channel
machinery, none of which had an inbound handler either. MSG_REVIEW,
MSG_PRODUCT and MSG_TABLE stay in the inv enum, unserved: CInv puts type
on the wire as an integer and MSG_TX and MSG_BLOCK are shared with every
other node.
Unknown commands are already ignored for extensibility, so peers still
running these lose nothing but an answer they never asked for. Verified
by a clean build of bitflash-node.
(cherry picked from commit ad9c393866c007a115f32d70a9017ce91066a7d1)
|
|
std::thread::hardware_concurrency() reports the MACHINE's core count. Inside a
container that is the wrong number: with --cpuset-cpus 0-5 on a 12-core host it
still answers 12, and every caller that sized a pool from it oversubscribed 2x.
Measured on the pool box:
- the pool node logged "Mining with 11 thread(s) of 12 core(s)" while
confined to 6 CPUs
- a burst miner launched with /hashthreads=2 filled the ~2 GB RandomX
dataset with 12 threads, alongside the pool node in the same 8 GB container
AvailableCoreCount() asks sched_getaffinity what the cpuset actually permits,
which is the number all of these meant. It cannot make the dataset build slower:
the CPU budget is identical either way, so fewer threads on the same cores is
the same work without the thrash. Falls back to hardware_concurrency where the
call is unavailable, and never returns 0 because callers subtract from it.
Not the cause of the burst miner dying -- that was a corrupt Berkeley DB
environment in the worker datadir -- but it is why the failure landed on the
dataset build, and it is wrong on its own terms.
(cherry picked from commit b3bee16f3eef8982bff1174bd9d0a9c21473732c)
|
|
Upstream #109's message-stall rework, hand-merged so it keeps the progress
timing this tree already had.
The old path consumed the header, found the payload short, inserted the header
back at the front of vRecv, then Sleep(100) inside the shared message thread.
A peer that announces a large payload and drips bytes made every pass pay an
O(n) insert and a 100 ms stall -- one peer slowing the thread that serves all
of them. Now the header is read from a copy and only skipped once the whole
body has arrived, so there is nothing to put back and nothing to sleep for.
Kept OURS, not upstream's, on the timeout: upstream restarts its clock only
when the (size, command) pair changes, which drops a peer sending one big
message slowly but honestly. This times the STALL -- the clock restarts on any
progress -- which is the distinction that matters on a slow link.
Also from #109: a header could announce 256 MB while serialize.h refuses
anything over MAX_SIZE (32 MB), so the check admitted an allocation the parser
would reject anyway and burned a peer slot for two minutes doing it.
MAX_PROTOCOL_MESSAGE_SIZE ties the two together. 8x less a peer can make us
hold, and this node takes public Stratum.
And #103's other half: ~CNode closed its socket with a raw closesocket(),
invisible to the accounting, and left hSocket set afterwards -- which is how a
descriptor number that now belongs to somebody else gets closed a second time.
(cherry picked from commit 2f25e2e07985e9bc732cadfbb3c9ac233895a814)
|
|
CheckBlock() validated block size against MAX_SIZE, the serializer's generic
32 MB guard, not a block rule. MAX_BLOCK_SIZE did not exist in this tree at
all: Bitcoin's 1 MB cap arrived in 0.3.x, after the 0.1.0 this fork came from.
A miner could produce a 32 MB block and every node would download, verify and
store it. Upstream #111 (from #110 by mtg0001).
The cap is a tightening, so a node that does not upgrade is the permissive one.
Measured upstream over 5709 blocks: largest 34,290 bytes, median 216, so 1 MB
rejects nothing that exists.
Upstream shipped the validation half only. Both places that ASSEMBLE a block
still filled to MAX_SIZE/2 (16 MB), which was consistent against a 32 MB
validation cap and is inverted by a 1 MB one -- the node would solve a block
and then reject it itself. Fixed both:
main.cpp CreateNewBlock, the node's own miner
rpc.cpp RebuildJob, the POOL's job builder -- the one that matters here,
because every miner on the pool would be working a template the
network rejects, and this pool is ~90% of hashrate
Half the cap in both, preserving the original margin: each test runs against a
transaction that is already counted, so the loop can overshoot by one.
(cherry picked from commit a3be9f04903447fa2741aff4eb529fe45f916877)
|
|
From the first of two wedges captured on the pool. It is a different
proximate fault from the second one, and this is the part that turns a
single bad socket into a node-wide outage.
A failed select() zeroes both descriptor sets, so no peer is serviced on
that pass. That is correct as far as it goes. What was missing is that
nothing stopped the offending descriptor coming straight back:
- the invalid socket was only marked fDisconnect, and a node is not
removed from vNodes until it has drained or DISCONNECT_DRAIN_SECS has
elapsed -- and it cannot drain, because its socket is gone;
- the select set was rebuilt from vNodes with no check, so the same dead
descriptor went back in on the very next pass and failed select()
again.
Measured in the capture: 1,027 consecutive 'its socket is no longer valid'
passes across 61 seconds -- the full drain timeout -- one bad socket, and
for the whole of it the node read nothing from anybody. The peer that
caused it had 57 bytes unsent, so it took the slow path out.
Now the descriptor is forgotten as soon as getsockopt says it is not ours:
select() succeeds again on the next pass and every other peer is served
normally while the dead node ages out as before.
Deliberately forgotten rather than closed. getsockopt has just said this
descriptor is no longer ours; calling close() on it is precisely how an
unrelated socket gets closed instead, which is the fault fixed in e742d0b.
(cherry picked from commit f14c513b4e39d6203b73584f6dc01acbb5a9546e)
|
|
Found auditing the previous commit for the push. Timing from the first
rewind would drop a peer whose message is large and merely slow, which
would be a fault of our own making rather than one we fixed.
Restart the clock whenever the buffer has grown by even one byte since the
last look. A transfer still making progress is never dropped however long
it takes; only one that has stopped growing while the node goes on
expecting more is, which is the actual failure.
(cherry picked from commit 5dddb26bae02013ce87719b11953aadf0bfa2594)
|
|
THIS IS THE WEDGE, and the note asking for this fix has been in the file
since Bitcoin 0.1: 'need a mechanism to give up waiting for overlong
message size error'.
ProcessMessages reads a header, finds the body has not all arrived, pushes
the header back onto the front of vRecv and breaks. Correct for a message
still in flight. For one that never completes it is permanent: every later
pass re-reads the same header and breaks in the same place, so that peer is
never parsed again for the life of the process -- no blocks, no inv, not
even its version message.
Nothing rescues it:
- a header may claim up to 256 MB (0x10000000) and still pass
CMessageHeader::IsValid(), so a garbage or truncated header is accepted
and then waited on forever;
- nLastRecv is refreshed by any byte that arrives (net.cpp), whether or
not it parses, so the inactivity timeout that drops a silent peer never
fires on a peer that is merely unreadable;
- the socket stays ESTABLISHED and select() keeps offering it, so nothing
at the socket layer notices either.
It also explains the signature that made no sense. A peer stuck before its
version message is parsed keeps nStartingHeight == -1, so a node with a full
vNodes still reports GetPeerMedianHeight() == -1 -- which is exactly what
was observed, and exactly what disabled the pool's behind-the-network gate
and let it fork.
Evidence, from two wedges captured on the pool by the watchdog:
MESSAGE-BREAK 2 ran 2014 times in the deaf window against 607 in the healthy
window before it, while blocks received went to zero and the node kept
processing miner traffic normally throughout.
Now: a peer that announces a body and does not deliver it within two minutes
is dropped, and says so in the log unconditionally. Two minutes is far
beyond any legitimate header-to-body gap and well inside
BTF_RECV_TIMEOUT_SECS.
(cherry picked from commit ca1e99288e03ac7beb2b97cf615a3255b4635d95)
|