Merge bitcoin/bitcoin#23486: rpc: Only allow specific types to be P2(W)SH wrapped in decodescript

9999342 rpc: Only allow specific types to be P2(W)SH wrapped in decodescript (MarcoFalke)

Pull request description:

  It seems confusing to return a P2SH wrapping address that is eventually either policy- or consensus-unspendable.

ACKs for top commit:
  laanwj:
    Code review re-ACK 9999342

Tree-SHA512: 3cd530442acee7c295d244995f0f17b2cae7212f1e0970bb5807621f8ff8e4308a3236b385d77087cd493d32ee524813d8edd15e91d937ef9a800094b7bc4946

Author: laanwj

src/rpc/rawtransaction.cpp

@@ -552,8 +552,10 @@ static RPCHelpMan decodescript()
                 {RPCResult::Type::STR, "asm", "Script public key"},
                 {RPCResult::Type::STR, "type", "The output type (e.g. " + GetAllOutputTypes() + ")"},
                 {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"},
-                {RPCResult::Type::STR, "p2sh", /* optional */ true, "address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH)"},
-                {RPCResult::Type::OBJ, "segwit", /* optional */ true, "Result of a witness script public key wrapping this redeem script (not returned if the script is a P2SH or witness)",
+                {RPCResult::Type::STR, "p2sh", /*optional=*/true,
+                 "address of P2SH script wrapping this redeem script (not returned for types that should not be wrapped)"},
+                {RPCResult::Type::OBJ, "segwit", /*optional=*/true,
+                 "Result of a witness script public key wrapping this redeem script (not returned for types that should not be wrapped)",
                  {
                      {RPCResult::Type::STR, "asm", "String representation of the script public key"},
                      {RPCResult::Type::STR_HEX, "hex", "Hex string of the script public key"},
@@ -584,22 +586,68 @@ static RPCHelpMan decodescript()
     std::vector<std::vector<unsigned char>> solutions_data;
     const TxoutType which_type{Solver(script, solutions_data)};
 
-    if (which_type != TxoutType::SCRIPTHASH) {
-        // P2SH cannot be wrapped in a P2SH. If this script is already a P2SH,
-        // don't return the address for a P2SH of the P2SH.
+    const bool can_wrap{[&] {
+        switch (which_type) {
+        case TxoutType::MULTISIG:
+        case TxoutType::NONSTANDARD:
+        case TxoutType::PUBKEY:
+        case TxoutType::PUBKEYHASH:
+        case TxoutType::WITNESS_V0_KEYHASH:
+        case TxoutType::WITNESS_V0_SCRIPTHASH:
+            // Can be wrapped if the checks below pass
+            break;
+        case TxoutType::NULL_DATA:
+        case TxoutType::SCRIPTHASH:
+        case TxoutType::WITNESS_UNKNOWN:
+        case TxoutType::WITNESS_V1_TAPROOT:
+            // Should not be wrapped
+            return false;
+        } // no default case, so the compiler can warn about missing cases
+        if (!script.HasValidOps() || script.IsUnspendable()) {
+            return false;
+        }
+        for (CScript::const_iterator it{script.begin()}; it != script.end();) {
+            opcodetype op;
+            CHECK_NONFATAL(script.GetOp(it, op));
+            if (op == OP_CHECKSIGADD || IsOpSuccess(op)) {
+                return false;
+            }
+        }
+        return true;
+    }()};
+
+    if (can_wrap) {
         r.pushKV("p2sh", EncodeDestination(ScriptHash(script)));
         // P2SH and witness programs cannot be wrapped in P2WSH, if this script
         // is a witness program, don't return addresses for a segwit programs.
-        if (which_type == TxoutType::PUBKEY || which_type == TxoutType::PUBKEYHASH || which_type == TxoutType::MULTISIG || which_type == TxoutType::NONSTANDARD) {
+        const bool can_wrap_P2WSH{[&] {
+            switch (which_type) {
+            case TxoutType::MULTISIG:
+            case TxoutType::PUBKEY:
             // Uncompressed pubkeys cannot be used with segwit checksigs.
             // If the script contains an uncompressed pubkey, skip encoding of a segwit program.
-            if ((which_type == TxoutType::PUBKEY) || (which_type == TxoutType::MULTISIG)) {
                 for (const auto& solution : solutions_data) {
                     if ((solution.size() != 1) && !CPubKey(solution).IsCompressed()) {
-                        return r;
+                        return false;
                     }
                 }
-            }
+                return true;
+            case TxoutType::NONSTANDARD:
+            case TxoutType::PUBKEYHASH:
+                // Can be P2WSH wrapped
+                return true;
+            case TxoutType::NULL_DATA:
+            case TxoutType::SCRIPTHASH:
+            case TxoutType::WITNESS_UNKNOWN:
+            case TxoutType::WITNESS_V0_KEYHASH:
+            case TxoutType::WITNESS_V0_SCRIPTHASH:
+            case TxoutType::WITNESS_V1_TAPROOT:
+                // Should not be wrapped
+                return false;
+            } // no default case, so the compiler can warn about missing cases
+            CHECK_NONFATAL(false);
+        }()};
+        if (can_wrap_P2WSH) {
             UniValue sr(UniValue::VOBJ);
             CScript segwitScr;
             if (which_type == TxoutType::PUBKEY) {
@@ -608,7 +656,6 @@ static RPCHelpMan decodescript()
                 segwitScr = GetScriptForDestination(WitnessV0KeyHash(uint160{solutions_data[0]}));
             } else {
                 // Scripts that are not fit for P2WPKH are encoded as P2WSH.
-                // Newer segwit program versions should be considered when then become available.
                 segwitScr = GetScriptForDestination(WitnessV0ScriptHash(script));
             }
             ScriptPubKeyToUniv(segwitScr, sr, /* include_hex */ true);

test/functional/data/rpc_decodescript.json

@@ -4,17 +4,15 @@
         {
             "asm": "1 eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee",
             "address": "bcrt1pamhwamhwamhwamhwamhwamhwamhwamhwamhwamhwamhwamhwamhqz6nvlh",
-            "type": "witness_v1_taproot",
-            "p2sh": "2Mt5gBng2UVL3xX4FUQinSBthq8gWQqs37g"
+            "type": "witness_v1_taproot"
         }
     ],
     [
         "5102eeee",
         {
             "asm": "1 -28398",
             "address": "bcrt1pamhqk96edn",
-            "type": "witness_unknown",
-            "p2sh": "2ND89Zqxi19tq7AjL5Y3un8fDWRwpwrk4tf"
+            "type": "witness_unknown"
         }
     ],
     [
@@ -38,38 +36,21 @@
         "6a00",
         {
             "asm": "OP_RETURN 0",
-            "type": "nulldata",
-            "p2sh": "2NG8CqGyR16jkZU5H7J9WM5xpCT6Fpw6bww"
+            "type": "nulldata"
         }
     ],
     [
         "6aee",
         {
             "asm": "OP_RETURN OP_UNKNOWN",
-            "type": "nonstandard",
-            "p2sh": "2NGU1bmCBhSooc3vkPYdea2ngDcwhNx8CeF",
-            "segwit": {
-                "asm": "0 44358a3abb4cc9f635f459edffb2a1210f849857aaf12106a1af645e034faa95",
-                "hex": "002044358a3abb4cc9f635f459edffb2a1210f849857aaf12106a1af645e034faa95",
-                "address": "bcrt1qgs6c5w4mfnylvd05t8kllv4pyy8cfxzh4tcjzp4p4aj9uq60422sw9mgmf",
-                "type": "witness_v0_scripthash",
-                "p2sh-segwit": "2N9xFeGJC4Z2BQcVEq7vyeNUZiVoANFbrX1"
-            }
+            "type": "nonstandard"
         }
     ],
     [
         "6a02ee",
         {
             "asm": "OP_RETURN [error]",
-            "type": "nonstandard",
-            "p2sh": "2N9JFV56rrkTYVnrJTMFSpKNsq6j5NbAdQr",
-            "segwit": {
-                "asm": "0 6f3d493995bda1f72a8f4de96663be22b583623a05f5ae98f38c45b8e03ca5da",
-                "hex": "00206f3d493995bda1f72a8f4de96663be22b583623a05f5ae98f38c45b8e03ca5da",
-                "address": "bcrt1qdu75jwv4hkslw250fh5kvca7y26cxc36qh66ax8n33zm3cpu5hdqdtm4gp",
-                "type": "witness_v0_scripthash",
-                "p2sh-segwit": "2N3TqW8vuVr987Z695CmLNmLLXobBRMmqho"
-            }
+            "type": "nonstandard"
         }
     ],
     [
@@ -91,30 +72,14 @@
         "ba",
         {
             "asm": "OP_CHECKSIGADD",
-            "type": "nonstandard",
-            "p2sh": "2MyX11u6v747zcKHTJMjXFgkj1vYZgHr4i1",
-            "segwit": {
-                "asm": "0 281c93990bac2c69cf372c9a3b66c406c86cca826d6407b68e644da22eef8186",
-                "hex": "0020281c93990bac2c69cf372c9a3b66c406c86cca826d6407b68e644da22eef8186",
-                "address": "bcrt1q9qwf8xgt4skxnneh9jdrkekyqmyxej5zd4jq0d5wv3x6yth0sxrqe2wl7r",
-                "type": "witness_v0_scripthash",
-                "p2sh-segwit": "2NBoeWVFMmZdEhLzP5kpvjnJ8c1GucsCbFK"
-            }
+            "type": "nonstandard"
         }
     ],
     [
         "50",
         {
             "asm": "OP_RESERVED",
-            "type": "nonstandard",
-            "p2sh": "2NEqnmDnSWcfTRBG2t6M53ey6mjc8ncHesN",
-            "segwit": {
-                "asm": "0 5c62e091b8c0565f1bafad0dad5934276143ae2ccef7a5381e8ada5b1a8d26d2",
-                "hex": "00205c62e091b8c0565f1bafad0dad5934276143ae2ccef7a5381e8ada5b1a8d26d2",
-                "address": "bcrt1qt33wpydccpt97xa045x66kf5yas58t3vemm62wq73td9kx5dymfqknplwh",
-                "type": "witness_v0_scripthash",
-                "p2sh-segwit": "2NEtjT3ku2KjZo53bnwKX2v928Mzx5sjdUh"
-            }
+            "type": "nonstandard"
         }
     ]
-]
+]