Skip to content

fix: guard deprecated service netInfo access #7344

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.

Sign up for GitHub

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

Open
thepastaclaw wants to merge 1 commit into
base: develop
Choose a base branch
from
+24 −6
Open

fix: guard deprecated service netInfo access #7344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 10 additions & 4 deletions src/evo/core_write.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ const std::map<std::string, RPCResult> RPCRESULT_MAP{{
RESULT_MAP_ENTRY("votingAddress", RPCResult::Type::STR, "Dash address used for voting"),
}};
#undef RESULT_MAP_ENTRY

template <typename Obj>
std::string GetDeprecatedServiceField(const Obj& obj)
{
return CHECK_NONFATAL(obj.netInfo)->GetPrimary().ToStringAddrPort();
}
} // anonymous namespace

RPCResult GetRpcResult(const std::string& key, bool optional, const std::string& override_name)
Expand Down Expand Up @@ -222,7 +228,7 @@ UniValue CDeterministicMNState::ToJson(MnType nType) const
UniValue obj(UniValue::VOBJ);
obj.pushKV("version", nVersion);
if (IsServiceDeprecatedRPCEnabled()) {
obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
obj.pushKV("service", GetDeprecatedServiceField(*this));
}
obj.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
obj.pushKV("registeredHeight", nRegisteredHeight);
Expand Down Expand Up @@ -309,7 +315,7 @@ UniValue CProRegTx::ToJson() const
ret.pushKV("collateralHash", collateralOutpoint.hash.ToString());
ret.pushKV("collateralIndex", collateralOutpoint.n);
if (IsServiceDeprecatedRPCEnabled()) {
ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
ret.pushKV("service", GetDeprecatedServiceField(*this));
}
ret.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
ret.pushKV("ownerAddress", EncodeDestination(PKHash(keyIDOwner)));
Expand Down Expand Up @@ -402,7 +408,7 @@ UniValue CProUpServTx::ToJson() const
ret.pushKV("type", std23::to_underlying(nType));
ret.pushKV("proTxHash", proTxHash.ToString());
if (IsServiceDeprecatedRPCEnabled()) {
ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
ret.pushKV("service", GetDeprecatedServiceField(*this));
}
ret.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
if (CTxDestination dest; ExtractDestination(scriptOperatorPayout, dest)) {
Expand Down Expand Up @@ -540,7 +546,7 @@ UniValue CSimplifiedMNListEntry::ToJson(bool extended) const
obj.pushKV("proRegTxHash", proRegTxHash.ToString());
obj.pushKV("confirmedHash", confirmedHash.ToString());
if (IsServiceDeprecatedRPCEnabled()) {
obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
obj.pushKV("service", GetDeprecatedServiceField(*this));
}
obj.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
obj.pushKV("pubKeyOperator", pubKeyOperator.ToString());
Expand Down
16 changes: 14 additions & 2 deletions test/functional/rpc_deprecated.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

class DeprecatedRpcTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
self.num_nodes = 3
self.setup_clean_chain = True
self.extra_args = [[], ["-deprecatedrpc=permissive_bool"]]
self.extra_args = [[], ["-deprecatedrpc=permissive_bool"], ["-deprecatedrpc=service"]]

def run_test(self):
# This test should be used to verify correct behaviour of deprecated
Expand All @@ -25,6 +25,7 @@ def run_test(self):
# self.generate(self.nodes[1], 1)
# self.log.info("No tested deprecated RPC methods")
self.test_permissive_bool()
self.test_malformed_deprecated_service_rawtransactions()

def test_permissive_bool(self):
self.log.info("Test -deprecatedrpc=permissive_bool")
Expand Down Expand Up @@ -58,5 +59,16 @@ def test_permissive_bool(self):
assert_raises_rpc_error(-8, "detailed must be true, false, yes, no, 1 or 0 (not '2')", self.nodes[1].protx, "list", "valid", 2)
assert_raises_rpc_error(-8, "detailed must be true, false, yes, no, 1 or 0 (not 'notabool')", self.nodes[1].protx, "list", "valid", "notabool")

def test_malformed_deprecated_service_rawtransactions(self):
self.log.info("Test malformed provider payloads do not crash -deprecatedrpc=service")
node = self.nodes[2]
block_count = node.getblockcount()
for tx_hex in [
"03000100000000000000020000", # ProRegTx with only nVersion=0 payload
"03000200000000000000020000", # ProUpServTx with only nVersion=0 payload

@thepastaclaw thepastaclaw Jun 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

💬 Nitpick: Brittle assertion couples test to helper parameter name

The substring "Internal bug detected: obj.netInfo" binds the test to the CHECK_NONFATAL stringification of the condition, which depends on the helper's parameter being named obj in src/evo/core_write.cpp. Renaming the parameter (e.g. to state or tx) would break this assertion for reasons unrelated to the bug being guarded. Matching on "Internal bug detected" alone would still confirm the non-fatal path was taken without binding to internal naming.

source: ['claude']

]:
assert_raises_rpc_error(-1, "Internal bug detected: obj.netInfo", node.decoderawtransaction, tx_hex)
assert_equal(node.getblockcount(), block_count)

if __name__ == '__main__':
DeprecatedRpcTest().main()
Loading