refactor: Operate on bytes instead of bits in Asmap code

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>

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;

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()) {

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) {
+        std::copy_n(std::as_bytes(std::span{IPV4_IN_IPV6_PREFIX}).begin(),
-            for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
+                    IPV4_IN_IPV6_PREFIX.size(), ip_bytes.begin());
-                ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1;
-            }
-        }
         uint32_t ipv4 = address.GetLinkedIPv4();
-        for (int i = 0; i < 32; ++i) {
+        for (int i = 0; i < 4; ++i) {
-            ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1;
+            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) {
+        assert(addr_bytes.size() == ip_bytes.size());
-            uint8_t cur_byte = addr_bytes[byte_i];
+        std::copy_n(std::as_bytes(std::span{addr_bytes}).begin(),
-            for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
+                    addr_bytes.size(), ip_bytes.begin());
-                ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1;
-            }
-        }
     }
-    uint32_t mapped_as = Interpret(m_asmap, ip_bits);
+    uint32_t mapped_as = Interpret(m_asmap, ip_bytes);
     return mapped_as;
 }
 

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

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);
+    std::vector<std::byte> asmap(test::data::asmap.begin(), test::data::asmap.end());
-    NetGroupManager ngm_asmap{asmap};
+    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);
+    std::vector<std::byte> asmap(test::data::asmap.begin(), test::data::asmap.end());
-    NetGroupManager ngm_asmap{asmap};
+    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);
+    std::vector<std::byte> asmap1(test::data::asmap.begin(), test::data::asmap.end());
-    NetGroupManager netgroupman{asmap1};
+    NetGroupManager netgroupman{std::move(asmap1)};
 
     const auto ratio = GetCheckRatio(m_node);
     auto addrman_asmap1 = std::make_unique<AddrMan>(netgroupman, DETERMINISTIC, ratio);

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 = {
+static const auto IPV4_PREFIX_ASMAP = "fb03ec0fb03fc0fe00fb03ec0fb03fc0fe00fb03ec0fb0fffffeff"_hex_v;
-    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
-};
 
 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;
+    std::vector<std::byte> asmap = ipv6 ? IPV6_PREFIX_ASMAP : IPV4_PREFIX_ASMAP;
-    asmap.reserve(asmap.size() + 8 * asmap_size);
+    std::ranges::copy(std::as_bytes(buffer.subspan(1, asmap_size)), std::back_inserter(asmap));
-    for (int i = 0; i < asmap_size; ++i) {
-        for (int j = 0; j < 8; ++j) {
-            asmap.push_back((buffer[1 + i] >> j) & 1);
-        }
-    }
     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);
 }

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);
+    auto asmap = BitsToBytes(buffer.first(sep_pos));
-    if (SanityCheckASMap(asmap, buffer.size() - 1 - 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;
+        for (size_t prefix_len = sep_pos - 1; prefix_len > 0; --prefix_len) {
-        while (!asmap_prefix.empty() && asmap_prefix.size() + 7 > asmap.size() && asmap_prefix.back() == false) {
+            auto prefix = BitsToBytes(buffer.first(prefix_len));
-            asmap_prefix.pop_back();
+            // 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
-        while (!asmap_prefix.empty()) {
+            // byte.
-            asmap_prefix.pop_back();
+            if (prefix.size() == asmap.size()) continue;
-            assert(!SanityCheckASMap(asmap_prefix, buffer.size() - 1 - sep_pos));
+            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);
     }
 }

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)};

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

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

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));

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();
+    for (auto bit_sizes_it = bit_sizes.begin(); bit_sizes_it != bit_sizes.end(); ++bit_sizes_it) {
-        bit_sizes_it != bit_sizes.end(); ++bit_sizes_it) {
         if (bit_sizes_it + 1 != bit_sizes.end()) {
-            if (bitpos == endpos) break;
+            if (bitpos >= data.size() * 8) break;
-            bit = *bitpos;
+            bit = ConsumeBitLE(bitpos, data);
-            bitpos++;
         } 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
+                if (bitpos >= data.size() * 8) return INVALID; // Reached EOF in mantissa
-                bit = *bitpos;
+                bit = ConsumeBitLE(bitpos, data);
-                bitpos++;
                 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();
+    size_t pos{0};
-    const std::vector<bool>::const_iterator endpos = asmap.end();
+    const size_t endpos{asmap.size() * 8};
-    uint8_t bits = ip.size();
+    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) {
+    while (pos < endpos) {
-        opcode = DecodeType(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 (ip_bit == ip_bits_end) break; // No input bits left
-            if (int64_t{jump} >= int64_t{endpos - pos}) break; // Jumping past EOF
+            if (int64_t{jump} >= static_cast<int64_t>(endpos - pos)) break; // Jumping past EOF
-            if (ip[ip.size() - bits]) {
+            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();
+    size_t pos{0};
-    std::vector<bool>::const_iterator pos = begin;
+    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() && pos >= jumps.back().first) return false; // There was a jump into the middle of the previous instruction
-        if (!jumps.empty() && offset >= jumps.back().first) return false; // There was a jump into the middle of the previous instruction
+        Instruction opcode = DecodeType(pos, asmap);
-        Instruction opcode = DecodeType(pos, endpos);
         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
+                    if (ConsumeBitLE(pos, asmap)) return false; // Nonzero padding bit
-                    ++pos;
                 }
                 return true; // Sanely reached EOF
             } else {
                 // Continue by pretending we jumped to the next instruction
-                offset = pos - begin;
+                if (pos != jumps.back().first) return false; // Unreachable code
-                if (offset != 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) {
+    std::vector<std::byte> buffer(length);
-        file >> cur_byte;
+    file.read(buffer);
-        for (int bit = 0; bit < 8; ++bit) {
+
-            bits.push_back((cur_byte >> bit) & 1);
+    if (!SanityCheckASMap(buffer, 128)) {
-        }
-    }
-    if (!SanityCheckASMap(bits, 128)) {
         LogWarning("Sanity check of asmap file %s failed", fs::quoted(fs::PathToString(path)));
         return {};
     }
-    return bits;
+
+    return buffer;
 }

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

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',