Merge bitcoin#33914: Change Parse descriptor argument to string_view

c0bfe72 Change Parse descriptor argument to string_view (Sjors Provoost)

Pull request description:

  While investigating a silent merge conflict in bitcoin#33135 I noticed that bitcoin#32983 changed the descriptor `Parse` function signature from `const std::string& descriptor` to `std::span<const char> descriptor`.

  Calling that new version of `Parse` with a string literal will trigger a confusing "Invalid characters in payload" due to the trailing "\0".

  It can be worked around by having (the test) wrap string literals in `std::string()`, but that's easy to forget.

  Using `string_view` is easier and more compact than (as a previous version of this PR did) checking for trailing `\0`.

  Also add a test.

ACKs for top commit:
  maflcko:
    review ACK c0bfe72 🍨
  enirox001:
    tACK c0bfe72
  stickies-v:
    ACK c0bfe72
  rkrux:
    crACK c0bfe72

Tree-SHA512: 6b20307f834dae66826c8763f6c2ba0071f4e369375184cb5ff8543b85220fcaf33a47ddb065e418d1af3ed9a3fac401a7854f8924f52aab2b000b1f65328f2c

Author: fanquake

src/script/descriptor.cpp

@@ -2743,12 +2743,13 @@ bool CheckChecksum(std::span<const char>& sp, bool require_checksum, std::string
     return true;
 }
 
-std::vector<std::unique_ptr<Descriptor>> Parse(std::span<const char> descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum)
+std::vector<std::unique_ptr<Descriptor>> Parse(std::string_view descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum)
 {
-    if (!CheckChecksum(descriptor, require_checksum, error)) return {};
+    std::span<const char> sp{descriptor};
+    if (!CheckChecksum(sp, require_checksum, error)) return {};
     uint32_t key_exp_index = 0;
-    auto ret = ParseScript(key_exp_index, descriptor, ParseScriptContext::TOP, out, error);
-    if (descriptor.empty() && !ret.empty()) {
+    auto ret = ParseScript(key_exp_index, sp, ParseScriptContext::TOP, out, error);
+    if (sp.empty() && !ret.empty()) {
         std::vector<std::unique_ptr<Descriptor>> descs;
         descs.reserve(ret.size());
         for (auto& r : ret) {

src/script/descriptor.h

@@ -175,7 +175,7 @@ struct Descriptor {
  * If a parse error occurs, or the checksum is missing/invalid, or anything
  * else is wrong, an empty vector is returned.
  */
-std::vector<std::unique_ptr<Descriptor>> Parse(std::span<const char> descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum = false);
+std::vector<std::unique_ptr<Descriptor>> Parse(std::string_view descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum = false);
 
 /** Get the checksum for a `descriptor`.
  *

src/test/descriptor_tests.cpp

@@ -1262,4 +1262,13 @@ BOOST_AUTO_TEST_CASE(descriptor_test)
     CheckUnparsable("tr(musig(tuus(oldepk(gg)ggggfgg)<,z(((((((((((((((((((((st)", "tr(musig(tuus(oldepk(gg)ggggfgg)<,z(((((((((((((((((((((st)","tr(): Too many ')' in musig() expression");
 }
 
+BOOST_AUTO_TEST_CASE(descriptor_literal_null_byte)
+{
+    // Trailing '\0' string literal should be ignored.
+    FlatSigningProvider keys;
+    std::string err;
+    auto descs = Parse("pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)", keys, err, /*require_checksum=*/false);
+    BOOST_REQUIRE_MESSAGE(!descs.empty(), err);
+}
+
 BOOST_AUTO_TEST_SUITE_END()