| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
|
|
|
|
app icon)
|
|
BITFLASH_NODE_BUILD)
|
|
# Conflicts:
# src/makefile.mingw
|
|
|
|
|
|
|
|
|
|
|
|
The legacy parser only accepted two decimal places (cents) and rejected
0.5, leaving six of the eight coin decimals unreachable by -sendto and the
send window. Parse up to 8 fractional digits Bitcoin-style, reject >8 to
avoid silent truncation. Adds the parse-money selftest (12 cases).
|
|
The coin type 4346950 (0x425446 = BTF) was merged into the SLIP-0044
registry on 2026-08-07. Rename HD_BIP44_COIN_TYPE_BITFLASH_PROVISIONAL to
HD_BIP44_COIN_TYPE_BITFLASH and update comments, logs and UI strings.
Value is unchanged, so derivation paths and addresses are identical.
|
|
The headless loop installed SIGINT/SIGTERM handlers only under #ifndef _WIN32,
so a headless node on Windows never reached the clean StopNode()+DBFlush() path:
Ctrl-C, closing the console window, a service stop and OS shutdown all killed it
mid-write, and a Berkeley DB wallet.dat left unflushed will not reopen.
Install a SetConsoleCtrlHandler that mirrors the POSIX path: set fShutdown and,
because the OS terminates us shortly after the handler returns on
CLOSE/LOGOFF/SHUTDOWN, block until the main loop signals the flush is done.
A 'taskkill /F' is still an uncatchable hard kill by design.
Closes #16.
|
|
|
|
setint64() extracted big-endian bytes with a signed left shift (n <<= 8).
When the top byte is set that overflows int64 -- undefined behaviour, which
the script fuzzer flagged under UBSan (bignum.h:158). The result was correct
on two's-complement hardware, so this is a latent UB inherited from the
original 0.1 code, not a behaviour change. Do the shift on a uint64.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
* fuzz: add P2P message parser harness
* fuzz: avoid duplicate legacy header includes
* fuzz: seed the corpus with our network magic, not Bitcoin's
The two seed inputs began f5 be b6 d9, which is Bitcoin's message
start. Ours is bf 20 5c fd. CMessageHeader::IsValid rejects on that
field first, so both seeds died at the first check and the smoke run
exercised the reject path and nothing else.
libFuzzer would have to mutate its way onto four exact bytes before
reaching any parser, which does not happen in 128 runs. The job would
have gone green while covering none of what it was added to cover.
---------
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
Co-authored-by: Bitflash-sh <btf01@proton.me>
|
|
Two bugs in RebuildJob(), the operator's Stratum template builder. It
is a separate path from BitcoinMiner() and had drifted from it.
A single pass over mapTransactions is not enough. That map is keyed by
hash and carries no dependency order, so a transaction spending another
still-unconfirmed transaction's output -- change from a recent send,
usually -- fails ConnectInputs whenever its parent sits later in map
order. One pass drops it, and map order does not change between
rebuilds, so every later rebuild drops it at the same point. That is
deterministic exclusion rather than bad luck: where pool operators mine
most blocks, a transaction of that shape can stay unconfirmed for as
long as its parent is. Now it retries until a pass adds nothing, which
is what the solo builder has always done.
And the fees reached nobody. nFees was declared inside the scan loop, so
each transaction reset it and the running total was discarded, while the
coinbase line passed a hardcoded 0. An operator collected fees into a
block and paid none of them out. The block stayed valid, since paying
less than allowed always is, so nothing ever complained.
Both the diagnosis and the fix are Vic-Nas's, from his fork of this
code. He deleted the fork, so this brings the work back rather than
letting it go with it. His description of the failure was precise enough
that finding the same two defects here took one read of our RebuildJob.
|
|
Every fatal startup path wrote to stderr and to printf, and on Windows
neither reaches anybody. printf is remapped to OutputDebugStringF, so it
lands in debug.log inside the data directory. A -mwindows binary started
by double-clicking has no stderr at all, and AttachTerminal() cannot
help: it attaches to a parent console, which a double-clicked exe does
not have.
So Bitflash exited silently. Someone whose wallet.dat needs a newer
build -- exactly what the wallet format guard was added to tell them --
double-clicked and watched nothing happen. The guard was correct and
mute, which is the failure it was written to prevent.
Fatal startup errors now go through one reporter: debug.log as before,
stderr as before, and a message box when there is a person in front of
the screen. Applied to the three that mean "your data will not open":
the block index failing to load, the block index throwing, and
wallet.dat failing or throwing.
The box is deliberately not shown for a headless node. Those run under
scheduled tasks and services, where a modal dialog waits forever for a
click nobody is there to make -- a hung process instead of an exit code.
Headless keeps stderr and the log.
LoadWallet() now also reports why it refused, in a sentence, instead of
only logging it. The unsupported-format case is the one that has to be
understood by whoever reads it, and "LoadWallet failed" is not that.
Verified by breaking a throwaway wallet on purpose and starting both
ways. Headless: exit code 1, no dialog, no process left behind. Windowed
with no console: a box naming the file, the Berkeley DB error, the usual
cause, and where to look for more.
|
|
(#134)
* wallet: refuse future encrypted wallet formats
* wallet: add encrypted key unlock core
* wallet: add encryptwallet rewrite command
* wallet: read passphrases from stdin or files
* wallet: keep the derivation across encryption, and take the logs with it
Two things stopped the encryption stack from being safe to merge. Both
were found against a wallet holding real coin, not in the self-tests.
1. Encrypting turned a BIP44 wallet into a legacy one.
LoadWallet() ends by resetting the schema and the derivation counters
when no seed is in memory. That was written when "no seed after loading"
could only mean the wallet had none. An encrypted wallet has a seed --
it just has not been decrypted yet -- so the reset threw away metadata
that had been read correctly a moment earlier. Unlocking later restores
the seed and nothing restores the rest.
Measured on the wallet from the #124 proof: after encrypting, index 1
moved off BAtgSgra2X8Dqo5PJzPCLJhPwrrC2thYVJ, the address actually
holding 1.00 BTF, and the audit went from "1.00 covered by the phrase"
to "0.00 covered, 1.00 wallet.dat-only". Encrypt your wallet and the
program told you your recovery phrase no longer covered your money.
Now the condition also accepts a wallet whose seed is present but
encrypted. Same wallet, before and after: schema bip44, same four
derived addresses, 1.00 BTF still reported as covered.
2. The plaintext keys stayed behind in the Berkeley DB logs.
wallet.dat comes out clean -- verified by scanning the raw file for the
118 bytes of a known private key. database/log.0000000002 did not: it
held the whole key, and still did after a full node restart, because
every record this wallet ever wrote passed through the log.
That defeats the feature for the case it exists for, since our own
backup advice is to copy the whole directory rather than wallet.dat
alone. log_archive() cannot fix it: it only drops logs Berkeley DB
considers obsolete and never the one it is writing to, and with one 10
MB log per directory that is precisely where the keys are. They also did
not get there at encryption time -- they accumulated over the wallet's
whole life -- so nothing done during the rewrite can archive them out.
So the encrypt command now closes the environment and deletes the logs
on its way out, after the flush that makes every .dat self-contained.
This is not secure erasure; the blocks are unlinked, not overwritten.
It closes the case of someone reading a copied data directory.
Getting there uncovered a third bug, older than both.
DBFlush(true) is not safe to call twice. The first call closes the
environment, and txn_checkpoint() on a closed environment segfaults
instead of returning an error, so the try/catch around it never sees
anything. EncryptWallet() already flushes before renaming the wallet,
and the command flushes again on the way out -- that pair crashed every
single time. Guarded now.
The same function has been calling log_archive() with the address of an
uninitialised pointer since 0.1.0, on every clean shutdown, in a path
whose catch(...) cannot catch a structured fault on Windows.
DB_ARCH_REMOVE requires that argument to be NULL. Fixed while here,
because the encryption cleanup depends on that call not being a coin
flip.
---------
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
The right icon has been in the exe the whole time. GLFW never found it.
GLFW registers its own window class and fills hIcon with
LoadImageW(module, L"GLFW_ICON", ...), falling back to IDI_APPLICATION
when that exact resource name is missing. Ours was named "bitflash", so
the lookup failed and every window this program has opened wore the
generic Windows icon -- in the title bar and in the taskbar -- while the
correct artwork sat unused a few kilobytes away.
One line, verified by looking at it: rebuilt, ran the window, and zoomed
in on both places the user pointed at. The title bar and the taskbar
button now show the green BF coin. Checked the shell path too, since it
resolves icons differently: ExtractAssociatedIcon on the new exe still
returns the coin, so renaming the resource did not cost the file icon in
Explorer.
|
|
* Add BIP32 path derivation foundation
* Add HD wallet schema metadata
* Add provisional BIP44 coin type metadata
* Activate BIP44 wallet derivation
* wallet: scan BIP44 restore depth per branch
* wallet: widen default BIP44 restore lookahead
---------
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
CmdSendTo() has been in walletcmd.cpp since #115 and nothing declared
it or called it. The flag was dead: `bitflash -sendto=ADDRESS,AMOUNT`
started a node and said nothing, because an unrecognised argument is
not an error here.
The v1.2.14 notes announce it under "A headless node can finally
spend", and the headless Linux binary is exactly the one that does not
have it -- `-help` there lists /newaddress and stops. The published
Windows binary does have it, because it was built from a working tree
that carried these two files uncommitted. So the artifacts disagree
with each other and neither matches the tag.
This adds only what was missing: the declaration in walletcmd.h, the
dispatch in main(), and the -help line. No behaviour change to
CmdSendTo itself.
Checked on a build of this branch, against a throwaway data directory:
-help lists /sendto=ADDRESS,AMOUNT
-sendto=NOTANADDRESS,1 "Not a valid address", exit 1
-sendto=<valid>,-5 "Not a valid amount", exit 1
-sendto=<valid>,999 refused on an empty wallet, exit 1
All five self-tests still pass.
|
|
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
Creating a recovery phrase from the window aborted with an ImGui
assertion on the success path:
Expression: (g.DisabledStackSize > 0) && "Calling EndDisabled() too many times!"
File: imgui/imgui.cpp Line: 8435
The Finish button is wrapped in BeginDisabled/EndDisabled guarded by
!g_phraseWrittenDown, and its own click handler sets g_phraseWrittenDown
to false. So the pair was opened while the checkbox was ticked and
closed after the handler had unticked it: BeginDisabled was never
called, EndDisabled was, and ImGui asserted on the unmatched close.
It fires on the only path that installs a seed -- tick the box, press
Finish -- so every phrase created from the window has hit it since the
feature shipped in 1.2.12. Three releases carried it because nothing
tests the window.
Reproduced on the published 1.2.14 binary and confirmed fixed on this
build, driving the real window: Wallet Safety -> Create recovery phrase
-> tick -> Finish. The old build raises the assertion; the fixed build
closes the dialog, reports "This wallet has a recovery phrase", and the
address changes to a derived one.
The seed itself was already written before the assertion, so the wallet
was never left half-installed -- and ImGui's IM_ASSERT_USER_ERROR_RET
returns before touching its state, so answering Ignore was safe. Abort
was not: it kills the process with no DBFlush.
The restore dialog next door already does this correctly, copying the
condition into a local first. This makes the create dialog match.
|
|
Restoring from a phrase gave up after the first batch of a hundred addresses that turned up nothing, and the wallet digs a wider gap than that by itself: a restore leaves nHDNext at the depth it scanned, the key pool derives KEYPOOL_SIZE more, and change takes the index after those. Measured on a real wallet with real coin -- restore, spend once, and the outputs land at indices 201 and 302; restoring again found only the first transaction and reported the spend as gone. With -restoredepth=500 the same wallet found both, which is what said this was reach and not loss.
Three quiet batches instead of one: 2 transaction(s) across 601 derived addresses on the same wallet, chain and phrase.
Worse after #109, not before it: change used to return to an input's key, a low index the scan had already passed. Making change recoverable moved it out to where the scan stopped looking.
Does not fix the gap growing with every restore -- pulling nHDNext back to the last used index is the structural answer and needs its own testing.
|
|
#113 added three accept threads and called the extras harmless against an un-upgraded relay. They are not: an old relay closes the previous registration whenever the same node registers again, so the threads evict each other in a loop. Measured on the live network -- the node whose registrations landed on the un-upgraded New Jersey relay made 189 registrations in fifteen minutes with zero pairings, unreachable through it and hammering it, while the node on the upgraded relay made 9 with 7 pairings. One relay of three is upgraded, so this would hit two thirds of every node's registrations.
The relay half of #113 stays; it is compatible in both directions and is the prerequisite for raising this again, which the comment now states.
|
|
A registration was consumed by the first caller and registering again replaced the previous one, so a node was reachable to one peer at a time and unreachable for the gap between being paired and registering again. Measured on a live node: descriptors resolved 53/54, our own registrations paired 11/12, nothing paired and went silent -- and half of every outbound dial failed, 15 of 30 in eleven minutes.
The relay keeps a queue per pubkey capped at four, pairs with the oldest still-alive registration and discards the dead; the node runs three accept threads, each holding its own registration. The wire protocol does not change, so either side can be deployed first.
Bench with both relays built from source and waves of concurrent dialers: old+1 65%, new+1 58%, old+3 68%, new+3 98%. Each half alone lands within noise of doing nothing.
|
|
CheckBlock() validated block size against MAX_SIZE, the serializer's generic 32 MB guard rather than a block rule; MAX_BLOCK_SIZE did not exist in this tree at all, because Bitcoin's 1 MB cap arrived in 0.3.x, after the 0.1.0 this fork came from. A miner could produce a 32 MB block and every node would download, verify and store it.
Original work by mtg0001 in #110, rebased over #109 with the self-test conflict resolved, plus a comment explaining why the inherited count-against-bytes test stays.
Measured before choosing the number: 5709 blocks read from blk0001.dat give a largest block of 34,290 bytes and a median of 216, so 1 MB leaves 29x of headroom and rejects nothing that exists. A node built from this branch then synced genesis to 5706 on the live network with zero size-limit rejections and zero CheckBlock errors -- the same test that caught BIP30 rejecting a real block at height 859.
|
|
Change from an HD wallet now goes to a newly derived phrase-backed key instead of reusing a spent input's key. Reusing an input key silently moved value back to a key the phrase cannot reproduce; if derivation fails the transaction is refused rather than creating unrecoverable change. Legacy wallets with no seed keep the 0.1.0 behaviour. Restore also scans past the pre-generated key-pool window before treating an empty batch as the end.
The message parser no longer consumes a header it cannot complete: it reads from a copy, so the O(n) reinsert and the 100 ms sleep inside the shared message thread are gone, and a peer that sits on an incomplete payload is disconnected after two minutes. The wire cap drops from 256 MB to MAX_SIZE. New net-message self-test covers complete, incomplete, stale and oversized headers.
Verified here: clean rebuild, all self-tests pass, lock order checked (no path takes cs_keyPool before cs_mapWallet), and a node built from this branch synced from genesis to the chain tip on the live network with zero rejections.
|
|
The README still said deterministic restore was an open issue, told people the mining mode was not remembered between restarts, and wrote every flag in the slash form that MSYS2 turns into a path before the node sees it. Rewritten around the phrase and the file as two backups covering different keys, the -restorephrase note for anyone who made a phrase on 1.2.12, -rescan, the diagnostics report, and the two upgrade reasons measured this week.
Also: -help printed nothing at all. This tree remaps printf to OutputDebugStringF, so the usage page went into debug.log, and a -mwindows binary has no console to write to either way -- the one command whose job is to tell you something returned 0 and said nothing. It now takes the real printf and attaches the terminal. The help text was missing -restoredepth, -showderived and the wallet-hd self-test.
|
|
does not cover (#104)
Follow-up to #102. Reserves derivation index 0 before deriving it, so a failure between installing the seed and writing the default key cannot leave the pool free to hand that index out as an ordinary key. Renames the address that stops being the default to 'Your Address (created before the recovery phrase)', which was otherwise left under the same name as the new one. Also documents why the key-pool clear precedes the seed write, correcting a review comment of mine that asked for the reverse.
|
|
-newphrase installed a seed but left the wallet's visible address as the pre-seed random key and the old random pool intact, so the next address handed out -- a mining payout included -- could be one the twelve words cannot reproduce, while the window said a recovery phrase existed. Installing a phrase now clears the stale pool records (private keys stay in wallet.dat), reserves a derived default receiving key, tolerates already-stored derived keys so repeated installs do not stall the pool, and -restorephrase reports what it recovered rather than the wallet's total transaction count. Self-test covers installing a phrase over a pre-existing random pool.
|
|
CNode::Disconnect() closed the socket and left the handle number in the object; ~CNode() then closed it again with a raw closesocket() the accounting never saw. There is no assignment of INVALID_SOCKET to CNode::hSocket anywhere in this tree, so the destructor's guard never fired. Between the two closes the OS can reissue that number, and the second close then takes down somebody else's connection. The same omission left a closed handle in the select() set, which fails every iteration and drives the recovery path that prints 'dropping node: its socket is no longer valid'. Measured A/B on the real network at matched socket volume: unpatched 552 sockets, 4 closes failing with 10038, 741 invalid-socket drops (all one peer), 11 peers held; patched 547 sockets, 0, 0, and 21 peers held. No protocol change.
|
|
A live node reported one close failing with 10038 (WSAENOTSOCK) at the tunnel's pump end. It leaks nothing; what it says is that a site's bookkeeping and its sockets disagree about ownership -- and Windows reissues a handle number only after its last close, so a site whose bookkeeping outlives its socket will eventually close a number that belongs to a live connection. BtfSocketTag now counts a handle arriving while another site still claims it, which names the site to go and read.
|
|
Windows lists a wildcard-bound socket twice in MSFT_NetTCPConnection, so counting Bound rows makes the number climb whenever the node talks to more peers -- 35 of a node's 38 Bound rows shared a local port with one of its own established connections. Counting orphans instead: a 1.2.10 node at 26h held 1262 and was still taking on 30-90 an hour; nodes on 1.2.11 and 1.2.12 held 3, transient. The leak was real and 1.2.11 ended it. Adds a live-unconn column (getpeername on every tagged socket, reports 1: the listener) and rewrites the notes that said otherwise.
|
|
sockcount.h was in neither makefile's HEADERS, so editing it rebuilt only the objects whose .cpp had also changed and the rest kept the old struct layout -- one object indexing a shared function-local static by the old layout and another by the new one, in the same binary. compat.h and font_roboto.h were missing from one list each. Verified with a full clean rebuild: no duplicate-section warning.
|
|
The 1.2.11 accounting answered 'which site' and stopped. On a node with 3h51m of uptime it reported 58 live sockets while Windows attributed 112 to the process, 47 of them bound and never connected, against 55 closes that had returned an error -- with nothing recording what those errors were. Close failures are now counted per code per site, and the table totals, so the live figure can be compared directly against what the OS attributes to the process. Instrumentation only.
|
|
Adds -newphrase, -restorephrase, -restoredepth, -newaddress and -showderived, plus the window buttons that reach the same code. Restore derives forward in batches of 100 and rescans after each, stopping when a whole batch turns up nothing; the phrase is shown on the terminal only, never in debug.log. Proved end to end on a live wallet: paid a derived address, deleted wallet.dat, restored from the twelve words and recovered the coin. Coins on the pre-seed default address do not come back, and the command says so.
|
|
Adds BIP32 derivation behind the wallet key pool, with no user-facing way to create a seed yet. wallet.dat gains hdmaster/hdchaincode/hdnext; the mnemonic is never stored. TopUpKeyPool derives m/n' when a seed exists and falls back to a random key, with a log line, if derivation fails. New -selftest=wallet-hd, wired into make tests on both platforms.
|
|
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
On Windows the self-test from #91 printed nothing. printf in this tree is
OutputDebugStringF, which writes to the data directory debug.log, and the
Windows binary is linked -mwindows with no console. A failure was still caught,
since a non-zero return stops make, but it arrived with no way to tell which
check failed.
And because the first line printed before strSetDataDir pointed at the temporary
directory, every line was appended to the real debug.log -- the file users are
asked to paste into issues.
The file now gets the real printf, the data directory is set before anything
prints, and on Windows a console is borrowed only when stdout has nowhere to go.
That last condition matters: the first version reopened stdout unconditionally
and took the output away from `make tests > log`, which is the case that counts.
|
|
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
A transaction only entered the wallet when its block arrived over the network,
so a key added afterwards was invisible: coins on the chain, private key in
wallet.dat, balance zero. /importwallet has always ended that way -- it writes
the keys and the money never shows up, which looks exactly like a failed import.
ScanForWalletTransactions walks the main chain and hands every transaction that
IsMine() to AddToWalletIfMine, the same two primitives the live path uses. Wired
into /importwallet, and offered as /rescan.
It is also the half that any recovery-phrase feature has to stand on: deriving
keys from twelve words is easy, and without this a restore restores nothing.
|
|
Two follow-ups to #89. The default backup path was inside the data directory --
same disk, same folder the README says to copy wholesale, same folder the user
eventually deletes, which is the moment the backup was for. It now defaults to
the home directory and the dialog says so when a path under the data directory
is chosen anyway.
And nLastWalletBackup was written by the GUI, so anyone backing up with
/backupwallet -- the method the README documents first -- was told forever that
they had never backed up. BackupWallet records it now, so both paths count.
|
|
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
fGenerateBitcoins and nMineMode only mean anything together, and a wallet.dat
can hold one without the other: Bitcoin 0.1.0 already wrote fGenerateBitcoins
when mining was toggled, years before this fork had a mode. #84 restored that
lone flag, which puts the node in the one state the miner cannot act on --
generate on, mode relay -- so every miner thread starts and returns at the relay
guard. The machine looks like it is mining and hashes nothing.
That happened to a 32-core node on its first 1.2.11 start: "Mining with 31
thread(s)" followed by 31 lines of "BitcoinMiner thread N returned true".
The flag is now believed only when the mode it belongs to was stored with it,
and the pair is made coherent either way.
|
|
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.
|
|
|
|
* Bump to 1.2.11
One change, and it is the reason a node on Windows kept collecting peers it
could not hear.
FD_SETSIZE was never defined in this tree, so it defaulted to 64 and select()
could watch the listen socket plus 63 peers -- everything past that stayed
connected and was never read again. Windows FD_SET stops adding once its array
is full and says nothing, so there was no error to notice. Because new
connections go on the end of vNodes, it was always the newest connection that
starved, including the ones dialed to fix connectivity.
Measured on two nodes: 277 and 90 CNode objects against 93 and 90 live
sockets, 316 and 355 sockets in a bound-but-never-connected state, and a status
bar reporting hundreds of peers for a node really talking to thirty.
1.2.11 raises FD_SETSIZE to 1024 on Windows, caps connections at 125, drops a
node stuck undeliverable after 60 seconds instead of holding it forever, stops
answering a failed select() with POSIX code that has no meaning on Windows, and
warns when there are more peers than select can watch.
* diag: let the node say what is wrong with it
Every hard problem in this project so far was found from outside the node --
sockets read over WMI, committed memory compared against a README, a grep over
somebody debug.log. In each case the node knew and had no way to say so.
One report, rendered identically for the GUI panel and the log: peers held vs
peers actually watched by select(), blocks received and how many arrived without
a parent, proof-of-work mode with the live miner thread count, and per peer the
age of the last message in each direction plus anything stuck unsent.
|
|
Co-authored-by: Gilcimar Duarte <ti@gpm.infra>
|
|
|
|
CreateTransaction kept the payee amount in nValue and then did
int64 nValueOut = nValue;
nValue += nFee;
with both lines inside the retry loop. The first pass is fine. When the fee
turns out to be too low the loop starts over, and on the second pass
nValueOut is read from an nValue that already carries the fee -- so the
recipient is sent the amount plus the fee, out of the sender's pocket, and
it compounds on every further retry.
Reachable, but it takes two things at once, which is why it has sat here:
the user must have set a transaction fee in Options (with the default of
zero, nValue += nFee adds nothing on the first pass and nothing is
corrupted), and the transaction must be big enough for GetMinFee to demand
more than that -- over 10 KB, so a wallet paying from many small inputs.
Keep nValue meaning what the payee asked for and carry nValue + nFee in a
separate nTotalValue, the way Bitcoin fixed it in 107d9e288.
Verified by building the same transaction on both versions, with a
synthetic wallet of 200 coins of 50, a configured fee of 0.05, and a
request of 5,000 that needs 101 inputs and 28 KB -- enough for GetMinFee to
demand 0.12 and force the retry:
before payee gets 5,000.05 change 49.83
after payee gets 5,000.00 change 49.88
Two other fixes from the same era were considered and left out. Satoshi's
CENT floor on the change output (496823249) exists to avoid a dust output
that his fee rules then penalised; this fee schedule has no such rule, so
adopting it would only donate sub-cent change to the miner. The
1-confirmation rule (a790fa46f) is a spending-policy change, not a bug fix,
and deserves its own decision.
|
|
Three functions open a cursor and none of them close it: CTxDB::LoadBlockIndex,
CTxDB::ReadOwnerTxes and CWalletDB::LoadWallet.
Berkeley DB requires every cursor to be closed before the handle it came
from. ~CDB() calls Close(), which calls pdb->close() inside a catch-all
that swallows whatever it returns -- so the violation was real and silent,
which is this codebase's favourite kind.
An open cursor also holds read locks, and that is not theoretical here: the
first version of the startup chain repair (#61) opened a write transaction
beside the cursor LoadBlockIndex was still holding and waited on it
forever, coming up, printing its diagnosis and hanging with nothing in any
log. The repair had to be moved to the caller to get clear of it.
A scope guard rather than a close call at each return, because all three
functions have several exits and the point is to stop relying on anyone
remembering. Same fix Bitcoin made in c5c7911da.
Verified: node starts, verifies 288 blocks, loads the wallet, runs the
spent-flag rescan and syncs the live chain to 4422 with no Berkeley DB
error anywhere and db.log still empty.
|
|
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.
|
|
Two defects that feed each other. Measured on a 32-core production node:
1287 of 4117 processed blocks arrived with a missing parent, 97 arrived
that the node already had, and 490 transactions arrived whose inputs were
unknown.
mapOrphanBlocks had no ceiling at all. Orphan transactions have had one
since 0.1.0 -- MAX_ORPHAN_TRANSACTIONS with LimitOrphanTx -- and blocks
never did. An orphan block whose parent never turns up stays in memory for
the life of the process, each one holding a whole block, and a peer can
send an unlimited number of them just by inventing the previous-block hash.
LimitOrphanBlocks mirrors the transaction one: random eviction, both maps
kept in step, and the CBlock deleted because mapOrphanBlocks owns it.
Every orphan block also fired a getblocks at the sender, and an inv for a
block already held as an orphan fired another, with no filtering. So a node
missing part of the chain asked the same peer the same question over and
over, and the duplicate answers made more orphans, which asked again.
CNode::PushGetBlocks remembers the last request and drops a repeat -- the
same fix Bitcoin made in e2c2648c1.
One trap found by testing rather than by reading: eviction picks at random
and may pick the block just inserted, so GetOrphanRoot has to be read out
of pblock before LimitOrphanBlocks runs, not after. The first version had
that backwards and would have read freed memory.
Verified with fabricated orphans, four of the seven sharing one
hashPrevBlock so the multimap path is exercised: 7 in, limit(3) leaves 3,
limit(0) leaves 0, both maps in step at every stage, no crash. Then a run
against the live network to 4348 with no scaffolding left behind.
The storm itself was not reproducible on demand -- the network is healthy
today and produced one orphan block in seven minutes -- so there is no
before/after on the metric that motivated this.
|
|
The mining loop tests pindexPrev != pindexBest every 256 hashes, but never
again once a solution turns up. A thread that found one just after somebody
else extended the chain submitted a block built on a stale parent -- a
block born orphaned, at a height already taken.
Every miner thread on a machine works from the same pindexPrev, so the
moment one wins, all the others are hashing a doomed template until their
next checkpoint. On a 32-core box that is thirty threads with a standing
chance of producing a guaranteed orphan.
Re-check the tip inside cs_main before submitting, and drop the solution if
it moved.
Verified on an isolated chain with three miner threads: 2790 blocks
accepted, so nothing legitimate is being discarded, and 2 solutions dropped
for a moved tip -- which is the race actually happening and being caught.
Honest about the size of it: two in 2790 is not what fills a wallet with
orphans. On the 32-core node in production, 1287 of 4117 processed blocks
arrive with a missing parent, which is a different problem and the one
worth chasing next.
|
|
The wallet list sorted on GetDepthInMainChain() ascending so pending
transactions came first. An orphaned coinbase is also depth 0 -- and it
stays depth 0 forever, because it is never going to be confirmed.
So every orphan a node had ever produced sat permanently at the top of the
list, above every block it had actually won. A miner with a week of
ordinary orphaned races opened the wallet and saw nothing but failures.
The balance was right the whole time; the list was telling a different
story. That is how this was found: a screenshot of a healthy,
correctly-mining node that looked like it was failing at everything.
Sort on (rank, depth) instead, where rank is 0 for pending, 1 for
confirmed, 2 for orphaned. Orphans go to the bottom, pending stays on top,
confirmed stays ordered most-recent-first.
Verified against a datadir whose chain had been reorganised away, so every
coinbase in it is orphaned: all 17 wallet transactions classify as rank 2.
Not tested against a mixed wallet -- the machine that has one is in
production with its datadir locked by the running node.
|
|
Every miner thread calls RandomXInitDataset at startup. Its only guard is
g_fFast, which is set at the very end -- after the ~2 GB dataset has been
filled, which takes tens of seconds. So all of them sailed past the guard
together and each allocated its own dataset.
Measured in production on a 32-core machine running 31 miner threads: the
log says "initializing ~2 GB dataset" 34 times, and the process holds
65,127 MB of private pages while its own startup line promises 2142 MB
"(~2080 MB shared RandomX dataset + ~2 MB per thread)". The word shared
was not true. A 4-core machine showed the same thing at its own scale: 4
allocations for 3 threads.
The waste was the mild half. g_dataset is assigned the moment memory is
allocated, before it holds anything, so the last thread to allocate swung
the pointer out from under threads still filling their own. A miner could
then hash against a dataset another thread had not finished writing, and
proof of work computed that way is not reproducible by any other node --
the block would be found invalid by everyone, including this node's own
verifier, which uses the cache rather than the dataset.
This only became reachable with multi-threaded mining (#52); with one
thread there was nobody to race.
Holding the lock across the whole initialisation is deliberate: a thread
arriving mid-init has nothing useful to do until the dataset exists, and
blocking is how it waits for exactly that.
Verified with three mining threads: one "initializing ~2 GB dataset", one
"dataset ready", and 2362 MB of private memory against 2086 MB advertised
-- the difference being the 256 MB cache. Before, three threads meant three
datasets.
|
|
relay.nostr.band went down and stayed down -- it resolves and refuses TCP
on 443, from several networks. The seed accumulated 1092 identical error
lines about it.
The log noise was the small half. The discovery loop walks every relay in
turn with a ten-second connect timeout, so a dead relay added ten seconds
to every pass, forever, and that loop is what finds peers -- a relay that
answers nobody was slowing down the ones that answer everybody.
Back off per relay: 30s after the first failure, doubling to a fifteen
minute ceiling, cleared the moment it answers again. Only when an attempt
happens changes; nothing about whether discovery is correct.
Also swapped relay.nostr.band for nostr.bitcoiner.social, on 206.245.132.32
-- neither Cloudflare nor Hetzner, which is the diversity rule the list
already follows.
Verified with a deliberately bogus relay in the list: backoff stepped
30s -> 60s -> 120s and the dead entry produced nine lines in four minutes
instead of one per pass, while the node still connected to five relays,
discovered 54 fresh peers and followed the chain.
|
|
fSpent lives in wallet.dat and is written when this node spends something.
A wallet.dat restored from backup, or copied and used on another machine,
carries whatever that flag was when the copy was taken -- so it can say
"unspent" about coins the chain shows as gone. The balance reads high and
nothing contradicts it until a send is attempted against coins that do not
exist. #47 is about this shape of problem and #40 is someone living through
it; Bitcoin fixed the same thing in 53d508072.
On startup, after the block index is loaded and before anything reports a
balance, walk the wallet and mark spent whatever the transaction index says
is spent.
One direction only. Marking spent when the chain says spent is safe;
clearing the flag because the chain has not caught up would offer up coins
already on their way out.
fSpent is one flag per transaction rather than per output, which is how
0.1.0 has always worked -- CommitTransactionSpent already marks a whole
previous transaction when it spends any part of it -- so this matches
rather than changing the model.
It reports every run, including the runs where it finds nothing, with a
breakdown of why:
RescanSpentFlags() : examined 17, already-spent 0, not-indexed 17, corrected 0
A check that speaks only when it finds something is indistinguishable from
one that never ran, and that confusion has cost this project real time. The
breakdown earned its place immediately: it is how the test below turned
from a guess into an answer.
Verified running against a wallet with 17 transactions and against an empty
one, with no false corrections in either. The correcting branch itself is
NOT exercised by a real case: on this chain no test wallet holds a
transaction that is both in the transaction index and spent, and the
counters show why -- all 17 were absent from the index, so the comparison
never had anything to compare.
|
|
A node that accepted blocks its current rules would reject -- because it
was running an older build, or because the tip it landed on was never valid
-- had no way to notice. It kept the branch, kept building on it, and kept
mining coins the rest of the network throws away. The only cure was
deleting blk*.dat by hand, which requires knowing something is wrong. A
node here once mined a 323-block fork for five days that way.
On load, walk back from the tip running CheckBlock on each block. If one
fails, move the best chain back to its parent, disconnect what came after,
and let the node re-download from peers. Satoshi added this after the block
74638 incident for the same reason: so it would not have needed anyone to
delete files.
Only the last 288 blocks are checked by default -- about ten hours at a
two-minute target -- because every check costs a RandomX hash.
/checkblocks=N changes it, 0 does the whole chain.
The repair deliberately runs in LoadBlockIndex() in main.cpp rather than in
CTxDB::LoadBlockIndex where the detection happens. CTxDB::LoadBlockIndex
walks the index with a cursor it never closes, so it still holds read locks
when it returns, and a write transaction opened alongside it waits on them
forever. That is measured, not guessed: the first version came up, printed
the diagnosis, and hung with nothing in any log. Closing that cursor is a
separate fix worth making on its own.
A failed rollback reports and starts anyway. Refusing to start would leave
the operator with no node to read the message from.
What this catches is what CheckBlock catches: proof of work, merkle root,
duplicate transactions, sigops, timestamps. It does not re-verify
signatures or spends, which would mean rebuilding the transaction index.
Verified by forcing a block at tip-5 to fail: the node reported the bad
block, moved back six, stayed up, and re-synced past its old tip. Without
the forced failure it verifies 288 and says so.
|
|
#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.
|
|
A node running this code cannot tell "in sync with everyone" from "not
hearing anyone". The version message carries no height -- 0.1.0 never had
the field and this fork inherited the gap -- so the only evidence of a
problem is a number on screen that stopped moving, which looks the same as
a network with nothing to report. It is the structural reason a broken node
here has always been indistinguishable from an idle one.
Append nBestHeight to the version message and keep what a peer announced in
CNode::nStartingHeight. The field goes last, so a node that predates it
stops reading after addr and treats the rest as trailing bytes, which cost
it one log line. Reading it is guarded by vRecv.empty() and never required:
peers that do not send it keep nStartingHeight at -1 and are left out of
the median. Every release so far sends the short form.
GetPeerMedianHeight() takes the median rather than the maximum so a single
peer claiming an absurd height cannot move it.
Then use it. After five minutes more than one block behind the peer median,
the node says so in the log -- not behind a debug category, because an
operator who already enabled -debug is not the one who needs telling -- and
the GUI carries the same sentence in its status area. "8 blocks behind for
27 minutes" is something a user can act on; a frozen counter is not.
Verified against two nodes on the live network: the one starting from an
empty datadir read its peer at 3797, reported 2636 then 1274 then 433
blocks behind as it caught up, and went quiet on its own once level. The
other logged "peer height = -1" for the older nodes it met on the real
network, which is the compatibility path doing its job.
Known limit: nStartingHeight is a snapshot from handshake time and goes
stale on a long-lived connection, so this catches a node that is receiving
something but not blocks, not one that has gone completely deaf. The
inactivity timeout from #55 covers that case and refreshes these heights
when it forces the reconnect.
|
|
v1.2.1 disabled the unbounded opcodes and capped a single pushed element at
520 bytes. What it did not add was a ceiling on how many items there can be,
so a script still pays for itself once, in bytes, and then multiplies: 520
bytes per stack entry means a script of N bytes made almost entirely of
OP_DUP occupies roughly 520N in memory, on every node that validates it.
Two limits, both from Bitcoin:
- at most 201 operations per script, counted outside the fExec test so
the cost of a script cannot depend on which branch a signature happens
to take. Pushes are exempt, being already bounded by the script length
and by MAX_SCRIPT_ELEMENT_SIZE.
- at most 1000 entries across both stacks together, counted together
because OP_TOALTSTACK would otherwise move the problem rather than
solve it.
This tightens validation, so a node that does not upgrade accepts a
superset of what this one accepts and is not stranded.
Verified against the live chain: a node with these rules synced all 3910
blocks from genesis and accepted every one, so nothing in use relies on
scripts these limits would reject.
|
|
Nothing in a coinbase belonged to the block it was in. The transaction was a
function of three things: the difficulty, a counter, and the payout key.
nBits holds still for long stretches, and bnExtraNonce is a local that
restarts at zero whenever the miner does, so the same combination comes
round again and produces a byte-identical transaction with the same txid.
Two blocks then name the same transaction. The index keeps whichever
arrived last, and the outputs of the first stop being reachable, with
nothing logged and nothing shown.
It has already happened twice on the main chain -- coinbase c9e56f7131 at
heights 841 and 859, e635f4441f at 845 and 860 -- and 100 BTF are gone
because of it. #58 has the full account.
Putting the height first makes two coinbases at different heights unequal
by construction, whatever the counter and the key do afterwards.
Nothing validates coinbase content today, so this changes only what this
node produces: blocks stay acceptable to every node already on the network,
with no coordination and no activation height. Requiring the height, and
enforcing BIP30 on top of it, is the separate coordinated step that this
one has to come before.
Verified by mining with the patch: the coinbase reads 5104ffff0f1f0101 --
OP_1 for height 1, then nBits, then the counter -- and the scriptSig is 8
bytes against the 100-byte ceiling in CheckTransaction.
|
|
(#55)
A connection whose path dies silently -- NAT drops an idle mapping, a relay
restarts, a route changes -- never delivers FIN. Nothing becomes readable,
no error is raised, and the node simply stops hearing from that peer while
the socket looks perfectly healthy. #51 bounded this for the rendezvous
registration; the connection that comes out of the rendezvous had the same
hole, and so did every ordinary peer.
Stamp nLastRecv and nLastSend on the socket path and act on them:
- nothing at all within 60s of connecting, disconnect. In testing this
fired twice per run against dead .btf descriptors handed out by the
relay, which is where the ghosts have been coming from.
- queued to send for 10 minutes with nothing going out, disconnect.
- nothing received for 30 minutes, disconnect. A healthy peer relays a
block inventory about every two minutes, so this is fifteen intervals
of silence. Bitcoin used ninety minutes against ten-minute blocks.
Disconnecting is the whole cure: reconnection already works, and a peer we
cannot hear is worth what no peer is worth.
A node with nothing to relay would now look dead to a peer applying the
same rule, so send "ping" after ten minutes of having said nothing. It
carries no payload and expects no answer -- arriving is the entire content,
because the receive path has already stamped nLastRecv by the time it is
parsed. Requiring an answer would have read every node predating this
change as dead, since those ignore unknown commands rather than replying.
Verified on the live network: synced 0 to 3790 with five peers and no
spurious disconnects, then again with the interval cut to 20s, which put 54
pings on the wire with no peer dropping us and no exceptions.
|
|
Satoshi left a note above ThreadBitcoinMiner in 2009:
//// todo: start one thread per processor,
//// use getenv("NUMBER_OF_PROCESSORS")
Nobody did, and it stopped being a nicety when the proof of work
became RandomX. SHA-256d on a 2009 desktop lost little to a
single thread. RandomX is built to be fed by every core at once,
so one thread on a 32-core machine mines at a thirty-second of
what the box can do -- and the only outward sign is a CPU that
looks suspiciously idle. A user reported exactly that: 30 minutes
of "Mining", no blocks, Bitflash sitting at 3.3% CPU while the
2 GB dataset was allocated and fast mode was active. Nothing was
broken. It was hashing on one core.
bitflash -gen # every core but one
bitflash -gen -genproclimit=8 # exactly eight
Automatic leaves one core to the rest of the node -- accepting
blocks, talking to peers, drawing the window -- because a node
that mines beautifully while falling behind the chain has won
nothing. /genproclimit overrides it, and a count several times
the core count is refused rather than quietly thrashing.
The wrapper was already built for this: RandomXCreateMinerVM says
"own VM, not shared", and the ~2 GB dataset is one allocation
every thread reads. Only the 2 MB scratchpad multiplies, so
sixteen threads cost about 32 MB more than one. The startup line
says so, because "why is it using 2 GB" is the first question
anyone asks.
Two things had to be right for this to be safe:
- Shutdown. Miners share slot 3 of vfThreadRunning and StopNode
waits for every slot before DBFlush tears the database down.
With several, the first one out announced that everyone had
finished while the rest were still calling ProcessBlock, so
they are counted now and only the last one clears the flag.
The three CheckForShutdown(3) calls inside the miner became
plain returns: _endthread() does not unwind on Windows, so
the bookkeeping would have been skipped there.
- Duplicated work. Each thread makes its own key, and the
coinbase pays it, so every thread has a different merkle root
and searches a different space. Block assembly was already
under cs_main and cs_mapTransactions; AddKey and ProcessBlock
take their own locks.
Verified on both platforms: four distinct thread ids reach the
hashing loop and all report back on shutdown, which exits 0 with
DBFlush running. Auto picks cores-1 (1 on a 2-core box, correctly
reported). Speedup itself needs a many-core machine to show and
has not been measured here -- the claim in this message is about
threads doing work, not a benchmark.
Not touched: RandomX's memory. RANDOMX_DATASET_BASE_SIZE and
SCRATCHPAD_L3 are part of the hash function, so a knob for "use
more RAM" would either do nothing or fork the chain. Threads are
the only real dial.
|
|
A node registers at a relay and waits to be dialled. The wait
had no bound, and ConnectTo() clears the socket timeout on
purpose -- a registration is idle by design and may sit for a
long time.
What that cannot survive is the path dying with neither end
saying so, which is the ordinary case and not the exotic one:
a NAT drops the idle mapping, a relay restarts, a route
changes. No FIN arrives, nothing becomes readable, and recv()
never returns. The thread stays alive and asleep, so
ThreadBtfAccept never reaches the `continue` that would
register again. The node is unreachable and every outward sign
says it is fine.
Caught on the bootstrap seed: 27 minutes without a block,
eight behind the network, relay reachable the whole time, and
a restart fixing it instantly. Nothing in the log marked the
moment it went deaf.
I tried TCP keepalive first and it does not work here. With
probes every 5s and a 2-probe limit, a socket whose replies
were being dropped was still ESTAB after four minutes with the
probe counter stuck at zero. It is measured, not assumed, and
that approach is not in this change.
So the bound is ours to enforce, at the one place that knows
what waiting means. RvServiceWaitPaired takes a timeout;
expiring is not an error but "nobody came, go round again",
which the loop above already handles. Cost is one reconnect
per relay per five minutes.
Verified by killing a registration's path silently -- dropping
only the relay's replies, so no RST or FIN is ever seen. With
a 60s bound for the test:
rendezvous: no dial at 92.246.128.180:8434 within 60s
(or it dropped us), re-registering
Registrations went 1 -> 2 and the node carried on. Before this,
the same treatment left it blocked past four minutes with no
log line at all.
The socket goes back to blocking once a dial lands, because
from that point it is a data tunnel and a tunnel is allowed to
sit quiet between messages.
|
|
The last door out. Everything else assumes Berkeley DB will
cooperate. When it will not -- a build that cannot read the
file, a truncation, an environment past recovering -- the keys
are usually still perfectly intact and were unreachable
anyway. #40 is one person's account of exactly that, and #46
only rescues the cases where BDB still opens the file at all.
bitflash -dumpwallet=keys.txt
bitflash -importwallet=keys.txt
One key per line with its address and label. No database, no
environment, no version coupling: anything that can read a
line can recover from it.
The format is the DER private key in hex, which round-trips
exactly through SetPrivKey. Not interchangeable with other
wallets -- worth having eventually, but a WIF encoder I got
subtly wrong would be worse than no encoder at all, and this
file exists for the case where nothing else works.
Care taken, because the failure modes here cost money:
- Refuses to overwrite an existing file. A mistyped path
must not eat the only copy of something irreplaceable.
- Written owner-only on POSIX, before any key reaches it.
Windows inherits the folder's ACL; the README says so
rather than pretending otherwise.
- Newlines in labels are flattened, so a label cannot forge
an extra entry on read-back.
- Import skips keys already present, so running it twice
does nothing, and one unreadable line is reported and
stepped over instead of costing the reader every other key
in the file.
Verified on Linux and Windows: a key exported from one wallet
imports into a different one and its address then appears
there; re-importing adds nothing; a corrupted line is skipped
while the rest still load; the dump is 0600 and refuses to
overwrite.
This does not solve #47. Keys still are not derived from a
seed, so a dump is as much a snapshot as any other backup.
What it does is make the keys reachable no matter what state
the database is in.
|
|
* wallet: make wallet.dat a file you can actually back up
Issue #40: a wallet.dat copied out on its own will not open
anywhere else. The file is intact -- same bytes, same keys --
and the node refuses it with "Db::put: Invalid argument",
thrown from a path that aborted the process. The reporter
believes they lost funds to this, having saved the one file
anybody would think to save.
The chain, from the bottom:
_endthread() is pthread_exit() on POSIX, and glibc implements
that by throwing abi::__forced_unwind through the stack. That
is not a real exception; catching it without rethrowing is a
fatal error by contract. CATCH_PRINT_EXCEPTION ended in a bare
catch(...), so every Linux shutdown ended:
UNKNOWN EXCEPTION bitflash-node in ThreadSocketHandler()
FATAL: exception not rethrown
An abort, mid-shutdown, so nothing after StopNode() ran.
Which mattered because Berkeley DB stamps every page with a
log sequence number belonging to the environment that wrote
it. Carried elsewhere, those numbers point at logs that do not
exist and BDB refuses to write. DBFlush(true) calls lsn_reset,
which is precisely the cure -- and DBFlush has been declared
since 2009 and called from nowhere. Even had it been called,
the abort stood in front of it.
So the wallet was never released from its directory, and the
one file users copy was the one file that could not travel.
Three fixes, each covering a different case:
- Rethrow abi::__forced_unwind, and call DBFlush(true) on
both shutdown paths. Shutdown completes, and wallet.dat is
left portable by construction.
- Adopt orphaned files on open: a freshly created database
environment beside pre-existing .dat files means they came
from elsewhere, so reset their sequence numbers. This is
what recovers wallets already stranded by older builds --
including, if they still hold the file, the reporter's.
- BackupWallet(), reachable as /backupwallet=FILE, which
checkpoints, copies, and clears the copy's sequence
numbers. Bitcoin 0.1.0 had a backup command; this tree
lost it in the wxWidgets removal and has shipped with no
way to take a backup since.
DBFlush's own txn_checkpoint and lsn_reset are wrapped too --
they throw, and throwing on the way out is how this started.
Verified on Linux and Windows: shutdown exits 0 rather than
134, DBFlush runs, and a lone wallet.dat opens in an empty
directory with no exceptions and byte-identical key records
(defaultkey, key, name, setting, version) before and after.
Not addressed here, and the larger half of #40: keys are
generated as they are needed rather than derived from a seed,
so any backup is a snapshot and coins paid to addresses
created after it are not in it. /backupwallet says so on every
run. The real answer is deterministic derivation, which is a
wallet format change and its own piece of work.
* README: document what a wallet backup actually requires
The PR description claimed this section existed and it did not.
Covers the two things that cost money and are not obvious: a lone
wallet.dat is refused elsewhere because Berkeley DB ties it to the
database/ directory beside it, and every backup is a snapshot
because keys are made on demand rather than derived from a seed.
|
|
The segfault in issue #5 was a format string: `%I64d` is MSVC-only,
glibc parses it as the GNU `I` flag with a width of 64 and an `int`
conversion, the vararg run desynchronises, and the following `%s`
dereferences an integer. It killed the node mid-sync at the first
block carrying a real transaction, deterministically.
GCC can catch this. It never got the chance, for two reasons that
had to be fixed together:
- `-w` sat in both makefiles and suppressed every diagnostic. It
cannot be narrowed; it wins over any -W or -Werror= that follows,
so it had to go. The four -Wno- entries replacing it are the
categories this 2009-era tree actually produces (measured: 248
deprecated-declarations, 157 write-strings, 18 narrowing, 1
unused-result), silenced by name instead of wholesale.
- Even with -Wformat on, nothing was checked. GCC only inspects
format strings for functions it knows are printf-like, and every
format string here goes through strprintf, error, my_snprintf, or
OutputDebugStringF -- `printf` itself is #defined to the last of
those. None carried a format attribute, so the entire tree was
invisible to the check. They are annotated now.
Annotating surfaced 77 warnings across 17 distinct sites, all fixed
here. Most were size_t or int64 passed to %d, harmless on x86-64 but
wrong. Two were not cosmetic:
- CBlockIndex::ToString printed pprev/pnext with %08x, truncating
both pointers to 32 bits, so the two fields that line exists to
show were unreliable on every 64-bit build.
- CInv::GetCommand threw "type=% unknown type", where the % ate the
space as a flag and the u as its conversion. The message read
"type= 3nknown type".
PRId64 is now one spelling for every target. Its MSVCRT branch was
what produced "I64d" in the first place, it is pointless against UCRT
which takes the C99 forms, the rest of the tree already prints int64
with %lld on Windows, and its only user (i64tostr) is dead code.
Collapsing it lets both builds share the gnu_printf archetype --
ms_printf would reject %zu, which this tree uses and UCRT supports,
while waving through the %I64d that crashes.
Verified on both platforms: clean build, and reintroducing the
issue #5 specifier now fails compilation rather than shipping.
main.h:410:50: error: unknown conversion type character 'I' in format
main.h:410:70: error: format '%s' expects argument of type 'char*',
but argument 3 has type 'long long int'
That second line is the crash itself, caught at build time.
Runtime checked on both: node syncs past the block from issue #5, and
the repaired log lines read correctly --
before: CTxOut(nValue= 50.%08I64d, scriptPubKey=(null))
after: CTxOut(nValue=50.00000000, scriptPubKey=0xE1C7EF3E706002)
Four '_WIN32_WINNT redefined' warnings remain on Windows. Silencing
them means letting a system header pick the Windows API level instead
of this tree, which is a separate decision from format checking.
|
|
nMineMode defaults to MINE_RELAY (0) and ParseStartupArguments() never changed
it, so a node started with /gen would spawn ThreadBitcoinMiner, enter
BitcoinMiner(), hit
if (nMineMode == MINE_RELAY)
return true;
and return before doing any work. The node then ran indefinitely looking
completely healthy -- it synced, it held peers, it reported no errors -- while
earning nothing. Nothing in the log said otherwise; the only hint was
`nMineMode = 0` in a startup line that reads like a status echo.
This affects every headless miner, because the GUI is the only other way to
select a mining mode.
/gen now implies MINE_SOLO unless /operator or /participant asked for something
else. Verify with `nMineMode = 1` at startup.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
|
ByteReverse(word32) in src/sha.h uses the x86 `bswap` instruction via inline
asm, guarded only by `#if defined(__GNUC__)`. GCC and Clang define __GNUC__ on
every architecture they target, so on aarch64 the guard passes and the
assembler then rejects the instruction:
Error: unknown mnemonic `bswap' -- `bswap %eax'
This makes the tree fail to build on any ARM64 host -- Apple Silicon, ARM
servers, a Raspberry Pi -- at a point far from anything the user did.
Narrow the guard to the architectures where the instruction exists. The
portable C fallback directly below is then used everywhere else; it was
already there and already correct.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
|
Berkeley DB throws when it cannot open a file written by
another platform's build (e.g. BDB 5.3 wallet read by 6.2).
Nothing on the call path caught these, so the exception
unwound into std::terminate -- surfacing on Windows as
STATUS_STACK_BUFFER_OVERRUN (0xC0000409) with no useful
message.
Three layers of defense:
1. main_gui.cpp: try/catch around LoadBlockIndex() and
LoadWallet(), printing the reason and exiting cleanly.
2. db.cpp LoadWallet: try/catch around the record-reading
loop (Satoshi's 2009 TODO), tracking the last record type
so the error message says what it choked on.
3. db.cpp cleanup paths: every dbenv.close(), pdb->close(),
txn_checkpoint() and log_archive() wrapped in try/catch
so the environment teardown after a failed open does not
throw a second exception into std::terminate.
Tested: cross-platform wallet (Linux BDB 5.3 → Windows 6.2)
now prints a clear message and exits with code 1, wallet
left untouched.
Closes #34
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|
|
The GUI build requires libGL at load time even when started with -nogui. The
dynamic loader resolves every dependency before main() runs, so the flag arrives
far too late -- on a clean server the binary dies with
error while loading shared libraries: libGL.so.1: cannot open shared object file
and nothing suggests it would otherwise have worked. A server is where a node
most naturally lives, so this is the deployment that mattered most and the one
that failed. The AppImage additionally needs FUSE, which minimal servers lack.
Bundling libGL into the AppImage would be the wrong fix: GL has to come from the
host's driver stack, and shipping our own breaks machines that have a real GPU.
So: a second binary, built from the same node objects without ImGui, GLFW or
OpenGL. `make linux-node` produces bitflash-node-<version>-x86_64, 2.5 MB
stripped, with no graphics or FUSE dependency at all. `make linux` now builds it
alongside the AppImage. Anyone wanting a window keeps using the AppImage.
Two small moves were needed. MainFrameRepaint is called from main.cpp and
net.cpp but defined in gui.cpp, so headless.cpp provides an empty one rather
than putting ifdefs at six call sites. DateTimeStr also lived in gui.cpp while
main.cpp logs block times with it -- nothing about formatting a date is GUI
work, so it moved to util.cpp and both builds share one definition.
Verified on Debian 13:
ldd bitflash-node | grep -iE 'libGL|glfw|X11' -> nothing
ldd bitflash | grep -iE 'libGL|glfw' -> libglfw.so.3, libGL.so.1, libGLdispatch.so.0
and at runtime, from an empty datadir:
btfseed: reached ygffz37...btf via 90.156.222.107:8434
rendezvous: registered at 90.156.222.107:8434, waiting for a dial
self-resolve OK (meeting_node=90.156.222.107:8434)
btfpeers: kept 15 of 15 descriptor(s)
first block at ~30s, 3182 blocks accepted, synced to the tip
The GUI builds on Linux and Windows were rebuilt and are unaffected.
|
|
* net: compiled-in bootstrap seeds, so a first run does not need a relay
A node with no cache has only the Nostr relays. If they are down, blocked or
slow, a first run has no way into the network at all -- and that is the one
moment a user has no patience for. Seeds are the floor under that.
A seed entry is an address and an encryption key. Deliberately no meeting node:
the rendezvous relay pairs on the service's public key, and a `.btf` address is
that key, so a client can find a seed by trying the known relays in turn.
Recording which relay a seed was on would rot the moment it failed over, which
is exactly the failure this project spent weeks chasing -- advertising a
rendezvous somebody is not registered at.
Seeds carry no authority. They serve the same signed, self-certifying
descriptors as any peer, so a hostile seed can stall a bootstrap but cannot
forge a peer or feed a false chain.
The compiled list is empty. Which nodes become permanent public bootstrap
targets is a decision for whoever runs the network, not a default, and it is
close to irreversible once binaries are distributed. Empty is valid: discovery
falls back to Nostr exactly as before. /btfseed=ADDRESS:ENCHEX (repeatable)
appends at runtime, for testing and for private networks.
Tested against the live network, clean datadir, no cache, three seeds carrying
no meeting node:
btfpeers: no cache yet, falling back to seeds
btfseed: trying 3 seed(s) across 3 relay(s)
btfseed: reached q5jxyzn...btf via 90.156.222.107:8434
btfseed: 3 seed(s) answered
* net: add the Almaty bootstrap node to the seed list
Dedicated node, no wallet balance, no mining, running beside the rendezvous
relay on the same host. It exists only to answer a first dial.
Fresh datadir, no flags, compiled list only:
btfpeers: no cache yet, falling back to seeds
btfseed: trying 1 seed(s) across 3 relay(s)
btfseed: reached ygffz37...btf via 90.156.222.107:8434
btfseed: 1 seed(s) answered
btfpeers: kept 2 of 2 descriptor(s)
5.2 seconds from nothing to a peer, against 36s for a cold start that had to go
through the relays. The seed then handed over two more peers by exchange, which
the cache keeps for next time.
|
|
Discovery has depended entirely on the Nostr relays. Two connected nodes never
told each other who else existed, so a relay outage left a running network
unable to grow, and a node that found one stale peer had no way to learn there
was anything better. The old `addr` gossip went out with the IRC seed.
Peers now trade signed descriptors over a new "btfpeers" message: our own
first, then peers that actually answered us. The receiver checks each Schnorr
signature against the key the `.btf` address decodes to, so the sender is
trusted for nothing -- forging an entry needs somebody else's secret key. That
leaves flooding with self-generated keys, which the per-message caps and the
"peers that answered" ordering are there to blunt.
Unknown commands are ignored (main.cpp), so nodes without this just drop the
message; nothing here is a protocol break.
The cache from the peer-cache change now keeps each peer's descriptor verbatim
alongside the parsed fields, because that signature is what makes a peer
relayable. Entries without one -- resolved through Nostr, or written before
this -- stay dialable, just not passed on. A dial never clears a descriptor
already on file.
|
|
A node could not become reachable. RvServiceRegister connected, sent its
'S'+pubkey header, and then blocked reading the relay's 0x01 byte -- which the
relay only writes when a client pairs with the service. So the call did not
return when the node was registered; it returned when somebody had already
dialled it.
ThreadBtfAccept called BtfSetActiveRelay after that. The result is a deadlock:
- to be dialled, a node must publish a descriptor naming its meeting node
- to publish, BtfActiveRelay() must be set
- it is only set once somebody has dialled
Before #26 this was masked. BtfActiveRelay() fell back to
vBtfMeetingRelays[0], so a node always published something, and ThreadBtfAccept
picks its relay at random -- so it worked whenever the two happened to match,
roughly one time in the number of relays. That is the flakiness this project has
been chasing: not a bad relay, an advertisement that usually named the wrong one.
#26 removed the fallback, which was right on its own terms and turned an
unreliable heuristic into a reliable deadlock. This is the other half of that
change and they belong together.
RvServiceRegister now returns as soon as the relay has the service listed.
RvServiceWaitPaired does the blocking read. ThreadBtfAccept registers, advertises,
then waits. RvClientConnect is untouched -- a dialer blocking until it is paired
is correct.
Registration failure also had no log line at all, which is why this stayed
invisible: a node that never registered looked identical to one waiting quietly
for a peer. There are now lines for registering, failing over, and being dropped.
Verified: a fresh node reports `registered at 90.156.222.107:8434, waiting for a
dial` and then self-resolves with `meeting_node=90.156.222.107:8434` -- the relay
it is actually on. Two nodes on one machine reach each other.
|
|
OutputDebugStringF resolved its path lazily with no lock:
static string strDebugFile;
if (strDebugFile.empty()) { ... strDebugFile = dir + "\\" + "debug.log"; }
FILE* fileout = fopen(strDebugFile.c_str(), "a");
if (fileout) { ... }
One thread could read that string while another was still assigning to it, and
at startup three threads log within milliseconds of each other. When it raced,
fopen received a torn path, returned NULL, and the `if (fileout)` dropped the
line with no error anywhere. Several threads calling fopen on the same file at
once had no protection either.
Measured over six runs, counting first-statement printfs that are unconditional
and must appear every time:
ThreadBtfAccept started 4/6
ThreadNostrSeed started 5/6
btfpeers: (cached peers) 2/6
Over eight runs after the change, all three are 8/8.
This matters beyond tidiness. A missing line was read as a thread that never
started, and a real investigation was spent on it. Any reasoning of the form
"it is not in the log, so it did not happen" was unsound against this build.
Path resolution and the write now share one critical section. The rest of the
function already had its own for the OutputDebugString accumulation path; only
the file half was unguarded.
|
|
net.cpp has described nListenPort as "tunable via /port" since it was written:
unsigned short nListenPort = DEFAULT_PORT; // local P2P port (tunable via /port)
Nothing ever parsed the option. The port was fixed at 8433, so a second node
could not start on a machine already running one -- StartNode fails the bind and
the node comes up with no networking at all. That makes local multi-node testing
impossible, which is how this was found: a two-node test could not be run.
Sets addrLocalHost.port alongside nListenPort, or the node announces a port it
never bound. An out-of-range value is rejected with a message and the default
kept, rather than silently wrapping through the unsigned short cast.
|
|
PR #25 was opened, reviewed and approved as a single-file change to net.cpp:
the .btf peer cache, +169/-0. I then pushed peer exchange and a /port option
onto that same branch while the PR was open, which silently widened it to six
files. The merge carried all of it in.
Nothing about the peer exchange had been validated. Its two-node test never
completed -- node B could not dial node A at all -- so what landed on main is
unproven network protocol code that no one agreed to take.
This restores main to what #25 actually described. The peer cache stays, with
its runtime numbers (36.3s to 2.1s to first peer). #26 stays. What comes out:
- the "btfpeers" message, both directions
- BtfPexCollect / BtfPexAccept and the per-message caps
- CNode::nLastPexRecv
- the `desc` field on the cached peer record
- BtfLocalDescriptor / BtfSecpContext
- /port
Peer exchange returns as its own pull request once its test passes. The /port
option is worth having on its own -- two nodes cannot share a machine without
it, which is what blocked the test -- and should go up separately rather than
ride along again.
Release v1.2.2 is unaffected: it was built from ba2de82, before either merge.
|
|
* net: remember .btf peers that answered, and dial them first on start
Discovery depends entirely on the Nostr relays. A node that has been up
for hours knows exactly which peers respond, and throws all of it away on
exit; the next start reaches for the relays again and walks the same
descriptor list, of which roughly 96% is dead. That is what makes a
restart take minutes before the first tunnel opens -- measured today at
about seven.
Every peer that completes a tunnel is now written to btfpeers.json with
its meeting node and encryption key, which is everything needed to redial
it. On start a thread tries those before the relays have answered
anything, stopping once eight are connected. Entries older than a week
are dropped, and the file holds at most fifty.
Cache only: nothing here changes how a first-ever run bootstraps, and a
missing or corrupt file just falls through to relay discovery.
* net: exchange signed .btf descriptors between peers
Discovery has depended entirely on the Nostr relays. Two connected nodes never
told each other who else existed, so a relay outage left a running network
unable to grow, and a node that found one stale peer had no way to learn there
was anything better. The old `addr` gossip went out with the IRC seed.
Peers now trade signed descriptors over a new "btfpeers" message: our own
first, then peers that actually answered us. The receiver checks each Schnorr
signature against the key the `.btf` address decodes to, so the sender is
trusted for nothing -- forging an entry needs somebody else's secret key. That
leaves flooding with self-generated keys, which the per-message caps and the
"peers that answered" ordering are there to blunt.
Unknown commands are ignored (main.cpp), so nodes without this just drop the
message; nothing here is a protocol break.
The cache from the peer-cache change now keeps each peer's descriptor verbatim
alongside the parsed fields, because that signature is what makes a peer
relayable. Entries without one -- resolved through Nostr, or written before
this -- stay dialable, just not passed on. A dial never clears a descriptor
already on file.
* main_gui: implement /port, which net.cpp already documented
net.cpp has said nListenPort is "tunable via /port" since it was written, but
nothing ever parsed the option. Two nodes could not share a machine, which is
exactly what testing peer exchange needs. Sets addrLocalHost.port too, so we
do not advertise a port we never bound.
|
|
BtfActiveRelay() fell back to vBtfMeetingRelays[0] whenever registration had
not completed yet. ThreadNostrSeed and ThreadBtfAccept start in parallel
(net.cpp), so at boot that fallback was exactly what PublishDescriptor wrote
into the descriptor -- including the one-time self-test, which publishes to
every relay in the list. Peers reading it dialed a rendezvous this node was
never registered at, and whenever the first seed happened to be down
(92.246.128.180 was, for part of today) every one of those dials was wasted.
BtfActiveRelay() now returns only the relay ThreadBtfAccept confirmed, and
PublishDescriptor skips publishing until there is one. The seed loop
republishes on every pass, so the only cost is the delay until registration
succeeds; the self-test waits up to 60s for it so it still measures the
publish/resolve chain instead of losing the race against it.
Also fix the WebSocket handshake reader. On headers past the cap it did a
bare break, leaving the socket parked in the middle of the header block --
and since the status line had already been read, the " 101 " check still
passed, so the caller got a "connected" socket whose next bytes were
leftover HTTP that the frame parser then read as garbage frames. It now
errors out instead, and the cap is 8192 because relays behind Cloudflare
send large header blocks.
|
|
MAX_SIZE bounds a block's bytes, not what it costs to check. Every
OP_CHECKSIG is an elliptic-curve verification, and blocks may be 32 MB
here, so one packed with signature operations takes every node minutes of
CPU to reject -- while at this chain's difficulty it stays cheap to mine.
There was no counter at all.
CheckBlock now sums signature operations across the block and rejects
past 20000, the figure Bitcoin settled on. OP_CHECKMULTISIG counts as 20
since it may verify that many.
Consensus change. Real blocks here carry one or two operations each --
coinbase pay-to-pubkey and standard pay-to-pubkey-hash spends -- so the
ceiling is four orders of magnitude above anything in the chain, but it
still needs verifying against the full history before merge.
|
|
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.
|
|
The node connects to public Nostr relays it does not run, and the
WebSocket reader trusted them completely.
A frame's payload length is read from the wire -- up to 64 bits when the
extended form is used -- and passed straight to string::resize(). One
frame header claiming 2^40 bytes is enough to make the node allocate
until it dies. Reassembly across continuation frames had no ceiling
either, so the same result was reachable with small frames. Frames are
now capped at 1 MiB and a reassembled message at 4 MiB; the frame cap
also keeps the length inside the int that ReadN takes, which a 64-bit
value would otherwise have narrowed, possibly to a negative.
Six message loops then did:
if (!j.is_array() || j.empty()) continue;
string type = j[0].get<string>();
j[0] was never checked to be a string, so a relay answering with [1]
threw an uncaught json::type_error and took the thread down. Same shape
as the descriptor bug fixed in #12, in six more places.
Nothing here is consensus -- this is the relay-facing edge only.
|
|
A transaction naming a parent the node does not have is never validated,
just stored in mapOrphanTransactions -- and there was no ceiling. Sending
them costs a peer nothing: no proof of work, no valid signature, no fee,
and the node keeps every one. Any peer could grow that map until the node
died of memory exhaustion.
Cap at 100, evicting at random rather than in map order so an attacker
cannot keep its own orphans resident by choosing hashes that sort low.
Local policy, not consensus -- no effect on block or transaction
validity.
|