Two behavior-preserving refactors flagged by the Phase-6 hygiene scoping:
* addrman: CAddrMan::Select_ contained two ~40-line copies of the same
bucket-table random walk, differing only in the table (vvTried/vvNew),
its bucket count, and a log label. Extract the shared loop into
SelectFromTable_(vvTable, nBucketCount, tableName); Select_ now just
dispatches to it. Verbatim move — clean compile proves self-containment.
* pow: the "Equihash uses powLimit, everything else uses powAlternate"
selection was copy-pasted as an if/else into GetNextWorkRequired,
CalculateNextWorkRequired, and lwmaCalculateNextWorkRequired. Extract
into PowLimitForAlgo(params). The CheckProofOfWork site (line ~892) is
left as-is: it has an extra `height <= 1` genesis special-case and is
NOT the same selection. On DragonX (RandomX) this always returns
powAlternate, exactly as before.
Validated: full build of dragonxd/cli/tx, isolated self-mine to height
336, verifychain 4 0 -> true (exercises PowLimitForAlgo on every block).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fifth phase of the code-hygiene remediation, done per-file (18 files) via an
18-agent Workflow with strict guardrails. No consensus or security VALUES
changed (verified: pow/hush_utils/hush_bitcoind/net.h/crypter/stratum have
zero non-comment numeric changes); built clean, self-mined, verifychain=true.
Totals: 57 stale comments corrected, 46 user-facing branding fixes, 6
behavior-preserving named constants, 4 stratum warnings, 77 items deliberately
left (consensus/security values, copyright headers).
- Stale comments -> DragonX: corrected HUSH3/HUSH/Komodo/Equihash/Arrakis
leftovers that misdescribed the active chain (pow AWT/lwma notes, txdb
"Equihash solution" -> RandomX, wallet CLTV/overwinter/Sapling@1 notes,
util datadir provenance, crypter's stale mapSproutSpendingKeys invariant).
Replaced unprofessional/editorializing comments (profane TODOs, "developers
are elite") with neutral technical notes, preserving the real design info.
- User-facing branding: dumpwallet header "created by Hush" -> "DragonX";
RPC help text hushd/hush-cli/hushprivkey/hushaddress/Agama -> dragonx*;
rebranded provably-dead "HUSH3" symbol-fallback literals to DRAGONX.
- Named constants (same values, non-consensus/non-security): addrman
peer-selection factors (kChanceFactorGrowth/kChanceScale/deprioritize),
sietch MIN_ZOUTS, an init notarization-DB cache size.
- Left the security macros (ECC/TFM_TIMING_RESISTANT=420, comment-clarified
only), the consensus subsidy/commission literals + 128/129 TRANSITION
(comment only), and copyright headers untouched.
- Added a prominent WARNING that the stratum server is Equihash-era and NOT
updated for DragonX's RandomX PoW (flagged, not rewritten).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run-time asserts are a horrible anti-pattern littered across code
inherited from BTC. One could maybe argue they are the right thing to do
in some situations but not when managing the peer database. Crashing our
full node and potentially corrupting our wallet or block index is INSANE
in the case of some inconsistencies in peers.dat .
We seem to have a bug that we don't find a peer when looking for a peer
in peers.dat . Instead of doing an assertion and crashing the node,
just keep iterating in the loop. The code which computes nId may have an
off-by-one error, such that it looks up an incorrect bucket position and
doesn't find a valid nId.
From BTC Core https://github.com/bitcoin/bitcoin/pull/9037/ with modifications to support our asmap.
This has a small part of code commented out that depends feeler connection code.
Any projects which want to use Hush code from now on will need to be licensed as
GPLv3 or we will send the lawyers: https://www.softwarefreedom.org/
Notably, Komodo (KMD) is licensed as GPLv2 and is no longer compatible to receive
code changes, without causing legal issues. MIT projects, such as Zcash, also cannot pull
in changes from the Hush Full Node without permission from The Hush Developers,
which may in some circumstances grant an MIT license on a case-by-case basis.
Adds several unittests for CAddrMan and CAddrInfo.
Increases the accuracy of addrman tests.
Removes non-determinism in tests by overriding the random number generator.
Extracts testing code from addrman class to test class.
Signed-off-by: Simon <simon@bitcartel.com>
Adds several unittests for addrman to verify it works as expected.
Makes small modifications to addrman to allow deterministic and targeted tests.
Signed-off-by: Simon <simon@bitcartel.com>
the Select_() function will loop endlessly trying to find an address,
and therefore eat up 100% cpu time on the 'opencon' thread.
Solution is to (1) add a delay to the loop and (2) restrict the number
of attempts to find an address. On exiting the loop, we return
to an outer loop in net.cpp which will sleep, add seed nodes and
calcualte new addresses.
On hosts that had spent some time with a failed internet connection their
nAttempts penalty was going through the roof (e.g. thousands for all peers)
and as a result the connect search was pegging the CPU and failing to get
more than a 4 connections after days of running (because it was taking so
long per try).
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
Give each address a single fixed location in the new and tried tables,
which become simple fixed-size arrays instead of sets and vectors.
This prevents attackers from having an advantages by inserting an
address multiple times.
This change was suggested as Countermeasure 1 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
It is also more efficient.
After the tinyformat switch sprintf() family functions support passing
actual std::string objects.
Remove unnecessary c_str calls (236 of them) in logging and formatting.
The function Get64(.) has a bug in case the width is not divisible by 64.
Since it is only ever used as Get64(0) this simply changes it to this
special case. Additionally, an assert is added, and a cast to prevent
a compiler error.
Use misc methods of avoiding unnecesary header includes.
Replace int typedefs with int##_t from stdint.h.
Replace PRI64[xdu] with PRI[xdu]64 from inttypes.h.
Normalize QT_VERSION ifs where possible.
Resolve some indirect dependencies as direct ones.
Remove extern declarations from .cpp files.
A function returned the element to remove from a bucket, instead of its
position in that bucket. This function was only called when a tried
bucket overflowed, which only happens after many outgoing connections
have been made.
Closes: #1065, #1156