Skip to content

BIP-155 addrv2 + Remove Tor v2 (OnionCat) support - #2

Closed
VictorLux wants to merge 12 commits into
masterfrom
feature/onion-v3-cleanup
Closed

BIP-155 addrv2 + Remove Tor v2 (OnionCat) support#2
VictorLux wants to merge 12 commits into
masterfrom
feature/onion-v3-cleanup

Conversation

@VictorLux

@VictorLux VictorLux commented Dec 11, 2025

Copy link
Copy Markdown
Owner

Summary

  • Implement BIP-155 (addrv2) protocol for Tor v3 address support
  • Remove deprecated Tor v2 (OnionCat) address handling completely
  • NET_ONION fully removed - All code now uses NET_TORV3 only (no aliases)
  • Add peers.dat serialization for v3 onion addresses

Changes

BIP-155 Implementation

  • Add sendaddrv2 message exchange during version handshake
  • Implement addrv2 serialization format with variable-length addresses
  • Support NET_TORV3, NET_I2P, NET_CJDNS network types
  • Update peers.dat to persist v3 onion addresses
  • Protocol version bumped to 170012

Onion v2 Removal (Complete)

  • Removed pchOnionCat[] constant (FD87:D87E:EB43 prefix)
  • Removed v2 16-char .onion parsing in SetSpecial()
  • Removed v2 base32 encoding in ToStringIP()
  • Removed #define NET_ONION NET_TORV3 alias
  • Replaced all NET_ONION references with NET_TORV3
  • Unified IsTor() to call IsTorV3()

Files Modified

File Changes
src/netbase.h Clean enum with NET_TORV3 only, BIP155 types
src/netbase.cpp v3 parsing only, 3 NET_TORV3 refs
src/init.cpp 6 NET_TORV3 refs, proxy setup
src/torcontrol.cpp 2 NET_TORV3 refs
src/net.h / src/net.cpp sendaddrv2 handshake, addrv2 handling
src/addrman.h / src/addrman.cpp peers.dat v2 format
src/test/netbase_tests.cpp v3 onion tests only
doc/BIP-155-IMPLEMENTATION-PLAN.md Updated to v3.0

Verification

grep -rn "NET_ONION" src/    # Returns 0 results
make -j8                      # Build OK
./src/test/test_bitcoin --run_test=netbase_tests  # 7 tests PASS

Test plan

  • Build compiles successfully (macOS ARM64)
  • netbase_tests pass (7 tests, no errors)
  • Daemon connects to Tor v3 .onion peers
  • BIP-155 sendaddrv2/addrv2 messages exchanged
  • peers.dat persists v3 addresses across restarts
  • grep NET_ONION returns 0 results (clean codebase)
  • Linux/Windows build testing needed

🤖 Generated with Claude Code

VictorLux and others added 12 commits December 9, 2025 09:40
Research-based implementation plan incorporating lessons learned from:
- Bitcoin Core PRs #19031, #19954, #20119, #20564
- Zcash issues zcash#5277, zcash#3051

Key issues addressed in plan:
- sendaddrv2 timing (send before verack, not after)
- Protocol version gating (don't send to old nodes)
- Address relay black holes prevention
- peers.dat migration strategy
- Spam prevention for unknown network types

4-phase implementation over ~4 weeks:
1. Data structures
2. Protocol messages
3. Address manager
4. Testing & hardening

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Bump protocol version to 170012
- Add sendaddrv2/addrv2 message handlers
- Add BIP155Network enum and address size constants
- Implement address serialization for variable-length addresses
- Add m_wants_addrv2 flag for peer negotiation
- Update address relay logic for addrv2-aware peers
- Add -enablebip155 config flag (default: on)
- Backward compatible with protocol 170011 peers

Tested: Tor v3 onion address advertised, legacy peers receive
legacy addr format, unexpected sendaddrv2 from old peers rejected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Accept sendaddrv2 at any time from 170012+ peers, not just before
verack. This improves compatibility with implementations that may
send sendaddrv2 slightly after verack while still rejecting it
from old protocol versions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When -proxy or -onion is set, also configure NET_TORV3 network:
- Set proxy for Tor v3 connections
- Mark NET_TORV3 as reachable (not limited)

This allows the node to make outbound connections to Tor v3
onion addresses and properly display them in getnetworkinfo.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit completes the BIP-155 implementation by adding proper
serialization support for variable-length addresses in peers.dat,
enabling Tor v3 addresses to persist across node restarts.

Changes aligned with Bitcoin Core's approach (PR #19954, #20284):

peers.dat format versioning:
- Format byte 0: version (V0_HISTORICAL, V1_DETERMINISTIC, V3_BIP155)
- Format byte 1: INCOMPATIBILITY_BASE(32) + lowest_compatible_version
- Old nodes see keysize=35 (not 32), fail gracefully with
  "Incorrect keysize in addrman deserialization"
- New nodes detect format and use addrv2 when format >= V3_BIP155

Serialization changes:
- src/serialize.h: Add SER_ADDRV2 flag for address serialization
- src/netbase.h: CNetAddr/CService serialize in BIP155 format when
  SER_ADDRV2 flag is set (network_id + CompactSize len + variable addr)
- src/protocol.h: CAddress uses CompactSize for services in addrv2
- src/addrman.h: CAddrMan Serialize/Unserialize handle both legacy
  and addrv2 formats with proper version detection

Migration behavior:
- Read: Detects old format (keysize=32) and reads legacy 16-byte addrs
- Write: Always writes V3_BIP155 format with addrv2 addresses
- Backward: Old nodes reject new peers.dat, create fresh database

This enables Zclassic nodes to store and relay Tor v3 onion addresses
(.onion 56-char), making Zclassic one of the first Zcash-based coins
to fully implement BIP-155.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Build fixes for the BIP-155 peers.dat implementation:

1. Replace static const with enum for INCOMPATIBILITY_BASE
   - static const/constexpr class members require out-of-line definitions
     when ODR-used (passed to strprintf varargs)
   - Using anonymous enum avoids linker "undefined symbol" errors
   - Removed FILE_FORMAT and LOWEST_COMPATIBLE, use V3_BIP155 directly

2. Fix CDataStream buffer access
   - CDataStream doesn't have data() method (unlike std::vector)
   - Use &ssAddr[0] instead of ssAddr.data() for write() calls

3. Add explicit int casts in strprintf
   - Enum values passed to varargs need explicit casting to avoid
     format specifier warnings

Tested: peers.dat now correctly writes V3_BIP155 format (03 23 header)
verified by xxd after flush - old format was (01 20), new is (03 23).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Two critical bugs were causing peers.dat load failures after addrv2 flush:

1. Missing nVersion read in addrv2 Unserialize (addrman.h)
   - CAddress with SER_DISK serializes: nVersion(4) + nTime(4) + nServices + CService
   - The addrv2 parsing code was missing the nVersion read, causing stream
     position misalignment and "ReadCompactSize(): size too large" errors
   - Fixed by adding `int nAddrVersion; s >> nAddrVersion;` before nAddrTime

2. m_net field not set after legacy format deserialization (netbase.h)
   - When loading legacy peers.dat, CNetAddr::m_net remained at default NET_IPV4
   - This caused GetBIP155Network() to return BIP155_IPV4 for all addresses
   - IPv6 addresses were incorrectly serialized as 4-byte IPv4, corrupting data
   - Fixed by detecting network type from IP content after legacy FLATDATA read:
     IsIPv4() -> NET_IPV4, IsTor() -> NET_ONION, else -> NET_IPV6

Testing verified:
- Legacy peers.dat (01 20) loads correctly, sets proper m_net values
- Addrv2 peers.dat (03 23) written on flush with correct network IDs
- Addrv2 peers.dat reloads correctly (tested with 2059 addresses)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Documentation updates:
- doc/bips.md: Add BIP-155 to list of implemented BIPs for Zclassic v2.1.2
- doc/BIP-155-IMPLEMENTATION-PLAN.md:
  - Document Bitcoin Core compatible peers.dat versioning scheme
  - Add upgrade instructions with mandatory peers.dat backup step
  - Document rollback procedures
  - List critical bug fixes (nVersion read, m_net detection)

peers.dat format details:
- Legacy format: [magic][0x01][0x20]... (format=1, compat=32)
- BIP155 format: [magic][0x03][0x23]... (format=3, compat=35)
- Versioning uses INCOMPATIBILITY_BASE(32) + lowest_compatible
- Old nodes reject new format gracefully ("Corrupt peers.dat")
- New nodes can read both legacy and addrv2 formats

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Mark implementation as tested on macOS ARM64 only
- Add TODO items for Linux and Windows testing
- Add requirement for extended testnet validation before mainnet release
- Document additional test results (peers.dat migration cycle)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove pchOnionCat constant and all OnionCat IPv6 prefix handling
- Remove v2 .onion (16-char) address parsing in SetSpecial()
- Remove v2 ToStringIP() encoding
- Unify IsTor() to just call IsTorV3()
- Remove NET_ONION enum, add compatibility alias to NET_TORV3
- Remove ADDR_TORV2_SIZE constant
- Update BIP155 handling to reject TORV2 addresses
- Update tests to use v3 onion addresses instead of OnionCat

Tor v2 has been deprecated since October 2021 and fully removed from
the Tor network. This cleanup removes ~30 lines of dead code and
simplifies the codebase for easier maintenance.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove #define NET_ONION NET_TORV3 compatibility alias
- Replace all NET_ONION references with NET_TORV3 in:
  - src/netbase.h (serialization)
  - src/netbase.cpp (ParseNetwork, GetNetwork, GetGroup)
  - src/init.cpp (proxy setup - cleaned up duplicate calls)
  - src/torcontrol.cpp (Tor authentication)
  - src/test/netbase_tests.cpp (v3 onion tests)
- Update BIP-155 documentation to v3.0
- Regenerate cleanup diagram with better layout

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Shows NET_ONION completely removed (not just aliased)
- Includes verification: grep NET_ONION = 0 results
- Documents all files modified with NET_TORV3 references

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@VictorLux VictorLux closed this in 63008e7 Jun 5, 2026
VictorLux added a commit that referenced this pull request Jun 9, 2026
… audit status)

Detailed record of the work integrated by this branch: the ConnectBlock
use-after-free fix (CVE-2024-52911 parity) with its gtest + ASan proof, the
deep-reorg parking IBD-gate review and three-layer-defense safety analysis,
the ZIP-209 checked-arithmetic hardening, and the open audit items (#2-ZclassicCommunity#5).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VictorLux added a commit that referenced this pull request Jun 9, 2026
…IBD-gate + ZIP-209 hardening

Integrates (see doc/security-hardening-2026-06.md for full detail):

- 28382a7  Fix use-after-free in ConnectBlock check-queue lifetime
             (CVE-2024-52911 / GHSA-fqr9-fxpx-rfpf parity). `control` was
             declared before `txdata`, so an early return destroyed `txdata`
             before ~CCheckQueueControl()'s Wait() drained the queued checks
             that dereference it -> heap-use-after-free, reachable as a remote
             DoS (-par>=2, block above checkpoint, late-failing valid-PoW block).
             Fixed by declaring txdata before control. Guarded by a deterministic
             gtest; ASan-proven (buggy order -> UAF, fixed order -> clean).

- dcbdf14  Deep-reorg parking: skip during IBD/reindex. Fixes the from-genesis
             reindex stall (spurious parking on out-of-order blk*.dat loads).
             Reviewed safe: IsInitialBlockDownload() latches to false after first
             catch-up so at-tip behaviour is unchanged; the skip window is
             backstopped by depth-10 auto-finalization and the 99-block
             MAX_REORG_LENGTH cap, both still active.

- bbea317  ZIP-209 pool accounting: checked CAmount delta arithmetic.

- fb585fd, b2da5f6  TVSP proposal (v2) docs.

Open audit items (#2 checkpoint-hash enforcement, #3 reindex size band-aids,
ZclassicCommunity#4 header-DoS hardening, ZclassicCommunity#5 timestamp) are tracked in the doc, NOT in this merge.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VictorLux added a commit that referenced this pull request Jun 9, 2026
ContextualCheckBlockHeader rejected only forks strictly below the last
checkpoint present in mapBlockIndex; it never rejected a header presented
AT a checkpoint height with the wrong hash, and depended on chain state. A
fresh/eclipsed node could therefore accept a forged chain that does not pass
through the compiled checkpoint hashes.

Add Checkpoints::CheckBlock(data, nHeight, hash) (mirrors upstream) and call
it in ContextualCheckBlockHeader under fCheckpointsEnabled: a header at a
checkpoint height whose hash != the compiled checkpoint hash is rejected
(DoS 100, REJECT_CHECKPOINT, "bad-fork-checkpoint"). Reads the hardcoded
checkpoint map directly, so it is independent of mapBlockIndex state and also
protects a fresh/eclipsed node during bootstrap.

Pure tightening: canonical headers at checkpoint heights match by definition,
so honest peers are never rejected; an empty checkpoint map (regtest) is a
no-op. Unit test covers no-checkpoint / match / mismatch branches.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VictorLux added a commit that referenced this pull request Jun 9, 2026
…rdening writeup

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VictorLux added a commit that referenced this pull request Jun 9, 2026
… header acceptance (audit #2)

Closes the audit #2 gap: ContextualCheckBlockHeader now rejects a header
presented AT a checkpoint height with the wrong hash, via the new
Checkpoints::CheckBlock reading the hardcoded checkpoint map (independent of
mapBlockIndex state -> protects fresh/eclipsed nodes during bootstrap). Pure
tightening; honest peers unaffected; regtest (empty checkpoints) is a no-op.
Unit test added; all Checkpoints_tests pass and the UAF gtest still passes.

doc/security-hardening-2026-06.md updated to mark #2 done (open items now #3/ZclassicCommunity#4/ZclassicCommunity#5).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VictorLux added a commit that referenced this pull request Jun 9, 2026
… fixes to README

The #1 (UAF) and #2 (checkpoint) commits added lines to main.cpp, shifting
several line references in doc/security-hardening-2026-06.md. Re-grepped and
corrected every reference against master 9bf04ef (substance unchanged; only
the cited line numbers were stale), and added a note that line numbers are
as-of-commit and each reference names its symbol.

README.md: the "Consensus Hardening" section listed only the older fixes
(ZIP-209, CR-01, reindex genesis-crash). Added the three just-merged items —
ConnectBlock UAF fix (CVE-2024-52911 parity), deep-reorg parking IBD-gate, and
checkpoint hash lock-in — with a link to the security-hardening writeup, and
clarified the deployment-notes line (which fixes are soft-fork-class vs
reliability-only).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant