Merge #6229: fix: creditOutputs in AssetLock tx json output should be an array of objects, not debug strings

9876c2d78b docs: add partial release notes (UdjinM6)
b330318db7 refactor: drop circular dependency (UdjinM6)
e54fe42ce8 refactor: use `key_to_p2pkh_script` in more places (UdjinM6)
3ed6246889 test: check `creditOutputs` format (UdjinM6)
ba0e64505b fix: `creditOutputs` in AssetLock tx json output should be an array of objects, not debug strings (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Txout-s in `creditOutputs` for AssetLock txes should be shown the way txout-s are shown in other places. We should not be using debug strings there.

  Example: `getrawtransaction 50757f651f335e22c5a810bd05c1e5aac0d95b132f6454e2a72683f88e3983f3 1`

  develop:
  ```
    "assetLockTx": {
      "version": 1,
      "creditOutputs": [
        "CTxOut(nValue=0.01000000, scriptPubKey=76a914cdfca4ae1cf2333056659a2c)"
      ]
    },
  ```
  This PR:
  ```
    "assetLockTx": {
      "version": 1,
      "creditOutputs": [
        {
          "value": 0.01000000,
          "valueSat": 1000000,
          "scriptPubKey": {
            "asm": "OP_DUP OP_HASH160 cdfca4ae1cf2333056659a2c8dc656f36d228402 OP_EQUALVERIFY OP_CHECKSIG",
            "hex": "76a914cdfca4ae1cf2333056659a2c8dc656f36d22840288ac",
            "address": "yf6c2VSpWGXUgmjQSHRpfEcTPsbqN4oL4c",
            "type": "pubkeyhash"
          }
        }
      ]
    },
  ```
  kudos to @coolaj86 for finding the issue

  ## What was done?
  Change `CAssetLockPayload::ToJson()` output to be closer to [`TxToUniv()`](https://github.com/dashpay/dash/blob/develop/src/core_write.cpp#L262-L272)

  NOTE: `refactor: use key_to_p2pkh_script in more places` commit is a bit unrelated but I decided to add it anyway to make it easier to follow assetlock creation vs getrawtransaction rpc check.

  ## How Has This Been Tested?
  Try example above, run tests

  ## Breaking Changes
  RPC output is different for AssetLock txes

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 9876c2d78b

Tree-SHA512: 158c98ac9e4979bb29c4f54cb1b71806f22aaec92218d92cd2b2e9b9f74df721563e7a6c5f517ea358ac74659fa79f51d1b683002a1cdceb1b8ee80f8fd79375

doc/release-notes-6229.md

@@ -0,0 +1,4 @@
+RPC changes
+-----------
+
+- `creditOutputs` entries in various RPCs that output transaction JSON are shown as objects now instead of being shown as strings.

src/evo/assetlocktx.h

@@ -22,6 +22,10 @@ namespace llmq {
 class CQuorumManager;
 } // namespace llmq
 
+// Forward declaration from core_io to get rid of circular dependency
+UniValue ValueFromAmount(const CAmount amount);
+void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex, bool include_addresses);
+
 class CAssetLockPayload
 {
 public:
@@ -51,14 +55,18 @@ public:
 
     [[nodiscard]] UniValue ToJson() const
     {
-        UniValue obj;
+        UniValue outputs(UniValue::VARR);
-        obj.setObject();
+        for (const CTxOut& credit_output : creditOutputs) {
-        obj.pushKV("version", int(nVersion));
+            UniValue out(UniValue::VOBJ);
-        UniValue outputs;
+            out.pushKV("value", ValueFromAmount(credit_output.nValue));
-        outputs.setArray();
+            out.pushKV("valueSat", credit_output.nValue);
-        for (const CTxOut& out : creditOutputs) {
+            UniValue spk(UniValue::VOBJ);
-            outputs.push_back(out.ToString());
+            ScriptPubKeyToUniv(credit_output.scriptPubKey, spk, /* fIncludeHex = */ true, /* include_addresses = */ false);
+            out.pushKV("scriptPubKey", spk);
+            outputs.push_back(out);
         }
+        UniValue obj(UniValue::VOBJ);
+        obj.pushKV("version", int(nVersion));
         obj.pushKV("creditOutputs", outputs);
         return obj;
     }

test/functional/feature_asset_locks.py

@@ -30,12 +30,9 @@ from test_framework.messages import (
 from test_framework.script import (
     CScript,
     OP_CHECKSIG,
-    OP_DUP,
-    OP_EQUALVERIFY,
-    OP_HASH160,
     OP_RETURN,
-    hash160,
 )
+from test_framework.script_util import key_to_p2pkh_script
 from test_framework.test_framework import DashTestFramework
 from test_framework.util import (
     assert_equal,
@@ -65,8 +62,8 @@ class AssetLocksTest(DashTestFramework):
         tmp_amount = amount
         if tmp_amount > COIN:
             tmp_amount -= COIN
-            credit_outputs.append(CTxOut(COIN, CScript([OP_DUP, OP_HASH160, hash160(pubkey), OP_EQUALVERIFY, OP_CHECKSIG])))
+            credit_outputs.append(CTxOut(COIN, key_to_p2pkh_script(pubkey)))
-        credit_outputs.append(CTxOut(tmp_amount, CScript([OP_DUP, OP_HASH160, hash160(pubkey), OP_EQUALVERIFY, OP_CHECKSIG])))
+        credit_outputs.append(CTxOut(tmp_amount, key_to_p2pkh_script(pubkey)))
 
         lockTx_payload = CAssetLockTx(1, credit_outputs)
 
@@ -282,7 +279,11 @@ class AssetLocksTest(DashTestFramework):
         self.check_mempool_result(tx=asset_lock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
         self.validate_credit_pool_balance(0)
         txid_in_block = self.send_tx(asset_lock_tx)
-        assert "assetLockTx" in node.getrawtransaction(txid_in_block, 1)
+        rpc_tx = node.getrawtransaction(txid_in_block, 1)
+        assert_equal(len(rpc_tx["assetLockTx"]["creditOutputs"]), 2)
+        assert_equal(rpc_tx["assetLockTx"]["creditOutputs"][0]["valueSat"] + rpc_tx["assetLockTx"]["creditOutputs"][1]["valueSat"], locked_1)
+        assert_equal(rpc_tx["assetLockTx"]["creditOutputs"][0]["scriptPubKey"]["hex"], key_to_p2pkh_script(pubkey).hex())
+        assert_equal(rpc_tx["assetLockTx"]["creditOutputs"][1]["scriptPubKey"]["hex"], key_to_p2pkh_script(pubkey).hex())
         self.validate_credit_pool_balance(0)
         node.generate(1)
         assert_equal(self.get_credit_pool_balance(node=node), locked_1)