Skip to content

Commit 509dc91

Browse files
committed
Merge bitcoin#33026: test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work
7f318e1 test: Add better coverage for Autofile size() (Fabian Jahr) b7af960 refactor: Add AutoFile::size (Fabian Jahr) ec0f758 refactor: Modernize logging in util/asmap.cpp (Fabian Jahr) 606a251 tests: add unit test vectors for asmap interpreter (Pieter Wuille) Pull request description: This contains some commits from bitcoin#28792 that can be easily reviewed and merged independently. I hope splitting this change off can make this part move a bit faster and reduce frequency of needed rebases for bitcoin#28792. The commits in order: - Add additional unit test vectors to the asmap interpreter (written by sipa). This helps to ensure that the further refactors in bitcoin#28792 don't change behavior. - Modernizes the logging in `util/asmap.cpp`, I added this while touching the rest of the file all over anyway. - Adds an `AutoFile::size` helper function with some additional test coverage in a separate commit ACKs for top commit: maflcko: review ACK 7f318e1 🏀 hodlinator: tACK 7f318e1 laanwj: Code review ACK 7f318e1 Tree-SHA512: 45156b74e4bd9278a7ec24521dfdafe4dab1ba3384243c7d589ef17e16ca374ee2af7178c86b7229e80ca262dbe78c4d456d80a6ee742ec31d2ab5243dac8b57
2 parents b126f98 + 7f318e1 commit 509dc91

File tree

6 files changed

+116
-8
lines changed

6 files changed

+116
-8
lines changed

src/streams.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,20 @@ int64_t AutoFile::tell()
5757
return *m_position;
5858
}
5959

60+
int64_t AutoFile::size()
61+
{
62+
if (IsNull()) {
63+
throw std::ios_base::failure("AutoFile::size: file handle is nullptr");
64+
}
65+
// Temporarily save the current position
66+
int64_t current_pos = tell();
67+
seek(0, SEEK_END);
68+
int64_t file_size = tell();
69+
// Restore the original position
70+
seek(current_pos, SEEK_SET);
71+
return file_size;
72+
}
73+
6074
void AutoFile::read(std::span<std::byte> dst)
6175
{
6276
if (detail_fread(dst) != dst.size()) {

src/streams.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,9 @@ class AutoFile
435435
/** Find position within the file. Will throw if unknown. */
436436
int64_t tell();
437437

438+
/** Return the size of the file. Will throw if unknown. */
439+
int64_t size();
440+
438441
/** Wrapper around FileCommit(). */
439442
bool Commit();
440443

src/test/netbase_tests.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <boost/test/unit_test.hpp>
2020

2121
using namespace std::literals;
22+
using namespace util::hex_literals;
2223

2324
BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup)
2425

@@ -610,4 +611,56 @@ BOOST_AUTO_TEST_CASE(isbadport)
610611
BOOST_CHECK_EQUAL(std::ranges::count_if(ports, IsBadPort), 85);
611612
}
612613

614+
615+
BOOST_AUTO_TEST_CASE(asmap_test_vectors)
616+
{
617+
// Randomly generated encoded ASMap with 128 ranges, up to 20-bit AS numbers.
618+
constexpr auto ASMAP_DATA{
619+
"fd38d50f7d5d665357f64bba6bfc190d6078a7e68e5d3ac032edf47f8b5755f87881bfd3633d9aa7c1fa279b3"
620+
"6fe26c63bbc9de44e0f04e5a382d8e1cddbe1c26653bc939d4327f287e8b4d1f8aff33176787cb0ff7cb28e3f"
621+
"daef0f8f47357f801c9f7ff7a99f7f9c9f99de7f3156ae00f23eb27a303bc486aa3ccc31ec19394c2f8a53ddd"
622+
"ea3cc56257f3b7e9b1f488be9c1137db823759aa4e071eef2e984aaf97b52d5f88d0f373dd190fe45e06efef1"
623+
"df7278be680a73a74c76db4dd910f1d30752c57fe2bc9f079f1a1e1b036c2a69219f11c5e11980a3fa51f4f82"
624+
"d36373de73b1863a8c27e36ae0e4f705be3d76ecff038a75bc0f92ba7e7f6f4080f1c47c34d095367ecf4406c"
625+
"1e3bbc17ba4d6f79ea3f031b876799ac268b1e0ea9babf0f9a8e5f6c55e363c6363df46afc696d7afceaf49b6"
626+
"e62df9e9dc27e70664cafe5c53df66dd0b8237678ada90e73f05ec60e6f6e96c3cbb1ea2f9dece115d5bdba10"
627+
"33e53662a7d72a29477b5beb35710591d3e23e5f0379baea62ffdee535bcdf879cbf69b88d7ea37c8015381cf"
628+
"63dc33d28f757a4a5e15d6a08"_hex};
629+
630+
// Convert to std::vector<bool> format that the ASMap interpreter uses.
631+
std::vector<bool> asmap_bits;
632+
asmap_bits.reserve(ASMAP_DATA.size() * 8);
633+
for (auto byte : ASMAP_DATA) {
634+
for (int bit = 0; bit < 8; ++bit) {
635+
asmap_bits.push_back((std::to_integer<uint8_t>(byte) >> bit) & 1);
636+
}
637+
}
638+
639+
// Construct NetGroupManager with this data.
640+
NetGroupManager netgroup{std::move(asmap_bits)};
641+
BOOST_CHECK(netgroup.UsingASMap());
642+
643+
// Check some randomly-generated IPv6 addresses in it (biased towards the very beginning and
644+
// very end of the 128-bit range).
645+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("0:1559:183:3728:224c:65a5:62e6:e991", false)), 961340);
646+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("d0:d493:faa0:8609:e927:8b75:293c:f5a4", false)), 961340);
647+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("2a0:26f:8b2c:2ee7:c7d1:3b24:4705:3f7f", false)), 693761);
648+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("a77:7cd4:4be5:a449:89f2:3212:78c6:ee38", false)), 0);
649+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("1336:1ad6:2f26:4fe3:d809:7321:6e0d:4615", false)), 672176);
650+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("1d56:abd0:a52f:a8d5:d5a7:a610:581d:d792", false)), 499880);
651+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("378e:7290:54e5:bd36:4760:971c:e9b9:570d", false)), 0);
652+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("406c:820b:272a:c045:b74e:fc0a:9ef2:cecc", false)), 248495);
653+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("46c2:ae07:9d08:2d56:d473:2bc7:57e3:20ac", false)), 248495);
654+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("50d2:3db6:52fa:2e7:12ec:5bc4:1bd1:49f9", false)), 124471);
655+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("53e1:1812:ffa:dccf:f9f2:64be:75fa:795", false)), 539993);
656+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("544d:eeba:3990:35d1:ad66:f9a3:576d:8617", false)), 374443);
657+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("6a53:40dc:8f1d:3ffa:efeb:3aa3:df88:b94b", false)), 435070);
658+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("87aa:d1c9:9edb:91e7:aab1:9eb9:baa0:de18", false)), 244121);
659+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("9f00:48fa:88e3:4b67:a6f3:e6d2:5cc1:5be2", false)), 862116);
660+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("c49f:9cc6:86ad:ba08:4580:315e:dbd1:8a62", false)), 969411);
661+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("dff5:8021:61d:b17d:406d:7888:fdac:4a20", false)), 969411);
662+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("e888:6791:2960:d723:bcfd:47e1:2d8c:599f", false)), 824019);
663+
BOOST_CHECK_EQUAL(netgroup.GetMappedAS(*LookupHost("ffff:d499:8c4b:4941:bc81:d5b9:b51e:85a8", false)), 824019);
664+
}
665+
613666
BOOST_AUTO_TEST_SUITE_END()

src/test/streams_tests.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ BOOST_AUTO_TEST_CASE(xor_file)
101101
BOOST_CHECK_EXCEPTION(xor_file << std::byte{}, std::ios_base::failure, HasReason{"AutoFile::write: file handle is nullptr"});
102102
BOOST_CHECK_EXCEPTION(xor_file >> std::byte{}, std::ios_base::failure, HasReason{"AutoFile::read: file handle is nullptr"});
103103
BOOST_CHECK_EXCEPTION(xor_file.ignore(1), std::ios_base::failure, HasReason{"AutoFile::ignore: file handle is nullptr"});
104+
BOOST_CHECK_EXCEPTION(xor_file.size(), std::ios_base::failure, HasReason{"AutoFile::size: file handle is nullptr"});
104105
}
105106
{
106107
#ifdef __MINGW64__
@@ -111,6 +112,7 @@ BOOST_AUTO_TEST_CASE(xor_file)
111112
#endif
112113
AutoFile xor_file{raw_file(mode), obfuscation};
113114
xor_file << test1 << test2;
115+
BOOST_CHECK_EQUAL(xor_file.size(), 7);
114116
BOOST_REQUIRE_EQUAL(xor_file.fclose(), 0);
115117
}
116118
{
@@ -121,6 +123,7 @@ BOOST_AUTO_TEST_CASE(xor_file)
121123
BOOST_CHECK_EQUAL(HexStr(raw), "fc01fd03fd04fa");
122124
// Check that no padding exists
123125
BOOST_CHECK_EXCEPTION(non_xor_file.ignore(1), std::ios_base::failure, HasReason{"AutoFile::ignore: end of file"});
126+
BOOST_CHECK_EQUAL(non_xor_file.size(), 7);
124127
}
125128
{
126129
AutoFile xor_file{raw_file("rb"), obfuscation};
@@ -130,6 +133,7 @@ BOOST_AUTO_TEST_CASE(xor_file)
130133
BOOST_CHECK_EQUAL(HexStr(read2), HexStr(test2));
131134
// Check that eof was reached
132135
BOOST_CHECK_EXCEPTION(xor_file >> std::byte{}, std::ios_base::failure, HasReason{"AutoFile::read: end of file"});
136+
BOOST_CHECK_EQUAL(xor_file.size(), 7);
133137
}
134138
{
135139
AutoFile xor_file{raw_file("rb"), obfuscation};
@@ -141,6 +145,7 @@ BOOST_AUTO_TEST_CASE(xor_file)
141145
// Check that ignore and read fail now
142146
BOOST_CHECK_EXCEPTION(xor_file.ignore(1), std::ios_base::failure, HasReason{"AutoFile::ignore: end of file"});
143147
BOOST_CHECK_EXCEPTION(xor_file >> std::byte{}, std::ios_base::failure, HasReason{"AutoFile::read: end of file"});
148+
BOOST_CHECK_EQUAL(xor_file.size(), 7);
144149
}
145150
}
146151

@@ -781,4 +786,41 @@ BOOST_AUTO_TEST_CASE(streams_hashed)
781786
BOOST_CHECK_EQUAL(hash_writer.GetHash(), hash_verifier.GetHash());
782787
}
783788

789+
BOOST_AUTO_TEST_CASE(size_preserves_position)
790+
{
791+
const fs::path path = m_args.GetDataDirBase() / "size_pos_test.bin";
792+
AutoFile f{fsbridge::fopen(path, "w+b")};
793+
for (uint8_t j = 0; j < 10; ++j) {
794+
f << j;
795+
}
796+
797+
// Test that usage of size() does not change the current position
798+
//
799+
// Case: Pos at beginning of the file
800+
f.seek(0, SEEK_SET);
801+
(void)f.size();
802+
uint8_t first{};
803+
f >> first;
804+
BOOST_CHECK_EQUAL(first, 0);
805+
806+
// Case: Pos at middle of the file
807+
f.seek(0, SEEK_SET);
808+
// Move pos to middle
809+
f.ignore(4);
810+
(void)f.size();
811+
uint8_t middle{};
812+
f >> middle;
813+
// Pos still at 4
814+
BOOST_CHECK_EQUAL(middle, 4);
815+
816+
// Case: Pos at EOF
817+
f.seek(0, SEEK_END);
818+
(void)f.size();
819+
uint8_t end{};
820+
BOOST_CHECK_EXCEPTION(f >> end, std::ios_base::failure, HasReason{"AutoFile::read: end of file"});
821+
822+
BOOST_REQUIRE_EQUAL(f.fclose(), 0);
823+
fs::remove(path);
824+
}
825+
784826
BOOST_AUTO_TEST_SUITE_END()

src/util/asmap.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,11 @@ std::vector<bool> DecodeAsmap(fs::path path)
200200
FILE *filestr = fsbridge::fopen(path, "rb");
201201
AutoFile file{filestr};
202202
if (file.IsNull()) {
203-
LogPrintf("Failed to open asmap file from disk\n");
203+
LogWarning("Failed to open asmap file from disk");
204204
return bits;
205205
}
206-
file.seek(0, SEEK_END);
207-
int length = file.tell();
206+
int64_t length{file.size()};
208207
LogInfo("Opened asmap file %s (%d bytes) from disk", fs::quoted(fs::PathToString(path)), length);
209-
file.seek(0, SEEK_SET);
210208
uint8_t cur_byte;
211209
for (int i = 0; i < length; ++i) {
212210
file >> cur_byte;
@@ -215,9 +213,8 @@ std::vector<bool> DecodeAsmap(fs::path path)
215213
}
216214
}
217215
if (!SanityCheckASMap(bits, 128)) {
218-
LogPrintf("Sanity check of asmap file %s failed\n", fs::quoted(fs::PathToString(path)));
216+
LogWarning("Sanity check of asmap file %s failed", fs::quoted(fs::PathToString(path)));
219217
return {};
220218
}
221219
return bits;
222220
}
223-

src/wallet/migrate.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -544,8 +544,7 @@ void BerkeleyRODatabase::Open()
544544
page_size = outer_meta.pagesize;
545545

546546
// Verify the size of the file is a multiple of the page size
547-
db_file.seek(0, SEEK_END);
548-
int64_t size = db_file.tell();
547+
const int64_t size{db_file.size()};
549548

550549
// Since BDB stores everything in a page, the file size should be a multiple of the page size;
551550
// However, BDB doesn't actually check that this is the case, and enforcing this check results

0 commit comments

Comments
 (0)