From 82310b09849580f36234180b5ac54bd460a18a5c Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Thu, 1 Feb 2024 17:15:20 +0200 Subject: [PATCH] feat(rpc): added optional block height in `getassetunlockstatuses` (#5849) ## Issue being fixed or feature implemented RPC `getassetunlockstatuses` is now accepting an extra optional parameter `height`. When a valid `height` is passed, then the RPC returns the status of AssetUnlock indexes up to this specific block. (Requested by Platform team) ## What was done? Note that in order to avoid cases that can lead to deterministic result, when `height` is passed, then the only `chainlocked` and `unknown` outcomes are possible. ## How Has This Been Tested? `feature_asset_locks.py` was updated. ## Breaking Changes n/a ## 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 - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --------- Co-authored-by: Konstantin Akimov Co-authored-by: UdjinM6 --- doc/release-notes-5776.md | 13 +++--- src/rpc/client.cpp | 2 + src/rpc/rawtransaction.cpp | 55 +++++++++++++++++--------- test/functional/feature_asset_locks.py | 16 ++++++-- 4 files changed, 58 insertions(+), 28 deletions(-) diff --git a/doc/release-notes-5776.md b/doc/release-notes-5776.md index ac1e102fa1..e7885e3dce 100644 --- a/doc/release-notes-5776.md +++ b/doc/release-notes-5776.md @@ -1,11 +1,14 @@ Added RPC -------- -- `getassetunlockstatuses` RPC allows fetching of Asset Unlock txs by their withdrawal index. The RPC accepts an array of indexes and returns status for each index. +- `getassetunlockstatuses` RPC allows fetching of Asset Unlock txs by their withdrawal index. +The RPC accepts an array of indexes and an optional block height. The possible outcomes per each index are: -- "chainlocked": If the Asset Unlock index is mined on a ChainLocked block. -- "mined": If no ChainLock information is available, and the Asset Unlock index is mined. -- "mempooled": If the Asset Unlock index is in the mempool. -- "unknown": If none of the above are valid. +- `chainlocked`: If the Asset Unlock index is mined on a ChainLocked block or up to the given block height. +- `mined`: If no ChainLock information is available for the mined Asset Unlock index. +- `mempooled`: If the Asset Unlock index is in the mempool. +- `unknown`: If none of the above are valid. + +Note: If a specific block height is passed on request, then only `chainlocked` and `unknown` outcomes are possible. Note: This RPC is whitelisted for the Platform RPC user. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index e9050e41ca..52eb7bb4d0 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -58,6 +58,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "listreceivedbylabel", 1, "addlocked" }, { "listreceivedbylabel", 2, "include_empty" }, { "listreceivedbylabel", 3, "include_watchonly" }, + { "getassetunlockstatuses", 0, "indexes" }, + { "getassetunlockstatuses", 1, "height" }, { "getbalance", 1, "minconf" }, { "getbalance", 2, "addlocked" }, { "getbalance", 3, "include_watchonly" }, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index be9301ee82..9f4779186e 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -438,13 +438,14 @@ static void getassetunlockstatuses_help(const JSONRPCRequest& request) { RPCHelpMan{ "getassetunlockstatuses", - "\nReturns the status of given Asset Unlock indexes.\n", + "\nReturns the status of given Asset Unlock indexes at the tip of the chain or at a specific block height if specified.\n", { {"indexes", RPCArg::Type::ARR, RPCArg::Optional::NO, "The Asset Unlock indexes (no more than 100)", { {"index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "An Asset Unlock index"}, }, }, + {"height", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The maximum block height to check"}, }, RPCResult{ RPCResult::Type::ARR, "", "Response is an array with the same size as the input txids", @@ -488,27 +489,43 @@ static UniValue getassetunlockstatuses(const JSONRPCRequest& request) throw JSONRPCError(RPC_INTERNAL_ERROR, "No blocks in chain"); } - const auto pBlockIndexBestCL = [&]() -> const CBlockIndex* { - if (llmq_ctx.clhandler->GetBestChainLock().IsNull()) { + std::optional poolCL{std::nullopt}; + std::optional poolOnTip{std::nullopt}; + std::optional nSpecificCoreHeight{std::nullopt}; + + if (!request.params[1].isNull()) { + nSpecificCoreHeight = request.params[1].get_int(); + if (nSpecificCoreHeight.value() < 0 || nSpecificCoreHeight.value() > chainman.ActiveChain().Height()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); + } + poolCL = std::make_optional(node.creditPoolManager->GetCreditPool(chainman.ActiveChain()[nSpecificCoreHeight.value()], Params().GetConsensus())); + } + else { + const auto pBlockIndexBestCL = [&]() -> const CBlockIndex* { + if (!llmq_ctx.clhandler->GetBestChainLock().IsNull()) { + return pTipBlockIndex->GetAncestor(llmq_ctx.clhandler->GetBestChainLock().getHeight()); + } // If no CL info is available, try to use CbTx CL information if (const auto cbtx_best_cl = GetNonNullCoinbaseChainlock(pTipBlockIndex)) { return pTipBlockIndex->GetAncestor(pTipBlockIndex->nHeight - cbtx_best_cl->second - 1); } - } - return nullptr; - }(); + // no CL info, no CbTx CL + return nullptr; + }(); - // We need in 2 credit pools: at tip of chain and on best CL to know if tx is mined or chainlocked - // Sometimes that's two different blocks, sometimes not and we need to initialize 2nd creditPoolManager - std::optional poolCL = pBlockIndexBestCL ? - std::make_optional(node.creditPoolManager->GetCreditPool(pBlockIndexBestCL, Params().GetConsensus())) : - std::nullopt; - auto poolOnTip = [&]() -> std::optional { - if (pTipBlockIndex != pBlockIndexBestCL) { - return std::make_optional(node.creditPoolManager->GetCreditPool(pTipBlockIndex, Params().GetConsensus())); - } - return std::nullopt; - }(); + // We need in 2 credit pools: at tip of chain and on best CL to know if tx is mined or chainlocked + // Sometimes that's two different blocks, sometimes not and we need to initialize 2nd creditPoolManager + poolCL = pBlockIndexBestCL ? + std::make_optional(node.creditPoolManager->GetCreditPool(pBlockIndexBestCL, Params().GetConsensus())) : + std::nullopt; + + poolOnTip = [&]() -> std::optional { + if (pTipBlockIndex != pBlockIndexBestCL) { + return std::make_optional(node.creditPoolManager->GetCreditPool(pTipBlockIndex, Params().GetConsensus())); + } + return std::nullopt; + }(); + } for (const auto i : irange::range(str_indexes.size())) { UniValue obj(UniValue::VOBJ); @@ -537,7 +554,7 @@ static UniValue getassetunlockstatuses(const JSONRPCRequest& request) return false; }); }(); - return is_mempooled ? "mempooled" : "unknown"; + return is_mempooled && !nSpecificCoreHeight.has_value() ? "mempooled" : "unknown"; }; obj.pushKV("status", status_to_push()); result_arr.push_back(obj); @@ -1975,7 +1992,7 @@ void RegisterRawTransactionRPCCommands(CRPCTable &t) static const CRPCCommand commands[] = { // category name actor (function) argNames // --------------------- ------------------------ ----------------------- ---------- - { "rawtransactions", "getassetunlockstatuses", &getassetunlockstatuses, {"indexes"} }, + { "rawtransactions", "getassetunlockstatuses", &getassetunlockstatuses, {"indexes","height"} }, { "rawtransactions", "getrawtransaction", &getrawtransaction, {"txid","verbose","blockhash"} }, { "rawtransactions", "getrawtransactionmulti", &getrawtransactionmulti, {"txid_map","verbose"} }, { "rawtransactions", "gettxchainlocks", &gettxchainlocks, {"txids"} }, diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py index acaa158cdd..71d8723367 100755 --- a/test/functional/feature_asset_locks.py +++ b/test/functional/feature_asset_locks.py @@ -349,8 +349,12 @@ class AssetLocksTest(DashTestFramework): txid = self.send_tx(asset_unlock_tx) assert "assetUnlockTx" in node.getrawtransaction(txid, 1) - indexes_statuses = self.nodes[0].getassetunlockstatuses(["101", "102", "300"]) - assert_equal([{'index': 101, 'status': 'mempooled'}, {'index': 102, 'status': 'unknown'}, {'index': 300, 'status': 'unknown'}], indexes_statuses) + tip = self.nodes[0].getblockcount() + indexes_statuses_no_height = self.nodes[0].getassetunlockstatuses(["101", "102", "300"]) + assert_equal([{'index': 101, 'status': 'mempooled'}, {'index': 102, 'status': 'unknown'}, {'index': 300, 'status': 'unknown'}], indexes_statuses_no_height) + indexes_statuses_height = self.nodes[0].getassetunlockstatuses(["101", "102", "300"], tip) + assert_equal([{'index': 101, 'status': 'unknown'}, {'index': 102, 'status': 'unknown'}, {'index': 300, 'status': 'unknown'}], indexes_statuses_height) + self.mempool_size += 1 self.check_mempool_size() @@ -524,8 +528,12 @@ class AssetLocksTest(DashTestFramework): node.generate(1) self.sync_all() - indexes_statuses = self.nodes[0].getassetunlockstatuses(["101", "102", "103"]) - assert_equal([{'index': 101, 'status': 'mined'}, {'index': 102, 'status': 'mined'}, {'index': 103, 'status': 'unknown'}], indexes_statuses) + tip = self.nodes[0].getblockcount() + indexes_statuses_no_height = self.nodes[0].getassetunlockstatuses(["101", "102", "103"]) + assert_equal([{'index': 101, 'status': 'mined'}, {'index': 102, 'status': 'mined'}, {'index': 103, 'status': 'unknown'}], indexes_statuses_no_height) + indexes_statuses_height = self.nodes[0].getassetunlockstatuses(["101", "102", "103"], tip) + assert_equal([{'index': 101, 'status': 'chainlocked'}, {'index': 102, 'status': 'chainlocked'}, {'index': 103, 'status': 'unknown'}], indexes_statuses_height) + self.log.info("generate many blocks to be sure that mempool is empty after expiring txes...") self.slowly_generate_batch(60)