Page MenuHomePhabricator

Introduce well-defined CAddress disk serialization
ClosedPublic

Authored by PiRK on Nov 8 2023, 08:03.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC7a098167fe03: Introduce well-defined CAddress disk serialization
Summary

Before this commit, CAddress disk serialization was messy. It stored
CLIENT_VERSION in the first 4 bytes, optionally OR'ed with ADDRV2_FORMAT.

  • All bits except ADDRV2_FORMAT were ignored, making it hard to use for actual future format changes.
  • ADDRV2_FORMAT determines whether or not nServices is serialized in LE64 format or in CompactSize format.
  • Whether or not the embedded CService is serialized in V1 or V2 format is determined by the stream's version having ADDRV2_FORMAT (as opposed to the nServices encoding, which is determined by the disk version).

To improve the situation, this commit introduces the following disk
serialization format, compatible with earlier versions, but better defined for
future changes:

  • The first 4 bytes store a format version number. Its low 19 bits are ignored (as it historically stored the CLIENT_VERSION), but its high 13 bits specify the serialization exactly:
    • 0x00000000: LE64 encoding for nServices, V1 encoding for CService
    • 0x20000000: CompactSize encoding for nServices, V2 encoding for CService
    • Any other value triggers an unsupported format error on deserialization, and can be used for future format changes.
  • The ADDRV2_FORMAT flag in the stream's version does not impact the actual serialization format; it only determines whether V2 encoding is permitted; whether it's actually enabled depends solely on the disk version number.

Operationally the changes to the deserializer are:

  • Failure when the stored format version number is unexpected.
  • The embedded CService's format is determined by the stored format version number rather than the stream's version number.

These do no introduce incompatibilities, as no code versions exist that write
any value other than 0 or 0x20000000 in the top 13 bits, and no code paths
where the stream's version differs from the stored version.

This is a backport of core#20516

Test Plan

ninja all check-all bitcoin-fuzzers

run a few hours of IBD, stop and restart the node, check that addresses are loaded successfully from peers.dat

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Nov 8 2023, 08:03

Why didn't you backport the fuzzer change ?

Fabien requested changes to this revision.Nov 8 2023, 10:03

Please run IBD as well in the test so the node runs for longer and connects many peers outside of the controlled test environment

This revision now requires changes to proceed.Nov 8 2023, 10:03

Why didn't you backport the fuzzer change ?

I can do that. I'll need https://github.com/bitcoin/bitcoin/pull/20355 first

PiRK edited the test plan for this revision. (Show Details)

rebase onto D14755 and include the fuzzer changes

This revision is now accepted and ready to land.Nov 8 2023, 13:38

Tail of the build log:

warning: some crates are on edition 2021 which defaults to `resolver = "2"`, but virtual workspaces default to `resolver = "1"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
    Finished release [optimized] target(s) in 0.11s
[2/12] Running utility command for _cargo-build_chronik-lib
[3/12] Generating ../cargo/build/x86_64-pc-windows-gnu/cxxbridge/chronik-bridge/src/ffi.rs.cc, ../cargo/build/x86_64-pc-windows-gnu/cxxbridge/chronik-lib/src/ffi.rs.cc
Generating cxx bridge files
[3/4] Run CPack packaging tool...
CPack: Create package using NSIS
CPack: Install projects
CPack: - Install directory: /work/doc
CPack: - Install project: bitcoin-abc []
CPack: Create package
CPack: - package: /work/abc-ci-builds/build-win64/bitcoin-abc-0.28.3-x86_64-w64-mingw32.exe generated.
CPack: Create package using ZIP
CPack: Install projects
CPack: - Install directory: /work/doc
CPack: - Install project: bitcoin-abc []
CPack: Create package
CPack: - package: /work/abc-ci-builds/build-win64/bitcoin-abc-0.28.3-x86_64-w64-mingw32.zip generated.
wine: created the configuration directory '/root/.wine'
002c:fixme:actctx:parse_depend_manifests Could not find dependent assembly L"Microsoft.Windows.Common-Controls" (6.0.0.0)
002c:fixme:winediag:loader_init wine-staging 8.19 is a testing version containing experimental patches.
002c:fixme:winediag:loader_init Please mention your exact version when filing bug reports on winehq.org.
004c:fixme:actctx:parse_depend_manifests Could not find dependent assembly L"Microsoft.Windows.Common-Controls" (6.0.0.0)
004c:err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded.
004c:err:winediag:nodrv_CreateWindow L"The explorer process failed to start."
004c:err:systray:initialize_systray Could not create tray window
004c:err:ole:StdMarshalImpl_MarshalInterface Failed to create ifstub, hr 0x80004002
004c:err:ole:CoMarshalInterface Failed to marshal the interface {6d5140c1-7436-11ce-8034-00aa006009fa}, hr 0x80004002
004c:err:ole:apartment_get_local_server_stream Failed: 0x80004002
0044:err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded.
0044:err:winediag:nodrv_CreateWindow L"Make sure that your X server is running and that $DISPLAY is set correctly."
002c:err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded.
002c:err:winediag:nodrv_CreateWindow L"Make sure that your X server is running and that $DISPLAY is set correctly."
0070:err:ntoskrnl:ServiceMain Failed to load L"C:\\windows\\system32\\win32k.sys"
0070:err:ntoskrnl:ServiceMain Failed to load L"C:\\windows\\system32\\drivers\\dxgkrnl.sys"
0070:err:ntoskrnl:ServiceMain Failed to load L"C:\\windows\\system32\\drivers\\dxgmms1.sys"
0088:err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded.
0088:err:winediag:nodrv_CreateWindow L"Make sure that your X server is running and that $DISPLAY is set correctly."
0090:err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded.
0090:err:winediag:nodrv_CreateWindow L"Make sure that your X server is running and that $DISPLAY is set correctly."
00d8:fixme:msg:pack_message msg 14 (WM_ERASEBKGND) not supported yet
00d8:err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded.
00d8:err:winediag:nodrv_CreateWindow L"Make sure that your X server is running and that $DISPLAY is set correctly."
0104:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
0104:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
0104:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
0104:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
wine: configuration in L"/root/.wine" has been updated.
0130:fixme:winspool:PerfOpen (null): stub
0130:fixme:winspool:PerfCollect L"Global", 00007FFFFE2FEA98, 00007FFFFE2FEA7C, 00007FFFFE2FEA80: stub
0130:fixme:winspool:PerfClose stub
Running 582 test cases...
0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0130:fixme:file:NtLockFile I/O completion on lock not implemented yet
0070:fixme:mountmgr:harddisk_ioctl Unsupported ioctl 90064 (device=9 access=0 func=19 method=0)
0  0070:fixme:mountmgr:harddisk_ioctl Unsupported ioctl 90064 (device=9 access=0 func=19 method=0)
0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  Assertion failed: nPreferredDownload == 0, file ../../src/net_processing.cpp, line 1997

Build build-win64 failed with exit code 3