feat(rpc): Asset Unlock status by index #5776

ogabrielides · 2023-12-19T10:33:01Z

Issue being fixed or feature implemented

Platform in the scope of credit withdrawals, need a way to get the status of an Asset Unlock by index.

What was done?

A new RPC was created getassetunlockchainlocks that accepts Asset Unlock indexes array as parameter and return corresponding status for each index.

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.

Note: This RPC is whitelisted for the Platform RPC user.

How Has This Been Tested?

Inserted on feature_asset_locks.py covering cases where Asset Unlock txs are in mempool, mined and not present.

Breaking Changes

no

Checklist:

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 (for repository code-owners and collaborators only)

src/rpc/rawtransaction.cpp

PastaPastaPasta · 2023-12-19T13:52:57Z

src/rpc/rawtransaction.cpp

+    CBlockIndex* pBlockIndex{nullptr};
+    bool chainlock_info = false;
+    llmq::CChainLockSig bestclsig = llmq_ctx.clhandler->GetBestChainLock();
+    if (bestclsig.IsNull()) {


we should create a separate feat PR which on ProcessBlock we will submit a blocks chain lock to the chain lock system

src/rpc/rawtransaction.cpp

thephez

~~What about calling it getassetunlockstatus instead since it returns one of several status values?~~
As discussed, getassetunlockstatuses looks like a better options since it can return status for multiple txs.

UdjinM6

utACK

thephez

Probably should update the release notes now too so they match

doc/release-notes-5776.md

Co-authored-by: thephez <[email protected]>

UdjinM6

re-utACK

test/functional/feature_asset_locks.py

src/rpc/rawtransaction.cpp

test/functional/feature_asset_locks.py

knst

ACK

knst

utACK

knst · 2023-12-21T09:32:03Z

src/rpc/rawtransaction.cpp

        else {
            UniValue jnull(UniValue::VNULL);
-            obj.pushKV(str_indexes[i].get_str(), jnull);
+            obj.pushKV("status", jnull);


nit: obj.pushKV("status", UniValue{}); does same, doesn't it?

PastaPastaPasta

Please see the following patch; note this changes behavior to not return a null but "unknown"

Subject: [PATCH] lots of lamdas
---
Index: src/rpc/rawtransaction.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
--- a/src/rpc/rawtransaction.cpp	(revision 22dd732df77f69163293b8adcea1e9af1f3c6c73)
+++ b/src/rpc/rawtransaction.cpp	(date 1703220226644)
@@ -388,25 +388,31 @@
     }
 
     CBlockIndex* pTipBlockIndex{WITH_LOCK(cs_main, return chainman.ActiveChain().Tip())};
-
-    CBlockIndex* pBlockIndexBestCL{nullptr};
-    const llmq::CChainLockSig bestclsig{llmq_ctx.clhandler->GetBestChainLock()};
-    if (bestclsig.IsNull()) {
-        // If no CL info is available, try to use CbTx CL information
-        if (const auto cbtx_best_cl = GetNonNullCoinbaseChainlock(pTipBlockIndex); cbtx_best_cl.has_value()) {
-            pBlockIndexBestCL = pTipBlockIndex->GetAncestor(pTipBlockIndex->nHeight - cbtx_best_cl->second - 1);
-        }
-    }
-
     if (!pTipBlockIndex) {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "No blocks in chain");
     }
 
+    const auto pBlockIndexBestCL = [&]() -> const CBlockIndex* {
+        if (llmq_ctx.clhandler->GetBestChainLock().IsNull()) {
+            // 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;
+    }();
+
     // 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<CCreditPool> poolCL;
-    if (pBlockIndexBestCL != nullptr) poolCL = node.creditPoolManager->GetCreditPool(pBlockIndexBestCL, Params().GetConsensus());
-    std::optional<CCreditPool> poolOnTip{};
+    std::optional<CCreditPool> poolCL = pBlockIndexBestCL ?
+            std::make_optional(node.creditPoolManager->GetCreditPool(pBlockIndexBestCL, Params().GetConsensus())) :
+            std::nullopt;
+    auto poolOnTip = [&]() -> std::optional<CCreditPool> {
+        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);
@@ -415,39 +421,32 @@
             throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid index");
         }
         obj.pushKV("index", index);
-        if (poolCL.has_value() && poolCL->indexes.Contains(index)) {
-            obj.pushKV("status", "chainlocked");
-            result_arr.push_back(obj);
-            continue;
-        }
-        if (pTipBlockIndex != pBlockIndexBestCL) {
-            if (!poolOnTip.has_value()) poolOnTip = node.creditPoolManager->GetCreditPool(pTipBlockIndex, Params().GetConsensus());
-            if (poolOnTip->indexes.Contains(index)) {
-                obj.pushKV("status", "mined");
-                result_arr.push_back(obj);
-                continue;
+
+        auto status_to_push = [&]() -> std::string {
+            if (poolCL.has_value() && poolCL->indexes.Contains(index)) {
+                return "chainlocked";
+            }
+            if (poolOnTip.has_value() && poolOnTip->indexes.Contains(index)) {
+                return "mined";
             }
-        }
-        LOCK(mempool.cs);
-        bool is_mempooled = false;
-        for (const CTxMemPoolEntry& e : mempool.mapTx) {
-            if (e.GetTx().nType == CAssetUnlockPayload::SPECIALTX_TYPE) {
-                CAssetUnlockPayload assetUnlockTx;
-                if (!GetTxPayload(e.GetTx(), assetUnlockTx)) {
-                    throw JSONRPCError(RPC_TRANSACTION_ERROR, "bad-assetunlocktx-payload");
-                }
-                if (index == assetUnlockTx.getIndex()) {
-                    is_mempooled = true;
-                    break;
-                }
-            }
-        }
-        if (is_mempooled)
-            obj.pushKV("status", "mempooled");
-        else {
-            UniValue jnull(UniValue::VNULL);
-            obj.pushKV("status", jnull);
-        }
+            bool is_mempooled = [&]() {
+                LOCK(mempool.cs);
+                return std::any_of(mempool.mapTx.begin(), mempool.mapTx.end(), [index](const CTxMemPoolEntry &e) {
+                    if (e.GetTx().nType == CAssetUnlockPayload::SPECIALTX_TYPE) {
+                        if (CAssetUnlockPayload assetUnlockTx; GetTxPayload(e.GetTx(), assetUnlockTx)) {
+                            return index == assetUnlockTx.getIndex();
+                        } else {
+                            throw JSONRPCError(RPC_TRANSACTION_ERROR, "bad-assetunlocktx-payload");
+                        }
+                    }
+                    return false;
+                });
+            }();
+            return is_mempooled ? "mempooled" : "unknown";
+        };
+
+        obj.pushKV("status", status_to_push());
+
         result_arr.push_back(obj);
     }

src/init.cpp

…l variable. This fix unifies these objects, but it should be deglobalized instead. Deglobalization in follow PRs

knst

utACK

knst · 2023-12-22T11:08:10Z

src/init.cpp

@@ -1924,8 +1925,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
                node.evodb = std::make_unique<CEvoDB>(nEvoDbCache, false, fReset || fReindexChainState);
                node.mnhf_manager.reset();
                node.mnhf_manager = std::make_unique<CMNHFManager>(*node.evodb);
-                node.creditPoolManager.reset();
-                node.creditPoolManager = std::make_unique<CCreditPoolManager>(*node.evodb);
+


nit: extra \n

UdjinM6

👍

utACK

doc/release-notes-5776.md

Co-authored-by: PastaPastaPasta <[email protected]>

PastaPastaPasta

utACK for squash merge

UdjinM6

utACK

ogabrielides added 4 commits December 19, 2023 12:19

feat(rpc): getassetunlockchainlocks

97e0bb1

linter fix

ee1774e

added release-notes

a1416bc

refactor + comments

6e16d19

ogabrielides requested review from UdjinM6, thephez, knst and PastaPastaPasta December 19, 2023 12:32

ogabrielides added this to the 20.1 milestone Dec 19, 2023

ogabrielides added the RPC Some notable changes to RPC params/behaviour/descriptions label Dec 19, 2023

PastaPastaPasta reviewed Dec 19, 2023

View reviewed changes

thephez reviewed Dec 19, 2023

View reviewed changes

suggestions

08ad2cb

ogabrielides requested review from PastaPastaPasta and thephez December 19, 2023 15:19

UdjinM6 previously approved these changes Dec 19, 2023

View reviewed changes

thephez reviewed Dec 19, 2023

View reviewed changes

doc/release-notes-5776.md Outdated Show resolved Hide resolved

Update doc/release-notes-5776.md

a3d076e

Co-authored-by: thephez <[email protected]>

ogabrielides dismissed UdjinM6’s stale review via a3d076e December 19, 2023 19:27

UdjinM6 previously approved these changes Dec 19, 2023

View reviewed changes

knst reviewed Dec 20, 2023

View reviewed changes

test/functional/feature_asset_locks.py Outdated Show resolved Hide resolved

knst reviewed Dec 20, 2023

View reviewed changes

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved

fix: mined status can be set for non-CL block

c4155c2

ogabrielides dismissed UdjinM6’s stale review via c4155c2 December 20, 2023 12:43

thephez approved these changes Dec 20, 2023

View reviewed changes

knst reviewed Dec 20, 2023

View reviewed changes

test/functional/feature_asset_locks.py Outdated Show resolved Hide resolved

suggestions

0bd48aa

knst previously approved these changes Dec 20, 2023

View reviewed changes

ogabrielides marked this pull request as draft December 20, 2023 17:58

change of reply model

22dd732

ogabrielides dismissed knst’s stale review via 22dd732 December 21, 2023 09:22

ogabrielides marked this pull request as ready for review December 21, 2023 09:22

ogabrielides requested review from UdjinM6, knst and thephez December 21, 2023 09:22

knst previously approved these changes Dec 21, 2023

View reviewed changes

PastaPastaPasta requested changes Dec 22, 2023

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

suggestions

2eccd9f

ogabrielides dismissed knst’s stale review via 2eccd9f December 22, 2023 09:27

ogabrielides requested review from PastaPastaPasta and knst December 22, 2023 09:29

fix: rpc uses NodeContext's creditPool but all other places use globa…

e8a1011

…l variable. This fix unifies these objects, but it should be deglobalized instead. Deglobalization in follow PRs

knst previously approved these changes Dec 22, 2023

View reviewed changes

UdjinM6 previously approved these changes Dec 22, 2023

View reviewed changes

PastaPastaPasta reviewed Dec 22, 2023

View reviewed changes

doc/release-notes-5776.md Outdated Show resolved Hide resolved

Update release-notes-5776.md

7ac07b7

Co-authored-by: PastaPastaPasta <[email protected]>

ogabrielides dismissed stale reviews from UdjinM6 and knst via 7ac07b7 December 22, 2023 20:04

ogabrielides requested review from PastaPastaPasta, knst and UdjinM6 December 22, 2023 20:05

trivial: make a few things const; indent properly

f155b32

PastaPastaPasta approved these changes Dec 22, 2023

View reviewed changes

UdjinM6 approved these changes Dec 22, 2023

View reviewed changes

PastaPastaPasta merged commit 563cc34 into dashpay:develop Dec 22, 2023
7 of 8 checks passed

ogabrielides deleted the get_assetunlock_chainlocks_rpc branch December 24, 2023 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rpc): Asset Unlock status by index #5776

feat(rpc): Asset Unlock status by index #5776

ogabrielides commented Dec 19, 2023 •

edited

Loading

PastaPastaPasta Dec 19, 2023

ogabrielides Dec 19, 2023

thephez left a comment •

edited

Loading

UdjinM6 left a comment

thephez left a comment

UdjinM6 left a comment

knst left a comment

knst left a comment

knst Dec 21, 2023

PastaPastaPasta left a comment

knst left a comment

knst Dec 22, 2023

UdjinM6 left a comment

PastaPastaPasta left a comment

UdjinM6 left a comment

feat(rpc): Asset Unlock status by index #5776

feat(rpc): Asset Unlock status by index #5776

Conversation

ogabrielides commented Dec 19, 2023 • edited Loading

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

PastaPastaPasta Dec 19, 2023

Choose a reason for hiding this comment

ogabrielides Dec 19, 2023

Choose a reason for hiding this comment

thephez left a comment • edited Loading

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

thephez left a comment

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

knst left a comment

Choose a reason for hiding this comment

knst left a comment

Choose a reason for hiding this comment

knst Dec 21, 2023

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

knst left a comment

Choose a reason for hiding this comment

knst Dec 22, 2023

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

ogabrielides commented Dec 19, 2023 •

edited

Loading

thephez left a comment •

edited

Loading