Replace MakeSpan helper with Span deduction guide

sipa commented at 8:36 pm on November 1, 2021: member

C++17 supports user-defined deduction guides, allowing class constructors to be invoked without specifying class template arguments. Instead, the code can contain rules to infer the template arguments from the constructor argument types.

This alleviates the need for the MakeSpan helper. Convert the existing MakeSpan rules into deduction rules for Span itself, and replace all invocations of MakeSpan with just Span ones.

DrahtBot commented at 9:04 pm on November 1, 2021: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#23595 (util: Add ParseHexstd::byte() helper by MarcoFalke)
#23226 (c++20: Opt-in to modeling view and borrowed_range for Span by theuni)
#17034 ([BIP 174] PSBT version, proprietary, and xpub fields by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Descriptors on Nov 1, 2021

DrahtBot added the label P2P on Nov 1, 2021

DrahtBot added the label RPC/REST/ZMQ on Nov 1, 2021

DrahtBot added the label TX fees and policy on Nov 1, 2021

DrahtBot added the label Wallet on Nov 1, 2021

MarcoFalke removed the label Wallet on Nov 2, 2021

MarcoFalke removed the label TX fees and policy on Nov 2, 2021

MarcoFalke removed the label RPC/REST/ZMQ on Nov 2, 2021

MarcoFalke removed the label P2P on Nov 2, 2021

MarcoFalke removed the label Descriptors on Nov 2, 2021

MarcoFalke added the label Refactoring on Nov 2, 2021

in src/span.h:222 in 94176a0c70 outdated

224-template <typename V> constexpr auto MakeSpan(V&& v SPAN_ATTR_LIFETIMEBOUND) -> typename std::enable_if<!std::is_lvalue_reference<V>::value, Span<const typename std::remove_pointer<decltype(v.data())>::type>>::type { return std::forward<V>(v); }
225-/** MakeSpan for (lvalue) references, supporting mutable output. */
226-template <typename V> constexpr auto MakeSpan(V& v SPAN_ATTR_LIFETIMEBOUND) -> Span<typename std::remove_pointer<decltype(v.data())>::type> { return v; }
227+// Deduction guides for Span
228+// For the pointer/size based constructor:
229+template <typename T> Span(T*, size_t) -> Span<T>;

MarcoFalke commented at 10:51 am on November 2, 2021:

This is missing the (T*, T*) or (It, It) deduction. See (1) in https://en.cppreference.com/w/cpp/container/span/deduction_guides

sipa commented at 2:20 pm on November 2, 2021:

Indeed, fixed.

in src/span.h:230 in 94176a0c70 outdated

232+// For the temporaries/rvalue references constructor, only supporting const output.
233+template <typename T> Span(T&&) -> Span<std::enable_if_t<!std::is_lvalue_reference_v<T>, const std::remove_pointer_t<decltype(std::declval<T&&>().data())>>>;
234+// For (lvalue) references, supporting mutable output.
235+template <typename T> Span(T&) -> Span<std::remove_pointer_t<decltype(std::declval<T&>().data())>>;
236+// For the copy constructor
237+template <typename T> Span(const Span<T>&) -> Span<T>;

MarcoFalke commented at 11:13 am on November 2, 2021:

why is this needed?

sipa commented at 1:58 pm on November 2, 2021:

It is not, after reading more about deductions.

MarcoFalke commented at 11:20 am on November 2, 2021: member

Concept ACK on moving Span closer to std::span. Though I am wondering some deductions are omitted.

See patch:

  0diff --git a/src/net.cpp b/src/net.cpp
  1index ad558dd598..b323d17134 100644
  2--- a/src/net.cpp
  3+++ b/src/net.cpp
  4@@ -738,7 +738,7 @@ std::optional<CNetMessage> V1TransportDeserializer::GetMessage(const std::chrono
  5     if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) {
  6         LogPrint(BCLog::NET, "Header error: Wrong checksum (%s, %u bytes), expected %s was %s, peer=%d\n",
  7                  SanitizeString(msg->m_command), msg->m_message_size,
  8-                 HexStr(Span<uint8_t>(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)),
  9+                 HexStr(Span{hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE}),
 10                  HexStr(hdr.pchChecksum),
 11                  m_node_id);
 12         out_err_raw_size = msg->m_raw_message_size;
 13@@ -1556,7 +1556,7 @@ void CConnman::SocketHandler()
 14             if (nBytes > 0)
 15             {
 16                 bool notify = false;
 17-                if (!pnode->ReceiveMsgBytes(Span<const uint8_t>(pchBuf, nBytes), notify))
 18+                if (!pnode->ReceiveMsgBytes(Span(pchBuf, nBytes), notify))
 19                     pnode->CloseSocketDisconnect();
 20                 RecordBytesRecv(nBytes);
 21                 if (notify) {
 22diff --git a/src/netaddress.cpp b/src/netaddress.cpp
 23index 97ad40aa6e..201b797a0d 100644
 24--- a/src/netaddress.cpp
 25+++ b/src/netaddress.cpp
 26@@ -303,7 +303,7 @@ CNetAddr::CNetAddr(const struct in_addr& ipv4Addr)
 27 
 28 CNetAddr::CNetAddr(const struct in6_addr& ipv6Addr, const uint32_t scope)
 29 {
 30-    SetLegacyIPv6(Span<const uint8_t>(reinterpret_cast<const uint8_t*>(&ipv6Addr), sizeof(ipv6Addr)));
 31+    SetLegacyIPv6(Span{reinterpret_cast<const uint8_t*>(&ipv6Addr), sizeof(ipv6Addr)});
 32     m_scope_id = scope;
 33 }
 34 
 35diff --git a/src/pubkey.h b/src/pubkey.h
 36index e4e3a9560b..dcbdca8ebd 100644
 37--- a/src/pubkey.h
 38+++ b/src/pubkey.h
 39@@ -242,7 +242,7 @@ public:
 40     explicit XOnlyPubKey(Span<const unsigned char> bytes);
 41 
 42     /** Construct an x-only pubkey from a normal pubkey. */
 43-    explicit XOnlyPubKey(const CPubKey& pubkey) : XOnlyPubKey(Span<const unsigned char>(pubkey.begin() + 1, pubkey.begin() + 33)) {}
 44+    explicit XOnlyPubKey(const CPubKey& pubkey) : XOnlyPubKey(Span{pubkey.begin() + 1, pubkey.begin() + 33}) {}
 45 
 46     /** Verify a Schnorr signature against this public key.
 47      *
 48diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
 49index 2d7f5f2894..c72f08a5ff 100644
 50--- a/src/rpc/util.cpp
 51+++ b/src/rpc/util.cpp
 52@@ -317,7 +317,7 @@ public:
 53         UniValue obj(UniValue::VOBJ);
 54         obj.pushKV("iswitness", true);
 55         obj.pushKV("witness_version", (int)id.version);
 56-        obj.pushKV("witness_program", HexStr(Span<const unsigned char>(id.program, id.length)));
 57+        obj.pushKV("witness_program", HexStr(Span{id.program, id.length}));
 58         return obj;
 59     }
 60 };
 61diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
 62index 95611fc909..b023d199d5 100644
 63--- a/src/script/descriptor.cpp
 64+++ b/src/script/descriptor.cpp
 65@@ -1251,7 +1251,7 @@ std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseS
 66 std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider)
 67 {
 68     if (ctx == ParseScriptContext::P2TR && script.size() == 34 && script[0] == 32 && script[33] == OP_CHECKSIG) {
 69-        XOnlyPubKey key{Span<const unsigned char>{script.data() + 1, script.data() + 33}};
 70+        XOnlyPubKey key{Span{script.data() + 1, script.data() + 33}};
 71         return std::make_unique<PKDescriptor>(InferXOnlyPubkey(key, ctx, provider));
 72     }
 73 
 74diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
 75index eafa9840d7..2d84a28981 100644
 76--- a/src/script/interpreter.cpp
 77+++ b/src/script/interpreter.cpp
 78@@ -1858,7 +1858,7 @@ uint256 ComputeTaprootMerkleRoot(Span<const unsigned char> control, const uint25
 79     uint256 k = tapleaf_hash;
 80     for (int i = 0; i < path_len; ++i) {
 81         CHashWriter ss_branch{HASHER_TAPBRANCH};
 82-        Span<const unsigned char> node(control.data() + TAPROOT_CONTROL_BASE_SIZE + TAPROOT_CONTROL_NODE_SIZE * i, TAPROOT_CONTROL_NODE_SIZE);
 83+        Span node{control.data() + TAPROOT_CONTROL_BASE_SIZE + TAPROOT_CONTROL_NODE_SIZE * i, TAPROOT_CONTROL_NODE_SIZE};
 84         if (std::lexicographical_compare(k.begin(), k.end(), node.begin(), node.end())) {
 85             ss_branch << k << node;
 86         } else {
 87@@ -1874,7 +1874,7 @@ static bool VerifyTaprootCommitment(const std::vector<unsigned char>& control, c
 88     assert(control.size() >= TAPROOT_CONTROL_BASE_SIZE);
 89     assert(program.size() >= uint256::size());
 90     //! The internal pubkey (x-only, so no Y coordinate parity).
 91-    const XOnlyPubKey p{Span<const unsigned char>{control.data() + 1, control.data() + TAPROOT_CONTROL_BASE_SIZE}};
 92+    const XOnlyPubKey p{Span{control.data() + 1, control.data() + TAPROOT_CONTROL_BASE_SIZE}};
 93     //! The output pubkey (taken from the scriptPubKey).
 94     const XOnlyPubKey q{program};
 95     // Compute the Merkle root from the leaf and the provided path.
 96diff --git a/src/signet.cpp b/src/signet.cpp
 97index aafd1999ee..68fbbc9388 100644
 98--- a/src/signet.cpp
 99+++ b/src/signet.cpp
100@@ -38,7 +38,7 @@ static bool FetchAndClearCommitmentSection(const Span<const uint8_t> header, CSc
101     std::vector<uint8_t> pushdata;
102     while (witness_commitment.GetOp(pc, opcode, pushdata)) {
103         if (pushdata.size() > 0) {
104-            if (!found_header && pushdata.size() > (size_t) header.size() && Span<const uint8_t>(pushdata.data(), header.size()) == header) {
105+            if (!found_header && pushdata.size() > (size_t)header.size() && Span{pushdata.data(), header.size()} == header) {
106                 // pushdata only counts if it has the header _and_ some data
107                 result.insert(result.end(), pushdata.begin() + header.size(), pushdata.end());
108                 pushdata.erase(pushdata.begin() + header.size(), pushdata.end());
109diff --git a/src/span.h b/src/span.h
110index 71de70b64f..b04fdd45d1 100644
111--- a/src/span.h
112+++ b/src/span.h
113@@ -218,16 +218,14 @@ public:
114 };
115 
116 // Deduction guides for Span
117-// For the pointer/size based constructor:
118-template <typename T> Span(T*, size_t) -> Span<T>;
119+// For the pointer/size based and iterator based constructor:
120+template <typename T, typename EndOrSize> Span(T*, EndOrSize) -> Span<T>;
121 // For the array constructor:
122 template <typename T, std::size_t N> Span(T (&)[N]) -> Span<T>;
123 // For the temporaries/rvalue references constructor, only supporting const output.
124 template <typename T> Span(T&&) -> Span<std::enable_if_t<!std::is_lvalue_reference_v<T>, const std::remove_pointer_t<decltype(std::declval<T&&>().data())>>>;
125 // For (lvalue) references, supporting mutable output.
126 template <typename T> Span(T&) -> Span<std::remove_pointer_t<decltype(std::declval<T&>().data())>>;
127-// For the copy constructor
128-template <typename T> Span(const Span<T>&) -> Span<T>;
129 
130 /** Pop the last element off a span, and return a reference to that element. */
131 template <typename T>
132diff --git a/src/test/fuzz/asmap.cpp b/src/test/fuzz/asmap.cpp
133index d402f8632c..dc37d068d9 100644
134--- a/src/test/fuzz/asmap.cpp
135+++ b/src/test/fuzz/asmap.cpp
136@@ -49,7 +49,7 @@ FUZZ_TARGET(asmap)
137     CNetAddr net_addr;
138     if (ipv6) {
139         assert(addr_size == ADDR_IPV6_SIZE);
140-        net_addr.SetLegacyIPv6(Span<const uint8_t>(addr_data, addr_size));
141+        net_addr.SetLegacyIPv6(Span{addr_data, addr_size});
142     } else {
143         assert(addr_size == ADDR_IPV4_SIZE);
144         in_addr ipv4;
145diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp
146index 8d2a06f11a..d625403fa0 100644
147--- a/src/test/fuzz/utxo_snapshot.cpp
148+++ b/src/test/fuzz/utxo_snapshot.cpp
149@@ -38,7 +38,7 @@ FUZZ_TARGET_INIT(utxo_snapshot, initialize_chain)
150     {
151         CAutoFile outfile{fsbridge::fopen(snapshot_path, "wb"), SER_DISK, CLIENT_VERSION};
152         const auto file_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)};
153-        outfile << Span<const uint8_t>{file_data};
154+        outfile << Span{file_data};
155     }
156 
157     const auto ActivateFuzzedSnapshot{[&] {
158diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
159index b1300d06ba..a97dc83a3e 100644
160--- a/src/test/util_tests.cpp
161+++ b/src/test/util_tests.cpp
162@@ -142,13 +142,13 @@ BOOST_AUTO_TEST_CASE(util_HexStr)
163         "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f");
164 
165     BOOST_CHECK_EQUAL(
166-        HexStr(Span<const unsigned char>(
167-               ParseHex_expected + sizeof(ParseHex_expected),
168-               ParseHex_expected + sizeof(ParseHex_expected))),
169+        HexStr(Span{
170+            ParseHex_expected + sizeof(ParseHex_expected),
171+            ParseHex_expected + sizeof(ParseHex_expected)}),
172         "");
173 
174     BOOST_CHECK_EQUAL(
175-        HexStr(Span<const unsigned char>(ParseHex_expected, ParseHex_expected)),
176+        HexStr(Span{ParseHex_expected, ParseHex_expected}),
177         "");
178 
179     std::vector<unsigned char> ParseHex_vec(ParseHex_expected, ParseHex_expected + 5);
180diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
181index 1f13b80f3e..48ed50b9a8 100644
182--- a/src/wallet/rpcdump.cpp
183+++ b/src/wallet/rpcdump.cpp
184@@ -35,7 +35,7 @@ std::string static EncodeDumpString(const std::string &str) {
185     std::stringstream ret;
186     for (const unsigned char c : str) {
187         if (c <= 32 || c >= 128 || c == '%') {
188-            ret << '%' << HexStr(Span<const unsigned char>(&c, 1));
189+            ret << '%' << HexStr(Span{&c, 1});
190         } else {
191             ret << c;
192         }

sipa force-pushed on Nov 2, 2021

sipa commented at 2:21 pm on November 2, 2021: member

Thanks, @MarcoFalke! I’ve incorporated your suggested changes, plus a few additional simplifications in a second commit.

sipa force-pushed on Nov 2, 2021

in src/net.cpp:1560 in 3576b9393c outdated

1556@@ -1557,7 +1557,7 @@ void CConnman::SocketHandler()
1557             if (nBytes > 0)
1558             {
1559                 bool notify = false;
1560-                if (!pnode->ReceiveMsgBytes(Span<const uint8_t>(pchBuf, nBytes), notify))
1561+                if (!pnode->ReceiveMsgBytes({pchBuf, (size_t)nBytes}, notify))

MarcoFalke commented at 10:34 am on November 3, 2021:

nit in the second commit, if you retouch:

Add {} to the if body?

sipa commented at 9:27 pm on November 17, 2021:

Done.

in src/script/descriptor.cpp:977 in abd1f61f6e outdated

972@@ -973,10 +973,10 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const S
973     }
974     KeyPath path;
975     DeriveType type = DeriveType::NO;
976-    if (split.back() == MakeSpan("*").first(1)) {
977+    if (split.back() == Span{"*"}.first(1)) {

MarcoFalke commented at 6:11 pm on November 10, 2021:

unrelated fun fact:

You could write Span{std::string_view{"*"}} and get rid of the .first() call.

(Same below)

sipa commented at 9:27 pm on November 17, 2021:

I don’t think that’s all that better.

in src/test/util_tests.cpp:152 in 3576b9393c outdated

151     BOOST_CHECK_EQUAL(
152-        HexStr(Span<const unsigned char>(ParseHex_expected, ParseHex_expected)),
153+        HexStr(Span{ParseHex_expected}.first(0)),
154         "");
155 
156     std::vector<unsigned char> ParseHex_vec(ParseHex_expected, ParseHex_expected + 5);

MarcoFalke commented at 6:25 pm on November 10, 2021:

3576b9393c678352b2ed8a446a62d9639b0a451c: now unused?

sipa commented at 9:27 pm on November 17, 2021:

Gone.

MarcoFalke commented at 6:29 pm on November 10, 2021: member

review ACK 3576b9393c678352b2ed8a446a62d9639b0a451c 🚛

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3review ACK 3576b9393c678352b2ed8a446a62d9639b0a451c 🚛
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUhMqAv/ZMtShuhel+aDB8nQ6xqbVI2SXNOyeXScpWHbu46r1c5dWTlVYgXtHrWM
 8EXuNFkzMJioCbuWkWbauvox8ccZopPkRBdKseU9jUi/jz+5zp4pK5g0X8WhU1v12
 9tYL5sQSGCTbl8yKY/L+mSNnoAe0rVAD7nuEAmCLeGU6c8Uv8nz0aj/w6AtDusAD0
10K7mJIJpks61r0svCodkCIwWSP+0EmUWOhoEdWdkPywV6F0QeJ2vgVL2Dj507xgMG
11BEcCbNX2IdPrYYkybTHKJBlgOaG7+QSMm2rq2MKukSq7Q+ctss0mlA/KT446tTp8
12L+5MkXZZDm6m4Kj63ceWU9VF8pVwWNF5F/yvzZobF8lB4wAJo8UORFEAzt8zr9n1
13FesE9xru65XvEFyFi0YFYz46D5zX33nbPfe+JRJ1RKRkRXCEJ6XDud7PLhJjjFtc
14Y+d8+mCe0A/Fs15kVbv5bZCy4pA1OO6hjYMHsjAzAqWVc4073GCmVIJpYgelcQfU
15932Fjcwc
16=KdJ0
17-----END PGP SIGNATURE-----

MarcoFalke commented at 6:31 pm on November 10, 2021: member

I haven’t tried, but to the second commit you might be able to add (after a rebase):

 0diff --git a/src/test/key_io_tests.cpp b/src/test/key_io_tests.cpp
 1index 0361618c82..02268dbcf5 100644
 2--- a/src/test/key_io_tests.cpp
 3+++ b/src/test/key_io_tests.cpp
 4@@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(key_io_valid_parse)
 5             privkey = DecodeSecret(exp_base58string);
 6             BOOST_CHECK_MESSAGE(privkey.IsValid(), "!IsValid:" + strTest);
 7             BOOST_CHECK_MESSAGE(privkey.IsCompressed() == isCompressed, "compressed mismatch:" + strTest);
 8-            BOOST_CHECK_MESSAGE(Span<const uint8_t>{privkey} == Span<const uint8_t>{exp_payload}, "key mismatch:" + strTest);
 9+            BOOST_CHECK_MESSAGE(Span{privkey} == Span{exp_payload}, "key mismatch:" + strTest);
10 
11             // Private key must be invalid public key
12             destination = DecodeDestination(exp_base58string);

sipa force-pushed on Nov 17, 2021

sipa commented at 9:28 pm on November 17, 2021: member

@MarcoFalke Done (it necessitated adding CKey::data, but that’s generally useful in any case).

EDIT: nvm, the CKey::data was just added, I noticed in rebase.

sipa force-pushed on Nov 17, 2021

sipa commented at 9:39 pm on November 17, 2021: member

Rebased and addressed some comments.

MarcoFalke commented at 10:44 am on November 24, 2021: member

re-ACK f90d5ae1cf5b002fe7ec1fa54e022513589721de 👤

Changes:

Additional change MakeSpan -> Span in the first commit after rebase on 5ccab7187b35142e03c15dd55390e45b3d233c52
In the second commit:
- Add {} for style
- Remove now-unused template arguments from a Span constructor call
- Remove now-unused ParseHex_vec symbol from tests

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3re-ACK f90d5ae1cf5b002fe7ec1fa54e022513589721de 👤
 4
 5Changes:
 6* Additional change MakeSpan -> Span in the first commit after rebase on 5ccab7187b35142e03c15dd55390e45b3d233c52
 7* In the second commit:
 8  - Add {} for style
 9  - Remove now-unused template arguments from a Span constructor call
10  - Remove now-unused ParseHex_vec symbol from tests 
11-----BEGIN PGP SIGNATURE-----
12
13iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
14pUhvOQv9Gk6tz4eLv4pt3y9sh4sUGDkgmTog83oA3Fom5bk7dQvvZjTL7aK655Vc
15DB9X4NNWQHlxB49cbKlBM5jsaoZU/cryuq2Mk1g0nkSgWeNN6MARCT79cW6t4R0S
16imBkqiDbyxzx8aErRbnM7MP5GZdf9Ax6XoRNfYmAIEkuo7rsn8kO8kMnsxL1DmX5
17PkKEZF5qm575xEOydZeAtvUJijnTS9J8gIRwSu6KfVeP/hBy6HGkfDxEmUJQBQ6T
18sswHcbNJ4BuGU2IdX/C3kIz2SzWGRpmHBm7KEhA5BoLcUm+KF1G6V8CEVMtp5ls1
19LAWv7M7WqgdsbkArKltsj4MiXapl1+x+skZP4Coisiq/McAzTg9XYditcS1zj6Xb
201GpnPu//ffdVr64VEwPYigHsogJejFBCHek/VL54aGuWgSRcYQ4D3oSKcDwpP2eq
21Zc06Xl+Gg/rtxGACnzditb/KpDGZbasy2S/3V2EW6hpf/kit/xHAJzUzQ0I+ARMn
22VFtCbN6b
23=oaY7
24-----END PGP SIGNATURE-----

DrahtBot added the label Needs rebase on Nov 24, 2021

MarcoFalke commented at 11:24 am on November 24, 2021: member

message.txt:

0re-ACK f90d5ae1cf5b002fe7ec1fa54e022513589721de
1
2Changes:
3* Additional change MakeSpan -> Span in the first commit after rebase on 5ccab7187b35142e03c15dd55390e45b3d233c52
4* In the second commit:
5  - Add {} for style
6  - Remove now-unused template arguments from a Span constructor call
7  - Remove now-unused ParseHex_vec symbol from tests

allowed_sig.txt:

0MarcoFalke_sign_github_comments_test ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDYEowis4rvh+A4j2/2CeE7Sx/go9mzv8D7HiFRICGGl

message.txt.sig:

0-----BEGIN SSH SIGNATURE-----
1U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgNgSjCKziu+H4DiPb/YJ4TtLH+C
2j2bO/wPseIVEgIYaUAAAAEZmlsZQAAAAAAAAAGc2hhNTEyAAAAUwAAAAtzc2gtZWQyNTUx
3OQAAAEBrIf/Uab6AwMKfzBPP67zQZGpePmSDs5qtrO6w7ympI3EuiCXl655I7TIhN+Y4jN
4DL5tCD8EJ5Kew8AR65PBAD
5-----END SSH SIGNATURE-----

To check:

0$ ssh-keygen -Y verify -f allowed_sig.txt -I MarcoFalke_sign_github_comments_test -n file -s message.txt.sig < message.txt

sipa commented at 10:16 pm on November 24, 2021: member

Rebased.

sipa force-pushed on Nov 24, 2021

DrahtBot removed the label Needs rebase on Nov 24, 2021

in src/test/util_tests.cpp:174 in e4aec1e0cb outdated

167@@ -170,6 +168,10 @@ BOOST_AUTO_TEST_CASE(span_write_bytes)
168     mut_bytes[1] = std::byte{0x11};
169     BOOST_CHECK_EQUAL(mut_arr.at(0), 0xaa);
170     BOOST_CHECK_EQUAL(mut_arr.at(1), 0x11);
171+    BOOST_CHECK_EQUAL(
172+        HexStr(Span{ParseHex_expected}.first(5)),
173+        "04678afdb0"
174+    );

MarcoFalke commented at 7:24 am on November 25, 2021:

wrong place for this test?

MarcoFalke commented at 8:23 am on November 29, 2021:

Do you plan to address this or leave it?

sipa commented at 11:07 pm on November 29, 2021:

Done.

MarcoFalke approved

MarcoFalke commented at 7:24 am on November 25, 2021: member

re-ACK e4aec1e0cbdc237a26713a34f465e2b485f892f6 🕙

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3re-ACK e4aec1e0cbdc237a26713a34f465e2b485f892f6 🕙
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUgCHAwAic9QG7p6GHs7HQoWstGWqMYJcHHK9R3unb65d8NqlQIGUF+5X4N/V0o/
 8LTAbZf+Ug+kiUwREEyldTOeJo9WGr1lRQFs5EyHTuB1QGZ0E2p8+0RNc31KUj3Ev
 9x5sCgGGsDVJrtEt4wbHclqsRebdmIl+AZVLqt3J/dP7sktKwoUU260TCjXrwQnpz
10q1S3d6ySfmMdhTU3pWCilGLO6EYZUwJdM2WQD8Ss75CnvkTRadOu1FJTKXbnPySL
11krttLbYmCFVqpm9R/MUZRrUKNZ5uPzBfpCRHv/b7XeL6CNJTXYAHZWI93Zlcr4dl
124CAPi5Xbd0tFF0CdLACxXwHQCF6DUEWFFWGhzX+oxKO5/yVOTbWQJBJLuwaLIR9C
13rT1GALRBw7LxPCPAZGYI8h6tC76k3BcSpurLjpOHqEFVZ8eXjfxCjsUXnVsV3PP9
14I09rN4lMHBRujs45GNG0QlHzpMYauVhKvjnxh4Z0uu1F/C9U90rqpwdsf3fPet5z
15r6nD5eYi
16=NXdG
17-----END PGP SIGNATURE-----

fanquake requested review from theuni on Nov 25, 2021

Replace MakeSpan helper with Span deduction guide 568dd2f839

More Span simplifications

Based on suggestions by MarcoFalke <falke.marco@gmail.com>

11daf6ceb1

sipa force-pushed on Nov 29, 2021

MarcoFalke commented at 12:45 pm on November 30, 2021: member

re-ACK 11daf6ceb1d9ea1f8d638b123eecfe39d162a7c3 Only change is removing a hunk in the tests 🌕

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3re-ACK 11daf6ceb1d9ea1f8d638b123eecfe39d162a7c3 Only change is removing a hunk in the tests 🌕
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUjhgwv9H9NYRuLgLxngpX1oUqgWdlwre+eOwKtTSrdbWwe16zq9g0+CKKo+UZS9
 8YOuhAicaZ8qdq2Oy65KTaZopQUSOFBV2A3B+BjwoKe4wOrUo7Wb6xcxR39/xbWzI
 9KyS8uutOqStRmeQUsyLUh/Hq1LmQFR5AJ4RVURA3sLr79BlMo94J52TacjjDUoFN
10jLr2UP4b86E4x42AIf9kVl8HRSsWdIK62OXujnY32DTlw5uoa12L6PuhE5wACb9l
11cD3kw6/a+v2S6Zxxke9tCG5vZ5C/ZwhHA5W4FSUHbDU5NTvcbigG9h92nEyINmGD
12i7ZhZmPo1UQcpohAZdF8DvXWAa2etLyvSOLN/PE37OyuSGaD21A5rTBLorp+EHeC
13Y5WlY/MKmSlFTU+AWKBQi3Q9eiNMAD8XK+36JSY3dlwWla9ClSiFXVVWH3vT+71B
14Nj8I69a0IMN+WpRjWAOinYk0gRf4UsxeqayaRTFjSVvSWCq+99XBO3e0oKj2iYSC
15utKtL3DU
16=fUa/
17-----END PGP SIGNATURE-----

DrahtBot commented at 8:37 am on December 3, 2021: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

DrahtBot added the label Needs rebase on Dec 3, 2021

MarcoFalke merged this on Dec 3, 2021

MarcoFalke closed this on Dec 3, 2021

in src/wallet/rpcdump.cpp:38 in 11daf6ceb1

34@@ -35,7 +35,7 @@ std::string static EncodeDumpString(const std::string &str) {
35     std::stringstream ret;
36     for (const unsigned char c : str) {
37         if (c <= 32 || c >= 128 || c == '%') {
38-            ret << '%' << HexStr(Span<const unsigned char>(&c, 1));

fanquake commented at 10:32 am on December 3, 2021:

If anyone is confused about the bot marking this as needing a rebase, and then the merge. The conflict was in src/wallet/rpcdump.cpp. The conflicting code was moved from there, to src/wallet/rpc/backup.cpp in #23647. You can see src/wallet/rpc/backup.cpp being updated in the merge commit: https://github.com/bitcoin/bitcoin/commit/8b1de78577a02300340cec37e26cd6418ef6e645#diff-c4eeb9bd8e84ac276465d100d6ac4c1c9b9ddc145827a03dfe11932598b028b9R38.

MarcoFalke commented at 10:45 am on December 3, 2021:

yeah, daily reminder to stop trusting GitHub metadata

sidhujag referenced this in commit 9f654d880a on Dec 3, 2021

RandyMcMillan referenced this in commit 52e9b7c14c on Dec 23, 2021

ftrader referenced this in commit 25d99c1b34 on Nov 30, 2022

DrahtBot locked this on Dec 3, 2022

Replace MakeSpan helper with Span deduction guide #23413

Conflicts