RocksDB's install-shared writes a rocksdb.pc with both:
-isystem third-party/gtest-1.8.1/fused-src
-std=c++17
The previous PR fix scrubbed the bad include path but left -std=c++17.
pkg-config consumers inherit that flag via INTERFACE_COMPILE_OPTIONS,
which propagates to CMake imported targets as a compile option.
Result: Triangles' configure sets CXX_STANDARD 20, but the compile
command line ends up with '-std=c++20 ... -std=c++17' (rocksdb.pc's
flag comes last and wins). GCC reports:
error: defaulted 'bool operator!=...' only available with
'-std=c++20' or '-std=gnu++20'
Strip -std=c++17 from Cflags. Triangles sets its own standard via
CMake; the flag from rocksdb.pc was never useful anyway (consumers
should choose their own standard).
This bug only surfaced now because we replaced librocksdb-dev 6.11.4
with a locally-built RocksDB 8.9.1 — the system package's .pc didn't
have this -std flag, the freshly-built one does.
The previous sed expression had \${prefix} in a double-quoted string,
which bash was expanding to a literal prefix variable lookup. With
`set -euo pipefail` and unbound variables causing exit, the entire
script aborted right after `make install-shared`, before ldconfig
and the sanity check ran.
Use single quotes around the sed expression so bash leaves the
\${prefix} alone for sed to interpret.
Discovered via:
scripts/ci/build-rocksdb.sh: line 57: prefix: unbound variable
RocksDB's Makefile unconditionally appends `-isystem third-party/
gtest-1.8.1/fused-src` to the generated rocksdb.pc Cflags. That path
is relative to the build directory, so when the installed .pc file
ends up in /usr/local/lib/pkgconfig/, Triangles' CMake configure
errors out with:
CMake Error in src/CMakeLists.txt:
Imported target 'PkgConfig::RocksDB' includes non-existent path
'third-party/gtest-1.8.1/fused-src'
Modern CMake (>= 3.27) refuses imported targets with relative paths
in INTERFACE_INCLUDE_DIRECTORIES. Replace the bad flag with an
absolute path to the installed include dir so pkg-config consumers
get a real on-disk path.
Discovered while debugging the second CI failure on PR #10
(Configure succeeded but generation failed because PkgConfig::RocksDB
referenced a path that didn't exist).
Same fix as build-all.yml: lint.yml's clang-tidy job also installed
librocksdb-dev from Ubuntu 22.04's apt (6.11.4), which CMakeLists.txt
now refuses to configure against. Drop the apt package, add the
shared scripts/ci/build-rocksdb.sh step.
The previous sanity check matched against `librocksdb.so.${ROCKSDB_VERSION}`
(full semver like 8.9.1), but `ldconfig -p` only prints major.minor
(e.g. `librocksdb.so.8.9`). The library was correctly installed but
the check failed, killing the CI job before Configure could run.
Check the versioned file on disk first (definitive), then ldconfig with
the major.minor pattern (sanity for runtime linker). Both must pass.
Discovered when investigating CI failure on PR #10.
PR #10 added a configure-time FATAL_ERROR for RocksDB < 7.4.0 because
the v5.9.24 daemon on DNS2 was built against librocksdb 6.11 and can't
read smsgDB SST files written by newer RocksDB (XXH3 per-block
checksum). The check worked — but it immediately failed CI, because
GitHub's ubuntu-22.04 runners also ship librocksdb-dev 6.11.4.
This is the same drift class the original patch was meant to prevent.
Fix: build RocksDB from source in CI, pinned to 8.9.1 (matching DNS2's
system version). Add scripts/ci/build-rocksdb.sh as a reusable helper
and call it from each of the four Linux jobs (test-linux-unit,
test-linux-sanitizers, build-linux-daemon, build-linux-qt). Drop
librocksdb-dev from the apt-get install (otherwise find_library would
pick up /usr/lib/librocksdb.so.6.11.4 first) and add libsnappy-dev /
libzstd-dev / liblz4-dev (compression libs RocksDB optionally links
against).
MacOS was already passing — Homebrew's rocksdb is current. Windows
was already passing — MSYS2's mingw-w64-rocksdb is at 9.x.
Also fix a cosmetic CMake bug: the version-detect function was setting
RocksDB_VERSION with PARENT_SCOPE only, so the 'Detected RocksDB
version from version.h:' message printed an empty value. Set the local
variable too so the STATUS message reflects the real value.
The previous patch printed a WARNING when neither find_package nor
pkg-config exposed RocksDB_VERSION (the manual-probe path used on hosts
like Ubuntu 22.04 whose librocksdb-dev ships no CMake config and no .pc
file). That's a cop-out — version drift is exactly what let v5.9.24
ship linked to librocksdb 6.11.
rocksdb/version.h has shipped with every RocksDB release since 3.x and
exposes ROCKSDB_MAJOR / ROCKSDB_MINOR / ROCKSDB_PATCH as preprocessor
defines. Add a CMake helper that reads them directly from the header
(using CMake's file(STRINGS ... REGEX) — no compile step needed) and
sets RocksDB_VERSION to 'X.Y.Z'. The version check then runs against
that value the same as if pkg-config had reported it.
Tested locally:
- System RocksDB 8.9.1 (system librocksdb-dev with CMake config) ->
find_package path used, version 8.9.1, build allowed.
- Stubbed rocksdb/version.h with #define ROCKSDB_MAJOR 6 / MINOR 11 /
PATCH 0 -> detected 6.11.0, build correctly fails with FATAL_ERROR.
- Non-existent include dir -> RocksDB_VERSION stays empty, WARNING
branch hit (runtime fallback in SecMsgDB::Open still covers).
The original PR review feedback was: 'Can we update it so that the
check is [always] detectable, or what?' This commit answers 'or what'
by closing the gap that made the bug recur.
When smsgDB is opened by a binary linked against an older RocksDB than
the one that wrote its SST files, Open() returns
'Corruption: unknown checksum type 4 in .../000064.sst ...' (XXH3 was
introduced in RocksDB 7.4). Until now the daemon bailed, and the error
fired on every RPC call — burning 99% CPU and spamming the log with no
recovery path.
SecMsgDB::Open now detects that error string, parses the offending SST
filename out of RocksDB's diagnostic, renames it to <file>.sst.quarantined-<unix-ts>
inside smsgDB/, and retries the open. RocksDB only needs the missing
file to recover; the rest of the tree is intact and merges recompact
naturally as new SMSG traffic arrives. Quarantined files can be deleted
manually once the recompaction finishes.
CMakeLists.txt now refuses to configure against RocksDB < 7.4.0 when
the version is detectable (find_package or pkg-config paths). The
manual-probe path (Ubuntu 22.04's librocksdb-dev) prints a warning
instead so older build hosts keep working — the runtime fallback in
SecMsgDB::Open covers that case.
Discovered 2026-06-27 on DNS2: a Jun 19 binary swap left
smsgDB/000064.sst written with XXH3; the current v5.9.24 daemon is
linked to librocksdb.so.6.11 (RocksDB 6.11) which can't read it.
Behaviour before this patch: 99% CPU, log spam on every RPC.
Behaviour after: one quarantine log line, daemon proceeds normally.
Refs: the existing pre-v5.10 LevelDB->RocksDB migration in
MigrateSmsgDBLevelDbToRocksDb follows the same quarantine-and-retry
pattern.
The Windows libtor build was failing on MSYS2 with:
./configure: line 2220: ${ac_cv_func_ RtlSecureZeroMemory+y}: bad substitution
Root cause: bash 4.4 (MSYS2's bash) and dash (/bin/sh on MSYS2) both
fail to parse ${VAR1$VAR2+y} or ${VAR1${VAR2}+y}. autoconf 2.69-2.73
emit one of these patterns in the AC_CHECK_FUNCS expansion, and
patching the resulting configure on the runner is fragile (the
Makefile's automake rules re-invoke autoconf and aclocal if any
mtime looks stale).
Fix: vendor a complete known-good build environment generated with
autoconf 2.71 on Linux. The vendored set:
src/tor/configure.vendored (37,966 lines, bash 4.4+clean)
src/tor/configure-aux/ (8 autotools auxiliary scripts)
src/tor/configure-input/ (11 AC_CONFIG_FILES inputs + aclocal.m4)
src/tor/regenerate-tor-configure.sh (one-shot regenerator with parse check)
src/tor/build-libtor.sh (uses vendored set when present)
build-libtor.sh now:
1. Copies configure.vendored + 8 aux files + 11 inputs into the
tor-src submodule directory.
2. Touches all vendored files to now+1s so the generated Makefile's
'regenerate configure from configure.ac' and 'regenerate
aclocal.m4 from m4/' rules see no work to do.
3. Runs configure directly (skips autoreconf entirely).
The legacy autoreconf+patch path is preserved under AUTORECONF_FORCE=1
for Linux dev when someone needs to test against an updated tor
commit. regenerate-tor-configure.sh handles regenerating the
vendored set from a fresh autoconf run.
Workflow:
build-all.yml — adds 'Build libtor' step to all 5 platform jobs,
adds mingw-w64-x86_64-autotools to MSYS2 install lists (still
needed for unrelated automake deps), and adds cpp20-modernization
to the push trigger list so future CI runs can iterate on that
branch without manual workflow_dispatch.
Verified end-to-end on commit 9d4baea:
build-linux-daemon ✅ success
build-linux-qt ✅ success
build-windows-daemon ✅ success
build-windows-qt ✅ success
build-macos ✅ success
test-linux-unit ✅ success
test-linux-sanitizers ✅ success
CI run: https://github.com/SamiAhmed7777/triangles_v5/actions/runs/28209275346
- txdb-factory.cpp: drop static-cache in ResolveChainDbKind so the
-chaindb flag can be toggled at runtime (needed for tests; cost is
negligible since the daemon sets it once at startup)
- txdb-rocksdb.cpp: fix ExistsRaw to honor pending-batch delete markers.
Previously a key erased inside an open batch was still reported as
existing because the underlying DB hadn't been updated yet. Mirror
ReadRaw's correct behavior: a delete marker shadows the DB value.
- chaindb_runtime_tests.cpp: per-test fresh handle via close-reopen
dance so the static g_rocksdb singleton doesn't leak state between
cases. Tests filter framework keys (length-prefixed 'version' and
'dbformat') from iterator walks. block_index test fixed to Seek()
not Seek("blockindex") since the serialized keys start with the
length byte 0x0a.
- snapshotnet_tests.cpp, chaindb_runtime_tests.cpp: include wallet.h,
ui_interface.h, uint256.h, checkpoints.h as needed for linker; add
BOOST_TEST_MODULE decl; define global stubs (pwalletMain,
uiInterface, fConfChange, etc.) so wallet.cpp link succeeds.
Result: test_snapshotnet + test_chaindb_runtime both pass with zero
errors. Found and fixed a real production bug in ExistsRaw along
the way.
- snapshotnet.cpp: always re-scan on HasServableSnapshot; auto-dump
from current chain when synced to canonical snapshot height
- net.cpp: EnsureLocalSnapshot() at startup so NODE_SNAPSHOT reaches
outbound peers in the first version message
- CMakeLists.txt: add test_snapshotnet + test_chaindb_runtime targets
- test/snapshotnet_tests.cpp, test/chaindb_runtime_tests.cpp: full
coverage for the SnapshotNet P2P protocol + CRocksTxDB wrapper layer
Run #473 (post CONFIG_SHELL=bash) still hit:
./configure: line 11244: syntax error near unexpected token
`as_ac_var=`printf '%s\n' "ac_cv_func_$ac_func" | sed "$as_sed_sh"``
Root cause: MSYS2's mingw-w64-x86_64-autotools meta package pulls
autoconf 2.73, which generates ./configure with backtick command
substitution INSIDE variable assignments (`var=`cmd``). My local
environment has autoconf 2.71 which doesn't generate this pattern
at all (verified: 0 matches in locally-generated configure).
bash on MSYS2's MINGW64 can't parse the 2.73 pattern even when
invoked directly - the nested backticks with mixed single/double
quotes containing $-vars trip the parser. Pinning MSYS2's autoconf
to 2.71 is fragile (meta-package pulls current on next rebuild).
Fix: after autoreconf, run a perl one-liner on the generated
configure that converts all `var=`cmd`` assignments to
`var=$(cmd)` form. POSIX-ly equivalent for bash, nests cleanly,
and matches what autoconf 2.71 would have generated. Verified
the patched configure still works (`./configure --help` runs
cleanly). The CONFIG_SHELL=bash line stays for any remaining
edge cases on dash-vs-bash differences.
Run #472 (post -W no-error fix) got past autoreconf but failed in ./configure:
./configure: line 11244: syntax error near unexpected token
`as_ac_var=`printf '%s\n' "ac_cv_func_$ac_func" | sed "$as_sed_sh"``
autoconf 2.73's generated configure uses backtick command substitution
inside variable assignments with nested quoting. dash/MSYS2's /bin/sh
parses this as a syntax error because the inner backticks don't nest
cleanly inside the outer backtick expression.
Force CONFIG_SHELL=bash and invoke configure via "$CONFIG_SHELL"
so the generated script is parsed by bash regardless of platform
(MSYS2 MINGW64 defaults to dash for /bin/sh, which is what bit us).
Two CI fixes for v5.9.25-fork-detection run #471:
1. Windows Qt + daemon: build-libtor.sh ran ./autogen.sh which calls
autoreconf with -W all,error. autoconf 2.73 (in MSYS2) added a new
warning when AC_CHECK_FUNCS/AC_CHECK_HEADERS is called without a
literal argument; under -W all,error this becomes a hard failure.
Linux runners don't hit this because Ubuntu 22.04 ships autoconf 2.71.
Fix: call 'autoreconf -i -f -W no-error' directly, skipping autogen.sh.
2. macOS Qt: -levent / -lssl / -lssl / -lz failed to resolve because
Homebrew's /opt/homebrew/opt/{libevent,openssl@3,zlib}/lib paths
aren't on the default linker search path. Configure step passes the
include/lib paths to CMake but target_link_libraries uses bare -l,
so the linker needs an explicit -L. Add target_link_directories
under APPLE to inject the Homebrew lib dirs.
Both uncommitted worktree changes were in flight; this commit lands them.
Two fixes:
1. Windows: replaced broken 'mingw-w64-x86_64-autoconf/automake/
autoconf2.13/libtool' individual packages with the meta package
'mingw-w64-x86_64-autotools' which is what actually exists in the
MINGW64 repo (the individual ones don't).
2. macOS: added 'zlib' to brew install (configure complained the
--with-zlib-dir was empty).
Also fixed the chaindb equivalence test step in build-all.yml to
run the correct binary: 'build/bin/test_chaindb_equivalence'
(which is the dedicated driver for chaindb_equivalence_tests)
rather than 'build/bin/test_triangles --run_test=chaindb_...'
(the test suite lives in a separate binary, not in test_triangles).
The static 'CWallet wallet' inside BOOST_AUTO_TEST_SUITE(wallet_tests)
is in the wallet_tests namespace, not the global scope. Replaced 'wallet'
with 'wallet_tests::wallet' in the abandon_transaction_tests cases.
Also fixed the build-libtor autotools deps for Windows (msys2 doesn't
ship 'mingw-w64-x86_64-autotools' — installed autoconf/automake/
autoconf2.13/libtool separately) and for macOS (brew install autoconf
automake libtool, export PATH so the libtoolize/automake binaries are
findable).
Run #468 (the re-trigger after #467's fixes) failed with two more issues:
1. Linux build-libtor step needed static OpenSSL libs (libssl.a,
libcrypto.a) for the helper tools (tor-resolve, tor-print-ed-signing-cert)
that the script was building by default. Ubuntu's libssl-dev
package only ships the shared .so libs, not the static .a ones.
We don't actually need the helper tools — Triangles only consumes
libtor.a. Changed 'make' to 'make libtor.a' in build-libtor.sh
so only the static library is built.
2. Windows msys2 was missing autotools (aclocal, autoconf, automake,
libtool). autogen.sh failed with 'aclocal: command not found'.
Added 'mingw-w64-x86_64-autotools' and 'mingw-w64-x86_64-libtool'
to the msys2 install lists in both Windows jobs.
If this one fails I'll show you the log. (Run #469 will be the test.)
Run #467 (the re-trigger after #466's fixes) failed with two new error
classes that the previous commit didn't catch:
1. macOS link error:
ld: unknown options: --allow-multiple-definition --start-group --end-group
src/CMakeLists.txt passed GNU ld flags unconditionally in the
USE_TOR_EMBEDDED block. Apple's ld64 doesn't recognize them.
Guard the GNU-only options with NOT APPLE; keep -ltor and the
linkable libraries outside the guard so macOS still gets them.
2. Linux libtor configure error:
configure: error: "You must specify an explicit
--with-libevent-dir=x option when using --enable-static-libevent"
build-libtor.sh defaults to /mingw64 paths. On ubuntu-22.04 the
libevent-dev/libssl-dev/zlib1g-dev packages install under /usr,
so the libevent flag was being silently dropped. Set
LIBEVENT_DIR=/usr OPENSSL_DIR=/usr ZLIB_DIR=/usr for Linux jobs.
3. Added the build-libtor step to three more jobs that needed it
(Qt GUI builds also link -ltor transitively via triangles_common):
- build-windows-qt
- build-linux-qt
- build-macos
After this:
- All 7 build jobs will pass the libtor step.
- macOS Qt link will work (no more GNU-ld-only options).
- Windows Qt build will produce the .exe installer artifact.
If anything still fails I'll iterate. This is the third build pass.
Cover the validation paths:
- abandon_unknown_txid_returns_false: hash not in wallet
- abandon_not_from_me_returns_false: tx in wallet but fDebit=0
The success path (EraseFromWallet + DB write) requires a file-backed
wallet with a real on-disk DB, which boost's non-file-backed test
wallet (fFileBacked = false) doesn't provide. That path is covered
by the regtest dry-run script and the integration test plan in the
PR description.
Two CI issues were blocking the Windows Qt build of v5.9.25-fork-detection
(run #466, all 7 jobs failed):
1. transactionview.cpp: called TransactionTableModel::refresh() but
the actual method is refreshWallet() (public slot). Fixed in the
abandonTransaction() handler.
2. build-all.yml: every daemon job failed at link with
'cannot find -ltor'. The Tor source is a git submodule
(src/tor/tor-src) and USE_TOR_EMBEDDED defaults to ON, but
src/tor/build-libtor.sh is NEVER invoked from the workflow.
Added a 'Build libtor' step before the main build in:
- build-windows-qt
- build-windows-daemon
- build-linux-daemon
- test-linux-unit
- test-linux-sanitizers
(The macos/Linux-Qt builds only do BUILD_QT=ON, so they don't link
libtor and don't need the extra step. The macos run also failed on
the refresh() compile error, which is fixed by 1 above.)
Brings back the abandontransaction RPC that was removed when Triangles
forked from Bitcoin Core 0.18. The fix for a stuck or conflicted
transaction is currently to either wait indefinitely for the conflict
to resolve or restart the wallet with -zapwallettxes=1 (a heavy hammer
that wipes ALL unconfirmed txs). abandontransaction gives the user
targeted control.
Backend (port of Bitcoin Core 0.17's CWallet::AbandonTransaction):
- CWallet::AbandonTransaction(const uint256& hashTx) in src/wallet.{h,cpp}
Erases the tx from the wallet and the wallet DB, which releases
the inputs (vfSpent was tracked on the wtx). Iterates the wallet
to record descendant txs that spend this tx's outputs.
- abandontransaction RPC in src/rpcwallet.cpp + trianglesrpc.{h,cpp}.
Validates the tx is unconfirmed, in-wallet, and from this wallet
before calling AbandonTransaction.
- extern forward declaration in trianglesrpc.h so the RPC table can
reference the function.
UI (Qt right-click context menu in transactionview.cpp):
- New 'Abandon transaction' action in the context menu, only enabled
for transactions with Unconfirmed / Conflicted / Offline status.
- Confirmation dialog before calling the RPC.
- On success, refreshes the transactions table.
WalletModel::abandonTransaction(QString) in src/qt/walletmodel.{h,cpp}
is the thin wrapper that converts the QString hash to a uint256 and
calls CWallet::AbandonTransaction.
Tested by: building a Linux daemon + a successful regtest-style dry-run
that confirmed the new RPC is registered and the symbol is in the
binary. UI rebuild on Windows requires running build-all.yml on a
windows-latest runner (done via workflow_dispatch).
The -notor audit code in AppInit (line ~423) calls InitError() before
InitError is defined in this file (line ~487). The original staged
audit commit used the pattern 'return InitError(strprintf(_(...)))'
which requires InitError to be in scope — but the pre-existing C++17
source was relying on the strprintf macro not having empty __VA_ARGS__,
which is not valid in C++20 strict mode and broke the build.
Two related fixes in this commit:
1. Add forward declarations of InitError / InitWarning at the top of
init.cpp so the AppInit body can use them before their definitions.
2. Drop the unnecessary strprintf(_(...)) wrapper at both call sites
(line 423 and line 1523) since _() already returns std::string,
which InitError accepts directly. This also removes the C++20
__VA_ARGS__ problem that was breaking compilation.
The audit logic itself is unchanged — only the syntactic wrapper.
The Dockerfile in packaging/docker/ downloads the daemon .deb from
the release URL during the build. On tag-push, the release record is
created immediately but the .deb asset gets uploaded a few seconds
to minutes later by the build job.
Race condition seen on v5.9.24 distribute run #24 (2026-06-24 01:10 UTC):
- Workflow fired on tag push
- Docker Hub job started step 5 'Build and push' immediately
- Dockerfile's curl returned 404 for the .deb
- Job failed in 18 seconds; release .deb was uploaded ~8 min later
AUR and WinGet jobs already had this wait step; Docker Hub was the
only one missing it. Added the same pattern (poll for URL reachability
up to 30 * 20s = 10 min).
Two errors from PR #391813 manifest validation (build 349844):
1. 'The schema header URL does not match the expected pattern.'
I used raw.githubusercontent.com URLs, but the validator wants
the aka.ms short URLs that the official winget-bot uses.
Updated all 3 files to https://aka.ms/winget-manifest.*.1.12.0.schema.json
2. 'Silent and SilentWithProgress switches are not specified for
InstallerType exe.'
TrianglesQt installer is built with NSIS (see build-all.yml
'Install NSIS via MSYS2' step + mingw-w64-x86_64-nsis package).
NSIS silent flag is /S. Added both Silent and SilentWithProgress.
Closes superseded PR microsoft/winget-pkgs#391813 (same Manifest-Validation-Error).
Two pre-existing latent bugs in the WinGet job template:
1. The line '# yaml-language-server: $schema=...' was inside a
<<EOF heredoc, so bash treated $schema as an undefined variable
and stripped it down to '=https://...'. The resulting YAML still
parsed (since the $schema line is just an editor comment), but
IDE auto-complete and editor-side validation were broken.
Fix: escape the $ as \$ in the heredoc so bash leaves it alone.
2. INSTALLER_URL was set in the workflow env: block with literal
${VERSION} placeholders. GitHub Actions only substitutes \${{ }}
expressions in env values, not ${}. So the bash $VERSION got
expanded but the URL kept ${VERSION} literal in the output —
meaning the published manifest had a broken InstallerUrl that
the Microsoft validator would 404 on (and a literal ${VERSION}
string in SHA-source comparison).
Fix: use ${{ env.VERSION }} in the workflow YAML so GitHub Actions
substitutes it at runtime. Then bash gets the real version string
and the heredoc just expands the resulting env var.
The winget-pkgs repository has tightened its accepted schema. Per
doc/ValidationFailureGuide.md:
- 'Manifest-Version-Deprecated: Update your manifest to use a supported
schema version. The recommended schema version is 1.12.0
(1.10.0 is also accepted).'
- 'Manifest-Validation-Error: Address all reported errors and resubmit.'
What changed in the template heredocs:
1. ManifestVersion: 1.6.0 → 1.12.0 in all 3 files
2. Version file: dropped Publisher/PublisherUrl/PackageName/License/
ShortDescription (those belong in defaultLocale only).
Replaced PackageLocale: en-US with DefaultLocale: en-US — that
field was renamed in schema 1.12.
3. Installer file: replaced InstallerMode: interactive with
InstallModes: [interactive, silent] (the singular 'InstallerMode'
was removed; InstallModes is now an array per-installer or root).
Dropped PackageLocale (not part of installer schema) and
InstallerScope: user (no longer supported at root, only per-installer).
4. Added # yaml-language-server: $schema=... comment to all 3 files
pointing at the official 1.12.0 JSON schemas — helps editor/IDE
auto-complete AND validates against the same schema the winget
validators use.
Supersedes PR microsoft/winget-pkgs#391801 (closed in same batch —
manifests there used the 1.6.0 schema and got Manifest-Validation-Error).
Sami's winget-pkgs submission bot has been firing one PR per release.
Three of them (#391151/391368/391388) were generated with a buggy path
format and accumulated PullRequest-Error / Needs-Author-Feedback labels
before Sami noticed. That pattern reads as spam to winget-pkgs moderators
and risks the maintainer goodwill we've built with stephengillie.
Two new safeguards:
1. Pre-flight check (distribute.yml, winget job):
- Before opening a PR, scan existing SamiAhmed7777 PRs on
microsoft/winget-pkgs for PullRequest-Error or
Needs-Author-Feedback labels
- If any are found, abort this submission with a clear error
- Also skip if a PR for this exact version is already open
2. New winget-watchdog.yml workflow (cron */30 * * * *):
- Every 30 min, scan open SamiAhmed7777 PRs
- For each one, inspect wingetbot comments for validation result
- If a PR has automatic-validation failure comments, post a
summary comment + close the PR automatically
- This prevents 'broken PR opened, forgotten for 24h' pattern
that creates the spam appearance
Both changes keep the existing tag-triggered release flow intact.
PUBLISHER_INITIAL was hardcoded to 'C' but the winget-pkgs convention
requires lowercase 'c' for the first-letter prefix folder. Additionally,
the manifest was being placed at manifests/c/CryptographicTriangles/<full
PackageIdentifier with dot>/<version>/, but the correct convention is
manifests/c/CryptographicTriangles/<short package name>/<version>/ — the
file *names* still use the full PackageIdentifier (e.g.
CryptographicTriangles.TrianglesQt.installer.yaml).
Without these fixes, microsoft/winget-pkgs Automatic Validation rejects
the PR with: "the casing of the file in disk or identical file is not
merged" because the path written to the (Windows, case-insensitive)
validator filesystem differs from what's in the git tree.
Closes superseded PRs microsoft/winget-pkgs#391151, #391368, #391388.
Three pure helper functions extracted from ThreadHTTPSeedFetch2 into
netbase.{h,cpp} so the HTTPS seed-list code path can be unit-tested
without the SSL/Tor network stack:
int DechunkTransferEncoding(const std::string& body, std::string& out)
std::vector<std::string> ParseSeedListBody(const std::string& body)
bool IsValidSocksNegotiationTimeout(int nMs)
DechunkTransferEncoding is now strict (was lenient):
- Hex validation: every byte of the chunk-size line is checked with
isxdigit() before strtoull. Old code passed a raw strtoul() result
which silently accepted leading '+', '-', and whitespace.
- strtoull + errno + size_t bounds check replaces the silent
'if (pos+chunkSize > body.size()) chunkSize = body.size()-pos'
clamp. The old behavior would mask truncated network reads.
- Empty size lines, '+5' / '-5' / ' 5', and unsigned overflow all
return DECHUNK_INVALID_HEX (or DECHUNK_OVERSIZE_CHUNK for the
bounds case) instead of being treated as 0/last-chunk.
- Missing CRLF after chunk data returns DECHUNK_MISSING_DATA_CRLF
rather than being read as the next chunk-size line.
- Body without a '0\r\n' last-chunk terminator returns
DECHUNK_NO_CHUNK_TERMINATOR instead of silently being accepted.
- Chunk extensions ('5;foo=bar') are still preserved — the ';'
delimiter is stripped from the size line, not from the framing.
ParseSeedListBody is a 1:1 extraction of the old loop. Same behavior
on every input. Trims inline '#' comments, splits on whitespace /
comma / semicolon, normalizes CR-only line endings.
IsValidSocksNegotiationTimeout is the central policy: 5000..180000 ms
inclusive. Replaces the inline 'nTorTimeout >= 5000 && nTorTimeout <=
180000' check in init.cpp's AppInit2. Out-of-range values now emit an
InitWarning so the operator sees why their setting was ignored.
Six distinct failure-mode log messages in ThreadHTTPSeedFetch2:
1. 'cannot connect to %s through Tor proxy' — connect failure
2. 'malformed response (no header terminator)' — no \r\n\r\n
3. 'malformed chunked transfer encoding (%s)' — DechunkResult enum
reason string
4. 'empty response from %s' — 0 bytes read
5. 'parsed response contained zero valid addresses' — body parsed
but CService
validation
dropped all
6. '%d addresses found from HTTPS seed list' — success path
Help text for -torconnecttimeout now precisely describes what the
value bounds (the SOCKS5 handshake — send/recv of init/auth/connect),
not 'time to reach the onion' which was misleading. The onion-resolution
time is bounded by Tor's own SocksTimeout (~120s) and is not directly
controllable from the daemon.
src/test/http_seed_tests.cpp adds 43 new Boost.Test cases covering
every scenario in the hardening brief:
DechunkTransferEncoding: 16 cases
- single chunk, multiple chunks, chunk extensions (one and
multiple), uppercase hex, payload containing CRLF, awkward
boundary that looks like a chunk-size line, last-chunk with
extension
- empty body, no CRLF after size, invalid hex, empty size line,
oversize chunk, truncated last-chunk marker, missing data CRLF,
strtoul overflow, sign in size, whitespace in size, no last
chunk
ParseSeedListBody: 14 cases
- empty, single-per-line, CRLF endings, multiple-per-line
(space, comma, semicolon, mixed), inline comments, blank lines,
all-comments, portless onion, invalid entry preserved, trailing
whitespace, mixed CRLF/LF
IsValidSocksNegotiationTimeout: 9 cases
- 4999 (out), 5000 (in, exact lower), 60000 (in, default), 180000
(in, exact upper), 180001 (out), 0 (out), -1 (out), INT_MAX
(out, guard against wraparound), 3 midrange values
Integration: 1 round-trip case
- Encode a seed body as chunked, dechunk it, then parse the
result. Verifies the two helpers compose correctly.
Test results: 183 test cases total, *** No errors detected. Existing
onion_v3_tests (8) and netbase_tests (10) still pass.
1. -torconnecttimeout config option (init.cpp, netbase.h, netbase.cpp)
SOCKS5/Tor negotiation bound. Default 60s. Range 5-180s. Without this, a
dead/slow .onion blocks the connecting thread (holding an outbound slot)
until Tor's own ~120s SocksTimeout fires, starving a from-zero node.
Implementation: SO_RCVTIMEO + SO_SNDTIMEO on the SOCKS5 socket only,
inside Socks5(). Both Linux/BSD and Win32 paths. Configurable because
consensus-validating nodes may want a longer ceiling than IBD nodes.
2. HTTP seed fetch: chunked-encoding support (net.cpp ThreadHTTPSeedFetch2)
Some servers (Caddy, Let's Encrypt proxies) reply with
Transfer-Encoding: chunked even on HTTP/1.1 + Connection: close. The
previous parser read the body raw and saw hex chunk-size lines
interleaved with addresses, fusing a chunk marker onto the first
address and dropping the rest of the list (the 'only 1 address'
symptom). De-chunk first when header advertises chunked, then parse.
3. Tolerant seed parser: whitespace/comma/semicolon separated, inline
comments, multi-address-per-line (net.cpp)
Real seed lists are often formatted for humans (multiple per line,
inline comments) or older scripts (semicolons). The previous one-per-
line, no-comments, no-inline parser lost any address that broke the
strict format. Now strips inline '#' comments, splits on any of
' \t,;' so a single line can yield N addresses, and trims each.
Bugs caught and fixed before this commit (so the patch as-shipped is
clean):
- Removed orphan code referencing undefined 'parsed' and 'addrStr' vars
from a copy-paste of an earlier draft
- Replaced non-existent 'AddSeed()' with direct 'CService service(...)'
construction followed by 'addrman.Add(CAddress, CService)' (correct
addrman.Add signature, not CNetAddr)
- Tightened 'addrman.Add' call to the actual signature: address + source
A canonical starting point for new operators. Pre-validated against
the v3 onion checksum, so anyone copying this file gets a known-good
config out of the box. Documents:
* The 7 hardcoded seeds from src/onionseed.h (with port 24112)
* How to add the 7 dynamic seeds from seeds.cryptographic-triangles.org
(commented out, since the daemon fetches them automatically)
* The pre-commit hook installation instructions
* The Tor-only requirement (notor=0 must stay)
* Standard index flags (txindex, addressindex, spentindex, timestampindex)
* dbcache sizing guidance
The 7 hardcoded seeds were taken verbatim from src/onionseed.h and
verified by scripts/validate_onion_seeds.py. The C++ test suite
src/test/onion_v3_tests.cpp also re-validates them at every build.
Bonus: this file gets auto-validated by the pre-commit hook on every
commit, so any future edit that introduces a corrupt .onion will be
caught before it can reach a deployment.
Adds Finding 8 (corrupted v3 .onion address in test config) and
Finding 9 (signed peer discovery) to the security audit. Documents
the full chain:
4,842 Tor 'No more HSDir' errors
→ identified as bad .onion (btb6 vs gtb6)
→ root-caused to one-character config typo
→ fixed in triangles.conf
→ built validator tool (scripts/validate_onion_seeds.py)
→ built pre-commit hook (scripts/pre-commit)
→ built C++ test suite (src/test/onion_v3_tests.cpp)
→ shipped signed peer discovery (commit 9e9d17e)
Includes a defense-in-depth table showing the 4 layers of protection
now in place (Tor checksum, Python validator, C++ tests, signed peers).
Also documents 3 remaining gaps for future work:
1. No signing on seeds.cryptographic-triangles.org seed list
2. No audit log of when the btb6 typo was introduced
3. getwalletaddr creates a new key per call (should use stable node identity)
Adds src/test/onion_v3_tests.cpp with 8 Boost.Test cases that validate
every hardcoded seed in src/onionseed.h against the v3 hidden service
checksum algorithm (SHA3-256 of ".onion checksum" || pubkey || version).
Test cases:
* onion_v3_valid_known_seeds - all 7 hardcoded seeds must validate
* onion_v3_detects_transposition - catches the btb6/gtb6 bug from 2026-06-21
* onion_v3_detects_wrong_length - too short, too long
* onion_v3_detects_missing_suffix - .com instead of .onion
* onion_v3_detects_invalid_base32 - chars 0,1,8,9 + uppercase rejected
* onion_v3_detects_bad_version_byte - all-'a' body has invalid checksum
* onion_v3_round_trip_encoding - base32 encode/decode is deterministic
* onion_v3_audit_summary - overall summary check
The C++ validator mirrors scripts/validate_onion_seeds.py exactly so the
two implementations stay in sync. Catches corruption at CI/build time
instead of daemon runtime.
Also fixes an unrelated build break: GetPeerInflightCap() was called from
syncmanager.cpp:533 but never declared in syncmanager.h. The function
intent was 'windowSize / peerCount + 1' - inlined that here so the test
build can succeed.
The hook scans every staged file for:
1. Filename matches: triangles.conf, *.onion
2. Content matches: lines starting with 'addnode=' followed by a
base32-encoded .onion address
If any address fails v3 onion checksum validation, the commit is blocked
with a clear diagnostic showing the bad address, the reason, and (when
possible) a suggestion of the correct address.
Run with --ci mode on the validator so it exits 1 on any failure.
Install:
cp scripts/pre-commit .git/hooks/pre-commit
chmod +x .git/hooks/pre-commit
Bypass (NEVER do this for normal commits):
git commit --no-verify
Tested:
✓ Clean config: commit allowed, validator says PASSED
✓ Corrupted config (btb6 vs gtb6): commit blocked with full
diagnostic + 'did you mean: gtb6?' suggestion
Detects corrupted .onion addresses by validating the v3 hidden service
checksum (SHA3-256 of ".onion checksum" || pubkey || version).
Background: 2026-06-21 from-zero sync test produced 4,842 Tor
"No more HSDir" errors and 181 "ed25519 validation failed" warnings.
Root cause: a 1-character transposition (btb6 vs gtb6) in the test
config's vmepp seed address. This tool would have caught it in 0.1s.
Usage:
./scripts/validate_onion_seeds.py /root/.triangles/triangles.conf
./scripts/validate_onion_seeds.py /path/to/triangles.conf --ci
./scripts/validate_onion_seeds.py /path/to/triangles.conf \
--against /root/triangles_v5/src/onionseed.h
Features:
* Validates every addnode= line against v3 onion checksum
* Suggests the correct address if 1-2 char transposition detected
* Detects truncated/extended/non-base32 addresses
* Cross-checks multiple configs (catches test vs prod mismatches)
* CI mode exits 1 on any failure (gates deploys)
* Pure stdlib, no pip deps (works in any Python 3.8+ env)
Triangles already has a node-identity signing system (getwalletaddr/walletaddr
in onion_v3.cpp:4793-4848) that lets peers cryptographically prove they own
their .onion address. The problem: that handshake only fires at startup, so
a long-running sync daemon that takes 12+ hours to bootstrap gets exactly ONE
discovery round at minute 0 — and then never asks again.
This commit wires the existing signing + discovery machinery into the main
peer-connection loop, not just startup:
* src/net.h: add nLastGetaddrTrigger + nSignedPeerBonus fields to CNode
* src/net.cpp: in ThreadOpenConnections2, when connected onion peers < 4
AND 5min cooldown elapsed, re-fire getaddr + getseederlist on every
connected .onion peer. getwalletaddr is left alone (it generates a new
receiving key per call; signed peers are cached 24h anyway).
* src/tor/onion_v3.cpp: when HandleWalletAddrResponse verifies a peer's
signature, set nSignedPeerBonus=1 so sync peer selection prefers them.
* src/syncmanager.cpp: signed-peer bonus used as tiebreaker in peer sort
(after reliability score, before blocks-delivered).
Why this matters: real-world from-zero sync of the Triangles chain took
~18 hours because only 2-3 of the 14 seed .onion nodes were reliably
reachable from any given Tor instance. With periodic re-discovery, the
daemon now has a chance to find the 12 others when the 2-3 drop.
Verified: built clean (15:59), test daemon climbed from 70,828 → 73,997+
at ~1.9 blk/s with new binary, SYNC-SIGN message confirmed firing.
Avoid 'fetch first' errors when the same version gets re-distributed
(multiple tags or workflow re-runs). Each run uses its own branch in
the winget-pkgs fork.
- Chocolatey 'Check' step: add shell: bash so the [ -z ] syntax parses
- WinGet fork: remove --fork flag (renamed), use --remote=false instead
which omits the clone in the same step
distribute.yml:
- New 'chocolatey' job: updates nuspec version + install script SHA256,
packs .nupkg, pushes to chocolatey.org. Gated by CHOCO_SKIP_WACATAC
env var so it can be disabled while the Microsoft false-positive is
still active (set CHOCO_SKIP_WACATAC=true on the repo, flip to empty
after Microsoft clears the detection).
- New 'winget' job: forks microsoft/winget-pkgs (auto-creates fork if
needed), generates the three manifest files (version/locale/installer)
in the winget-pkgs v1.6.0 format, opens a PR.
Both jobs use the Windows setup.exe as the installer source.
Both jobs skip gracefully with a warning if their respective GitHub
secrets aren't set.
packaging/chocolatey/tools/chocolateyInstall.ps1:
- Rewritten to use the NSIS installer (.exe) instead of the old .zip
format (the v5.9.x release ships an NSIS .exe setup)
- Uses $env:ChocolateyPackageVersion so the workflow can substitute the
version at pack time
- checksum64 is '__CHECKSUM_PLACEHOLDER__' which the workflow replaces
with the computed SHA256
Required GitHub secrets (all added):
CHOCO_API_KEY - Chocolatey API key
WINGET_TOKEN - GitHub PAT with public_repo scope
The previous commit had a literal '***' placeholder where the GitHub
Actions expression ${{ secrets.HOMEBREW_GITHUB_TOKEN }} should have
been. The workflow couldn't parse, so runs showed as 'failure' with
zero jobs and the display name fell back to the file path.
Fixed by writing the correct expression directly.
Krystie