| Age | Commit message (Collapse) | Author |
|---|
|
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
|
|
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
|
|
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)
|
|
THE WEDGE.
CNode::Disconnect() closed hSocket and left the member holding the stale
number. ~CNode's only guard is 'hSocket != INVALID_SOCKET' and nothing in
the tree ever assigned that sentinel, so the destructor closed the same
number a second time.
The two closes are five minutes apart by design: ThreadSocketHandler2
disconnects a node, sets nReleaseTime to GetTime() + 5*60, and parks it in
vNodesDisconnected; GetRefCount() reports it busy until that expires, so
'delete pnode' -- and the second close -- cannot run before then.
Linux hands out the lowest free descriptor. Measured on the pool: 202 open
descriptors with a high-water mark of 216, so a freed number is reissued
within seconds -- to a Nostr relay TLS socket, a rendezvous socket, a
tunnel end, or a miner connection. Five minutes later the destructor
closes it. The victim gets no error and no log line. It simply stops.
That is the whole signature, including the part that never made sense:
.btf block delivery and Nostr descriptor lookup share no lock, no queue
and no thread, so they should not fail together. They share the file
descriptor table, and a stray close is drawn from all of it.
It also explains why #78 did not help and may have hurt. #78 drops stuck
nodes -- it fired 41 times in one window -- and every drop is another
Disconnect(), which is another delayed close armed against an innocent
socket.
Second bug, same symptom, different subsystem: the pooled relay sockets.
ResolveDescriptor returned the instant a descriptor verified, leaving the
relay's EOSE unread on a socket that goes straight back into g_relayPool,
so the next caller read our leftovers as its own answer -- permanently one
message out of step, and no code path anywhere inspected the subscription
id that would have caught it. Worse, BtfResolve set lease.fOk = true on
every failure path including a dead RecvText, labelling it 'exchange
completed, just no match'. fOk = false is the only thing that ever closes
a pooled socket or clears fConnected, so a relay connection that died was
pinned in the pool, reported healthy, and reused for the life of the
process. That is 'no Nostr descriptor found' for a descriptor that nak can
see, cleared only by a restart.
Now: read to EOSE before returning, match the subscription id, and keep a
pooled socket only when the relay actually finished the conversation.
(cherry picked from commit ed4640c306cc46dbdbd92cb6dc55e7528651269c)
|
|
The peer-height gate replaced a tip-age check that was wrong in both
directions. It is a strict improvement and it is still blind in the one
case that matters most.
When the node goes deaf -- peers still in vNodes, nothing arriving -- no
peer announces anything, GetPeerMedianHeight() returns -1, and the
fallback mines our best chain. That fallback is right for a node that has
not been told yet and wrong for a node that has stopped being able to
hear, and a height nobody sent cannot tell those apart. A wedged node is
exactly a node with no peer height information, so the gate was silent
for the whole of heights 3966-4085: 120 blocks, 5890.50 BTF voided.
Time can tell them apart. GetPeerHeightAgeSecs() measures how long since
any peer reported a height, counting from first use rather than the epoch
so a starting node does not refuse jobs before its first handshake. Past
20 minutes the pool stops issuing jobs and says so unconditionally in the
log. /poolmineblind overrides it: without an escape hatch a genuinely
isolated pool that is 90% of the hashrate could never restart its own
chain, which is the deadlock the tip-age check used to cause.
Also stop nStartingHeight from going stale. It was a snapshot taken once
during the version handshake and never touched again, so on a long-lived
connection the median is built from heights the peers had hours ago, and
the behind-the-network check drifts toward 'everyone is behind us' and
quietly stops firing. A peer that hands us a block is demonstrably at
least at that block's height.
(cherry picked from commit 0800c18faf703b4acde1b131d0b742d37a86d92b)
|
|
The sync gate compared the tip timestamp against 30 minutes. That cannot
distinguish "we are not synced" from "the network is slow", and it failed
in both directions on the same day.
A pool mining its own fork keeps its tip fresh, so the check passed for
the whole of heights 3966-4085 -- 120 consecutive blocks, all orphaned,
5890.50 BTF of payouts voided, and nothing in the log to say so. Then,
once the pool stopped finding blocks, the same check refused to issue
jobs against a tip that was the network's own best. The pool is ~90% of
the hashrate, so nobody else could move the chain either: stale tip, no
jobs; no jobs, stale tip.
Gate on the height peers announce in their version message instead.
Median, so one peer cannot stall us with an absurd claim, and a block of
slack to match WarnIfBehind. Peers predating the announcement report -1
and are ignored; with no reports at all we mine our best chain, which is
what the node does anyway without a pool attached.
(cherry picked from commit 4e76e34f37a82c0885f75db7dfb9e49db410171d)
|
|
The exec queue was memory-only, and FlushMaturePayouts erased a matured entry
from pending_payouts.json *before* handing it over. Any restart in that window
lost those payments with no record that anyone was owed anything. It was not
hypothetical: a container restart on 2026-07-30 silently dropped a backlog of
~1,156 BTF, and there was nothing left to reconstruct it from.
Re-queueing blindly would have double-paid whoever had already been sent to,
so the payment needs to be idempotent rather than merely persistent.
In-flight work is now a persisted list of per-payment records
(paying_payouts.json), one per (block, address) -- the granularity SendMoney
works at, and therefore the granularity at which a payment can be half-done
when the process dies. Records are written, via a temp file and rename so a
crash cannot truncate them, *before* the ledger entry is erased, so the debt
is always described by at least one of the two files. Each attempt updates the
record (retries, deferrals, backoff); settling it removes the record and
rewrites the file, which is what stops a restart from paying twice.
On startup ReconcilePayouts runs before the payout thread:
- a record whose payment the wallet already shows -- matched on the payout
comment plus the exact amount and destination -- is dropped, not resent.
SendMoney commits the transaction to the wallet before it returns, so a
crash immediately after sending is recoverable.
- a ledger entry that has already been exploded into records is dropped,
which is the crash-in-the-window case where both files describe one debt.
The wallet check is deliberately biased towards "already paid": skipping a
payment is visible in the log and recoverable, paying twice is neither.
Verified live: sampling paying_payouts.json during a payout burst caught 3
records / 18.56 BTF mid-flight, cleared once paid, and the paid lines now
carry the txid.
(cherry picked from commit d0248f2e65dbe745f2e9b1d09e1f53a4354d3640)
|
|
Two fixes.
uint256's default constructor is empty (uint256.h:514), so the blockHash
field added in 6963862 was uninitialised garbage on every PendingPayout
built without an explicit hash -- which is every entry loaded from a ledger
written before that commit. Those entries were meant to be grandfathered
past the orphan check on a zero hash; instead each carried a random value
that matched no block in mapBlockIndex, and the first maturity sweep after
deploy voided the whole loaded ledger: 100 entries, 4950 BTF. Only one
"was orphaned -- voiding" line reached the log because LogPrint collapsed
the other 99 as duplicates, so the destruction was nearly silent. Nothing
had been paid at that point and the file was restored from a pre-deploy
backup, so no obligation was lost. Assigning 0 explicitly is all that was
missing.
Separately, four of the twelve relays are dead. Probed all twelve from two
independent networks (a residential line and the pool's VPS) over v4 and v6,
with DNS confirmed against 1.1.1.1:
relay.current.fyi, relay.nostr.bg -- NXDOMAIN, no A and no AAAA. The
domains are gone. (A getaddrinfo call on the VPS reports an address for
them, but that is the VPS's own IPv6 handed back by its local resolver,
not a real host.)
relay.nostr.band, nostr.fmt.wiz.biz -- DNS resolves and TCP connects, but
the TLS handshake never completes, from either network. Whether they are
down or refusing non-browser clients, this node cannot use them.
The remaining eight all complete a WebSocket upgrade. A dead relay is not
free: each holds a connect/retry slot and delays discovery for no added
redundancy.
(cherry picked from commit 144bc96f150d23507d57d29cb339f177dcd6b092)
|
|
A 280-block reorg (heights 3075-3354) orphaned 292 of the pool's 707 blocks
and destroyed 14,600 BTF of coinbase. The payout ledger did not notice: it
records only matureAtHeight, and a reorg replaces the block at a height
without changing the height, so ~14.4k BTF of obligations outlived the coin
that was supposed to fund them. The pool then paid fork-era miners out of
real-chain coin until the wallet was empty -- 17,827 BTF paid against 17,900
BTF of matured supply, leaving under 1.1 BTF spendable and every subsequent
payment failing.
Three changes, all in the payout path:
- Record the funding block's hash and height, persist both, and void an
obligation whose block is no longer in the main chain. Checked both at
maturity and again in the exec queue, since an entry can sit there for
hours across retries and the reorg can land at any point in that window.
Entries loaded from a file written before this are grandfathered rather
than voided on a missing field.
- Stop spending the retry budget on maturity. SelectCoins failing for want
of spendable coin is not an error, but it consumed one of 1440 retries
every 60s, so a payment the pool fully intended to honour would hit
GIVING UP 24h later and the miner would silently never be paid. Those
attempts now defer without incrementing retries, logged hourly rather
than every minute. GIVING UP now means what it says: 1440 real failures
against a block still on-chain with the money available.
- Schedule at COINBASE_SPENDABLE_DEPTH instead of COINBASE_MATURITY. At
depth 100 a block's own coinbase is 20 blocks short of spendable, so its
payout had to come from unrelated mature coin and the pool carried a
permanent ~19-block float. The "96% of 934 payments went through at depth
100" note that justified the shorter wait was measured while this node was
alone on a private fork paying itself. The extra 20 blocks also put the
maturity gate past the reorg that caused all of the above.
(cherry picked from commit 696386220a3d2fac88cb8045931392f4909e0985)
|
|
SendMoney's "oversized transaction that requires a transaction fee of
0.00" is not an oversized transaction and there is no fee. That branch is
taken when nValue + nFeeRequired > GetBalance() -- an empty spendable
balance -- and the fee reads 0.00 because CreateTransaction failed before
setting one. 3,267 of these in debug.log sent me looking for a
transaction-size problem that does not exist. Report what is short, by how
much, and against what balance.
Name COINBASE_SPENDABLE_DEPTH in main.h and derive GetBlocksToMaturity
from it, because the distinction it captures is genuinely surprising:
CMerkleTx::GetCredit values a mined coinbase at zero until
COINBASE_MATURITY+20 deep, not COINBASE_MATURITY.
I first changed the payout gate to that deeper value, on the theory that
each payout was firing 20 blocks before its own coinbase could be spent.
Measuring it refuted that: across 934 payments, 96% succeeded at depth
100, the moment they were queued, which is only possible because a payout
is funded from whatever coin is already mature rather than from its own
block. The gate stays at COINBASE_MATURITY, with a comment recording the
measurement so the next person does not re-make the same change.
The real shortfall is structural and not addressed here: the pool pays out
~99% of each block's subsidy while only matured coinbases are spendable,
so it runs with almost no buffer, and any orphaned block it queued a
payout for drains that buffer permanently. The 24h retry loop absorbs this
-- 0 payouts forfeited -- but the buffer is what needs fixing.
(cherry picked from commit e28df01f90d621b0cd818f28d70a20b8ab5d1a8d)
|
|
Upstream #30 split rendezvous registration from waiting for a dial, so
RvServiceRegister now returns as soon as the relay lists the service and
no longer consumes the relay's 0x01 pairing byte. It updated its own
caller in net.cpp and left this one behind.
AcceptOneFn went straight from RvServiceRegister to BtfServiceWrap, so
BtfServiceWrap read that 0x01 as the first byte of the miner's 32-byte
ephemeral key and every byte after it was shifted by one. The X25519
handshake has no confirmation step, so it completed against a garbage
shared secret, the tunnel was handed to the Stratum event loop, and
nothing failed loudly: the pool logged a miner arriving and then dropped
it as "never authorized", while the miner logged a pool that closed on it
with gotSubscribeAck=0. Loopback miners were unaffected, which is exactly
what made it look like a relay problem.
Block in RvServiceWaitPaired before wrapping, and close and re-register if
the relay drops us first.
test_rvaccept.cpp covers both shapes against a real bitflashd on loopback:
the pre-fix order establishes a tunnel that carries no bytes, the fixed
order round-trips. Also corrects the btftunnel.h contract comment, which
still described the pre-#30 behaviour and is what the old code was written
against -- upstream's own test_relayproc.cpp, test_btftunnel.cpp and
test_btfrv.cpp are still on that stale contract and fail the same way.
(cherry picked from commit 3e3a36b2bf1e6ffb4861507089b743688619377f)
|
|
SSL_CTX_new leaves verification at SSL_VERIFY_NONE and loads no trust
store, and nothing here ever changed that. SNI was set, a certificate was
received, and it was then ignored: every wss:// relay connection this node
has ever made would have completed against an attacker's certificate.
Signed events limit the damage -- a MITM cannot forge an announcement
attributable to our key -- but it can drop the rendezvous registration so
miners never find the pool, censor subscriptions, or answer them with
arbitrary content, which is the input the allocation bounds exist to
contain.
Load the system trust store, require SSL_VERIFY_PEER, and bind the check
to the host we dialed with SSL_set1_host, since chain validation alone
would accept any certificate a trusted CA ever issued for any name. Floor
the protocol at TLS 1.2. Verification failures report the specific X509
reason rather than a generic handshake error, and a missing CA store is a
hard failure instead of a silent downgrade to trusting everything.
(cherry picked from commit 6020740e68463deb1cc3c5b477372fcd1a245adb)
|
|
The min-2-threads floor applied even when the user explicitly asked
for 1 thread, so /hashthreads=1 silently spawned 2.
(cherry picked from commit a67b9e84f4f7607d307081b4a9873f5d9e78cf8a)
|
|
Descriptor (38501) still published so miners can resolve the pool;
only the public pool listing event (38503) is suppressed.
(cherry picked from commit 17bbdae1ec97945c37c1d5b2486e5c01a4d7ad87)
|
|
A pool that just found its first blocks has zero mature balance until
100 blocks later; orphaned coinbases (observed: reorgs orphaned our
blocks 2651-2655) make that worse. 30 one-minute retries forfeited
payments that would have succeeded hours later.
(cherry picked from commit 49e80023ca23baee8a4b80cfb891e7a358dc39ba)
|
|
A pool started with /rvrelay=ONE still spawned an accept thread for every
Nostr-discovered relay (BtfAllRelays always merged g_discoveredRelays), and
each thread optimistically overwrote the advertised meeting node before
registering -- so a discovered Tailscale-only relay (a mesh host)
flapped the published .btf descriptor and public miners could not connect.
fBtfRelayPinned (set only when /rvrelay parses) makes BtfAllRelays return
exactly the pinned list. Also validate the /rvrelay value at parse time:
with the pin in place no discovered relay can mask a typo.
(cherry picked from commit 0a4d2b02fa5382c5e788b5f0e2f8a844695dec47)
|
|
- RebuildJob: no jobs with zero peers or a tip older than 30min;
mining unsynced produced orphaned blocks and payouts that can
never be paid (observed in production: 8 orphan 'found blocks')
- payout exec: on SendMoney failure requeue with 60s backoff, up to
30 attempts, instead of silently forfeiting the payment
- nostr: add oxtr.dev, nos.social, fmt.wiz.biz, current.fyi, nostr.bg
(12 relays total) for announcement redundancy
(cherry picked from commit f1986cd7e4a6f69469a2256eb96ff03f3f146792)
|
|
auto-managed-tor into main
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
app icon)
|
|
BITFLASH_NODE_BUILD)
|
|
# Conflicts:
# src/makefile.mingw
|
|
|
|
|
|
|
|
|
|
|
|
The legacy parser only accepted two decimal places (cents) and rejected
0.5, leaving six of the eight coin decimals unreachable by -sendto and the
send window. Parse up to 8 fractional digits Bitcoin-style, reject >8 to
avoid silent truncation. Adds the parse-money selftest (12 cases).
|
|
The coin type 4346950 (0x425446 = BTF) was merged into the SLIP-0044
registry on 2026-08-07. Rename HD_BIP44_COIN_TYPE_BITFLASH_PROVISIONAL to
HD_BIP44_COIN_TYPE_BITFLASH and update comments, logs and UI strings.
Value is unchanged, so derivation paths and addresses are identical.
|
|
The headless loop installed SIGINT/SIGTERM handlers only under #ifndef _WIN32,
so a headless node on Windows never reached the clean StopNode()+DBFlush() path:
Ctrl-C, closing the console window, a service stop and OS shutdown all killed it
mid-write, and a Berkeley DB wallet.dat left unflushed will not reopen.
Install a SetConsoleCtrlHandler that mirrors the POSIX path: set fShutdown and,
because the OS terminates us shortly after the handler returns on
CLOSE/LOGOFF/SHUTDOWN, block until the main loop signals the flush is done.
A 'taskkill /F' is still an uncatchable hard kill by design.
Closes #16.
|
|
|
|
setint64() extracted big-endian bytes with a signed left shift (n <<= 8).
When the top byte is set that overflows int64 -- undefined behaviour, which
the script fuzzer flagged under UBSan (bignum.h:158). The result was correct
on two's-complement hardware, so this is a latent UB inherited from the
original 0.1 code, not a behaviour change. Do the shift on a uint64.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
* fuzz: add P2P message parser harness
* fuzz: avoid duplicate legacy header includes
* fuzz: seed the corpus with our network magic, not Bitcoin's
The two seed inputs began f5 be b6 d9, which is Bitcoin's message
start. Ours is bf 20 5c fd. CMessageHeader::IsValid rejects on that
field first, so both seeds died at the first check and the smoke run
exercised the reject path and nothing else.
libFuzzer would have to mutate its way onto four exact bytes before
reaching any parser, which does not happen in 128 runs. The job would
have gone green while covering none of what it was added to cover.
---------
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
Co-authored-by: Bitflash-sh <btf01@proton.me>
|
|
Two bugs in RebuildJob(), the operator's Stratum template builder. It
is a separate path from BitcoinMiner() and had drifted from it.
A single pass over mapTransactions is not enough. That map is keyed by
hash and carries no dependency order, so a transaction spending another
still-unconfirmed transaction's output -- change from a recent send,
usually -- fails ConnectInputs whenever its parent sits later in map
order. One pass drops it, and map order does not change between
rebuilds, so every later rebuild drops it at the same point. That is
deterministic exclusion rather than bad luck: where pool operators mine
most blocks, a transaction of that shape can stay unconfirmed for as
long as its parent is. Now it retries until a pass adds nothing, which
is what the solo builder has always done.
And the fees reached nobody. nFees was declared inside the scan loop, so
each transaction reset it and the running total was discarded, while the
coinbase line passed a hardcoded 0. An operator collected fees into a
block and paid none of them out. The block stayed valid, since paying
less than allowed always is, so nothing ever complained.
Both the diagnosis and the fix are Vic-Nas's, from his fork of this
code. He deleted the fork, so this brings the work back rather than
letting it go with it. His description of the failure was precise enough
that finding the same two defects here took one read of our RebuildJob.
|
|
Every fatal startup path wrote to stderr and to printf, and on Windows
neither reaches anybody. printf is remapped to OutputDebugStringF, so it
lands in debug.log inside the data directory. A -mwindows binary started
by double-clicking has no stderr at all, and AttachTerminal() cannot
help: it attaches to a parent console, which a double-clicked exe does
not have.
So Bitflash exited silently. Someone whose wallet.dat needs a newer
build -- exactly what the wallet format guard was added to tell them --
double-clicked and watched nothing happen. The guard was correct and
mute, which is the failure it was written to prevent.
Fatal startup errors now go through one reporter: debug.log as before,
stderr as before, and a message box when there is a person in front of
the screen. Applied to the three that mean "your data will not open":
the block index failing to load, the block index throwing, and
wallet.dat failing or throwing.
The box is deliberately not shown for a headless node. Those run under
scheduled tasks and services, where a modal dialog waits forever for a
click nobody is there to make -- a hung process instead of an exit code.
Headless keeps stderr and the log.
LoadWallet() now also reports why it refused, in a sentence, instead of
only logging it. The unsupported-format case is the one that has to be
understood by whoever reads it, and "LoadWallet failed" is not that.
Verified by breaking a throwaway wallet on purpose and starting both
ways. Headless: exit code 1, no dialog, no process left behind. Windowed
with no console: a box naming the file, the Berkeley DB error, the usual
cause, and where to look for more.
|
|
(#134)
* wallet: refuse future encrypted wallet formats
* wallet: add encrypted key unlock core
* wallet: add encryptwallet rewrite command
* wallet: read passphrases from stdin or files
* wallet: keep the derivation across encryption, and take the logs with it
Two things stopped the encryption stack from being safe to merge. Both
were found against a wallet holding real coin, not in the self-tests.
1. Encrypting turned a BIP44 wallet into a legacy one.
LoadWallet() ends by resetting the schema and the derivation counters
when no seed is in memory. That was written when "no seed after loading"
could only mean the wallet had none. An encrypted wallet has a seed --
it just has not been decrypted yet -- so the reset threw away metadata
that had been read correctly a moment earlier. Unlocking later restores
the seed and nothing restores the rest.
Measured on the wallet from the #124 proof: after encrypting, index 1
moved off BAtgSgra2X8Dqo5PJzPCLJhPwrrC2thYVJ, the address actually
holding 1.00 BTF, and the audit went from "1.00 covered by the phrase"
to "0.00 covered, 1.00 wallet.dat-only". Encrypt your wallet and the
program told you your recovery phrase no longer covered your money.
Now the condition also accepts a wallet whose seed is present but
encrypted. Same wallet, before and after: schema bip44, same four
derived addresses, 1.00 BTF still reported as covered.
2. The plaintext keys stayed behind in the Berkeley DB logs.
wallet.dat comes out clean -- verified by scanning the raw file for the
118 bytes of a known private key. database/log.0000000002 did not: it
held the whole key, and still did after a full node restart, because
every record this wallet ever wrote passed through the log.
That defeats the feature for the case it exists for, since our own
backup advice is to copy the whole directory rather than wallet.dat
alone. log_archive() cannot fix it: it only drops logs Berkeley DB
considers obsolete and never the one it is writing to, and with one 10
MB log per directory that is precisely where the keys are. They also did
not get there at encryption time -- they accumulated over the wallet's
whole life -- so nothing done during the rewrite can archive them out.
So the encrypt command now closes the environment and deletes the logs
on its way out, after the flush that makes every .dat self-contained.
This is not secure erasure; the blocks are unlinked, not overwritten.
It closes the case of someone reading a copied data directory.
Getting there uncovered a third bug, older than both.
DBFlush(true) is not safe to call twice. The first call closes the
environment, and txn_checkpoint() on a closed environment segfaults
instead of returning an error, so the try/catch around it never sees
anything. EncryptWallet() already flushes before renaming the wallet,
and the command flushes again on the way out -- that pair crashed every
single time. Guarded now.
The same function has been calling log_archive() with the address of an
uninitialised pointer since 0.1.0, on every clean shutdown, in a path
whose catch(...) cannot catch a structured fault on Windows.
DB_ARCH_REMOVE requires that argument to be NULL. Fixed while here,
because the encryption cleanup depends on that call not being a coin
flip.
---------
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
The right icon has been in the exe the whole time. GLFW never found it.
GLFW registers its own window class and fills hIcon with
LoadImageW(module, L"GLFW_ICON", ...), falling back to IDI_APPLICATION
when that exact resource name is missing. Ours was named "bitflash", so
the lookup failed and every window this program has opened wore the
generic Windows icon -- in the title bar and in the taskbar -- while the
correct artwork sat unused a few kilobytes away.
One line, verified by looking at it: rebuilt, ran the window, and zoomed
in on both places the user pointed at. The title bar and the taskbar
button now show the green BF coin. Checked the shell path too, since it
resolves icons differently: ExtractAssociatedIcon on the new exe still
returns the coin, so renaming the resource did not cost the file icon in
Explorer.
|
|
* Add BIP32 path derivation foundation
* Add HD wallet schema metadata
* Add provisional BIP44 coin type metadata
* Activate BIP44 wallet derivation
* wallet: scan BIP44 restore depth per branch
* wallet: widen default BIP44 restore lookahead
---------
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
CmdSendTo() has been in walletcmd.cpp since #115 and nothing declared
it or called it. The flag was dead: `bitflash -sendto=ADDRESS,AMOUNT`
started a node and said nothing, because an unrecognised argument is
not an error here.
The v1.2.14 notes announce it under "A headless node can finally
spend", and the headless Linux binary is exactly the one that does not
have it -- `-help` there lists /newaddress and stops. The published
Windows binary does have it, because it was built from a working tree
that carried these two files uncommitted. So the artifacts disagree
with each other and neither matches the tag.
This adds only what was missing: the declaration in walletcmd.h, the
dispatch in main(), and the -help line. No behaviour change to
CmdSendTo itself.
Checked on a build of this branch, against a throwaway data directory:
-help lists /sendto=ADDRESS,AMOUNT
-sendto=NOTANADDRESS,1 "Not a valid address", exit 1
-sendto=<valid>,-5 "Not a valid amount", exit 1
-sendto=<valid>,999 refused on an empty wallet, exit 1
All five self-tests still pass.
|
|
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
Creating a recovery phrase from the window aborted with an ImGui
assertion on the success path:
Expression: (g.DisabledStackSize > 0) && "Calling EndDisabled() too many times!"
File: imgui/imgui.cpp Line: 8435
The Finish button is wrapped in BeginDisabled/EndDisabled guarded by
!g_phraseWrittenDown, and its own click handler sets g_phraseWrittenDown
to false. So the pair was opened while the checkbox was ticked and
closed after the handler had unticked it: BeginDisabled was never
called, EndDisabled was, and ImGui asserted on the unmatched close.
It fires on the only path that installs a seed -- tick the box, press
Finish -- so every phrase created from the window has hit it since the
feature shipped in 1.2.12. Three releases carried it because nothing
tests the window.
Reproduced on the published 1.2.14 binary and confirmed fixed on this
build, driving the real window: Wallet Safety -> Create recovery phrase
-> tick -> Finish. The old build raises the assertion; the fixed build
closes the dialog, reports "This wallet has a recovery phrase", and the
address changes to a derived one.
The seed itself was already written before the assertion, so the wallet
was never left half-installed -- and ImGui's IM_ASSERT_USER_ERROR_RET
returns before touching its state, so answering Ignore was safe. Abort
was not: it kills the process with no DBFlush.
The restore dialog next door already does this correctly, copying the
condition into a local first. This makes the create dialog match.
|
|
Restoring from a phrase gave up after the first batch of a hundred addresses that turned up nothing, and the wallet digs a wider gap than that by itself: a restore leaves nHDNext at the depth it scanned, the key pool derives KEYPOOL_SIZE more, and change takes the index after those. Measured on a real wallet with real coin -- restore, spend once, and the outputs land at indices 201 and 302; restoring again found only the first transaction and reported the spend as gone. With -restoredepth=500 the same wallet found both, which is what said this was reach and not loss.
Three quiet batches instead of one: 2 transaction(s) across 601 derived addresses on the same wallet, chain and phrase.
Worse after #109, not before it: change used to return to an input's key, a low index the scan had already passed. Making change recoverable moved it out to where the scan stopped looking.
Does not fix the gap growing with every restore -- pulling nHDNext back to the last used index is the structural answer and needs its own testing.
|
|
#113 added three accept threads and called the extras harmless against an un-upgraded relay. They are not: an old relay closes the previous registration whenever the same node registers again, so the threads evict each other in a loop. Measured on the live network -- the node whose registrations landed on the un-upgraded New Jersey relay made 189 registrations in fifteen minutes with zero pairings, unreachable through it and hammering it, while the node on the upgraded relay made 9 with 7 pairings. One relay of three is upgraded, so this would hit two thirds of every node's registrations.
The relay half of #113 stays; it is compatible in both directions and is the prerequisite for raising this again, which the comment now states.
|
|
A registration was consumed by the first caller and registering again replaced the previous one, so a node was reachable to one peer at a time and unreachable for the gap between being paired and registering again. Measured on a live node: descriptors resolved 53/54, our own registrations paired 11/12, nothing paired and went silent -- and half of every outbound dial failed, 15 of 30 in eleven minutes.
The relay keeps a queue per pubkey capped at four, pairs with the oldest still-alive registration and discards the dead; the node runs three accept threads, each holding its own registration. The wire protocol does not change, so either side can be deployed first.
Bench with both relays built from source and waves of concurrent dialers: old+1 65%, new+1 58%, old+3 68%, new+3 98%. Each half alone lands within noise of doing nothing.
|
|
CheckBlock() validated block size against MAX_SIZE, the serializer's generic 32 MB guard rather than a block rule; MAX_BLOCK_SIZE did not exist in this tree at all, because 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.
Original work by mtg0001 in #110, rebased over #109 with the self-test conflict resolved, plus a comment explaining why the inherited count-against-bytes test stays.
Measured before choosing the number: 5709 blocks read from blk0001.dat give a largest block of 34,290 bytes and a median of 216, so 1 MB leaves 29x of headroom and rejects nothing that exists. A node built from this branch then synced genesis to 5706 on the live network with zero size-limit rejections and zero CheckBlock errors -- the same test that caught BIP30 rejecting a real block at height 859.
|