Skip to content

Commit

Permalink
Merge bitcoin#31540: refactor: std::span compat fixes
Browse files Browse the repository at this point in the history
fa494a1 refactor: Specify const in std::span constructor, where needed (MarcoFalke)
faaf480 Allow std::span in stream serialization (MarcoFalke)
faa5391 refactor: test: Return std::span from StringBytes (MarcoFalke)
fa86223 refactor: Avoid passing span iterators when data pointers are expected (MarcoFalke)
faae6fa refactor: Simplify SpanPopBack (MarcoFalke)
facc4f1 refactor: Replace fwd-decl with proper include (MarcoFalke)
fac3a78 refactor: Avoid needless, unsafe c-style cast (MarcoFalke)

Pull request description:

  The `std::span` type is already used in some parts of the codebase, and in most contexts can implicitly convert to and from `Span`. However, the two types are not identical in behavior and trying to use one over the other can result in compile failures in some contexts.

  Fix all those issues by allowing either `Span` or `std::span` in any part of the codebase.

  All of the changes are also required for the scripted-diff to replace `Span` with `std::span` in bitcoin#31519

ACKs for top commit:
  sipa:
    utACK fa494a1
  fjahr:
    Code review ACK fa494a1
  achow101:
    ACK fa494a1
  theuni:
    utACK fa494a1.
  adamandrews1:
    utACK bitcoin@fa494a1

Tree-SHA512: 9440941823e884ff5d7ac161f58b9a0704d8e803b4c91c400bdb5f58f898e4637d63ae627cfc7330e98a721fc38285a04641175aa18d991bd35f8b69ed1d74c4
  • Loading branch information
achow101 committed Dec 30, 2024
2 parents a137b0b + fa494a1 commit e6f1424
Show file tree
Hide file tree
Showing 11 changed files with 17 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/base58.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ std::string EncodeBase58Check(Span<const unsigned char> input)
// add 4-byte hash check to the end
std::vector<unsigned char> vch(input.begin(), input.end());
uint256 hash = Hash(vch);
vch.insert(vch.end(), (unsigned char*)&hash, (unsigned char*)&hash + 4);
vch.insert(vch.end(), hash.data(), hash.data() + 4);
return EncodeBase58(vch);
}

Expand Down
8 changes: 4 additions & 4 deletions src/script/miniscript.h
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
// Get threshold
int next_comma = FindNextChar(in, ',');
if (next_comma < 1) return false;
const auto k_to_integral{ToIntegral<int64_t>(std::string_view(in.begin(), next_comma))};
const auto k_to_integral{ToIntegral<int64_t>(std::string_view(in.data(), next_comma))};
if (!k_to_integral.has_value()) return false;
const int64_t k{k_to_integral.value()};
in = in.subspan(next_comma + 1);
Expand Down Expand Up @@ -1954,15 +1954,15 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
} else if (Const("after(", in)) {
int arg_size = FindNextChar(in, ')');
if (arg_size < 1) return {};
const auto num{ToIntegral<int64_t>(std::string_view(in.begin(), arg_size))};
const auto num{ToIntegral<int64_t>(std::string_view(in.data(), arg_size))};
if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {};
constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, *num));
in = in.subspan(arg_size + 1);
script_size += 1 + (*num > 16) + (*num > 0x7f) + (*num > 0x7fff) + (*num > 0x7fffff);
} else if (Const("older(", in)) {
int arg_size = FindNextChar(in, ')');
if (arg_size < 1) return {};
const auto num{ToIntegral<int64_t>(std::string_view(in.begin(), arg_size))};
const auto num{ToIntegral<int64_t>(std::string_view(in.data(), arg_size))};
if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {};
constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, *num));
in = in.subspan(arg_size + 1);
Expand All @@ -1974,7 +1974,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
} else if (Const("thresh(", in)) {
int next_comma = FindNextChar(in, ',');
if (next_comma < 1) return {};
const auto k{ToIntegral<int64_t>(std::string_view(in.begin(), next_comma))};
const auto k{ToIntegral<int64_t>(std::string_view(in.data(), next_comma))};
if (!k.has_value() || *k < 1) return {};
in = in.subspan(next_comma + 1);
// n = 1 here because we read the first WRAPPED_EXPR before reaching THRESH
Expand Down
2 changes: 1 addition & 1 deletion src/script/solver.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@

#include <attributes.h>
#include <script/script.h>
#include <span.h>

#include <string>
#include <optional>
#include <utility>
#include <vector>

class CPubKey;
template <typename C> class Span;

enum class TxoutType {
NONSTANDARD,
Expand Down
2 changes: 2 additions & 0 deletions src/serialize.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ template<typename Stream> inline void Serialize(Stream& s, int64_t a ) { ser_wri
template<typename Stream> inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); }
template <typename Stream, BasicByte B, int N> void Serialize(Stream& s, const B (&a)[N]) { s.write(MakeByteSpan(a)); }
template <typename Stream, BasicByte B, std::size_t N> void Serialize(Stream& s, const std::array<B, N>& a) { s.write(MakeByteSpan(a)); }
template <typename Stream, BasicByte B, std::size_t N> void Serialize(Stream& s, std::span<B, N> span) { s.write(std::as_bytes(span)); }
template <typename Stream, BasicByte B> void Serialize(Stream& s, Span<B> span) { s.write(AsBytes(span)); }

template <typename Stream, CharNotInt8 V> void Unserialize(Stream&, V) = delete; // char serialization forbidden. Use uint8_t or int8_t
Expand All @@ -278,6 +279,7 @@ template<typename Stream> inline void Unserialize(Stream& s, int64_t& a ) { a =
template<typename Stream> inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); }
template <typename Stream, BasicByte B, int N> void Unserialize(Stream& s, B (&a)[N]) { s.read(MakeWritableByteSpan(a)); }
template <typename Stream, BasicByte B, std::size_t N> void Unserialize(Stream& s, std::array<B, N>& a) { s.read(MakeWritableByteSpan(a)); }
template <typename Stream, BasicByte B, std::size_t N> void Unserialize(Stream& s, std::span<B, N> span) { s.read(std::as_writable_bytes(span)); }
template <typename Stream, BasicByte B> void Unserialize(Stream& s, Span<B> span) { s.read(AsWritableBytes(span)); }

template <typename Stream> inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); }
Expand Down
5 changes: 2 additions & 3 deletions src/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,8 @@ template <typename T>
T& SpanPopBack(Span<T>& span)
{
size_t size = span.size();
ASSERT_IF_DEBUG(size > 0);
T& back = span[size - 1];
span = Span<T>(span.data(), size - 1);
T& back = span.back();
span = span.first(size - 1);
return back;
}

Expand Down
2 changes: 1 addition & 1 deletion src/streams.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class VectorWriter
memcpy(vchData.data() + nPos, src.data(), nOverwrite);
}
if (nOverwrite < src.size()) {
vchData.insert(vchData.end(), UCharCast(src.data()) + nOverwrite, UCharCast(src.end()));
vchData.insert(vchData.end(), UCharCast(src.data()) + nOverwrite, UCharCast(src.data() + src.size()));
}
nPos += src.size();
}
Expand Down
3 changes: 1 addition & 2 deletions src/test/script_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1700,9 +1700,8 @@ BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors)
BOOST_CHECK_EQUAL(HexStr(sighash), input["intermediary"]["sigHash"].get_str());

// To verify the sigmsg, hash the expected sigmsg, and compare it with the (expected) sighash.
BOOST_CHECK_EQUAL(HexStr((HashWriter{HASHER_TAPSIGHASH} << Span{ParseHex(input["intermediary"]["sigMsg"].get_str())}).GetSHA256()), input["intermediary"]["sigHash"].get_str());
BOOST_CHECK_EQUAL(HexStr((HashWriter{HASHER_TAPSIGHASH} << std::span<const uint8_t>{ParseHex(input["intermediary"]["sigMsg"].get_str())}).GetSHA256()), input["intermediary"]["sigHash"].get_str());
}

}
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/serialize_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ class Base
if (s.template GetParams<BaseFormat>().m_base_format == BaseFormat::RAW) {
s << m_base_data;
} else {
s << Span{HexStr(Span{&m_base_data, 1})};
s << std::span<const char>{HexStr(Span{&m_base_data, 1})};
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/test/util/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <netaddress.h>
#include <node/connection_types.h>
#include <node/eviction.h>
#include <span.h>
#include <sync.h>
#include <util/sock.h>

Expand All @@ -28,9 +29,6 @@

class FastRandomContext;

template <typename C>
class Span;

struct ConnmanTestMsg : public CConnman {
using CConnman::CConnman;

Expand Down
3 changes: 1 addition & 2 deletions src/util/hasher.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
#include <crypto/common.h>
#include <crypto/siphash.h>
#include <primitives/transaction.h>
#include <span.h>
#include <uint256.h>

#include <cstdint>
#include <cstring>

template <typename C> class Span;

class SaltedTxidHasher
{
private:
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/test/db_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ inline std::ostream& operator<<(std::ostream& os, const std::pair<const Serializ

namespace wallet {

static Span<const std::byte> StringBytes(std::string_view str)
inline std::span<const std::byte> StringBytes(std::string_view str)
{
return AsBytes<const char>({str.data(), str.size()});
return std::as_bytes(std::span{str});
}

static SerializeData StringData(std::string_view str)
Expand Down

0 comments on commit e6f1424

Please sign in to comment.