| Age | Commit message (Collapse) | Author |
|---|
|
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.
|
|
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)
|
|
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)
|
|
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)
|
|
- 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)
|
|
|
|
|
|
|
|
|
|
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.
|
|
A node was found holding 1502 sockets in Windows bound state, one ephemeral port
each, 9% of the machine range in nineteen hours. Reading the code did not find
it: every connect() failure path here closes its socket.
So count. Every socket is tagged at creation and untagged at close, across all
ten sites, with a counter for closes that fail. Already rules out three
explanations: nothing is created or closed behind the counter (untagged closes
are zero), no close is failing, and the leaked sockets are not in the live set --
tracked live matches what the OS reports as established, while the bound pile is
something the program already closed successfully.
|
|
The wallet made a key per address and per block mined, so a backup only ever
contained the keys that existed the moment it was taken -- mine one block and
the coin lands on a key the backup has never heard of. The README warned about
it, which is an admission rather than a fix.
Keys are now generated a hundred at a time and handed out one at a time, each
written to wallet.dat before it can be pointed at. A backup covers the next
hundred addresses. Deterministic derivation (#47) stays open; this buys back the
distance between a backup and the next mistake.
|
|
Three independent bounds on what an untrusted peer can make this node allocate:
- serialize.h: reject a compact size above MAX_SIZE before reading a string or a
vector, instead of discovering it read by read. MAX_SIZE moves from main.h to
serialize.h, where the checks live; headers_core.h includes serialize.h first,
so every existing user still sees it.
- main.cpp: cap an orphan transaction at 10,000 inputs. Orphans are stored and
never validated, so mapOrphanTransactionsByPrev grew with one entry per input
of every orphan a peer cared to send.
- rpc.cpp: drop a Stratum line nested deeper than 20 before json::parse recurses
into it. The scan tracks string state, so a worker name containing { or [ is
no longer mistaken for nesting.
Verified: builds clean on UCRT64; the depth scanner passes a table of legitimate
Stratum lines and nesting attacks; a node built from this branch synced the chain
from genesis to 4462 with no deserialization failure and no rejected block.
|
|
#59 changed BitcoinMiner and stopped there. RebuildJob in rpc.cpp builds
its own coinbase for the pool operator's job template, and that template is
mined and submitted like any other block, so it had the same hole: nothing
in the transaction belonged to the block, and a repeated combination of
difficulty, counter and key produces a byte-identical coinbase.
The claim in #59 that two coinbases at different heights can no longer be
identical only becomes true with this.
|
|
Everything an unauthenticated socket can reach in the pool server, in one
pass.
A line that never ends. RecvLine appended to the per-miner buffer and
looked for a newline; a peer that sends bytes and no newline grew that
buffer for as long as it kept writing, before authorizing or subscribing.
Capped at 64 KB, which drops the miner.
No limit on connections. Every accepted socket became a Miner with its
own buffer and nothing bounded the list. Capped at 1000.
Malformed requests crashed the node, not just the pool. ThreadRPCServer
has no exception handler, so anything thrown from line handling reached
the top of the thread and terminated the process. Three ways in, all
valid JSON that parses cleanly:
[1,2,3] -- value() on a non-object throws
{"method": 1} -- value() with the wrong type throws
submit with params [1,2,3] -- get<std::string>() on a number throws
mining.authorize had the subtler shape of the same thing: it tested
p.size() without testing p.is_array(), and indexing a json object with a
number throws while size() happily returns its key count.
Each is now rejected on its own terms, and the call is wrapped so a throw
drops that miner instead of the node.
|
|
It reads:
Save BEFORE moving to exec queue: if we crash between the save and
SendMoney, LoadPendingPayouts re-queues them from the exec-queue
section of the file.
There is no exec-queue section. SavePendingPayouts serialises
gPendingPayouts and nothing else, and a matured payout is erased from it
and written out before ever reaching the queue, which lives only in
memory. Crash in between and those miners are never paid, with no record
they were owed anything.
Only the comment changes here. The behaviour needs idempotent payment --
record the txid, look for it in the wallet on restart -- because simply
persisting the queue and re-running it would double-pay anyone already
sent to, which is the worse failure.
|
|
* script: fix OP_RETURN spend bypass, disable unbounded opcodes
OP_RETURN let anyone spend anyone else's outputs.
VerifySignature evaluates scriptSig and scriptPubKey as one concatenated
script. OP_RETURN was implemented as 'pc = pend', which jumps past
everything that follows, and EvalScript ends with
CastToBool(stack.back()). So a scriptSig of <OP_TRUE OP_RETURN> skipped
the entire scriptPubKey -- OP_CHECKSIG included -- and the final check
read the attacker's own OP_TRUE. No signature required, any output
spendable. This is the 0.1.0 behaviour Bitcoin fixed in 0.3.5.
OP_RETURN now fails the script outright.
Also disables the opcodes Bitcoin removed for being unbounded, and caps
pushed elements at 520 bytes:
OP_CAT grows a stack element with no limit -- <x> OP_DUP OP_CAT
repeated doubles it each time, so a script that fits in a transaction
exhausts memory on every node that validates it. OP_LSHIFT shifts by
an attacker-supplied amount with no ceiling, same effect. There were
no element, stack or opcode-count limits of any kind.
Nothing on this chain uses these opcodes -- all outputs are standard
pay-to-pubkey-hash or coinbase pay-to-pubkey -- so no existing
transaction changes validity.
* main: reject blocks containing a duplicate transaction (CVE-2012-2459)
BuildMerkleTree duplicates the trailing hash whenever a tree level has
an odd width. Appending a copy of the last transaction therefore
produces a block whose merkle root -- and whose block hash -- match a
legitimate block exactly.
A node handed the forgery rejects it (the duplicated inputs are already
spent) and remembers that hash as bad. When the genuine block arrives
carrying the same hash, it is refused too, and the node stops following
the real chain.
CheckBlock now rejects any block that carries the same transaction
twice, which is what the mutation requires.
* main: halve the subsidy on the block's height, not the node's tip
GetBlockValue derived the halving from nBestHeight -- the validating
node's own chain tip -- rather than from the height of the block being
valued. The subsidy a block may pay is a property of that block, so
reading it off local state makes validity depend on how far the node has
synced: across a halving boundary a node still catching up would allow
50 BTF where a synced node allows 25, and the two would follow different
chains. Reorgs have the same problem in miniature.
Harmless today (every node computes nHeight/210000 == 0 at height ~2400)
and it changes no existing block's validity, but it splits the network
at height 210000.
The height is now passed in explicitly: pindex->nHeight when connecting,
tip + 1 when mining or building pool work.
|
|
* Add Linux GUI installer via Wine
* Fix launcher creation and add BTF_RELAY env var support
* Pool server, Linux GUI, unified binary, README update
* Root Makefile: make linux / make windows, cleaner README
* Remove headless.cpp
* Fix XPM color count mismatch causing toolbar image errors on Linux
* XPM icons: same code on Windows and Linux, no ifdefs around image loading
* Fix GTK icon crash and Linux printf format
* btfrv: 10s socket timeout on relay connects
* Faster peer discovery: more dials per pass, shorter backoff and sleep
* Parallel btf peer connections
* Fix GetMyExternalIP - was disabled, now uses modern IP echo services
* Fix tunnel disconnect on idle and restore external IP detection
* Filter stale Nostr descriptors
* Sort peers by freshness, fresh first
* Pool: add .btf Stratum listener — miners connect via rendezvous, no port forwarding needed
* btf pool: self-replacing listeners, one per relay, no queuing between miners
* Mining modes: solo/operator/participant — pool server on-demand, participant Stratum client
* Status bar: show external IP and .btf address
* README: clean rewrite, concept-first, minimal
* make linux produces AppImage
* make linux produces AppImage
* Pool dashboard: connect instructions, tabs, share bars
* Perf fixes: GetMyExternalIP threaded, btf listener counted
* Perf fixes: GetMyExternalIP threaded, btf listener counted
* Fix pool payouts: defer until coinbase maturity
* Pool: clean mode switching, IP warning
* Pool: clean mode switching
* Fix gPoolRunning: remove static
* AppImage: use real icon from bitcoin.ico
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Windows build: use mingw32-make
* Windows build: simpler make invocation
* Add deps-windows target
* Windows build fixes
* Windows build fixes
* Windows makefile: fix obj dir
* Windows: output to repo root
* Windows: bundle DLLs into zip
* Replace wxWidgets with ImGui+GLFW: same build on Linux and Windows
* Fix %I64d that slipped back through rebase (main.h, net.h)
* MinGW: add _mkdir shim for non-MSVC Windows; fix %I64d
* Fix gPoolRunning extern outside ifndef _WIN32
* Remove -lboost_system: boost is header-only in this codebase
* Status bar: copy buttons for IP and .btf address
* Windows: add app icon via windres, strip wx stuff from rc file
* Clean GUI: single Options menu item, bigger font, better spacing, cleaner options dialog; update README
* GUI: embed Roboto font at 16px for clear readability on all platforms
* Windows: windres depends on obj dir target
* Fix balance hidden off-screen: put on own line below address bar
* Windows: inline mkdir in every rule, remove dir target that confused make
* Windows: move all target to top so it's the default rule
* Fix pool server not starting: always start when operator selected, re-sync dialog state on open
* Write debug.log to data directory (AppData/Roaming/Bitflash on Windows)
* Participant miner: use wallet address as username so payouts go to wallet
* Add .gitattributes to lock LF line endings, prevent CRLF drift
* Refactor pool mining to btf-only transport
* Persist pool settings and startup wiring
* Prune stale peers and expose pool queries
* Fix build regressions and add CLI mode/arg parity
* Fix rpc miner stats declaration order
* Centralize pool runtime extern declarations
* Separate operator/participant dashboards and restore peer stats
* Add live telemetry strip and clearer per-mode runtime metrics
* Reduce mining dashboard clutter and remove manual pool entry
* Count only authorized pool miners
* Seed initial pool job on startup
* Use a real share target for pool work
* Switch pool accounting to round-based shares
* Fix nostr pool announcer stats call sites
* Fix pool stats definition order
* Improve pool worker status display
* Add mutex include for participant status
* Replace ImClamp in dashboard layout
* Clarify participant work status
* Log participant status transitions
* Reduce peer discovery log noise
* Add participant status badge
* Track participant auth acknowledgements
* Log and validate pool job delivery
* Stop persisting mining mode/pool settings across restarts
LoadWallet() runs after CLI flags are parsed in main_gui.cpp, and was
restoring nMineMode/strParticipantPool/strPoolName/strPoolDashboardUrl/
dPoolFeePercent/fGenerateBitcoins from wallet.dat every time, silently
overwriting whatever was just set on the command line or in the current
Options session -- no error, no log line. This is why pool name/address
could show correctly in the UI but stay stale in the logs (UI held the
in-memory value just set; logs eventually reflected whatever LoadWallet
clobbered it back to), and why mode changed behavior across restarts.
Mode/pool config is now decided fresh every launch: CLI flags if given,
otherwise the compiled-in default. Nothing is written back to wallet.dat
for these settings anymore, so there's nothing left to go stale or fight
over load order.
* Add explicit Relay mode as the new startup default
4th mining mode: node runs/syncs, never mines, no pool server -- an
explicit option instead of an implicit side-effect of 'Solo + never
click Start Mining'. Now the compiled-in default (was Solo).
- main.h: MINE_RELAY = 0, renumbered other modes (nothing compares
nMineMode against raw integers elsewhere, verified).
- main.cpp: default nMineMode = MINE_RELAY; BitcoinMiner() early-returns
for Relay, mirroring the existing Participant-mode guard.
- gui.cpp: added Relay radio button, status label, and telemetry-strip
text. Start/Stop Mining button is hidden for Relay (meaningless -- it
never mines by definition); OK forces fGenerateBitcoins off if you
switch into Relay while previously mining.
* Announce pool changes immediately instead of waiting up to 60s
ThreadBtfPoolAnnouncer re-read strPoolName/dPoolFeePercent/etc live, but
only once per iteration of its 60s sleep loop, so a pool name/fee change
in Options could take up to a minute to reach other nodes.
- nostr.h/.cpp: add gAnnounceNow flag; the announcer's sleep loop now
wakes early and publishes immediately when it's set, then clears it.
- gui.cpp: Options OK handler sets gAnnounceNow when operator pool
settings actually changed (name/dashboard/fee/mode), so a saved
change is announced right away instead of on the next tick.
* Give each pool miner a distinct extranonce2 so jobs no longer overlap
mining.notify previously broadcast one identical 80-byte header (baked
from a single server-side extraNonce) to every connected miner, so any
two miners on the same job searched the exact same nNonce space instead
of disjoint ranges.
Each Miner connection is now assigned a unique extranonce2 on connect.
BuildMinerBlock() takes the shared job template and appends that miner's
extranonce2 to the coinbase scriptSig, recomputing the merkle root, so
every miner gets its own header variant of the same job. mining.submit
validates and, on a full block, assembles the block using that same
miner-specific variant (via m->extranonce2), not the shared template.
No client-side protocol change needed: main.cpp's participant miner
already just takes whatever header bytes mining.notify hands it and
submits (jobId, nonce) back, so it works unmodified against per-miner
headers.
Still simplified vs standard Stratum (no coinb1/coinb2/merkle-branch
wire split, no mining.extranonce.subscribe reply payload) since this
protocol was already a custom simplification pre-existing this fix.
Syntax-checked with g++ -fsyntax-only (full link needs secp256k1/
RandomX/GLFW not present in this sandbox).
* Fix debug.log never being written on the real Windows build
printf-to-debug.log routing was gated on:
#if defined(_WINDOWS) || !defined(_WIN32)
_WINDOWS is only ever defined by the legacy, unused MSVC makefile.vc.
The actual Windows build (makefile.mingw) never defines it, and _WIN32
IS defined there (mingw builtin), so on real Windows this condition was
always false || false = false. printf() fell through to plain libc
printf (stdout only), so debug.log was never created, and output was
only visible if the process happened to be run attached to a console.
OutputDebugStringF() was already platform-safe internally (always
writes debug.log; only echoes to console when not on Windows), so the
outer condition was pure dead weight. Made the routing unconditional.
* Reduce base UI font size one more step (16px -> 14px)
* Remove Start/Stop Mining button; mining state follows selected mode
The button let a user configure a pool then click a separate toggle,
creating states where the button's label/action didn't match what the
selected mode actually needed. OK now starts/stops the miner thread
itself based on nMineMode: Relay clears fGenerateBitcoins, the other
three modes set it and spawn ThreadBitcoinMiner if it isn't already
running (guarded by vfThreadRunning[3]).
* Fix participant miner stuck waiting for notify after buffered multi-line recv
Root cause of the operator/participant pairing successfully but no work
ever flowing: PoolParticipantMiner()'s receive loop only pulled one line
out of the socket per select() wakeup. The server sends the authorize
reply, mining.set_difficulty, and mining.notify back-to-back on the same
connection, and these often land in a single TCP segment — one recv()
inside StratumRecvLine() buffers all three lines into 'buf', but only the
first was extracted and processed. The remaining lines (frequently
mining.notify itself) sat in 'buf' indefinitely, because select() only
reports readability of the OS socket, not whether our own buffer still
holds unprocessed complete lines — and the server had nothing further to
send once the initial burst was over, so select() never fired again.
Fix: drain any complete line already sitting in 'buf' first; only fall
back to select()+recv() when 'buf' has no complete line left.
* Fix mining.submit rejecting every share due to wrong param-count check
Server required params.size() >= 4 for mining.submit, but the client
only ever sends 3: [username, jobId, nonceHex] (indices 0-2, matching
what the handler actually reads: p[1]=jobId, p[2]=nonceHex). Every
submission was bounced with 'bad params' before its nonce was ever
checked against the job, producing an endless share-sent/share-rejected
loop with no other visible cause. Likely a leftover from the standard
5-field Stratum mining.submit shape, which this simplified 3-field
protocol never matched. Threshold corrected to size() >= 3.
* Fix mining.submit rejecting legitimate shares that weren't also winning blocks
Pool share difficulty is intentionally ~256x easier than full network
difficulty (MakeShareTarget() shifts the block target left 8 bits), so
miners are expected to submit many shares per actual block found. But
the handler replied to the miner based on ProcessBlock()/CheckBlock()
success — validation against the much harder network target — rather
than on whether the share met pool difficulty. Every legitimate share
that wasn't also a winning block (the vast majority, by design) failed
CheckBlock()'s nBits check and got reported to the miner as rejected.
Now: reply(true) as soon as a submission clears pool-share difficulty.
Only if it ALSO clears full network difficulty (job.target, not
job.shareTarget) does it get submitted as a block — with no second
reply sent, since the share was already acknowledged.
* Fix dead-peer dial priority: freshness sort was undone by full shuffle
ConnectDiscoveredBtfPeers sorted candidates newest-first, then immediately
shuffled the whole list, making the freshness sort a no-op — every pass was
fully random. Meanwhile g_btfPeerFails was tracked but never used to
prioritize, so a peer that had failed 20 times in a row got equal footing
with an untried or previously-successful peer; the only effect was backoff,
capped at 15 minutes, so dead descriptors cycled back into the dial pool
constantly.
- Track successful connections (g_btfPeerEverConnected) and use it, plus
fail history, to order candidates in tiers: known-good, then untried,
then known-bad (worst offenders last). Freshness order is preserved
within each tier instead of being discarded.
- Raise the backoff cap for chronic failures (6+ in a row) from 15 minutes
to 6 hours, so descriptors that are clearly dead stop consuming dial
budget every pass while still getting retried eventually.
* Batch-resolve .btf descriptors to stop hammering public relays during dial passes
Root cause of 'stuck at 1 peer' despite good peer discovery: each parallel
dial thread in ConnectDiscoveredBtfPeers called ConnectNodeBtf, which itself
walked all configured relays (relay.damus.io, nos.lol, relay.nostr.band,
relay.snort.social) to resolve that one peer's descriptor. With up to
BTF_DIALS_PER_PASS (12) threads dialing at once, a single pass could fire up
to 12 * nNostrRelays near-simultaneous fresh connections at the same handful
of public relays -- easily enough to trip rate limits or get handshakes
refused (exactly what the logs showed: 'handshake refused by relay.damus.io',
repeated connect failures to relay.nostr.band / relay.snort.social). Peers
that were genuinely reachable looked dead simply because their descriptor
couldn't be fetched under that load, and then got penalized as known-bad by
the dial-priority tiering from the previous fix.
- Add BtfResolveMany: resolves every still-unresolved candidate's descriptor
in a single Nostr REQ per relay (multiple authors in one filter), instead
of one relay connection per candidate. A 12-candidate pass now costs at
most a few relay round-trips total instead of dozens.
- Add ConnectNodeBtfResolved / ConnectNodeBtfTail (net.cpp): given an
already-resolved descriptor, dial straight to the peer's rendezvous node
with zero further relay contact. ConnectDiscoveredBtfPeers now batch-
resolves all candidates up front, then fans out tunnel-only connect
threads in parallel -- no thundering herd against the relays.
- Candidates that fail to resolve are recorded as a failure immediately
(same backoff/fail-count bookkeeping as before) instead of spawning a
thread with nothing to dial.
* Stop reconnecting to relays constantly: pool one persistent connection per relay
Real root cause of the ongoing 'handshake refused' / 'connect failed' spam
and peer count stuck at 0-1: every relay-touching operation (startup self-
test, per-pass seeding, batch descriptor resolve, pool announce, pool query)
opened its own brand-new CWebSocket every single time it ran. Two independent
threads (ThreadNostrSeed and ThreadBtfPoolAnnouncer) each reconnect from
scratch on their own ~60s cycle, so a node could fire well over a dozen
fresh TCP+TLS+WS handshakes at the same 4 public relays inside any given
minute. Public relays -- relay.damus.io especially -- rate-limit new
connections per source IP fairly aggressively, so this node was effectively
DoS-ing itself: peers were genuinely reachable, but their descriptors
couldn't be fetched because the relay had already started refusing this
node's own repeated reconnects.
- Add a small per-relay connection pool (g_relayPool) and an RAII
CRelayLease that reuses a relay's live socket across calls instead of
reconnecting every time, only reconnecting when a socket actually drops
or was never established.
- Route every relay call site through it: the startup self-test, the
self-test loop, SeedFromRelay, BtfResolve, BtfResolveMany,
BtfPublishPoolAnnouncement, BtfQueryPoolAnnouncement.
- Only log 'connected to <relay>' on an actual fresh (re)connect, not on
every pass that's just reusing an already-live socket, so the log
reflects what's really happening instead of implying constant churn.
This cuts fresh relay connections from several per minute down to
essentially one per relay for the life of the process (until a relay
actually drops the connection), which should make the rate-limit-driven
refusals go away and let descriptor resolution -- and therefore dialing --
actually succeed at the rate the peer list implies it should.
* Fix wallet crash on rejected send; remove legacy IP-based peer discovery
- SendMoney() no longer throws when a committed tx is rejected by
AcceptTransaction() (e.g. raced input already spent). It now unwinds
the wallet state via RollbackTransactionSpent() and returns false,
which the GUI already handles as 'Transaction failed.'
- Removed the dead IP-based peer discovery/connect path now that
discovery is entirely Nostr/.btf: CAddrDB/addr.dat, mapAddresses,
AddAddress, ConnectNode(CAddress), FindNode(CAddress), ConnectSocket
(SOCKS4 proxy helper), addrProxy, ThreadOpenConnections/2 class-C
dialer and its thread spawn, the addr/getaddr P2P handlers, and the
dead vAddrToSend/setAddrKnown fields on CNode.
- Deleted irc.cpp/irc.h (already-disabled legacy seed code) and
scrubbed references from headers_core.h and all four makefiles.
* Raise peer connection target and add dial diversity; fix IPv4-only relay connect
- BTF_TARGET_CONN: 8 -> 24. With ~100 known .btf peers, average degree
needs only to exceed ln(N)~=4.6 for the network to stay connected
with overwhelming probability (Erdos-Renyi); 24 is a healthy margin.
- ConnectDiscoveredBtfPeers now splits each pass's dial budget ~70/30
between the existing reliability-tiered order and a random sample
from the full fresh peer pool. Pure reliability bias means every
node converges on the same 'known good' hub set, which correlates
peer sets across the network -- a relay outage then drops the same
edges everywhere at once instead of independent ones. The random
slice keeps a floor of uncorrelated edges.
- CWebSocket::Connect (relay websocket) now resolves with AF_UNSPEC
and tries every returned address (IPv6 or IPv4) in turn instead of
hard-failing on AF_INET-only resolution, which was one plausible
cause of the relay.nostr.band:443 connect failures (CDN-fronted
relays increasingly rate-limit/filter the v4 path).
* Remove accidentally committed build binary; ignore src/bitflash
* Fix pool participants getting stuck after the first job
JobWatcherFn only rebuilt/broadcast a new Stratum job when a new block
arrived on this node or no job existed yet. Once a job was built after
startup, gHaveJob stayed true forever unless *this* node found the
block, so with no new block (slow interval, or this node just not
seeing one) every connected miner sat hashing the exact same job --
stale timestamp, stale mempool snapshot -- indefinitely. From the
miner's side that's indistinguishable from being stuck: it worked at
connect (gets the one-time job sent on authorize) and then nothing
ever arrives again.
Added a 60s periodic refresh so the job (and therefore mining.notify)
gets rebroadcast on a timer too, same as a normal stratum pool, not
only on new-block/no-job events.
* Diversify Nostr discovery relay list
Went from 4 relays to 7, and reduced Cloudflare concentration: previously
2 of 4 (damus.io, snort.social) sat behind the same CDN, so a Cloudflare-
side issue could take out half the list even though they're nominally
independent relays. Kept damus.io (most established) as the one CF-fronted
entry and filled the rest with different hosts (nos.lol, nostr.band on
Hetzner, nostr21.com, offchain.pub, nostr.mom, primal.net) so peer
discovery doesn't have a single shared upstream as a point of failure.
7 relays is within the commonly recommended 5-10 range for redundancy.
* Fix data race in RebuildJob: chain-state reads weren't holding cs_main
pindexBest, nBestHeight, GetNextWorkRequired() and GetMedianTimePast()
were all read without cs_main. A block being accepted concurrently by
the P2P thread could interleave with this: pindexPrev captured from the
old best block, but nBestHeight already bumped to the new one by the
time job.height is computed, or pindexBest reassigned mid-read. That
mismatch would make QueuePayouts() schedule a payout for the wrong
maturity height for whatever block gets found against that job.
Wrapped the whole chain-state snapshot in one CRITICAL_BLOCK(cs_main),
consistent with how the rest of the codebase touches this state.
* add operator fee
* Add categorized, burst-collapsing logging for pool/worker/payout flow
debug.log was growing unboundedly because every printf() call wrote
unconditionally, so loops that log once per item (discovered peers,
connect attempts) produced a near-identical line per address with no
cap. Add LogPrint(category, ...): a drop-in printf replacement that
tags each call with a subsystem (pool/worker/payout/net/nostr) and
collapses repeated calls from the same call site into one line per
3s window plus a '(xN similar in 3s)' summary, so log growth is
bounded by elapsed time instead of event count.
Wire it through the whole pool -> work -> payout pipeline (rpc.cpp
operator side, main.cpp participant/worker side, net.cpp/nostr.cpp
peer & relay discovery), and along the way fix several previously
*silent* failure points that produced exactly the reported symptom
(worker stuck asking for work, nothing logged):
- Relay listener registration failing: no log at all, so a pool with
every relay down looked identical to a healthy idle pool.
- No relays configured: silent infinite 5s retry loop.
- Initial job seed failing at pool startup: silent.
- JobWatcher: added a stuck-job watchdog that, after 15s with no
job available, logs a specific diagnosis (no chain tip yet vs.
RebuildJob() failing) instead of retrying forever in silence.
- QueuePayouts(): a block found with zero recorded shares queued no
payout and logged nothing.
- FlushMaturePayouts(): an unparseable payout address was skipped
with no log.
- Participant miner: the 'waiting for pool notify' stall was only
ever shown as a transient GUI status string, never written to
debug.log. Now logs on a 15s cadence while stalled, and after 120s
proactively drops the tunnel so the existing reconnect-in-10s loop
gets a fresh attempt instead of hanging indefinitely.
- Participant miner: share rejections were only a transient GUI
status string; now logged with the pool's rejection reason.
All edited files verified with g++ -fsyntax-only (gnu++14).
* Fix two bugs causing worker stuck + log bloat
1. Worker stuck asking for work (main.cpp):
The participant miner checked msg["result"].is_boolean() to detect
the mining.subscribe acknowledgement (id==1), but the pool sends
an array ([[["mining.notify",sid],sid,4]]) as per Stratum spec,
not a boolean. is_boolean() always returned false, so gotSubscribeAck
was never set true. This didn't directly block authorize (id==2 path
is independent), but it did mean the status never advanced through
"subscribed", leaving the progress logging misleading. More
importantly, the same wrong assumption meant any pool that also
gated job dispatch on a prior subscribe confirmation would leave the
worker stuck forever with no diagnosis.
Fix: accept any non-null result for id==1 (the array is the
session/extranonce info; its contents don't matter for our use case).
2. Log bloat from ProcessMessages (main.cpp):
ProcessMessages() used a bare printf() which fired unconditionally
for every P2P message received -- the bulk of debug.log was this one
line repeating thousands of times. Convert to LogPrint("net", ...)
so it is gated behind fDebug (off by default) and burst-collapsed
by the dedup introduced in the previous commit.
* Fix log bloat, em-dashes, dead code, status bar, Connected/Detected peers
Log bloat:
- Gate all per-message/per-block/per-loop printf in main.cpp, net.h,
net.cpp behind LogAcceptsCategory("net") -- only visible with -debug.
This includes: received:/sending: hex dumps, got inventory, askfor,
getblocks, getdata, received block, AddToWallet, RelayWalletTransactions,
ProcessMessage FAILED, mining loop N-transactions line, retarget block,
socket recv/send errors. AddToBlockIndex new best and ProcessBlock
ACCEPTED intentionally left always-on (one line per block).
- PoolParticipantMiner: status -> now LogPrint(worker) not printf.
Em-dashes:
- Replace all UTF-8 em-dashes (0xE2 0x80 0x94) with ASCII '--' in
every source file (rpc.cpp, gui.cpp, main.cpp, main_gui.cpp, net.cpp,
nostr.cpp, util.cpp, btfrv.cpp, db.cpp, db.h, main.h, headers.h,
headers_core.h, util.h). 40 occurrences across 14 files.
Dead code:
- Remove unused gJobSeq atomic from rpc.cpp (written, never read).
- Fix minerReward: was computed but payout loop used blockReward directly
(operator fee was advertised but never deducted). Now payout correctly
uses minerReward = blockReward * (1 - feeFrac).
Status bar / peers:
- Add GetDiscoveredPeerCount() to nostr.cpp/nostr.h (reads g_btfPeers
size under cs_btfPeers).
- Replace scrollable inline status text with DrawStatusBar(): a separate
ImGui overlay window pinned to bottom of screen (NoScrollbar,
NoBringToDisplayFront). Labels changed from 'Peers N' to
'Connected peers: N Detected peers: N'.
* Rewrite gui.cpp: proper fixed status bar, scrollable content child
Layout architecture:
- DrawMainWindow() covers the full display width, height minus STATUS_BAR_H
(22px). NoScrollbar + NoScrollWithMouse on the outer window so the
chrome (menu, address/balance strip, telemetry strip, separators) never
scrolls.
- A BeginChild("##scroll") fills the remaining height after the fixed
chrome, sized to GetContentRegionAvail().y -- which is already the space
between the cursor and the bottom of the outer window. All mode-specific
content (transaction table, participant pool list, operator workers/payouts)
lives inside this child. Content that overflows gets a scrollbar; nothing
bleeds past the outer window edge.
- DrawStatusBar() is a separate overlay window pinned to the bottom 22px,
drawn last so it is always on top. NoBringToDisplayFront is NOT used here
-- drawing last is sufficient and avoids interaction issues with dialogs.
- Status bar background is fully opaque (not alpha-blended) so it never
shows content behind it.
Labels and content:
- 'Peers N' replaced with 'Connected peers: N Detected peers: N'
using GetDiscoveredPeerCount().
- Mining indicator: '* Mode' instead of the 'filled circle' Unicode glyph
that renders as boxes on some Linux font configs.
- FmtAge used consistently; 'blocks deep' shortened to 'N deep' in tx list.
- Options dialog: em-dashes removed from RadioButton labels (ASCII '--').
- Removed usedY dead variable (GetCursorPosY() result was unused).
* Fix blocking bugs, dead code, pool/participant flow correctness
Bugs fixed:
StratumRecvLine blocking (main.cpp):
The participant miner's recv loop called recv() with no timeout after
select() said the socket was readable. If the server sent a partial
line (split across two TCP segments), the second recv() would block
indefinitely waiting for the rest -- the classic 'stuck asking for
work' hang even though the pool was alive and had sent data.
Fix: single non-blocking recv() pass (MSG_DONTWAIT on Linux,
FIONREAD+recv on Windows). If no complete line is in the buffer yet,
return false; the outer select() loop retries when more data arrives.
BroadcastJob holding mutex during send() (rpc.cpp):
BroadcastJob() held gMinersMutex across all SendLine() calls.
send() can block if a slow miner's TCP receive buffer is full.
With the mutex held, MinerThreadFn could not acquire it to remove a
disconnecting miner, causing a deadlock-adjacent stall: broadcast
blocked on slow miner, disconnect blocked on broadcast, new jobs
not delivered. Fix: snapshot {fd, authorised, address, extranonce2}
under the mutex, release it, then send to each fd outside the lock.
Dead code removed (rpc.cpp):
- gRpcUser, gRpcPassword: defined, never read or set by anything.
- dHashesPerSec, nHPSTimerStart: leftover Bitcoin 0.1 stubs, unused.
- IsInitialBlockDownload(): stub that returned pindexBest==NULL,
never called anywhere in this codebase.
- struct Share: declared with three fields, never instantiated.
- Duplicate comment block before payout section.
Other cleanup (main.cpp):
- STUCK_NO_JOB_TIMEOUT_SECS moved from inside the loop body to file
scope (was 'static const' inside a loop -- correct but confusing).
- pblock->print() in solo miner gated behind LogAcceptsCategory("net")
-- was unconditionally dumping a full block to stdout on every solve.
- ERROR in BitcoinMiner printf converted to LogPrint("net").
* Pool/participant: fix recv blocking, BroadcastJob mutex, listener spawn, escaped em-dash
main.cpp -- StratumRecvLine rewrite:
The previous version called recv() in a while loop -- if select() said
the socket was readable but only a partial line had arrived, the second
recv() call would block indefinitely waiting for the rest of the line.
This was the root cause of the recurring stuck-worker bug.
Rewrite: single non-blocking recv (MSG_DONTWAIT on Linux, FIONREAD+recv
on Windows). If no complete newline is in the buffer yet, return false
immediately; the outer 50ms select() loop retries when more data arrives.
This means the hash loop always stays responsive regardless of network
fragmentation.
rpc.cpp -- BroadcastJob mutex held across send():
BroadcastJob held gMinersMutex for the entire duration of sending jobs
to all connected miners. send() can block if a slow miner's TCP buffer
is full. With the mutex held, MinerThreadFn could not acquire it to
clean up a disconnecting miner, stalling the whole broadcast.
Fix: snapshot {fd, authorised, address, extranonce2} under the mutex,
release it, then send to each fd without holding any lock.
rpc.cpp -- LaunchThread failure not handled in BtfStratumAcceptOneFn:
The replacement listener thread was launched via LaunchThread() which
wraps _beginthread() but discarded the return value. If _beginthread
fails (thread limit, OOM), the listener count stayed at 1 with no thread
behind it -- that relay would get no new listener until the 30s outer
loop in BtfStratumAcceptFn fired. Fix: call _beginthread() directly,
check the return value, decrement and log on failure.
rpc.cpp -- escaped em-dash in log string:
BtfStratumAcceptFn had a literal \xe2\x80\x94 in a LogPrint format
string that survived the em-dash sweep (it was source-encoded, not a
UTF-8 byte in the file). Replaced with '--'.
* Rewrite pool server: single event loop, no per-miner threads
Replace the old architecture (one thread per connected miner + separate
JobWatcher thread + BtfStratumAcceptFn thread with listener count tracking)
with a clean two-layer design:
Accept layer (one thread per relay, AcceptOneFn):
Each relay gets exactly one persistent thread. It loops: calls blocking
RvServiceRegister (waits for a miner to knock), then BtfServiceWrap
(echan handshake), then pushes the finished plain socket to a small
lock-protected queue, then loops immediately to accept the next miner.
No listener count tracking. No replacement thread spawning. No races
around spawn failure. The relay always has exactly one pending accept.
Event loop (PoolEventLoop, runs on the ThreadRPCServer thread):
Single thread owns all connected miner sockets. Each tick:
1. Drain the accept queue -- pick up any newly-connected miners.
2. Check for a new block -- rebuild job and broadcast if so.
3. No-job watchdog -- log and retry if job is missing too long.
4. Periodic job refresh -- restamp nTime every 60s.
5. select() over all miner fds with 50ms timeout.
6. For each readable fd: non-blocking RecvLine, drain all complete
lines, dispatch each to HandleLine, collect dead fds.
7. Refresh GUI stats snapshot every 2s.
Because the event loop is the only writer to gMiners, no mutex is needed
on the miner list. The only mutexes remaining are:
- gStatsMutex: guards the GUI-visible snapshot (StatsSnapshot,
gWorkerStats, gPayoutViews) written by the event loop and read by
GUI threads via GetPoolOperatorStats/GetPoolWorkerStats/GetPendingPayouts.
- gReadyMutex: guards the accept queue between AcceptOneFn threads
and the event loop.
Other improvements in this rewrite:
- gPendingPayouts is event-loop-only (no mutex needed).
- Round share tracking lives in the event loop (roundShareCount map +
roundShareTotal counter) -- zeroed on block found, no per-miner stat
mutex needed mid-round.
- QueuePayouts takes the share snapshot as a parameter instead of
reaching into gMinerStats under a separate mutex.
- BroadcastJob no longer snapshots miners to avoid a mutex -- it runs
directly in the event loop with exclusive access to gMiners.
- AcceptOneFn backs off 30s on relay registration failure instead of
returning -- one relay going down doesn't kill the accept thread.
- No goto. No per-thread dynamic allocation for listener contexts on
each accepted connection.
* Thorough logging and correctness fixes across the full pool/participant flow
Every failure point in the operator->miner->payout chain now logs exactly
what broke, with enough context to diagnose without a debugger.
Operator side (rpc.cpp):
RebuildJob: logs 'no chain tip (pindexBest is null)' on failure instead
of silently returning false.
HandleLine:
- JSON parse errors logged with the raw line (first 80 chars)
- subscribe: logs fd and session id
- authorize: logs address, worker, and whether the address is valid
- submit: logs stale-job case with submitted vs current job id;
logs high-hash with actual hash and target values;
logs valid shares with nonce and running session count;
logs BLOCK CANDIDATE separately from BLOCK ACCEPTED/REJECTED so
a ProcessBlock failure is unambiguous
- unknown method logged (reveals miner software quirks)
- reply() logs send failures with method name
QueuePayouts: logs total payout amount, miner count, fee applied, and
how many miners were below dust threshold.
FlushMaturePayouts: logs 'insufficient funds or wallet locked?' on
SendMoney failure instead of just 'FAILED'.
BroadcastJob: logs job id and miner count on every broadcast.
Event loop: logs when each new miner socket enters the loop; logs
disconnect with session and round share counts.
No-job watchdog: RebuildJob retried every 5s (not every 50ms tick),
eliminating cs_main hammering when there is no chain tip.
AcceptLoopFn: re-polls BtfAllRelays every 60s so relays discovered
after startup are picked up; logs each new relay thread spawned.
AcceptOneFn: logs relay name on start/stop; logs BtfServiceWrap
(echan handshake) failure separately from RvServiceRegister failure.
RecvLine: disconnected out-param distinguishes 'socket closed' from
'no complete line yet', preventing incorrect disconnection on partial recv.
Participant side (main.cpp):
StratumRecvLine: same disconnected out-param fix as RecvLine in rpc.cpp.
Previously a false return could mean either 'disconnect' or 'no data yet',
causing spurious reconnects when a partial line arrived.
PoolParticipantMiner rewritten with step-by-step logging:
Step 1 resolve: logs specific failure (no descriptor vs bad format)
Step 2 tunnel: logs relay and failure reason
Step 3 address: warns explicitly if payout address cannot be determined
Step 4 subscribe/authorize: logs each send and each response
Step 5 RandomX: logs VM mode
Step 6 main loop:
- malformed mining.notify (wrong param count or short header/target)
logged with sizes instead of silently ignored
- authorize rejected: logs reason, does not disconnect (some pools
reject bad addresses but still send work)
- share accepted/rejected: logs share id and rejection reason
- unknown server method logged
- pool disconnect logged with handshake state at that moment
(gotSubscribeAck, gotAuthorizeAck, haveJob) so you know exactly
how far the connection got
- nonce space exhausted logged (was silent before)
- on exit: logs total hashes, shares sent, shares accepted
* Fix build error: ImGuiWindowFlags_NoBringToDisplayFront -> NoBringToFrontOnFocus
NoBringToDisplayFront does not exist in this version of ImGui (v1.89.x).
The correct flag is ImGuiWindowFlags_NoBringToFrontOnFocus (bit 13).
Used in two places: ##statusbar and ##main windows.
Also remove dead variable usedY (GetCursorPosY() result assigned but
never read) and replace the confusing 'ImGuiWindowFlags_NoScrollbar * 0'
expression (evaluates to 0 but looks like a mistake) with a plain 0 in
the BeginChild call.
* Remove duplicate status logging from SetParticipantMiningStatus
Every transition in PoolParticipantMiner already has a direct LogPrint
with full context (addresses, job ids, nonces, error reasons). The
LogPrint inside SetParticipantMiningStatus was firing on top of those,
producing redundant 'PoolParticipantMiner: status -> ...' lines after
every meaningful log entry. SetParticipantMiningStatus now just updates
the GUI string silently.
* Implement vardiff: stable share rate instead of fixed difficulty
Previously the pool set shareTarget = blockTarget >> 8 for every miner
forever. This meant at low network difficulty a fast miner found shares
every few nonces (dozens per second), while at high difficulty a slow
miner might go hours without one -- both are bad for fair payout
accounting and pool stability.
Pool (rpc.cpp):
Per-miner difficulty that adjusts to target one share every 15 seconds.
After every 5 shares, check actual seconds/share vs target. If the
ratio is off by more than 50% (VARDIFF_RETARGET_RATIO), recompute:
new_difficulty = current_difficulty / (actual_secs_per_share / target)
i.e. shares coming too fast -> difficulty goes up; too slow -> down.
Only apply if the change exceeds 10% to avoid jitter.
Difficulty is bounded to [0.001, 1e12].
Each miner gets their own shareTarget derived from their difficulty via
ShareTargetFromDifficulty(), which maps Stratum difficulty to a uint256
target using the Bitcoin difficulty-1 base (0x1d00ffff compact).
New difficulty is sent to the miner via mining.set_difficulty.
Participant (main.cpp):
Apply the received difficulty value from mining.set_difficulty.
Previously the handler was a no-op ('acknowledge only'); now it parses
the difficulty double and logs the change. The share target used for
comparison comes from the pool's mining.notify p[3] field, which the
pool updates after each vardiff adjustment -- this is the standard
approach and matches how cpuminer, xmrig etc. handle it.
With vardiff, share rate is roughly constant regardless of miner speed
or network difficulty. Payouts are proportional to shares submitted,
so each miner's payout accurately reflects their CPU contribution.
* Fix share target mismatch: pool rejected shares it should have accepted
Root cause: the pool put job.shareTarget (from blockTarget<<8, a fixed
per-job value) into mining.notify p[3], but checked m->shareTarget
(from ShareTargetFromDifficulty, the vardiff value) on submit. With
difficulty=1.0, ShareTargetFromDifficulty gives a much HARDER target
than blockTarget<<8, so shares that the participant thought were valid
(passed the easier notify target) were rejected by the pool (failed the
harder difficulty-1 target).
Fix: MakeNotifyParams now takes the per-miner shareTarget as an explicit
parameter. SendJob passes m->shareTarget. Both sides -- the participant
reading p[3] and HandleLine checking m->shareTarget on submit -- now use
exactly the same value. Single source of truth.
Cleanup:
- Remove shareTarget field from StratumJob (was only set, never read
after the MakeNotifyParams call was fixed).
- Remove MakeShareTarget() function (was only used to populate that
now-dead field).
- Participant set_difficulty log: 'initial' vs 'updated' instead of
'unchanged' (first connect sends 1.0 which matches the default 1.0,
so it was misleadingly logged as 'unchanged').
* Fix double broadcast, correct block reward calculation, improve payout log
Double broadcast (event loop vs HandleLine):
When a pool miner finds a block, HandleLine calls RebuildJob() +
BroadcastJob(true) immediately. The event loop then detects
nBestHeight changed (step 2) and was calling RebuildJob() +
BroadcastJob(true) again -- miners received duplicate job broadcasts.
Fix: step 2 now checks gHaveJob before rebuilding. If HandleLine
already built and broadcast a job for this height (gHaveJob=true),
step 2 just logs 'job already broadcast by submit handler' and skips.
If the block came from an external peer (not our miner), gHaveJob is
false and step 2 handles it as before.
Block reward calculation in QueuePayouts:
Was calling CBlock().GetBlockValue(0) on an empty block object.
GetBlockValue uses nBestHeight globally, which by the time QueuePayouts
runs has already been incremented to the height AFTER the found block.
Fix: compute subsidy directly as '50 COIN >> (blockHeight / 210000)'
using the actual found height. At current scale this makes no difference
(same subsidy for thousands of blocks) but is now correct by construction.
Payout log now shows:
- Full block subsidy
- Miner share after fee
- Operator fee amount in BTF (not just percentage)
- Number of recipients, maturity height, dust-filtered count
Example: '[payout] block 2096: subsidy 50.00 BTF, miners get 49.00
(fee 2.00% = 1.00 to operator), 3 recipients, matures height 2196'
* Fix 6 bugs found in full flow re-read
Bug 1 -- Double broadcast (re-applied fix that was lost):
When HandleLine accepted a block it called RebuildJob()+BroadcastJob(true).
The event loop then detected nBestHeight changed and did it again. Miners
received two back-to-back mining.notify clean=true, wasting their work.
Fix: step 2 of event loop checks gHaveJob before rebuilding. If HandleLine
already built a fresh job (gHaveJob=true), step 2 just logs and skips.
Bug 3 -- Disconnect log showed global totals, not session totals:
gParticipantSharesSent/Accepted accumulate since node start, not since the
last connect. Added sessionSharesSent/sessionSharesAccepted local counters
that reset each time PoolParticipantMiner() is called.
Bug 4 -- Dead function ParticipantShareTargetFromDiff (main.cpp):
Defined but never called anywhere. Removed.
Bug 5 -- Invalid-address miner shares silently burned:
If a miner authorized with an invalid address, their share of the block
reward was neither paid out nor logged -- it just vanished. QueuePayouts
now validates addresses up front, logs the forfeited amount explicitly
('X BTF forfeited (bad address)'), and excludes those entries so the
payout log is accurate.
Bug 6 -- SendMoney called from event loop (stalls miner I/O):
FlushMaturePayouts called SendMoney which takes cs_main. If the P2P
thread held cs_main (processing a block), the event loop would stall
for seconds, causing all connected miners to time out waiting for
responses. Fix: FlushMaturePayouts now only moves matured entries to
gPayoutExecQueue (fast, no locks except a brief mutex). A new
PayoutThreadFn thread drains the queue and calls SendMoney
independently, so the event loop is never blocked by wallet I/O.
Bug 7 -- Dead goto + dead HandleLine return-false path:
HandleLine always returns true; the goto next_miner path was never
reached. Removed goto and simplified the select drain loop.
* Fix share-target scale mismatch and dead pool hashrate reporting
- ShareTargetFromDifficulty() based Stratum difficulty 1 on Bitcoin's
SHA256 diff-1 constant (0x1d00ffff, ~2^32 hashes/share) instead of
this chain's actual PoW floor (bnProofOfWorkLimit = ~uint256(0)>>12,
~2^12 hashes/share). That's a ~2^20 (~1,048,576x) mismatch: at the
supposedly-easy difficulty of 1.0, workers were being asked for
shares roughly a million times harder than the chain's own easiest
target. Vardiff never gets a chance to correct this because it only
retargets after real shares come in, and none were coming in. Fixed
by basing the diff-1 reference on bnProofOfWorkLimit instead.
- Pool hashrate was never computed anywhere despite the data (per-miner
difficulty + share timing) being available: ThreadBtfPoolAnnouncer in
nostr.cpp hardcoded ann.hashRate = 0.0, PoolWorkerStatView had no
hashrate field, and the GUI workers table had no column for it.
Added an EWMA hashrate estimate per miner (updated on each accepted
share from expected-hashes-at-difficulty / time-since-last-share),
threaded it through GetPoolOperatorStats/GetPoolWorkerStats, the pool
announcement payload, and the GUI (new Hashrate column + totals in
the telemetry line).
Verified: full clean build via src/Makefile (secp256k1 + RandomX deps
built from source, g++ 13, all four touched files compile and link).
* Remove stale wxWidgets-era build files and orphaned assets
These predate the ImGui/GLFW migration (83af065) and are dead weight:
- src/makefile.linux, build-linux.sh: superseded by src/Makefile +
root Makefile (make linux). The old ones reference obj/uibase.o and
obj/ui.o, which don't exist -- they never worked post-migration.
- src/makefile.vc: MSVC build for the old wx UI (obj\ui.obj,
obj\uibase.obj, obj\irc.obj) -- irc.cpp doesn't even exist anymore,
and nothing invokes this makefile. Windows builds go through
makefile.mingw via the root Makefile's target.
- src/ui.rc, src/uiproject.fbp: wxFormBuilder project/resource files
for the old wx UI. The current Windows resource file is bitflash.rc.
- src/rc/{addressbook,send}*.bmp, check.ico, bitcoin_bc_original.ico:
icons used only by the old wx UI. bitcoin.ico (used by bitflash.rc)
is kept.
Verified nothing includes uibase.h/ui.h/irc.h anywhere, and did a full
clean rebuild afterward (secp256k1 + RandomX from source, src/Makefile)
to confirm removal doesn't break the real build.
* Mine with RandomX fast (2GB dataset) mode instead of light mode forever
RandomXInitDataset() was fully implemented and ready to switch mining
into fast mode, but nothing ever called it -- not in PoolParticipantMiner()
(pool worker), not in the solo BitcoinMiner() loop. RandomXCreateMinerVM()
falls back to light mode whenever the dataset isn't built, so every miner,
participant or solo, has always been hashing in light/cache-only mode.
Light mode exists for cheap single-hash verification (which is why the
pool's own share verification correctly stays on it) -- it recomputes
dataset items on the fly for every hash, which is dramatically slower
than fast mode's direct dataset lookups. Combined with the share-target
scale bug fixed in e87a905, this compounds into exactly the symptom
reported: a participant sitting there hashing for many minutes per share
instead of the vardiff-targeted ~15s, even after the target fix landed.
Fix: build the ~2GB dataset once (multi-threaded, sized to
hardware_concurrency) before mining starts, in both the participant and
solo paths, so RandomXCreateMinerVM() actually gets to use fast mode.
Verified: full clean rebuild via src/Makefile.
* Add vardiff stall watchdog: force difficulty down on zero shares, not just share count
ShareTargetFromDifficulty already fixed the diff-1 scale mismatch (chain
floor vs Bitcoin's SHA256 diff-1), so difficulty 1 correctly means ~4096
expected hashes on this chain, not ~2^32. That part was already correct.
But the normal vardiff retarget in HandleLine only runs after
VARDIFF_SHARES_PER_CHECK (5) shares arrive. A miner that never finds even
one share -- e.g. stuck in RandomX light/cache mode instead of the fast
2GB dataset, or just slower hardware than the target assumes -- never
reaches that check, so it can sit at its initial/current difficulty
indefinitely with vardiff having no samples to act on.
Add a time-based fallback in PoolEventLoop: if a miner's vardiff window
has been open for VARDIFF_STALL_SECS (90s, 6x the 15s target) with zero
shares, force difficulty down by VARDIFF_STALL_CUT (4x) regardless of
share count, and re-notify via mining.set_difficulty. Once a miner is
already at VARDIFF_MIN, log it plainly as a hashrate/connectivity issue
instead of a difficulty issue, so it's not confused with the same-looking
but different problem this was actually fixed for before.
* Fix PrintException rethrow killing the whole node on any transient exception
The uploaded participant log shows the real bug behind the mining stalls:
the node process itself was dying and restarting from scratch every
minute or two ('UNKNOWN EXCEPTION ... in ThreadMessageHandler()' /
'ThreadSocketHandler()'), wiping the RandomX dataset/cache and losing all
progress toward a share -- not a difficulty problem.
Every background thread (ThreadMessageHandler, ThreadSocketHandler,
ThreadBtfConnect, the Nostr helpers) wraps its real work in a
'try { Foo2() } catch(...) { log, sleep 5s, loop again }' self-healing
retry pattern specifically so one bad exception doesn't take the whole
node down. But PrintException(), called from inside every one of those
catch blocks, ended with 'if (!fShutdown) throw;' -- re-throwing the just-
caught exception during ordinary operation. Nothing above the retry loop
catches it, so it escapes the thread and std::terminate()s the entire
process on the very first transient error (a flaky relay connect, a
malformed message, whatever). The comment's intent (suppress noise while
already shutting down) was inverted from what the surrounding retry loops
actually need.
Removed the rethrow. Threads now log the exception and let their own
retry loop handle it, as designed.
* Push a fresh job immediately after any difficulty change, not just on next broadcast
mining.set_difficulty alone doesn't change what the worker actually hashes
against -- the participant only reads its share target from
mining.notify's params[3] (see main.cpp's mining.notify handler); the
set_difficulty message just updates a locally-logged currentDifficulty
value. So both the normal vardiff retarget (in HandleLine, on 5 shares)
and the new stall watchdog were updating m->shareTarget and telling the
worker about the new difficulty, but the worker kept hashing against its
old target until the next natural job broadcast -- a new block, or the
60s trickle refresh. That could delay any difficulty correction by up to
a minute for no reason.
Call SendJob(m, false) right after each set_difficulty send so the
updated shareTarget reaches the worker in the very next tick instead of
waiting on the next unrelated broadcast.
* Build RandomX dataset before stratum handshake, not after
Worker was sending mining.subscribe/authorize, then blocking for the
duration of the synchronous ~2GB RandomX dataset build, and only then
reading the socket. The pool's replies (subscribe ack, authorize ack,
set_difficulty, first job) sat unread in the OS receive buffer for that
entire window -- nothing was lost, but with the build taking anywhere
from seconds to a couple minutes depending on core count, the worker
looked and logged as permanently stuck on 'waiting for pool response'
when it just wasn't listening yet.
Move the dataset build ahead of the subscribe/authorize sends so the
socket is actively being read the moment the pool replies. Also add
missing RandomXDestroyMinerVM cleanup on the two early-return paths
that now happen after VM creation.
* Reset round shares on externally-found blocks, not just our own
Round shares were only cleared in HandleLine() after our own miner's
block got accepted. When the chain height advanced because someone
else found the block, the main loop rebuilt the job but left
roundShareCount/roundShareTotal/Miner::roundShares untouched, so they
kept accumulating across rounds instead of resetting.
* Show pool hashrate in participant view; show immature coinbase value with maturity countdown
* Persist pending payouts to disk -- survive operator restarts
* Fix log escapes, stall timeout 90->45s, save-before-exec ordering, Immature tx status
* Sort transactions by nTimeReceived not chain median time -- new txs always on top
* Sort txs by chain depth not nTimeReceived -- survives resync, unconfirmed always first
* Fix orphan block false-positive payout; meaningful tx status instead of raw depth
* Show Orphaned status in red for orphaned coinbases; distinguish from genuinely unconfirmed
---------
Co-authored-by: Vic-Nas <nasci.victorio@gmail.com>
Co-authored-by: Bitflash Pool Dev <pool@bitflash>
Co-authored-by: Victorio Nascimento <155357371+Vic-Nas@users.noreply.github.com>
Co-authored-by: Claude <claude@anthropic.com>
|
|
* Add Linux GUI installer via Wine
* Fix launcher creation and add BTF_RELAY env var support
* Pool server, Linux GUI, unified binary, README update
* Root Makefile: make linux / make windows, cleaner README
* Remove headless.cpp
* Fix XPM color count mismatch causing toolbar image errors on Linux
* XPM icons: same code on Windows and Linux, no ifdefs around image loading
* Fix GTK icon crash and Linux printf format
* btfrv: 10s socket timeout on relay connects
* Faster peer discovery: more dials per pass, shorter backoff and sleep
* Parallel btf peer connections
* Fix GetMyExternalIP - was disabled, now uses modern IP echo services
* Fix tunnel disconnect on idle and restore external IP detection
* Filter stale Nostr descriptors
* Sort peers by freshness, fresh first
* Pool: add .btf Stratum listener — miners connect via rendezvous, no port forwarding needed
* btf pool: self-replacing listeners, one per relay, no queuing between miners
* Mining modes: solo/operator/participant — pool server on-demand, participant Stratum client
* Status bar: show external IP and .btf address
* README: clean rewrite, concept-first, minimal
* make linux produces AppImage
* make linux produces AppImage
* Pool dashboard: connect instructions, tabs, share bars
* Perf fixes: GetMyExternalIP threaded, btf listener counted
* Perf fixes: GetMyExternalIP threaded, btf listener counted
* Fix pool payouts: defer until coinbase maturity
* Pool: clean mode switching, IP warning
* Pool: clean mode switching
* Fix gPoolRunning: remove static
* AppImage: use real icon from bitcoin.ico
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Fix exit crash; Mining menu
* Windows build: use mingw32-make
* Windows build: simpler make invocation
* Add deps-windows target
* Windows build fixes
* Windows build fixes
* Windows makefile: fix obj dir
* Windows: output to repo root
* Windows: bundle DLLs into zip
* Replace wxWidgets with ImGui+GLFW: same build on Linux and Windows
* Fix %I64d that slipped back through rebase (main.h, net.h)
* MinGW: add _mkdir shim for non-MSVC Windows; fix %I64d
* Fix gPoolRunning extern outside ifndef _WIN32
* Remove -lboost_system: boost is header-only in this codebase
* Status bar: copy buttons for IP and .btf address
* Windows: add app icon via windres, strip wx stuff from rc file
* Clean GUI: single Options menu item, bigger font, better spacing, cleaner options dialog; update README
* GUI: embed Roboto font at 16px for clear readability on all platforms
* Windows: windres depends on obj dir target
* Fix balance hidden off-screen: put on own line below address bar
* Windows: inline mkdir in every rule, remove dir target that confused make
* Windows: move all target to top so it's the default rule
* Fix pool server not starting: always start when operator selected, re-sync dialog state on open
* Write debug.log to data directory (AppData/Roaming/Bitflash on Windows)
* Participant miner: use wallet address as username so payouts go to wallet
* Add .gitattributes to lock LF line endings, prevent CRLF drift
* Refactor pool mining to btf-only transport
* Persist pool settings and startup wiring
* Prune stale peers and expose pool queries
* Fix build regressions and add CLI mode/arg parity
* Fix rpc miner stats declaration order
* Centralize pool runtime extern declarations
* Separate operator/participant dashboards and restore peer stats
* Add live telemetry strip and clearer per-mode runtime metrics
* Reduce mining dashboard clutter and remove manual pool entry
* Count only authorized pool miners
* Seed initial pool job on startup
* Use a real share target for pool work
* Switch pool accounting to round-based shares
* Fix nostr pool announcer stats call sites
* Fix pool stats definition order
* Improve pool worker status display
* Add mutex include for participant status
* Replace ImClamp in dashboard layout
* Clarify participant work status
* Log participant status transitions
* Reduce peer discovery log noise
* Add participant status badge
* Track participant auth acknowledgements
* Log and validate pool job delivery
---------
Co-authored-by: Bitflash Pool Dev <pool@bitflash>
|