Merge bitcoin/bitcoin#35952: kernel: prevent dangling iterators from temporary ranges

fc0dcf950f kernel: keep range iterators tied to their owner (Lőrinc)
0936c55f62 test: characterize kernel range iterators (Lőrinc)

Pull request description:

  **Problem:** Kernel wrapper methods return `Range` views by value, but their iterators point to the `Range` object.
  Saving an iterator from a temporary view, such as `block.Transactions().begin()`, leaves it pointing to the destroyed view, so later use has undefined behavior.

  **Fix:** Make range iterators point to the underlying Kernel wrapper object and use the range's compile-time getter for element access.
  Remove `operator->`, which returned elements by value and could not easily support arrow expressions.

ACKs for top commit:
  purpleKarrot:
    ACK fc0dcf950f
  yuvicc:
    ACK fc0dcf950f
  sedited:
    ACK fc0dcf950f

Tree-SHA512: 85ae1f8d4c62a762a10c546ebb312f126efc5ef349557dd235c81b585bef92cbb6c4a565734d85d315641a169f48b6032b859f5a9f070bc347b1424b05473e5d
This commit is contained in:
merge-script
2026-08-18 15:25:33 +02:00
2 changed files with 12 additions and 7 deletions

View File

@@ -190,7 +190,7 @@ T check(T ptr)
return ptr;
}
template <typename Collection, typename ValueType>
template <typename Collection, typename ValueType, auto GetFunc>
class Iterator
{
public:
@@ -209,8 +209,7 @@ public:
Iterator(const Collection* ptr, size_t idx) : m_collection{ptr}, m_idx{idx} {}
// This is just a view, so return a copy.
auto operator*() const { return (*m_collection)[m_idx]; }
auto operator->() const { return (*m_collection)[m_idx]; }
auto operator*() const { return std::invoke(GetFunc, *m_collection, m_idx); }
auto& operator++() { m_idx++; return *this; }
auto operator++(int) { Iterator tmp = *this; ++(*this); return tmp; }
@@ -226,7 +225,7 @@ public:
auto operator-(const Iterator& other) const { return static_cast<difference_type>(m_idx) - static_cast<difference_type>(other.m_idx); }
ValueType operator[](difference_type n) const { return (*m_collection)[m_idx + n]; }
ValueType operator[](difference_type n) const { return *(*this + n); }
auto operator<=>(const Iterator& other) const { return m_idx <=> other.m_idx; }
@@ -249,7 +248,7 @@ class Range
public:
using value_type = std::invoke_result_t<decltype(GetFunc), const Container&, size_t>;
using difference_type = std::ptrdiff_t;
using iterator = Iterator<Range, value_type>;
using iterator = Iterator<Container, value_type, GetFunc>;
using const_iterator = iterator;
private:
@@ -261,8 +260,8 @@ public:
static_assert(std::ranges::random_access_range<Range>);
}
iterator begin() const { return iterator(this, 0); }
iterator end() const { return iterator(this, size()); }
iterator begin() const { return iterator(m_container, 0); }
iterator end() const { return iterator(m_container, size()); }
const_iterator cbegin() const { return begin(); }
const_iterator cend() const { return end(); }

View File

@@ -745,6 +745,12 @@ BOOST_AUTO_TEST_CASE(btck_block)
CheckHandle(block, block_100);
Block block_tx{hex_string_to_byte_vec(REGTEST_BLOCK_DATA[205])};
CheckRange(block_tx.Transactions(), block_tx.CountTransactions());
auto transactions{block_tx.Transactions()};
auto transactions_copy{transactions};
BOOST_CHECK(transactions.begin() == transactions_copy.begin());
BOOST_CHECK(transactions.begin() == block_tx.Transactions().begin());
auto transaction_it{transactions.begin()};
BOOST_CHECK((*transaction_it).Txid() == block_tx.GetTransaction(0).Txid());
auto invalid_data = hex_string_to_byte_vec("012300");
BOOST_CHECK_THROW(Block{invalid_data}, std::runtime_error);
auto empty_data = hex_string_to_byte_vec("");