Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement new RPC getislocks and add hex field to RPC getbestchainlock #6455

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions doc/release-notes-6455.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## New RPCs

- **`getislocks`**
- Retrieves the InstantSend lock data for the given transaction IDs (txids).
Returns the lock information in both human-friendly JSON format and binary hex-encoded zmq-compatible format.
Comment on lines +3 to +5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance the documentation for getislocks.

The documentation needs more details to be complete and useful:

  1. Document the required parameters (txids)
  2. Add example usage with sample input/output
  3. Document possible error cases
  4. Fix the list indentation (should be 2 spaces)

Here's a suggested improvement:

- - **`getislocks`**
-     - Retrieves the InstantSend lock data for the given transaction IDs (txids).
-     Returns the lock information in both human-friendly JSON format and binary hex-encoded zmq-compatible format.
+ - **`getislocks txids`**
+   - Retrieves the InstantSend lock data for the given transaction IDs.
+   - Arguments:
+     - txids (array, required): A JSON array of transaction IDs
+   - Returns:
+     - Object containing both JSON and hex-encoded formats of InstantSend locks
+   - Example:
+     ```
+     > dash-cli getislocks '["txid1", "txid2"]'
+     {
+       "locks": [...],
+       "hex": "..."
+     }
+     ```
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- **`getislocks`**
- Retrieves the InstantSend lock data for the given transaction IDs (txids).
Returns the lock information in both human-friendly JSON format and binary hex-encoded zmq-compatible format.
- **`getislocks txids`**
- Retrieves the InstantSend lock data for the given transaction IDs.
- Arguments:
- txids (array, required): A JSON array of transaction IDs
- Returns:
- Object containing both JSON and hex-encoded formats of InstantSend locks
- Example:
🧰 Tools
🪛 Markdownlint (0.37.0)

4-4: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


## Updated RPCs

- **`getbestchainlock` Changes**
- A new hex field has been added to the getbestchainlock RPC, which returns the ChainLock information in zmq-compatible, hex-encoded binary format.
Comment on lines +9 to +10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance the documentation for getbestchainlock changes.

The documentation needs more details about the new hex field and proposed changes:

  1. Document the hex field format and purpose
  2. Add example output showing the new field
  3. Fix the list indentation (should be 2 spaces)
  4. Document the proposed "verbose" argument mentioned in PR comments

Here's a suggested improvement:

- - **`getbestchainlock` Changes**
-     - A new hex field has been added to the getbestchainlock RPC, which returns the ChainLock information in zmq-compatible, hex-encoded binary format.
+ - **`getbestchainlock (verbose)`**
+   - A new hex field has been added which returns the ChainLock information in zmq-compatible, hex-encoded binary format
+   - Arguments:
+     - verbose (boolean, optional, default=true): If false, return only the raw hex data
+   - Returns when verbose=true:
+     - Object containing both JSON data and hex-encoded format
+   - Returns when verbose=false:
+     - String containing only the hex-encoded format
+   - Example:
+     ```
+     > dash-cli getbestchainlock
+     {
+       "blockhash": "...",
+       "height": ...,
+       "hex": "..."
+     }
+     ```
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- **`getbestchainlock` Changes**
- A new hex field has been added to the getbestchainlock RPC, which returns the ChainLock information in zmq-compatible, hex-encoded binary format.
- **`getbestchainlock (verbose)`**
- A new hex field has been added which returns the ChainLock information in zmq-compatible, hex-encoded binary format
- Arguments:
- verbose (boolean, optional, default=true): If false, return only the raw hex data
- Returns when verbose=true:
- Object containing both JSON data and hex-encoded format
- Returns when verbose=false:
- String containing only the hex-encoded format
- Example:
🧰 Tools
🪛 Markdownlint (0.37.0)

10-10: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


19 changes: 14 additions & 5 deletions src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,29 +241,38 @@ static RPCHelpMan getbestchainlock()
{RPCResult::Type::NUM, "height", "The block height or index"},
{RPCResult::Type::STR_HEX, "signature", "The ChainLock's BLS signature"},
{RPCResult::Type::BOOL, "known_block", "True if the block is known by our node"},
{RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for best ChainLock"},
}},
RPCExamples{
HelpExampleCli("getbestchainlock", "")
+ HelpExampleRpc("getbestchainlock", "")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
UniValue result(UniValue::VOBJ);

const NodeContext& node = EnsureAnyNodeContext(request.context);

const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
const llmq::CChainLockSig clsig = llmq_ctx.clhandler->GetBestChainLock();
if (clsig.IsNull()) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to find any ChainLock");
}

UniValue result(UniValue::VOBJ);

result.pushKV("blockhash", clsig.getBlockHash().GetHex());
result.pushKV("height", clsig.getHeight());
result.pushKV("signature", clsig.getSig().ToString());

const ChainstateManager& chainman = EnsureChainman(node);
LOCK(cs_main);
result.pushKV("known_block", chainman.m_blockman.LookupBlockIndex(clsig.getBlockHash()) != nullptr);
{
const ChainstateManager& chainman = EnsureChainman(node);
LOCK(cs_main);
result.pushKV("known_block", chainman.m_blockman.LookupBlockIndex(clsig.getBlockHash()) != nullptr);
}

CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
ssTx << clsig;
result.pushKV("hex", HexStr(ssTx));

return result;
},
};
Expand Down
1 change: 1 addition & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "gettransaction", 1, "include_watchonly" },
{ "gettransaction", 2, "verbose" },
{ "getrawtransaction", 1, "verbose" },
{ "getislocks", 0, "txids" },
{ "getrawtransactionmulti", 0, "transactions" },
{ "getrawtransactionmulti", 1, "verbose" },
{ "gettxchainlocks", 0, "txids" },
Expand Down
82 changes: 82 additions & 0 deletions src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ static RPCHelpMan getrawtransactionmulti() {
{"verbose", RPCArg::Type::BOOL, RPCArg::Default{false},
"If false, return a string, otherwise return a json object"},
},
// TODO: replace RPCResults to proper annotation
RPCResults{},
RPCExamples{
HelpExampleCli("getrawtransactionmulti",
Expand Down Expand Up @@ -366,6 +367,86 @@ static RPCHelpMan getrawtransactionmulti() {
};
}

static RPCHelpMan getislocks()
{
return RPCHelpMan{"getislocks",
"\nReturns the raw InstantSend lock data for each txids. Returns Null if there is no known IS yet.",
{
{"txids", RPCArg::Type::ARR, RPCArg::Optional::NO, "The transaction ids (no more than 100)",
{
{"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A transaction hash"},
},
},
},
RPCResult{
RPCResult::Type::ARR, "", "Response is an array with the same size as the input txids",
{{RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::STR_HEX, "txid", "The transaction id"},
{RPCResult::Type::ARR, "inputs", "The inputs",
{
{RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::STR_HEX, "txid", "The transaction id"},
{RPCResult::Type::NUM, "vout", "The output number"},
},
},
}},
{RPCResult::Type::STR_HEX, "cycleHash", "The Cycle Hash"},
{RPCResult::Type::STR_HEX, "signature", "The InstantSend's BLS signature"},
{RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"},
}},
RPCResult{"if no InstantSend Lock is known for specified txid",
RPCResult::Type::STR, "data", "Just 'None' string"
},
}},
RPCExamples{
HelpExampleCli("getislocks", "'[\"txid\",...]'")
+ HelpExampleRpc("getislocks", "'[\"txid\",...]'")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const NodeContext& node = EnsureAnyNodeContext(request.context);

UniValue result_arr(UniValue::VARR);
UniValue txids = request.params[0].get_array();
if (txids.size() > 100) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Up to 100 txids only");
}

const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
for (const auto idx : irange::range(txids.size())) {
Comment on lines +417 to +418
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add null check for llmq_ctx.isman.

The code directly accesses llmq_ctx.isman without checking if it's null.

 const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
+if (!llmq_ctx.isman) {
+    throw JSONRPCError(RPC_INTERNAL_ERROR, "InstantSend manager not available");
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
for (const auto idx : irange::range(txids.size())) {
const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
if (!llmq_ctx.isman) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "InstantSend manager not available");
}
for (const auto idx : irange::range(txids.size())) {

const uint256 txid(ParseHashV(txids[idx], "txid"));

if (const llmq::CInstantSendLockPtr islock = llmq_ctx.isman->GetInstantSendLockByTxid(txid); islock != nullptr) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could use const auto here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to see a type here 🤷 but I have no strong opinion to remove it

Comment on lines +419 to +421
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for invalid transaction IDs.

The function should validate transaction IDs before attempting to retrieve InstantSend locks.

         const uint256 txid(ParseHashV(txids[idx], "txid"));
+        if (txid.IsNull()) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid txid at index %d", idx));
+        }
 
         if (const llmq::CInstantSendLockPtr islock = llmq_ctx.isman->GetInstantSendLockByTxid(txid); islock != nullptr) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const uint256 txid(ParseHashV(txids[idx], "txid"));
if (const llmq::CInstantSendLockPtr islock = llmq_ctx.isman->GetInstantSendLockByTxid(txid); islock != nullptr) {
const uint256 txid(ParseHashV(txids[idx], "txid"));
if (txid.IsNull()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid txid at index %d", idx));
}
if (const llmq::CInstantSendLockPtr islock = llmq_ctx.isman->GetInstantSendLockByTxid(txid); islock != nullptr) {

UniValue objIS(UniValue::VOBJ);
objIS.pushKV("txid", islock->txid.ToString());
UniValue inputs(UniValue::VARR);
for (const auto out : islock->inputs) {
UniValue outpoint(UniValue::VOBJ);
outpoint.pushKV("txid", out.hash.ToString());
outpoint.pushKV("vout", static_cast<int64_t>(out.n));
inputs.push_back(outpoint);
}
objIS.pushKV("inputs", inputs);
objIS.pushKV("cycleHash", islock->cycleHash.ToString());
objIS.pushKV("signature", islock->sig.ToString());
{
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
ssTx << *islock;
objIS.pushKV("hex", HexStr(ssTx));
}
result_arr.push_back(objIS);
} else {
result_arr.push_back("None");
}
}
return result_arr;

},
};
}

static RPCHelpMan gettxchainlocks()
{
return RPCHelpMan{
Expand Down Expand Up @@ -2088,6 +2169,7 @@ static const CRPCCommand commands[] =
{ "rawtransactions", &getassetunlockstatuses, },
{ "rawtransactions", &getrawtransaction, },
{ "rawtransactions", &getrawtransactionmulti, },
{ "rawtransactions", &getislocks, },
{ "rawtransactions", &gettxchainlocks, },
{ "rawtransactions", &createrawtransaction, },
{ "rawtransactions", &decoderawtransaction, },
Expand Down
6 changes: 6 additions & 0 deletions test/functional/interface_zmq_dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ def run_test(self):
self.zmq_context = zmq.Context()
# Initialize the network
self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
self.log.info("Test RPC hex getbestchainlock before any CL appeared")
assert_raises_rpc_error(-32603, "Unable to find any ChainLock", self.nodes[0].getbestchainlock)
self.wait_for_sporks_same()
self.activate_v19(expected_activation_height=900)
self.log.info("Activated v19 at height:" + str(self.nodes[0].getblockcount()))
Expand Down Expand Up @@ -263,6 +265,7 @@ def test_chainlock_publishers(self):
assert_equal(uint256_to_string(zmq_chain_lock.blockHash), rpc_chain_lock_hash)
assert_equal(zmq_chain_locked_block.hash, rpc_chain_lock_hash)
assert_equal(zmq_chain_lock.sig.hex(), rpc_best_chain_lock_sig)
assert_equal(zmq_chain_lock.serialize().hex(), self.nodes[0].getbestchainlock()['hex'])
# Unsubscribe from ChainLock messages
self.unsubscribe(chain_lock_publishers)

Expand All @@ -285,6 +288,7 @@ def test_instantsend_publishers(self):
# Create two raw TXs, they will conflict with each other
rpc_raw_tx_1 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)
rpc_raw_tx_2 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)
assert_equal(['None'], self.nodes[0].getislocks([rpc_raw_tx_1['txid']]))
# Send the first transaction and wait for the InstantLock
rpc_raw_tx_1_hash = self.nodes[0].sendrawtransaction(rpc_raw_tx_1['hex'])
self.wait_for_instantlock(rpc_raw_tx_1_hash, self.nodes[0])
Expand All @@ -304,6 +308,8 @@ def test_instantsend_publishers(self):
assert_equal(zmq_tx_lock_tx.hash, rpc_raw_tx_1['txid'])
zmq_tx_lock = msg_isdlock()
zmq_tx_lock.deserialize(zmq_tx_lock_sig_stream)
assert_equal(rpc_raw_tx_1['txid'], self.nodes[0].getislocks([rpc_raw_tx_1['txid']])[0]['txid'])
assert_equal(zmq_tx_lock.serialize().hex(), self.nodes[0].getislocks([rpc_raw_tx_1['txid']])[0]['hex'])
assert_equal(uint256_to_string(zmq_tx_lock.txid), rpc_raw_tx_1['txid'])
# Try to send the second transaction. This must throw an RPC error because it conflicts with rpc_raw_tx_1
# which already got the InstantSend lock.
Expand Down
Loading