Fork summary: signed auto-update, headless dashboard, pool fallback, keypool, and more -- integrated through #46 - #50
Open
Vic-Nas wants to merge 272 commits into
Open
Fork summary: signed auto-update, headless dashboard, pool fallback, keypool, and more -- integrated through #46#50Vic-Nas wants to merge 272 commits into
Vic-Nas wants to merge 272 commits into
Conversation
…aner options dialog; update README
…sh#37); fix /gen not actually mining (port of Bitflash-sh#42) Two real gaps, both pulled from upstream, both squarely in "the command line has been neglected" territory. ## Headless build (Bitflash-sh#37) The GUI binary needs libGL present at load time even when started with -nogui/-daemon: the dynamic loader resolves every dependency before main() runs, so the flag arrives far too late. A clean server -- where a node most naturally lives -- fails outright with a shared- library error that gives no hint the program would otherwise work fine. The AppImage additionally needs FUSE, which minimal servers lack. `make linux-node-build` (wired into both `linux` and `linux-self`) produces a second binary, `$(APPNAME)-node-<version>-x86_64`, from the same node objects, without ImGui/GLFW/OpenGL at all -- no graphics or FUSE dependency. Two small moves were needed: headless.cpp provides an empty MainFrameRepaint() (declared in headers_core.h, previously only ever defined in gui.cpp, which a headless build doesn't compile), and DateTimeStr moved from gui.cpp to util.cpp since it's not actually GUI-specific. Adapted rather than copied: our src/Makefile carries extra objects (nostr/compactblock/update) and the mandatory REPO/UPDATE_PUBKEY (or NO_UPDATE=1) machinery upstream doesn't have. Caught one real bug while wiring the root Makefile's linux-node-build target: forwarding NO_UPDATE unconditionally (even as an empty value) would have made `ifdef NO_UPDATE` true in the child invocation regardless -- GNU Make's ifdef checks whether a variable was set at all, not whether it's non-empty -- silently disabling the updater in every node build, including regular (non-self) ones. Branched explicitly instead. ## /gen not mining (Bitflash-sh#42) nMineMode defaults to MINE_RELAY and nothing in ParseStartupArguments ever changed it for the plain /gen case (only /operator and /participant set their own mode) -- so a node started with /gen alone spawned ThreadBitcoinMiner, hit BitcoinMiner()'s relay guard, and returned immediately. The node ran indefinitely looking completely healthy -- synced, held peers, no errors -- while mining nothing, with the only hint being nMineMode = 0 in a startup line that reads like a status echo. This affects every headless miner, since the GUI was the only other way to select a mining mode. /gen now implies MINE_SOLO unless /operator or /participant asked for something else.
…RM64 build (port of Bitflash-sh#41); gitignore: cover key files (port of Bitflash-sh#43) ## Bootstrap seeds (Bitflash-sh#36) A node with an empty cache has only the Nostr relays for discovery -- if those are down, blocked, or just slow, a first run has no way into the network at all, which is exactly the moment a user has the least patience. Seeds are the floor under that: a compiled-in list of long-lived peers, tried automatically only when the peer cache is empty. A seed entry is deliberately just an address and an encryption key, no meeting node -- the rendezvous relay pairs on the service's public key, and a .btf address is that key, so a seed is findable by walking the known relays. Recording which relay a seed was on would rot the moment it failed over, which is the exact "advertising a rendezvous nobody is registered at" failure this project has chased before. Seeds carry no special authority: they hand out the same signed, self-certifying descriptors any peer does, so a hostile seed can stall a bootstrap but can't forge a peer or feed a false chain. The compiled list here is empty, deliberately -- upstream shipped their own dedicated bootstrap node's address in their version of this commit, but that's their infrastructure, not something this fork has any relationship to or authority to point users at. /btfseed= ADDRESS:ENCHEX (repeatable, CLI-only) covers testing and private networks without needing a compiled entry. Adapted the hex-decode call to our own BtfPeerCacheHexToBytes rather than a generic HexToBytes upstream doesn't have under that name. ## ARM64 build failure (Bitflash-sh#41) ByteReverse(word32) in sha.h used the x86 bswap instruction via inline asm, guarded only by #if defined(__GNUC__) -- true on every architecture GCC/Clang target, so on aarch64 the assembler rejected the instruction outright. The 64-bit ByteReverse right below it already had the correct guard (&& defined(__x86_64__)); the 32-bit one just never got it. Fails the build on Apple Silicon, ARM servers, or a Raspberry Pi -- all plausible headless-node hosts. ## .gitignore gaps (Bitflash-sh#43) *.dat/wallet.dat were covered, but btf_enc.key (this node's .btf identity) and nostr.key (its Nostr signing key) are not .dat files and are exactly as sensitive -- already listed explicitly in our .gitignore from an earlier session, but walletrpc.token (bearer authority to spend from the wallet over the wallet API) was missing entirely, along with a general *.key catch-all and patterns for key backup archives, mempool dumps, and stray .patch/.rej/.orig files. A node run with its data directory inside a working tree -- the obvious thing to do while developing -- would otherwise leave all of this untracked but committable.
StartUpdateCheck only ever ran from inside RunGUI, so a node started with -nogui/-daemon (or the new headless bitflash-node binary) never checked for an update at all -- and that's exactly the deployment most likely to run unattended for a long stretch, which is when missing a critical update matters most. No modal to show in a log-only context, so it's a plain log line instead: same 30-minute interval the GUI uses, checked inline in the existing sleep loop rather than a new thread. A no-op on a -self build, same as everywhere else Update_CheckLatest is called (it's a stub there).
…ith the GUI Both settings were only ever changeable from the GUI's Options dialog -- a headless node was stuck with a randomly generated nickname forever and a transaction fee of 0 with no way to override either without a window it doesn't have. Persisted the same way the GUI does (CWalletDB::WriteSetting), so a later GUI session on the same wallet.dat picks up whatever was set here.
…ash-sh#40) BitcoinMiner() called key.MakeNewKey() once per mining session and paid every block's reward to that key -- generated fresh, with no backup covering it until AddKey() ran, and that only happened *after* a block was successfully found. Two real ways to lose funds: 1. A wallet.dat backup taken before mining starts doesn't contain this key. Mine blocks, restore that backup later, and the reward is unrecoverable even though the coins are still on-chain. Reported exactly this way against this codebase in Bitflash-sh#40. 2. AddKey() only ran after a block was found and about to be processed. A crash between broadcasting the block and that save meant the reward's private key existed only in memory and was simply gone -- no backup/restore needed to trigger this one. Both are the same class of bug the keypool already exists to prevent, just never applied here. vchMinerPubKey = GetKeyFromPool() instead: the key is durably in wallet.dat (via TopUpKeyPool's AddKey, already run before the key is ever handed out) from the moment mining starts, not from the moment a block happens to be found. The old save-then-generate-next block after a found block is now just draw-the-next-one, since there's nothing left to save at that point.
Was badly stale: referenced v1.1.0 filenames, a hardcoded 'Bitflash-*' naming pattern that hasn't been true since release automation shipped, an 'Options' menu that's been renamed to 'Mine', and build instructions (plain 'make linux'/'make windows') that fail outright now that REPO/UPDATE_PUBKEY are mandatory -- someone following this doc today would hit a wall immediately. Also added real content that was never documented at all: the headless bitflash-node binary (no libGL dependency, the actual recommended path for a server), the periodic signed-update-check system and how it fails closed, the address book/peers panel, node nicknames, and current CLI flags. And a prominent backup warning (Bitflash-sh#40): wallet.dat alone isn't a safe backup without its database/ subfolder, and a backup is a point-in-time snapshot, not something that stays valid forever.
Cut from 173 to 111 lines. Kept: what it works like, how to build, mining modes, headless deployment, the backup warning. Dropped the separate Peers/Updates sections as their own headers -- folded the one-line versions of each into Quick start and Headless instead.
Chose the lower-risk path over adopting a TUI library (FTXUI/ cpp-terminal were the two real candidates, both cross-platform and well-maintained, but pulling either in means integrating a new dependency into two separate build systems for a display-only feature). Raw ANSI escape codes instead: cursor-home + per-line clear each redraw, no full-screen flicker, works on any terminal that understands ANSI (which is everywhere this app already targets -- Linux, macOS, and Windows 10+ consoles once VT100 processing is enabled per-handle, done here via SetConsoleMode). Shows height, peer count, mining mode/status, and -- for participant mode -- owed balance, round shares, and the overdue-payout warning already built earlier this session; for operator mode, miner count and blocks found this session. Update-check status folds in too, same 30-minute background check as before, just displayed instead of only logged. Auto-detects whether stdout is an actual interactive terminal (isatty/_isatty) and falls back to the existing plain scrolling log lines otherwise -- a systemd service, a pipe, anything redirected to a file gets exactly the old behavior, since ANSI redraw codes in a log file would be either noise or literally corrupted-looking text, and nobody's watching a redirected stream live to read a dashboard anyway. Real risk this could have introduced: background threads' printf/ LogPrint already echo to the console on non-Windows (confirmed in util.h), which would interleave with and corrupt an in-place- redrawing display. Added g_fSuppressConsoleEcho, checked right at that echo site -- set only while the dashboard owns the screen, so debug.log keeps getting every line as always, console output just comes from the dashboard alone during that time.
…ndored, MIT) The previous version was raw ANSI escape codes with no real cross-platform testing behind it -- exactly the kind of one-off terminal-handling code that looks fine in review and breaks on a real Windows console (a failed VT100 enable had no fallback, would have shown literal escape-code garbage on screen instead of a dashboard). Correctly called out as the wrong trade-off: I'd avoided a new build dependency at the cost of correctness on the actual thing being built. Vendored FTXUI's official amalgamated release (v7.0.1, MIT, from GitHub Releases -- same integration shape as ImGui already gets in this repo: two files, no CMake, no package manager) instead. It's packaged for Debian/Ubuntu/Arch/vcpkg/Conan and used in exactly this category of app already (resource monitors, CPU meters), which is the actual point: battle-tested where the hand-rolled version wasn't. Confirmed by actually building and running a test program against the vendored files before committing to this, not just reading docs. src/dashboard.cpp uses FTXUI's real DOM API (vbox/hbox/text/ separator, Screen::ResetPosition for in-place redraw) instead of raw escape sequences, and needs none of the manual Windows console-mode code the old version had -- FTXUI handles that internally (confirmed in its vendored source, and by the test program running correctly with zero setup). FTXUI requires C++17; the rest of this codebase stays gnu++14. FTXUI_CXXFLAGS derives a C++17 variant of the existing flags (same defines/includes, just the standard swapped) so only ftxui.cpp and dashboard.cpp build under it -- nothing else changes standard. Wired into both Makefiles identically (Linux and Windows/MSYS2), added to NODE_OBJS so both the GUI binary (-nogui/-daemon at runtime) and the headless bitflash-node binary link it. Actually compiled this, not just inspected it -- installed the missing dev packages in this sandbox to get a real build, which caught a real bug before it could ship: `using ftxui::Dimension;` is invalid (Dimension is a namespace, not a type), which would have failed to build outright. Fixed by qualifying ftxui::Dimension::Full() directly at its one call site. Confirmed clean compiles across all three build variants (with-updater, -self, and BITFLASH_NO_GUI) plus spot-checked that untouched core files (main.cpp, net.cpp, rpc.cpp, util.cpp) still compile cleanly after the util.h change from the previous pass (g_fSuppressConsoleEcho, still needed: FTXUI doesn't know about this codebase's own console-echo path in OutputDebugStringF, so background threads' log lines would still interleave with and corrupt the redraw without it).
… duration format Went back and actually compared against the GUI instead of assuming the dashboard's original scope was right. Real gap: the GUI's header shows the wallet address and balance first, before anything else -- same place here now, for the same reason (it's what anyone checking on a running node wants to see first). Nickname added too, small but free given it's already loaded. Deliberately still not mirroring the GUI's full surface: the transaction list, peer/cached-peer/address-book tables, and pool selection are genuinely list/management views that don't fit a fixed non-scrolling frame the way a scrollable GUI panel does -- peer count stays a count, not an attempt to cram a table into 80 columns. That's a real design choice, not the same kind of oversight the missing balance/address was. Also matched the GUI's uptime/duration format (seconds under a minute, minutes under an hour, hours after) instead of the dashboard's own minutes-only format, which read as '150m ago' instead of '2h ago' on anything long-running -- same FormatDuration used for uptime, the update-check age, and the overdue-payout warning, so all three read consistently instead of each doing their own thing. Recompiled against the real headers after these changes (same process as the initial FTXUI port) -- clean, no new errors.
Previously the dashboard could only tell you an update existed -- 'run the GUI build to apply it there' was a dead end for anyone running bitflash-node unattended on a box with no GUI anywhere on it. Rebuilt on FTXUI's component module (Renderer + CatchEvent) instead of the previous static print loop, using ScreenInteractive/Loop -- already-vendored, already-tested library code, not hand-rolled keyboard input handling. Verified by compiling and partially linking against the real headers again (same process as the initial FTXUI port): confirmed every component-module symbol used here (Renderer, CatchEvent, ScreenInteractive, Loop, Event) resolves cleanly from the vendored ftxui.cpp, with zero FTXUI-related link errors. Ctrl-C is deliberately left to FTXUI's own default handling rather than mixing in a custom signal() call for this path -- confirmed via FTXUI's own issue tracker that combining an external SIGINT handler with the library's internal terminal/signal management is a known way to end up with a hung terminal on exit. Periodic redraw and the update check itself run on a background thread using screen.PostEvent(Event::Custom), the same pattern FTXUI's maintainers point people to across multiple discussions for exactly this 'redraw on a timer, not just on input' case. Pressing 'u' only does anything once an update is actually shown as available (a no-op otherwise); it hands off to Update_ApplyAndRelaunch on its own thread, same signature-verification path as the GUI's Update Now button, same fail-closed behavior.
…date modal g_showUpdateDialog was doing two jobs -- whether the modal is open, and whether the status bar thinks an update exists -- so clicking Skip (which correctly only closes the modal) also incorrectly made the status bar forget the update was ever found. Split into two flags: g_showUpdateDialog controls only the modal now, g_updateAvailable is set once a check finds something newer and stays true regardless of dismissing the modal, until an actual update succeeds (which relaunches the process, making the state moot) or the app restarts. Status bar's Update button now also re-opens the modal when clicked, giving a way back to it after a dismiss.
GetDepthInMainChain() returns 0 for both a genuinely unconfirmed tx and an orphaned coinbase (block not in main chain) -- same value, so every depth-0 row had no defined order relative to each other and fell back to mapWallet's raw hash ordering, which has nothing to do with time. Visible in a screenshot as orphaned 'Generated' rows with dates jumping around non-chronologically. Added time as a tiebreaker within equal depth (most recent first), kept depth as the primary key exactly as before -- a resync re-stamping many txs with the same time just means those particular ties fall back to hash order again, no worse than the prior behavior, but the common case (normal operation, not mid-resync) now sorts correctly.
…ional auto-return Participant mode retried a dead pool forever with no way out short of manually switching modes. New: /fallbacktosolo=MINUTES (GUI: "Fall back to solo mining after") — after the pool's been unreachable that long, switch to solo automatically. 0/unset keeps the old behavior (retry forever). /noautoreturn (GUI: unchecking "Automatically go back to the pool") controls whether it then periodically retries the original pool, or just stays solo until the user intervenes. Settings are CLI-flag/GUI-session only, not persisted to wallet.dat — same reasoning already established for nMineMode/strParticipantPool in db.cpp's LoadWallet: a saved value would silently override whatever was just requested on the command line, with no error. Thread-safety was the real design constraint here, not the settings. BitcoinMiner() is invoked once per thread launch (ThreadBitcoinMiner doesn't loop), and the solo-mining hot loop's exit condition is only `while (fGenerateBitcoins)` — it never re-checks nMineMode. Naively flipping nMineMode out from under a running thread would leave it mining solo forever regardless, while a second thread started alongside it in participant mode — two mining threads at once. Both transition directions instead follow the same shape: the thread that's currently running is the one that decides to stop, spawns its own successor (via the existing _beginthread(ThreadBitcoinMiner, ...) call the GUI's Start Mining button already uses) with the new mode already set, and only then returns, ending itself. No other thread is ever running when the mode changes, in either direction. The auto-return check lives at the very top of the solo loop's outer iteration, before that iteration's RandomX VM is created — the one place in the loop where returning early needs no cleanup at all. g_fAutoFallenBackToSolo (whether the current solo session exists *because of* an automatic fallback, vs. the user picking solo deliberately) is cleared on any manual mode change from the GUI — otherwise a stale flag from an old auto-fallback could later hijack a solo session the user chose on purpose, switching it back to participant mode against their explicit choice. Compiled all three touched files for real against the actual headers (main.cpp, gui.cpp, main_gui.cpp) before committing, given this touches mining thread control flow directly -- clean across all three.
Prompted by a maintainer's root-cause of Bitflash-sh#5 (the segfault at block 1062): %I64d, an MSVC-only specifier inherited from the original Bitcoin 0.1 codebase, desyncs vararg consumption on glibc -- the compiler had been warning about it the whole time, and -w (used project-wide, here too) silently threw the warning away. We don't have %I64d anywhere in our tree (checked exhaustively, zero hits outside properly-guarded vendored ImGui code) -- but the lesson is about the suppression, not that specific bug: -w hides exactly the warning class that would have caught it, project-wide, permanently. -Wformat -Wformat-security -Werror=format added back after -w on both Makefiles, narrow enough to not fight the blanket suppression everywhere else. Verified clean on Linux by actually compiling every major source file (main/net/gui/main_gui/util/db/rpc/dashboard.cpp) plus the vendored FTXUI source with these flags live -- zero hits. Windows verified more narrowly: installed a real mingw-w64 cross compiler in this sandbox and confirmed %lld (what this codebase actually uses throughout) doesn't false-positive under it, but a full cross-compile of the real Windows build wasn't feasible here (would need a mingw-built OpenSSL/websockets/curl chain this sandbox doesn't have) -- lower confidence than the Linux side, worth watching the first real Windows CI run after this lands.
…r no-op fix) My previous attempt (adding -Wformat -Werror=format after -w in CXXFLAGS) did nothing. Verified by direct test: a genuine format bug (%d given a long long) still compiles clean with -w present in the command line, regardless of where -Wformat/-Werror=format are placed relative to it. -w overrides them unconditionally. Ported and adapted Bitflash-sh#44 properly instead, which diagnosed this exact problem against the same root cause (the Bitflash-sh#5 segfault) and fixed it correctly: -w removed entirely, replaced with -Wno- deprecated-declarations/-Wno-write-strings/-Wno-narrowing/-Wno- unused-result (the categories this tree's own code legitimately triggers) plus -Wformat -Wformat-security -Werror=format. Also required BF_FORMAT annotations on strprintf/my_snprintf/ErrorImpl/ OutputDebugStringF -- printf itself is #defined to the last of those, and GCC only checks format strings for functions it knows are printf-like, so without the attributes -Wformat had nothing to inspect regardless of -w. gnu_printf archetype specifically: MinGW's alternative, ms_printf, models legacy msvcrt and would reject %zu (used here, supported by UCRT) while accepting %I64d (the specifier that actually crashes on glibc) -- exactly backwards for what this needs to catch. Collapsed PRId64/PRIu64/PRIx64 to one spelling (dropping the MSVCRT branch that produced %I64d in the first place), needed for both platforms to share one format archetype. Adapted rather than copied where our fork has diverged: PrintBlockTree doesn't exist here (removed earlier this session as dead code, so its fixes in the upstream diff don't apply), and `error` here is a macro over ErrorImpl (ours), not a plain function (theirs). Removing -w surfaced real bugs beyond what the upstream PR's diff covers, since this fork carries additional code: a SECP256K1_STATIC macro-redefinition warning in nostr.cpp/btfaddr.cpp (guarded the same way upstream did), and one more size_t-into-%d mismatch in util.cpp's AddTimeData logging that wasn't in their tree. Verified by actually compiling every source file in the tree with the real flags (not just the ones this diff touches) -- main/net/gui/ main_gui/util/db/rpc/script/update/market/dashboard.cpp all clean. nostr/btfaddr/compactblock/randomx_pow.cpp couldn't be verified this same way here (their C dependencies -- secp256k1, libsodium, RandomX -- aren't vendored in this sandbox, unrelated to this change), but nostr.cpp and btfaddr.cpp got the one warning each that could be checked without those headers (the macro redefinition, both fixed).
…itflash-sh#46) Issue Bitflash-sh#40's first half: a wallet.dat copied out on its own doesn't open anywhere else, even though it's byte-identical and holds the same keys. Root cause chain, from the bottom: _endthread() is pthread_exit() on POSIX, which glibc implements by throwing abi::__forced_unwind through the stack -- not a real exception, and catching it without rethrowing is a fatal error by contract. CATCH_PRINT_EXCEPTION's bare catch(...) did exactly that, so every Linux shutdown aborted mid-way rather than completing. That mattered because Berkeley DB stamps every page with a log sequence number tied to the environment that wrote it; DBFlush(true) is what clears that (via lsn_reset) before the file is safe to move, and DBFlush has been declared since 2009 and never actually called on either shutdown path. So the wallet was never released from its directory, and the one file anyone would think to back up was the one file that couldn't travel. Three fixes: - Rethrow abi::__forced_unwind rather than swallowing it (util.h), and call the now-effective DBFlush(true) on both shutdown paths (headless and GUI) in main_gui.cpp. - Adopt orphaned wallet.dat/blkindex.dat on open: a freshly created database/ environment beside pre-existing .dat files means those files came from elsewhere, so their sequence numbers get reset. Recovers wallets already stranded by older builds. - BackupWallet(), reachable as /backupwallet=FILE: checkpoints, copies, clears the copy's sequence numbers. Bitcoin 0.1.0 had a backup command; this tree hasn't had one since the wxWidgets removal. DBFlush's own lsn_reset call is now wrapped too (its txn_checkpoint already was, from this fork's earlier BDB-exception fix) -- letting it throw on the way out is how this whole chain started. Adapted rather than copied: this fork's dead-code sweep earlier this session correctly removed FileExists as unused at the time -- it has a real caller again now (both the orphan-adoption check and BackupWallet), restored scoped to db.cpp rather than re-exposed in util.h, since that's the only file that needs it. DBFlush's txn_checkpoint wrapping already existed here from a prior fix; only lsn_reset needed adding. Updated the README's backup guidance to point at /backupwallet as the correct way to do this, rather than copying the data directory by hand. Still not addressed, and the larger half of Bitflash-sh#40: keys are generated on demand rather than derived from a seed, so a backup is a snapshot and coins paid to addresses created after it aren't in it. That's a wallet-format change and its own piece of work, matching what upstream's own PR description says about scope. Verified by compiling every touched and dependent file for real against the actual headers, with the format-checking flags from the previous commit live -- clean throughout.
…rsion Two real UX bugs: 1. The pool-fallback feature flipped nMineMode to MINE_SOLO directly, which is also what the GUI's radio buttons and Mine dialog read from -- so a temporary automatic fallback looked identical to the user having deliberately switched to solo, with no way to tell which one had happened, and no easy path back to participant mode short of re-selecting it manually. Split nMineMode (what's actually happening, still what the mining code branches on) from the new nMineModeIntended (what the user actually configured). The Mine dialog now reads intended mode when seeding its radio buttons, so reopening it during a fallback still shows Participant selected, not Solo. Status bar shows a distinct, clearly-labeled line during an active fallback: which pool it's retrying, in how long, or that auto-return is off and picking Participant again is what restarts the retry. 2. StartUpdateCheck's periodic recheck (every 30 min) reopened the update modal every time it ran, for as long as a newer version stayed the newer version -- meaning clicking Skip only bought 30 minutes of peace, not a real dismissal. Added g_strSkippedVersion, set on Skip; the periodic check still runs and still keeps the status bar's persistent 'Update available' indicator correct, it just doesn't reopen the modal for a version already explicitly declined. A genuinely newer version still pops it right back open. Compiled gui.cpp/main.cpp/main_gui.cpp for real against the actual headers with the format-checking flags live -- clean.
Went back and actually integrated this instead of leaving it as a
note. It was excluded from the previous pass with a weaker reason
('scope already covered this pass') than Bitflash-sh#46 -- which was pulled in
the same pass and is comparably sized and no less delicate -- really
warranted. Bitflash-sh#47 stays out; that one's a genuinely different kind of
change (wallet format, not a bug fix), which is a real distinction,
not just a smaller version of this one.
/dumpwallet=FILE and /importwallet=FILE: the case Bitflash-sh#46 doesn't cover,
where Berkeley DB itself won't cooperate at all (unreadable version,
truncated file, an unrecoverable environment) rather than just being
tied to the wrong directory. A plain text file, one key per line
(hex private key, address, label), no database, no environment, no
version coupling.
Refuses to overwrite an existing dump (a mistyped path must not eat
the only copy of something irreplaceable), written owner-only on
POSIX before any key reaches it, labels with embedded newlines are
flattened so they can't forge an extra entry on read-back, and
import skips keys already present so running it twice is a no-op --
one unreadable line is reported and stepped over rather than costing
every other key in the file.
FileExists (restored locally in db.cpp for Bitflash-sh#46) and HexStr (already
in util.h) covered both new call sites without needing anything
else. README gets a short mention in our condensed style rather than
upstream's fuller section.
Compiled db.cpp and main_gui.cpp for real against the actual headers
with the format-checking flags live -- clean.
…ught Real Windows build failure (build-windows job): RandAddSeed()'s Windows-only branch (RegQueryValueEx/HKEY_PERFORMANCE_DATA) passed nSize -- unsigned long -- to %d. Slipped through every check in the previous commit because that code sits inside #ifdef WIN32 and no Linux compile, however thorough, ever touches it; this sandbox has no way to fully cross-compile the real Windows build (mingw-w64 is available, but the project's own dependencies -- secp256k1, libsodium, websockets, curl -- aren't packaged for it here), which is exactly the gap flagged honestly in that commit's message. Re-audited every #ifdef WIN32/_WIN32 branch across the whole tree for the same class of bug, this time with correct nested #if/#else/#endif stack tracking (the first pass's simple depth counter could miss or misattribute nested blocks) -- this was the only one.
Real build-windows failure: FTXUI's header hard-errors without
UNICODE, _UNICODE, and NOMINMAX defined on Windows. Added to
FTXUI_CXXFLAGS specifically, not the shared CXXFLAGS -- this
codebase calls several WinAPI functions with an explicit ANSI ('A')
suffix (db.cpp's FileExists among them), which assume narrow
strings; defining UNICODE build-wide would silently break those.
Scoped to just the two files that actually need it (ftxui.cpp,
dashboard.cpp), it can't.
Also made db.cpp's GetFileAttributes call explicitly GetFileAttributesA
-- found while checking for exactly this class of risk, since psz is
already a narrow char* there and the unsuffixed name would resolve
differently if UNICODE ever ended up defined for that translation
unit by some other path.
Verified with a real mingw-w64 cross compiler (installed in this
sandbox for this): both ftxui.cpp and dashboard.cpp now compile past
the UNICODE/NOMINMAX check entirely (previously a hard error);
dashboard.cpp still can't fully compile here for the same reason as
before (mingw-built OpenSSL/etc. aren't available in this sandbox),
unrelated to this fix.
…d transactions, and never paying fees to the coinbase Reported symptom: a sent transaction never confirmed, blocks after blocks. Root cause found in RebuildJob() (the operator's Stratum block template, separate code path from BitcoinMiner()'s solo-mining block builder since operator mode never reaches that function at all): 1. Single-pass scan over mapTransactions, unlike solo mining's multi-pass retry loop. mapTransactions has no dependency order -- it's keyed by hash -- so a transaction spending another still- unconfirmed transaction's output (change from a recent send, for instance) fails ConnectInputs if its parent happens to be visited later in map order on that pass. A single pass drops it from the block permanently, and because map order doesn't change between job rebuilds, every future rebuild hits the exact same ordering and skips it again -- deterministic exclusion, not an occasional fluke, for as long as the transaction depends on something still unconfirmed. If most blocks on this network come from pool operators (plausible -- a pool aggregates hashrate), a transaction with this shape could stay unconfirmed indefinitely through no fault of its own. Fixed to match main.cpp's solo-mining builder exactly: retry the whole scan until nothing more can be added. 2. Coinbase value used a hardcoded 0 for fees (GetBlockValue(height, 0)) even though fees were being collected during the scan (into a per-transaction-scoped nFees that was silently discarded each iteration) -- every fee-paying transaction in an operator's block was quietly not paying its fee to anyone. Moved nFees to function scope, accumulated across the whole scan, actually passed to GetBlockValue. Unrelated to the reported symptom but found in the same function while fixing it. 3. Found and fixed in the same pass: the coinbase key here was generated fresh (key.MakeNewKey()) rather than drawn from the keypool -- the exact backup-safety issue fixed in main.cpp's solo BitcoinMiner() earlier this session, missed here because operator mode builds its block template through this separate function, never through BitcoinMiner() at all. Same fix: GetKeyFromPool(). The explicit AddKey(key) call after block construction is now redundant and removed -- a keypool key is already durably in wallet.dat before it's ever handed out. Compiled rpc.cpp and every file that could plausibly be affected for real against the actual headers with the format-checking flags live -- clean throughout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Same fork as #31 (closed), continuing to update as the fork develops. Opening this one to stay open rather than reuse the old one, per your preference for reports/findings over a merge attempt of the branch itself.
Integrated through
PRs: #27 through #49 inclusive.
Issues: #1 through #40 reviewed; #5 and #40 specifically cross-checked against our own code, not just read.
Two of these were independently re-derived here before I found your fix already existed, then reconciled against yours once I did:
-woverrides any-Wformat/-Werror=formatplaced after it on the command line, regardless of order), then found and adapted your build: check format strings, which nothing ever did #44, which diagnosed and fixed that correctly./gennot mining) and sha: restrict the x86 bswap asm to x86 #41 (ARM64bswap): found the same bugs independently, confirmed your fixes matched, ported directly.Reviewed, not integrated, noted for whoever picks this up next:
Where this fork adds things not in Bitflash-sh/bitflash yet
Not a claim that these should be merged -- just what's different, for whoever's deciding:
bitflash-nodebinary with no libGL/FUSE dependency./btfseed=flag for cold starts.Building and running on Windows and Linux, mining solo/operator/participant, pool payouts flowing end to end -- the goal this fork set out with is reached. Your call whether any of this is worth pulling in; not expecting a merge of the branch as a whole.