refactor: Operate on bytes instead of bits in Asmap code
What changed, and why it matters
This commit is a code cleanup (refactor) in Bitcoin Core's asmap feature. It changes the internal representation of asmap data from a sequence of individual bits (std::vector<bool>) to a sequence of bytes (std::vector<std::byte>). The visible behavior of IP-to-ASN mapping, sanity checks, and file loading is intended to remain the same. There is no security fix or vulnerability indicated in the commit message or diff.
No security action required. Treat as a normal maintainability refactor. Reviewers may optionally verify that the bit/byte conversion preserves the exact same decoding semantics, particularly the little-endian vs big-endian bit consumption in Interpret/SanityCheckASMap and the updated fuzz target prefix logic.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors the asmap interpreter and its callers to operate on std::vector
Changed components
src/util/asmap.cppsrc/util/asmap.hsrc/netgroup.cppsrc/netgroup.hsrc/init.cppsrc/test/fuzz/asmap.cppsrc/test/fuzz/asmap_direct.cppsrc/test/fuzz/util/net.hsrc/test/addrman_tests.cppsrc/test/netbase_tests.cppsrc/test/util/setup_common.cppsrc/bench/addrman.cpptest/functional/feature_asmap.pyInspect captured patch +142 / −160
diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp
index 907c7d2e..3f900b36 100644
--- a/src/bench/addrman.cpp
+++ b/src/bench/addrman.cpp
@@ -24,7 +24,7 @@
static constexpr size_t NUM_SOURCES = 64;
static constexpr size_t NUM_ADDRESSES_PER_SOURCE = 256;
-static NetGroupManager EMPTY_NETGROUPMAN{std::vector<bool>()};
+static NetGroupManager EMPTY_NETGROUPMAN{{}};
static constexpr uint32_t ADDRMAN_CONSISTENCY_CHECK_RATIO{0};
static std::vector<CAddress> g_sources;
diff --git a/src/init.cpp b/src/init.cpp
index 571d8b9c..6c14f501 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -96,6 +96,7 @@
#include <algorithm>
#include <cerrno>
#include <condition_variable>
+#include <cstddef>
#include <cstdint>
#include <cstdio>
#include <fstream>
@@ -1561,7 +1562,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
{
// Read asmap file if configured
- std::vector<bool> asmap;
+ std::vector<std::byte> asmap;
if (args.IsArgSet("-asmap") && !args.IsArgNegated("-asmap")) {
fs::path asmap_path = args.GetPathArg("-asmap");
if (asmap_path.empty()) {
diff --git a/src/netgroup.cpp b/src/netgroup.cpp
index 474d8a9c..970aa2b4 100644
--- a/src/netgroup.cpp
+++ b/src/netgroup.cpp
@@ -8,6 +8,8 @@
#include <logging.h>
#include <util/asmap.h>
+#include <cstddef>
+
uint256 NetGroupManager::GetAsmapChecksum() const
{
if (!m_asmap.size()) return {};
@@ -81,33 +83,27 @@ std::vector<unsigned char> NetGroupManager::GetGroup(const CNetAddr& address) co
uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const
{
uint32_t net_class = address.GetNetClass();
- if (m_asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) {
+ if (m_asmap.empty() || (net_class != NET_IPV4 && net_class != NET_IPV6)) {
return 0; // Indicates not found, safe because AS0 is reserved per RFC7607.
}
- std::vector<bool> ip_bits(128);
+ std::vector<std::byte> ip_bytes(16);
if (address.HasLinkedIPv4()) {
// For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits)
- for (int8_t byte_i = 0; byte_i < 12; ++byte_i) {
- for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
- ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1;
- }
- }
+ std::copy_n(std::as_bytes(std::span{IPV4_IN_IPV6_PREFIX}).begin(),
+ IPV4_IN_IPV6_PREFIX.size(), ip_bytes.begin());
uint32_t ipv4 = address.GetLinkedIPv4();
- for (int i = 0; i < 32; ++i) {
- ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1;
+ for (int i = 0; i < 4; ++i) {
+ ip_bytes[12 + i] = std::byte((ipv4 >> (24 - i * 8)) & 0xFF);
}
} else {
// Use all 128 bits of the IPv6 address otherwise
assert(address.IsIPv6());
auto addr_bytes = address.GetAddrBytes();
- for (int8_t byte_i = 0; byte_i < 16; ++byte_i) {
- uint8_t cur_byte = addr_bytes[byte_i];
- for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
- ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1;
- }
- }
+ assert(addr_bytes.size() == ip_bytes.size());
+ std::copy_n(std::as_bytes(std::span{addr_bytes}).begin(),
+ addr_bytes.size(), ip_bytes.begin());
}
- uint32_t mapped_as = Interpret(m_asmap, ip_bits);
+ uint32_t mapped_as = Interpret(m_asmap, ip_bytes);
return mapped_as;
}
diff --git a/src/netgroup.h b/src/netgroup.h
index 74dc7503..399876e5 100644
--- a/src/netgroup.h
+++ b/src/netgroup.h
@@ -8,6 +8,7 @@
#include <netaddress.h>
#include <uint256.h>
+#include <cstddef>
#include <vector>
/**
@@ -15,7 +16,7 @@
*/
class NetGroupManager {
public:
- explicit NetGroupManager(std::vector<bool> asmap)
+ explicit NetGroupManager(std::vector<std::byte>&& asmap)
: m_asmap{std::move(asmap)}
{}
@@ -70,7 +71,7 @@ private:
*
* This is initialized in the constructor, const, and therefore is
* thread-safe. */
- const std::vector<bool> m_asmap;
+ const std::vector<std::byte> m_asmap;
};
#endif // BITCOIN_NETGROUP_H
diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index 4debdc16..9cd667ba 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -24,7 +24,7 @@ using namespace std::literals;
using node::NodeContext;
using util::ToString;
-static NetGroupManager EMPTY_NETGROUPMAN{std::vector<bool>()};
+static NetGroupManager EMPTY_NETGROUPMAN{{}};
static const bool DETERMINISTIC{true};
static int32_t GetCheckRatio(const NodeContext& node_ctx)
@@ -46,20 +46,6 @@ static CService ResolveService(const std::string& ip, uint16_t port = 0)
return serv.value_or(CService{});
}
-
-static std::vector<bool> FromBytes(std::span<const std::byte> source)
-{
- int vector_size(source.size() * 8);
- std::vector<bool> result(vector_size);
- for (int byte_i = 0; byte_i < vector_size / 8; ++byte_i) {
- uint8_t cur_byte{std::to_integer<uint8_t>(source[byte_i])};
- for (int bit_i = 0; bit_i < 8; ++bit_i) {
- result[byte_i * 8 + bit_i] = (cur_byte >> bit_i) & 1;
- }
- }
- return result;
-}
-
BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(addrman_simple)
@@ -598,8 +584,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy)
// 101.8.0.0/16 AS8
BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket)
{
- std::vector<bool> asmap = FromBytes(test::data::asmap);
- NetGroupManager ngm_asmap{asmap};
+ std::vector<std::byte> asmap(test::data::asmap.begin(), test::data::asmap.end());
+ NetGroupManager ngm_asmap{std::move(asmap)};
CAddress addr1 = CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE);
CAddress addr2 = CAddress(ResolveService("250.1.1.1", 9999), NODE_NONE);
@@ -652,8 +638,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket)
BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket)
{
- std::vector<bool> asmap = FromBytes(test::data::asmap);
- NetGroupManager ngm_asmap{asmap};
+ std::vector<std::byte> asmap(test::data::asmap.begin(), test::data::asmap.end());
+ NetGroupManager ngm_asmap{std::move(asmap)};
CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE);
CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE);
@@ -730,8 +716,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket)
BOOST_AUTO_TEST_CASE(addrman_serialization)
{
- std::vector<bool> asmap1 = FromBytes(test::data::asmap);
- NetGroupManager netgroupman{asmap1};
+ std::vector<std::byte> asmap1(test::data::asmap.begin(), test::data::asmap.end());
+ NetGroupManager netgroupman{std::move(asmap1)};
const auto ratio = GetCheckRatio(m_node);
auto addrman_asmap1 = std::make_unique<AddrMan>(netgroupman, DETERMINISTIC, ratio);
diff --git a/src/test/fuzz/asmap.cpp b/src/test/fuzz/asmap.cpp
index fe019f93..40dde196 100644
--- a/src/test/fuzz/asmap.cpp
+++ b/src/test/fuzz/asmap.cpp
@@ -6,28 +6,18 @@
#include <netgroup.h>
#include <test/fuzz/fuzz.h>
#include <util/asmap.h>
+#include <util/strencodings.h>
#include <cstdint>
#include <vector>
+using namespace util::hex_literals;
+
//! asmap code that consumes nothing
-static const std::vector<bool> IPV6_PREFIX_ASMAP = {};
+static const std::vector<std::byte> IPV6_PREFIX_ASMAP = {};
//! asmap code that consumes the 96 prefix bits of ::ffff:0/96 (IPv4-in-IPv6 map)
-static const std::vector<bool> IPV4_PREFIX_ASMAP = {
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
- true, true, false, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, // Match 0xFF
- true, true, false, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true // Match 0xFF
-};
+static const auto IPV4_PREFIX_ASMAP = "fb03ec0fb03fc0fe00fb03ec0fb03fc0fe00fb03ec0fb0fffffeff"_hex_v;
FUZZ_TARGET(asmap)
{
@@ -37,13 +27,8 @@ FUZZ_TARGET(asmap)
bool ipv6 = buffer[0] & 128;
const size_t addr_size = ipv6 ? ADDR_IPV6_SIZE : ADDR_IPV4_SIZE;
if (buffer.size() < size_t(1 + asmap_size + addr_size)) return;
- std::vector<bool> asmap = ipv6 ? IPV6_PREFIX_ASMAP : IPV4_PREFIX_ASMAP;
- asmap.reserve(asmap.size() + 8 * asmap_size);
- for (int i = 0; i < asmap_size; ++i) {
- for (int j = 0; j < 8; ++j) {
- asmap.push_back((buffer[1 + i] >> j) & 1);
- }
- }
+ std::vector<std::byte> asmap = ipv6 ? IPV6_PREFIX_ASMAP : IPV4_PREFIX_ASMAP;
+ std::ranges::copy(std::as_bytes(buffer.subspan(1, asmap_size)), std::back_inserter(asmap));
if (!SanityCheckASMap(asmap, 128)) return;
const uint8_t* addr_data = buffer.data() + 1 + asmap_size;
@@ -57,6 +42,6 @@ FUZZ_TARGET(asmap)
memcpy(&ipv4, addr_data, addr_size);
net_addr.SetIP(CNetAddr{ipv4});
}
- NetGroupManager netgroupman{asmap};
+ NetGroupManager netgroupman{std::move(asmap)};
(void)netgroupman.GetMappedAS(net_addr);
}
diff --git a/src/test/fuzz/asmap_direct.cpp b/src/test/fuzz/asmap_direct.cpp
index 1d693e5e..fa775532 100644
--- a/src/test/fuzz/asmap_direct.cpp
+++ b/src/test/fuzz/asmap_direct.cpp
@@ -12,6 +12,24 @@
#include <cassert>
+std::vector<std::byte> BitsToBytes(std::span<const uint8_t> bits) noexcept
+{
+ std::vector<std::byte> ret;
+ uint8_t next_byte{0};
+ int next_byte_bits{0};
+ for (uint8_t val : bits) {
+ next_byte |= (val & 1) << (next_byte_bits++);
+ if (next_byte_bits == 8) {
+ ret.push_back(std::byte(next_byte));
+ next_byte = 0;
+ next_byte_bits = 0;
+ }
+ }
+ if (next_byte_bits) ret.push_back(std::byte(next_byte));
+
+ return ret;
+}
+
FUZZ_TARGET(asmap_direct)
{
// Encoding: [asmap using 1 bit / byte] 0xFF [addr using 1 bit / byte]
@@ -28,22 +46,24 @@ FUZZ_TARGET(asmap_direct)
}
if (!sep_pos_opt) return; // Needs exactly 1 separator
const size_t sep_pos{sep_pos_opt.value()};
- if (buffer.size() - sep_pos - 1 > 128) return; // At most 128 bits in IP address
+ const size_t ip_len{buffer.size() - sep_pos - 1};
+ if (ip_len > 128) return; // At most 128 bits in IP address
// Checks on asmap
- std::vector<bool> asmap(buffer.begin(), buffer.begin() + sep_pos);
- if (SanityCheckASMap(asmap, buffer.size() - 1 - sep_pos)) {
+ auto asmap = BitsToBytes(buffer.first(sep_pos));
+ if (SanityCheckASMap(asmap, ip_len)) {
// Verify that for valid asmaps, no prefix (except up to 7 zero padding bits) is valid.
- std::vector<bool> asmap_prefix = asmap;
- while (!asmap_prefix.empty() && asmap_prefix.size() + 7 > asmap.size() && asmap_prefix.back() == false) {
- asmap_prefix.pop_back();
- }
- while (!asmap_prefix.empty()) {
- asmap_prefix.pop_back();
- assert(!SanityCheckASMap(asmap_prefix, buffer.size() - 1 - sep_pos));
+ for (size_t prefix_len = sep_pos - 1; prefix_len > 0; --prefix_len) {
+ auto prefix = BitsToBytes(buffer.first(prefix_len));
+ // We have to skip the prefixes of the same length as the original
+ // asmap, since they will contain some zero padding bits in the last
+ // byte.
+ if (prefix.size() == asmap.size()) continue;
+ assert(!SanityCheckASMap(prefix, ip_len));
}
+
// No address input should trigger assertions in interpreter
- std::vector<bool> addr(buffer.begin() + sep_pos + 1, buffer.end());
+ auto addr = BitsToBytes(buffer.subspan(sep_pos + 1));
(void)Interpret(asmap, addr);
}
}
diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h
index a7b1bfd5..d72ec2f8 100644
--- a/src/test/fuzz/util.h
+++ b/src/test/fuzz/util.h
@@ -65,11 +65,6 @@ template<typename B = uint8_t>
return ret;
}
-[[nodiscard]] inline std::vector<bool> ConsumeRandomLengthBitVector(FuzzedDataProvider& fuzzed_data_provider, const std::optional<size_t>& max_length = std::nullopt) noexcept
-{
- return BytesToBits(ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length));
-}
-
[[nodiscard]] inline DataStream ConsumeDataStream(FuzzedDataProvider& fuzzed_data_provider, const std::optional<size_t>& max_length = std::nullopt) noexcept
{
return DataStream{ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length)};
diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h
index 90c018a4..93aabcc3 100644
--- a/src/test/fuzz/util/net.h
+++ b/src/test/fuzz/util/net.h
@@ -21,6 +21,7 @@
#include <util/sock.h>
#include <chrono>
+#include <cstddef>
#include <cstdint>
#include <limits>
#include <memory>
@@ -234,9 +235,9 @@ public:
[[nodiscard]] inline NetGroupManager ConsumeNetGroupManager(FuzzedDataProvider& fuzzed_data_provider) noexcept
{
- std::vector<bool> asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider);
+ std::vector<std::byte> asmap{ConsumeRandomLengthByteVector<std::byte>(fuzzed_data_provider)};
if (!SanityCheckASMap(asmap, 128)) asmap.clear();
- return NetGroupManager(asmap);
+ return NetGroupManager(std::move(asmap));
}
inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept
diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
index 266d952f..37997845 100644
--- a/src/test/netbase_tests.cpp
+++ b/src/test/netbase_tests.cpp
@@ -325,7 +325,7 @@ BOOST_AUTO_TEST_CASE(subnet_test)
BOOST_AUTO_TEST_CASE(netbase_getgroup)
{
- NetGroupManager netgroupman{std::vector<bool>()}; // use /16
+ NetGroupManager netgroupman{{}}; // use /16
BOOST_CHECK(netgroupman.GetGroup(ResolveIP("127.0.0.1")) == std::vector<unsigned char>({0})); // Local -> !Routable()
BOOST_CHECK(netgroupman.GetGroup(ResolveIP("257.0.0.1")) == std::vector<unsigned char>({0})); // !Valid -> !Routable()
BOOST_CHECK(netgroupman.GetGroup(ResolveIP("10.0.0.1")) == std::vector<unsigned char>({0})); // RFC1918 -> !Routable()
@@ -630,17 +630,8 @@ BOOST_AUTO_TEST_CASE(asmap_test_vectors)
"33e53662a7d72a29477b5beb35710591d3e23e5f0379baea62ffdee535bcdf879cbf69b88d7ea37c8015381cf"
"63dc33d28f757a4a5e15d6a08"_hex};
- // Convert to std::vector<bool> format that the ASMap interpreter uses.
- std::vector<bool> asmap_bits;
- asmap_bits.reserve(ASMAP_DATA.size() * 8);
- for (auto byte : ASMAP_DATA) {
- for (int bit = 0; bit < 8; ++bit) {
- asmap_bits.push_back((std::to_integer<uint8_t>(byte) >> bit) & 1);
- }
- }
-
// Construct NetGroupManager with this data.
- NetGroupManager netgroup{std::move(asmap_bits)};
+ NetGroupManager netgroup{std::vector(ASMAP_DATA.begin(), ASMAP_DATA.end())};
BOOST_CHECK(netgroup.UsingASMap());
// Check some randomly-generated IPv6 addresses in it (biased towards the very beginning and
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 4e886639..454f7a74 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -344,7 +344,7 @@ TestingSetup::TestingSetup(
if (!opts.setup_net) return;
- m_node.netgroupman = std::make_unique<NetGroupManager>(/*asmap=*/std::vector<bool>());
+ m_node.netgroupman = std::make_unique<NetGroupManager>(/*asmap=*/std::vector<std::byte>{});
m_node.addrman = std::make_unique<AddrMan>(*m_node.netgroupman,
/*deterministic=*/false,
m_node.args->GetIntArg("-checkaddrman", 0));
diff --git a/src/util/asmap.cpp b/src/util/asmap.cpp
index c7388c77..23cf6d16 100644
--- a/src/util/asmap.cpp
+++ b/src/util/asmap.cpp
@@ -13,6 +13,7 @@
#include <algorithm>
#include <bit>
#include <cassert>
+#include <cstddef>
#include <cstdio>
#include <utility>
#include <vector>
@@ -21,16 +22,28 @@ namespace {
constexpr uint32_t INVALID = 0xFFFFFFFF;
-uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos, uint8_t minval, const std::vector<uint8_t> &bit_sizes)
+inline bool ConsumeBitLE(size_t& bitpos, std::span<const std::byte> bytes) noexcept
+{
+ const bool bit = (std::to_integer<uint8_t>(bytes[bitpos / 8]) >> (bitpos % 8)) & 1;
+ ++bitpos;
+ return bit;
+}
+
+inline bool ConsumeBitBE(uint8_t& bitpos, std::span<const std::byte> bytes) noexcept
+{
+ const bool bit = (std::to_integer<uint8_t>(bytes[bitpos / 8]) >> (7 - (bitpos % 8))) & 1;
+ ++bitpos;
+ return bit;
+}
+
+uint32_t DecodeBits(size_t& bitpos, const std::vector<std::byte>& data, uint8_t minval, const std::vector<uint8_t>& bit_sizes)
{
uint32_t val = minval;
bool bit;
- for (std::vector<uint8_t>::const_iterator bit_sizes_it = bit_sizes.begin();
- bit_sizes_it != bit_sizes.end(); ++bit_sizes_it) {
+ for (auto bit_sizes_it = bit_sizes.begin(); bit_sizes_it != bit_sizes.end(); ++bit_sizes_it) {
if (bit_sizes_it + 1 != bit_sizes.end()) {
- if (bitpos == endpos) break;
- bit = *bitpos;
- bitpos++;
+ if (bitpos >= data.size() * 8) break;
+ bit = ConsumeBitLE(bitpos, data);
} else {
bit = 0;
}
@@ -38,9 +51,8 @@ uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, const std::vector
val += (1 << *bit_sizes_it);
} else {
for (int b = 0; b < *bit_sizes_it; b++) {
- if (bitpos == endpos) return INVALID; // Reached EOF in mantissa
- bit = *bitpos;
- bitpos++;
+ if (bitpos >= data.size() * 8) return INVALID; // Reached EOF in mantissa
+ bit = ConsumeBitLE(bitpos, data);
val += bit << (*bit_sizes_it - 1 - b);
}
return val;
@@ -58,69 +70,68 @@ enum class Instruction : uint32_t
};
const std::vector<uint8_t> TYPE_BIT_SIZES{0, 0, 1};
-Instruction DecodeType(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
+Instruction DecodeType(size_t& bitpos, const std::vector<std::byte>& data)
{
- return Instruction(DecodeBits(bitpos, endpos, 0, TYPE_BIT_SIZES));
+ return Instruction(DecodeBits(bitpos, data, 0, TYPE_BIT_SIZES));
}
const std::vector<uint8_t> ASN_BIT_SIZES{15, 16, 17, 18, 19, 20, 21, 22, 23, 24};
-uint32_t DecodeASN(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
+uint32_t DecodeASN(size_t& bitpos, const std::vector<std::byte>& data)
{
- return DecodeBits(bitpos, endpos, 1, ASN_BIT_SIZES);
+ return DecodeBits(bitpos, data, 1, ASN_BIT_SIZES);
}
const std::vector<uint8_t> MATCH_BIT_SIZES{1, 2, 3, 4, 5, 6, 7, 8};
-uint32_t DecodeMatch(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
+uint32_t DecodeMatch(size_t& bitpos, const std::vector<std::byte>& data)
{
- return DecodeBits(bitpos, endpos, 2, MATCH_BIT_SIZES);
+ return DecodeBits(bitpos, data, 2, MATCH_BIT_SIZES);
}
const std::vector<uint8_t> JUMP_BIT_SIZES{5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30};
-uint32_t DecodeJump(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
+uint32_t DecodeJump(size_t& bitpos, const std::vector<std::byte>& data)
{
- return DecodeBits(bitpos, endpos, 17, JUMP_BIT_SIZES);
+ return DecodeBits(bitpos, data, 17, JUMP_BIT_SIZES);
}
}
-uint32_t Interpret(const std::vector<bool> &asmap, const std::vector<bool> &ip)
+uint32_t Interpret(const std::vector<std::byte>& asmap, const std::vector<std::byte>& ip)
{
- std::vector<bool>::const_iterator pos = asmap.begin();
- const std::vector<bool>::const_iterator endpos = asmap.end();
- uint8_t bits = ip.size();
+ size_t pos{0};
+ const size_t endpos{asmap.size() * 8};
+ uint8_t ip_bit{0};
+ const uint8_t ip_bits_end = ip.size() * 8;
uint32_t default_asn = 0;
uint32_t jump, match, matchlen;
Instruction opcode;
- while (pos != endpos) {
- opcode = DecodeType(pos, endpos);
+ while (pos < endpos) {
+ opcode = DecodeType(pos, asmap);
if (opcode == Instruction::RETURN) {
- default_asn = DecodeASN(pos, endpos);
+ default_asn = DecodeASN(pos, asmap);
if (default_asn == INVALID) break; // ASN straddles EOF
return default_asn;
} else if (opcode == Instruction::JUMP) {
- jump = DecodeJump(pos, endpos);
+ jump = DecodeJump(pos, asmap);
if (jump == INVALID) break; // Jump offset straddles EOF
- if (bits == 0) break; // No input bits left
- if (int64_t{jump} >= int64_t{endpos - pos}) break; // Jumping past EOF
- if (ip[ip.size() - bits]) {
+ if (ip_bit == ip_bits_end) break; // No input bits left
+ if (int64_t{jump} >= static_cast<int64_t>(endpos - pos)) break; // Jumping past EOF
+ if (ConsumeBitBE(ip_bit, ip)) {
pos += jump;
}
- bits--;
} else if (opcode == Instruction::MATCH) {
- match = DecodeMatch(pos, endpos);
+ match = DecodeMatch(pos, asmap);
if (match == INVALID) break; // Match bits straddle EOF
matchlen = std::bit_width(match) - 1;
- if (bits < matchlen) break; // Not enough input bits
+ if ((ip_bits_end - ip_bit) < matchlen) break; // Not enough input bits
for (uint32_t bit = 0; bit < matchlen; bit++) {
- if ((ip[ip.size() - bits]) != ((match >> (matchlen - 1 - bit)) & 1)) {
+ if (ConsumeBitBE(ip_bit, ip) != ((match >> (matchlen - 1 - bit)) & 1)) {
return default_asn;
}
- bits--;
}
} else if (opcode == Instruction::DEFAULT) {
- default_asn = DecodeASN(pos, endpos);
+ default_asn = DecodeASN(pos, asmap);
if (default_asn == INVALID) break; // ASN straddles EOF
} else {
break; // Instruction straddles EOF
@@ -130,50 +141,47 @@ uint32_t Interpret(const std::vector<bool> &asmap, const std::vector<bool> &ip)
return 0; // 0 is not a valid ASN
}
-bool SanityCheckASMap(const std::vector<bool>& asmap, int bits)
+bool SanityCheckASMap(const std::vector<std::byte>& asmap, int bits)
{
- const std::vector<bool>::const_iterator begin = asmap.begin(), endpos = asmap.end();
- std::vector<bool>::const_iterator pos = begin;
+ size_t pos{0};
+ const size_t endpos{asmap.size() * 8};
std::vector<std::pair<uint32_t, int>> jumps; // All future positions we may jump to (bit offset in asmap -> bits to consume left)
jumps.reserve(bits);
Instruction prevopcode = Instruction::JUMP;
bool had_incomplete_match = false;
while (pos != endpos) {
- uint32_t offset = pos - begin;
- if (!jumps.empty() && offset >= jumps.back().first) return false; // There was a jump into the middle of the previous instruction
- Instruction opcode = DecodeType(pos, endpos);
+ if (!jumps.empty() && pos >= jumps.back().first) return false; // There was a jump into the middle of the previous instruction
+ Instruction opcode = DecodeType(pos, asmap);
if (opcode == Instruction::RETURN) {
if (prevopcode == Instruction::DEFAULT) return false; // There should not be any RETURN immediately after a DEFAULT (could be combined into just RETURN)
- uint32_t asn = DecodeASN(pos, endpos);
+ uint32_t asn = DecodeASN(pos, asmap);
if (asn == INVALID) return false; // ASN straddles EOF
if (jumps.empty()) {
// Nothing to execute anymore
if (endpos - pos > 7) return false; // Excessive padding
while (pos != endpos) {
- if (*pos) return false; // Nonzero padding bit
- ++pos;
+ if (ConsumeBitLE(pos, asmap)) return false; // Nonzero padding bit
}
return true; // Sanely reached EOF
} else {
// Continue by pretending we jumped to the next instruction
- offset = pos - begin;
- if (offset != jumps.back().first) return false; // Unreachable code
+ if (pos != jumps.back().first) return false; // Unreachable code
bits = jumps.back().second; // Restore the number of bits we would have had left after this jump
jumps.pop_back();
prevopcode = Instruction::JUMP;
}
} else if (opcode == Instruction::JUMP) {
- uint32_t jump = DecodeJump(pos, endpos);
+ uint32_t jump = DecodeJump(pos, asmap);
if (jump == INVALID) return false; // Jump offset straddles EOF
- if (int64_t{jump} > int64_t{endpos - pos}) return false; // Jump out of range
+ if (int64_t{jump} > static_cast<int64_t>(endpos - pos)) return false; // Jump out of range
if (bits == 0) return false; // Consuming bits past the end of the input
--bits;
- uint32_t jump_offset = pos - begin + jump;
+ uint32_t jump_offset = pos + jump;
if (!jumps.empty() && jump_offset >= jumps.back().first) return false; // Intersecting jumps
jumps.emplace_back(jump_offset, bits);
prevopcode = Instruction::JUMP;
} else if (opcode == Instruction::MATCH) {
- uint32_t match = DecodeMatch(pos, endpos);
+ uint32_t match = DecodeMatch(pos, asmap);
if (match == INVALID) return false; // Match bits straddle EOF
int matchlen = std::bit_width(match) - 1;
if (prevopcode != Instruction::MATCH) had_incomplete_match = false;
@@ -184,7 +192,7 @@ bool SanityCheckASMap(const std::vector<bool>& asmap, int bits)
prevopcode = Instruction::MATCH;
} else if (opcode == Instruction::DEFAULT) {
if (prevopcode == Instruction::DEFAULT) return false; // There should not be two successive DEFAULTs (they could be combined into one)
- uint32_t asn = DecodeASN(pos, endpos);
+ uint32_t asn = DecodeASN(pos, asmap);
if (asn == INVALID) return false; // ASN straddles EOF
prevopcode = Instruction::DEFAULT;
} else {
@@ -194,27 +202,24 @@ bool SanityCheckASMap(const std::vector<bool>& asmap, int bits)
return false; // Reached EOF without RETURN instruction
}
-std::vector<bool> DecodeAsmap(fs::path path)
+std::vector<std::byte> DecodeAsmap(fs::path path)
{
- std::vector<bool> bits;
FILE *filestr = fsbridge::fopen(path, "rb");
AutoFile file{filestr};
if (file.IsNull()) {
LogWarning("Failed to open asmap file from disk");
- return bits;
+ return {};
}
int64_t length{file.size()};
LogInfo("Opened asmap file %s (%d bytes) from disk", fs::quoted(fs::PathToString(path)), length);
- uint8_t cur_byte;
- for (int i = 0; i < length; ++i) {
- file >> cur_byte;
- for (int bit = 0; bit < 8; ++bit) {
- bits.push_back((cur_byte >> bit) & 1);
- }
- }
- if (!SanityCheckASMap(bits, 128)) {
+
+ std::vector<std::byte> buffer(length);
+ file.read(buffer);
+
+ if (!SanityCheckASMap(buffer, 128)) {
LogWarning("Sanity check of asmap file %s failed", fs::quoted(fs::PathToString(path)));
return {};
}
- return bits;
+
+ return buffer;
}
diff --git a/src/util/asmap.h b/src/util/asmap.h
index 87260869..b107ad25 100644
--- a/src/util/asmap.h
+++ b/src/util/asmap.h
@@ -7,14 +7,15 @@
#include <util/fs.h>
+#include <cstddef>
#include <cstdint>
#include <vector>
-uint32_t Interpret(const std::vector<bool> &asmap, const std::vector<bool> &ip);
+uint32_t Interpret(const std::vector<std::byte>& asmap, const std::vector<std::byte>& ip);
-bool SanityCheckASMap(const std::vector<bool>& asmap, int bits);
+bool SanityCheckASMap(const std::vector<std::byte>& asmap, int bits);
/** Read asmap from provided binary file */
-std::vector<bool> DecodeAsmap(fs::path path);
+std::vector<std::byte> DecodeAsmap(fs::path path);
#endif // BITCOIN_UTIL_ASMAP_H
diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py
index 65298270..762f5a14 100755
--- a/test/functional/feature_asmap.py
+++ b/test/functional/feature_asmap.py
@@ -18,7 +18,7 @@ from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal
ASMAP = 'src/test/data/asmap.raw' # path to unit test skeleton asmap
-VERSION = 'fec61fa21a9f46f3b17bdcd660d7f4cd90b966aad3aec593c99b35f0aca15853'
+VERSION = '6dfbc157b8a97b6e9fc7fc08d4e43d30247bbf62055eaa1098f46db9885855e3'
def expected_messages(filename):
return [f'Opened asmap file "{filename}" (59 bytes) from disk',
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.