Differential D9178

Restore compatibility with old CSubNet serialization
ClosedPublic
Actions

Authored by Fabien on Feb 5 2021, 08:56.

Details

Reviewers

majcosta

Group Reviewers

Restricted Project

Commits

rABCab15f29c5be2: Restore compatibility with old CSubNet serialization

Summary

#19628 changed CSubNet for IPv4 netmasks, using the first 4 bytes of
netmask rather than the last 4 to store the actual mask. Unfortunately,
CSubNet objects are serialized on disk in banlist.dat, breaking
compatibility with existing banlists (and bringing them into an
inconsistent state where entries reported in listbanned cannot be
removed).

Fix this by reverting to the old format (just for serialization). Also
add a sanity check to the deserializer so that nonsensical banlist.dat
entries are ignored (which would otherwise be possible if someone added
IPv4 entries after #19628 but without this PR).

Backport of core PR20140.

Depends on D9176.

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Feb 5 2021, 08:56

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 5 2021, 08:56

Fabien requested review of this revision.Feb 5 2021, 08:56

teamcity edited the summary of this revision. (Show Details)Feb 5 2021, 08:56

Harbormaster completed remote builds in B15137: Diff 27574.Feb 5 2021, 09:05

Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

[384/440] Running utility command for check-bitcoin-sighashtype_tests
[385/440] bitcoin: testing wallet_tests
[386/440] Running utility command for check-bitcoin-finalization_tests
[387/440] Running utility command for check-bitcoin-wallet_tests
[388/440] bitcoin: testing sync_tests
[389/440] bitcoin: testing torcontrol_tests
[390/440] Running utility command for check-bitcoin-sync_tests
[391/440] bitcoin: testing settings_tests
[392/440] Running utility command for check-bitcoin-torcontrol_tests
[393/440] bitcoin: testing scriptpubkeyman_tests
[394/440] Running utility command for check-bitcoin-settings_tests
[395/440] Running utility command for check-bitcoin-scriptpubkeyman_tests
[396/440] bitcoin: testing uint256_tests
[397/440] Running utility command for check-bitcoin-uint256_tests
[398/440] bitcoin: testing walletdb_tests
[399/440] Running utility command for check-bitcoin-walletdb_tests
[400/440] bitcoin: testing undo_tests
[401/440] Running utility command for check-bitcoin-undo_tests
[402/440] bitcoin: testing txvalidationcache_tests
[403/440] bitcoin: testing serialize_tests
[404/440] Running utility command for check-bitcoin-txvalidationcache_tests
[405/440] Running utility command for check-bitcoin-serialize_tests
[406/440] bitcoin: testing validation_chainstatemanager_tests
[407/440] bitcoin: testing radix_tests
[408/440] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[409/440] Running utility command for check-bitcoin-radix_tests
[410/440] bitcoin: testing ismine_tests
[411/440] bitcoin: testing schnorr_tests
[412/440] bitcoin: testing blockcheck_tests
[413/440] Running utility command for check-bitcoin-ismine_tests
[414/440] Running utility command for check-bitcoin-schnorr_tests
[415/440] Running utility command for check-bitcoin-blockcheck_tests
[416/440] bitcoin: testing cashaddr_tests
[417/440] bitcoin: testing crypto_tests
[418/440] Running utility command for check-bitcoin-cashaddr_tests
[419/440] Running utility command for check-bitcoin-crypto_tests
[420/440] bitcoin: testing ref_tests
[421/440] Running utility command for check-bitcoin-ref_tests
[422/440] bitcoin: testing script_tests
[423/440] bitcoin: testing util_tests
[424/440] Running utility command for check-bitcoin-util_tests
[425/440] Running utility command for check-bitcoin-script_tests
[426/440] bitcoin: testing validation_tests
[427/440] Running utility command for check-bitcoin-validation_tests
[428/440] bitcoin: testing cuckoocache_tests
[429/440] bitcoin: testing skiplist_tests
[430/440] bitcoin: testing coinselector_tests
FAILED: src/test/CMakeFiles/check-bitcoin-coinselector_tests 
cd /work/abc-ci-builds/build-clang/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-clang/test/log/bitcoin-coinselector_tests.log /work/abc-ci-builds/build-clang/src/test/test_bitcoin --run_test=coinselector_tests --logger=HRF,message:JUNIT,message,bitcoin-coinselector_tests.xml --catch_system_errors=no
Segmentation fault (core dumped)
[431/440] Running utility command for check-bitcoin-cuckoocache_tests
[432/440] Running utility command for check-bitcoin-skiplist_tests
[433/440] bitcoin: testing op_reversebytes_tests
[434/440] Running utility command for check-bitcoin-op_reversebytes_tests
[435/440] bitcoin: testing transaction_tests
[436/440] Running utility command for check-bitcoin-transaction_tests
[437/440] bitcoin: testing coins_tests
[438/440] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

majcosta accepted this revision.Feb 5 2021, 17:17

This revision is now accepted and ready to land.Feb 5 2021, 17:17

This revision was landed with ongoing or failed builds.Feb 5 2021, 17:37

Closed by commit rABCab15f29c5be2: Restore compatibility with old CSubNet serialization (authored by Pieter Wuille <pieter@wuille.net>, committed by Fabien). · Explain Why

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABCab15f29c5be2: Restore compatibility with old CSubNet serialization.