-
Notifications
You must be signed in to change notification settings - Fork 37.7k
refactor: replace util::Ref with std::any (C++17) #21366
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
refactor: replace util::Ref with std::any (C++17) #21366
Conversation
Concept ACK on removing no-longer needed abstraction. Currently failing to compile: CXX wallet/libbitcoin_wallet_a-coincontrol.o
In file included from /usr/include/c++/9/bits/move.h:55,
from /usr/include/c++/9/bits/nested_exception.h:40,
from /usr/include/c++/9/exception:144,
from /usr/include/c++/9/new:40,
from /usr/include/c++/9/any:37,
from ./rpc/request.h:9,
from ./rpc/server.h:10,
from rpc/server.cpp:6:
/usr/include/c++/9/type_traits: In instantiation of ‘struct std::__and_<std::is_copy_constructible<JSONRPCRequest>, std::__not_<std::is_constructible<JSONRPCRequest, const JSONRPCRequest&> >, std::__not_<std::__is_in_place_type<JSONRPCRequest> > >’:
/usr/include/c++/9/type_traits:150:27: required from ‘constexpr const bool std::__and_v<std::is_copy_constructible<JSONRPCRequest>, std::__not_<std::is_constructible<JSONRPCRequest, const JSONRPCRequest&> >, std::__not_<std::__is_in_place_type<JSONRPCRequest> > >’
/usr/include/c++/9/any:192:27: required by substitution of ‘template<class _ValueType, class _Tp, class _Mgr, typename std::enable_if<__and_v<std::is_copy_constructible<_Tp>, std::__not_<std::is_constructible<_Tp, _ValueType&&> >, std::__not_<std::__is_in_place_type<_Tp> > >, bool>::type <anonymous> > std::any::any(_ValueType&&) [with _ValueType = const JSONRPCRequest&; _Tp = JSONRPCRequest; _Mgr = std::any::_Manager_external<JSONRPCRequest>; typename std::enable_if<__and_v<std::is_copy_constructible<_Tp>, std::__not_<std::is_constructible<_Tp, _ValueType&&> >, std::__not_<std::__is_in_place_type<_Tp> > >, bool>::type <anonymous> = <missing>]’
/usr/include/c++/9/type_traits:883:12: required from ‘struct std::is_constructible<JSONRPCRequest, const JSONRPCRequest&>’
/usr/include/c++/9/type_traits:901:12: required from ‘struct std::__is_copy_constructible_impl<JSONRPCRequest, true>’
/usr/include/c++/9/type_traits:907:12: required from ‘struct std::is_copy_constructible<JSONRPCRequest>’
/usr/include/c++/9/type_traits:131:12: required from ‘struct std::__and_<std::is_copy_constructible<JSONRPCRequest>, std::is_constructible<JSONRPCRequest, const JSONRPCRequest&> >’
/usr/include/c++/9/any:181:58: required by substitution of ‘template<class _ValueType, class _Tp, class _Mgr, typename std::enable_if<std::__and_<std::is_copy_constructible<_Tp>, std::is_constructible<_Tp, _ValueType&&> >::value, bool>::type <anonymous>, typename std::enable_if<(! std::__is_in_place_type<_Tp>::value), bool>::type <anonymous> > std::any::any(_ValueType&&) [with _ValueType = const JSONRPCRequest&; _Tp = JSONRPCRequest; _Mgr = std::any::_Manager_external<JSONRPCRequest>; typename std::enable_if<std::__and_<std::is_copy_constructible<_Tp>, std::is_constructible<_Tp, _ValueType&&> >::value, bool>::type <anonymous> = <missing>; typename std::enable_if<(! std::__is_in_place_type<_Tp>::value), bool>::type <anonymous> = <missing>]’
rpc/server.cpp:91:32: required from here
/usr/include/c++/9/type_traits:136:12: error: incomplete type ‘std::is_copy_constructible<JSONRPCRequest>’ used in nested name specifier
136 | struct __and_<_B1, _B2, _B3, _Bn...>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/9/type_traits: In instantiation of ‘constexpr const bool std::__and_v<std::is_copy_constructible<JSONRPCRequest>, std::__not_<std::is_constructible<JSONRPCRequest, const JSONRPCRequest&> >, std::__not_<std::__is_in_place_type<JSONRPCRequest> > >’:
/usr/include/c++/9/any:192:27: required by substitution of ‘template<class _ValueType, class _Tp, class _Mgr, typename std::enable_if<__and_v<std::is_copy_constructible<_Tp>, std::__not_<std::is_constructible<_Tp, _ValueType&&> >, std::__not_<std::__is_in_place_type<_Tp> > >, bool>::type <anonymous> > std::any::any(_ValueType&&) [with _ValueType = const JSONRPCRequest&; _Tp = JSONRPCRequest; _Mgr = std::any::_Manager_external<JSONRPCRequest>; typename std::enable_if<__and_v<std::is_copy_constructible<_Tp>, std::__not_<std::is_constructible<_Tp, _ValueType&&> >, std::__not_<std::__is_in_place_type<_Tp> > >, bool>::type <anonymous> = <missing>]’
/usr/include/c++/9/type_traits:883:12: required from ‘struct std::is_constructible<JSONRPCRequest, const JSONRPCRequest&>’
/usr/include/c++/9/type_traits:901:12: required from ‘struct std::__is_copy_constructible_impl<JSONRPCRequest, true>’
/usr/include/c++/9/type_traits:907:12: required from ‘struct std::is_copy_constructible<JSONRPCRequest>’
/usr/include/c++/9/type_traits:131:12: required from ‘struct std::__and_<std::is_copy_constructible<JSONRPCRequest>, std::is_constructible<JSONRPCRequest, const JSONRPCRequest&> >’
/usr/include/c++/9/any:181:58: required by substitution of ‘template<class _ValueType, class _Tp, class _Mgr, typename std::enable_if<std::__and_<std::is_copy_constructible<_Tp>, std::is_constructible<_Tp, _ValueType&&> >::value, bool>::type <anonymous>, typename std::enable_if<(! std::__is_in_place_type<_Tp>::value), bool>::type <anonymous> > std::any::any(_ValueType&&) [with _ValueType = const JSONRPCRequest&; _Tp = JSONRPCRequest; _Mgr = std::any::_Manager_external<JSONRPCRequest>; typename std::enable_if<std::__and_<std::is_copy_constructible<_Tp>, std::is_constructible<_Tp, _ValueType&&> >::value, bool>::type <anonymous> = <missing>; typename std::enable_if<(! std::__is_in_place_type<_Tp>::value), bool>::type <anonymous> = <missing>]’
rpc/server.cpp:91:32: required from here
/usr/include/c++/9/type_traits:150:27: error: ‘value’ is not a member of ‘std::__and_<std::is_copy_constructible<JSONRPCRequest>, std::__not_<std::is_constructible<JSONRPCRequest, const JSONRPCRequest&> >, std::__not_<std::__is_in_place_type<JSONRPCRequest> > >’
150 | inline constexpr bool __and_v = __and_<_Bn...>::value;
| ^~~~~~~
make[2]: *** [Makefile:8807: rpc/libbitcoin_server_a-server.o] Error 1 |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
1ec2ebd
to
4cab1de
Compare
Meh, there seems to be a problem with the (default) copy-constructor of
No idea why this compiles locally with clang11 and obviously also for the CI jobs for MacOS and ARM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this followup!
9309094
to
6595378
Compare
Force-pushed with suggestions by @ryanofsky (#21366 (comment)): introduced a helper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review almost-ack 6595378. JSONRPCRequest std::any constructor isn't as safe as it could be because it can take precedence over the copy constructor
6595378
to
003e76c
Compare
Another batch of simplifications you could consider. There's no need really to require request objects to have contexts assigned. diff
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index 87c63593051..7d246325e44 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -131,7 +131,7 @@ static bool AppInit(int argc, char* argv[])
// If locking the data directory failed, exit immediately
return false;
}
- fRet = AppInitInterfaces(node) && AppInitMain(context, node);
+ fRet = AppInitInterfaces(node) && AppInitMain(node);
}
catch (const std::exception& e) {
PrintExceptionContinue(&e, "AppInit()");
diff --git a/src/httprpc.cpp b/src/httprpc.cpp
index d33e671ee42..cd45eccc4ed 100644
--- a/src/httprpc.cpp
+++ b/src/httprpc.cpp
@@ -159,7 +159,8 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
return false;
}
- JSONRPCRequest jreq(context);
+ JSONRPCRequest jreq;
+ jreq.context = context;
jreq.peerAddr = req->GetPeer().ToString();
if (!RPCAuthorized(authHeader.second, jreq.authUser)) {
LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr);
@@ -294,7 +295,7 @@ bool StartHTTPRPC(const std::any& context)
if (!InitRPCAuthentication())
return false;
- auto handle_rpc = [&context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req); };
+ auto handle_rpc = [context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req); };
RegisterHTTPHandler("/", true, handle_rpc);
if (g_wallet_init_interface.HasWalletSupport()) {
RegisterHTTPHandler("/wallet/", false, handle_rpc);
diff --git a/src/init.cpp b/src/init.cpp
index efb92802126..c166af10f1f 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -785,7 +785,7 @@ static bool InitSanityCheck()
return true;
}
-static bool AppInitServers(const std::any& context, NodeContext& node)
+static bool AppInitServers(NodeContext& node)
{
const ArgsManager& args = *Assert(node.args);
RPCServer::OnStarted(&OnRPCStarted);
@@ -794,9 +794,9 @@ static bool AppInitServers(const std::any& context, NodeContext& node)
return false;
StartRPC();
node.rpc_interruption_point = RpcInterruptionPoint;
- if (!StartHTTPRPC(context))
+ if (!StartHTTPRPC(&node))
return false;
- if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(context);
+ if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(&node);
StartHTTPServer();
return true;
}
@@ -1274,7 +1274,7 @@ bool AppInitInterfaces(NodeContext& node)
return true;
}
-bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
+bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
{
const ArgsManager& args = *Assert(node.args);
const CChainParams& chainparams = Params();
@@ -1379,7 +1379,7 @@ bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAn
*/
if (args.GetBoolArg("-server", false)) {
uiInterface.InitMessage_connect(SetRPCWarmupStatus);
- if (!AppInitServers(context, node))
+ if (!AppInitServers(node))
return InitError(_("Unable to start HTTP server. See debug log for details."));
}
diff --git a/src/init.h b/src/init.h
index fe1c66202bf..d8fcb1e0ce3 100644
--- a/src/init.h
+++ b/src/init.h
@@ -59,7 +59,7 @@ bool AppInitInterfaces(NodeContext& node);
* @note This should only be done after daemonization. Call Shutdown() if this function fails.
* @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called.
*/
-bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
+bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
/**
* Register all arguments with the ArgsManager
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 8cf0dee5b4e..7d195688032 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -76,7 +76,7 @@ public:
}
bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override
{
- return AppInitMain(m_context_ref, *m_context, tip_info);
+ return AppInitMain(*m_context, tip_info);
}
void appShutdown() override
{
@@ -218,7 +218,8 @@ public:
CFeeRate getDustRelayFee() override { return ::dustRelayFee; }
UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override
{
- JSONRPCRequest req(m_context_ref);
+ JSONRPCRequest req;
+ req.context = m_context;
req.params = params;
req.strMethod = command;
req.URI = uri;
@@ -287,14 +288,8 @@ public:
void setContext(NodeContext* context) override
{
m_context = context;
- if (context) {
- m_context_ref = context;
- } else {
- m_context_ref.reset();
- }
}
NodeContext* m_context{nullptr};
- std::any m_context_ref;
};
bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock, const CChain& active)
diff --git a/src/rest.cpp b/src/rest.cpp
index 452435f3e92..e2af5128121 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -317,7 +317,8 @@ static bool rest_chaininfo(const std::any& context, HTTPRequest* req, const std:
switch (rf) {
case RetFormat::JSON: {
- JSONRPCRequest jsonRequest(context);
+ JSONRPCRequest jsonRequest;
+ jsonRequest.context = context;
jsonRequest.params = UniValue(UniValue::VARR);
UniValue chainInfoObject = getblockchaininfo().HandleRequest(jsonRequest);
std::string strJSON = chainInfoObject.write() + "\n";
@@ -687,7 +688,7 @@ static const struct {
void StartREST(const std::any& context)
{
for (const auto& up : uri_prefixes) {
- auto handler = [&context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); };
+ auto handler = [context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); };
RegisterHTTPHandler(up.prefix, false, handler);
}
}
diff --git a/src/rpc/request.h b/src/rpc/request.h
index 82d8fdc63ae..7a78d382cc9 100644
--- a/src/rpc/request.h
+++ b/src/rpc/request.h
@@ -35,18 +35,7 @@ public:
std::string URI;
std::string authUser;
std::string peerAddr;
- const std::any& context;
-
- explicit JSONRPCRequest(const std::any& context) : id(NullUniValue), params(NullUniValue), fHelp(false), context(context) {}
-
- //! Initializes request information from another request object and the
- //! given context. The implementation should be updated if any members are
- //! added or removed above.
- JSONRPCRequest(const JSONRPCRequest& other, const std::any& context)
- : id(other.id), strMethod(other.strMethod), params(other.params), fHelp(other.fHelp), URI(other.URI),
- authUser(other.authUser), peerAddr(other.peerAddr), context(context)
- {
- }
+ std::any context;
void parse(const UniValue& valRequest);
};
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index 3d5216055ce..f16adf96d3e 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -88,7 +88,7 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest&
vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front()));
sort(vCommands.begin(), vCommands.end());
- JSONRPCRequest jreq(&helpreq);
+ JSONRPCRequest jreq(helpreq);
jreq.fHelp = true;
jreq.params = UniValue();
diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
index 4ffc2478696..d2749c89599 100644
--- a/src/test/rpc_tests.cpp
+++ b/src/test/rpc_tests.cpp
@@ -33,8 +33,8 @@ UniValue RPCTestingSetup::CallRPC(std::string args)
boost::split(vArgs, args, boost::is_any_of(" \t"));
std::string strMethod = vArgs[0];
vArgs.erase(vArgs.begin());
- std::any context{&m_node};
- JSONRPCRequest request(context);
+ JSONRPCRequest request;
+ request.context = &m_node;
request.strMethod = strMethod;
request.params = RPCConvertValues(strMethod, vArgs);
request.fHelp = false;
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index c91ede94f95..46533d8704c 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -514,7 +514,9 @@ public:
{
for (const CRPCCommand& command : GetWalletRPCCommands()) {
m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) {
- return command.actor({request, &m_context}, result, last_handler);
+ JSONRPCRequest wrapped = request;
+ wrapped.context = &m_context;
+ return command.actor(wrapped, result, last_handler);
}, command.argNames, command.unique_id);
m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
}
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index e48f7b4d4f5..a9c20921abd 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -213,8 +213,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
key.pushKV("timestamp", newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1);
key.pushKV("internal", UniValue(true));
keys.push_back(key);
- std::any context;
- JSONRPCRequest request(context);
+ JSONRPCRequest request;
request.params.setArray();
request.params.push_back(keys);
@@ -265,8 +264,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
AddWallet(wallet);
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
}
- std::any context;
- JSONRPCRequest request(context);
+ JSONRPCRequest request;
request.params.setArray();
request.params.push_back(backup_file);
@@ -281,8 +279,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
LOCK(wallet->cs_wallet);
wallet->SetupLegacyScriptPubKeyMan();
- std::any context;
- JSONRPCRequest request(context);
+ JSONRPCRequest request;
request.params.setArray();
request.params.push_back(backup_file);
AddWallet(wallet); |
003e76c
to
6b6b899
Compare
Force-pushed with feedback from @JeremyRubin (#21366 (comment)) and @ryanofsky (#21366 (comment)) taken into account:
Good idea. I think those changes are out of scope for this PR, as the proposed simplifications are not connected to the |
Concept ACK +89 −155 -- very nice! :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK 6b6b899. Looks good! Only change since last review is simplifying AnyPtr function.
6b6b899
to
cb31209
Compare
Rebased on master. |
3906949
to
916ab01
Compare
Rebased on master and took in all suggestions by @hebasto (#21366 (review)) -- thanks a lot for reviewing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK 916ab01. Changes since last review: rebase and replacing types with auto
. I might have used const auto*
and auto*
instead of plain auto
because I think the qualifiers are useful, but this is all good.
This bug doesn't have any effects currently because it only affects external signer RPCs which aren't currently using the wallet context, but it does cause an appveyor failure in a upcoming PR: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882 This bug is subtle and could have been avoided if JSONRPCRequest didn't have constructors that were so loose with type checking. Suggested change bitcoin#21366 (comment) eliminates these and would be a good followup for a future PR.
This just makes an additional simplification after bitcoin#21366 replaced util::Ref with std::any. It was originally suggested bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin#21572.
This bug doesn't have any effects currently because it only affects external signer RPCs which aren't currently using the wallet context, but it does cause an appveyor failure in a upcoming PR: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882 This bug is subtle and could have been avoided if JSONRPCRequest didn't have constructors that were so loose with type checking. Suggested change bitcoin#21366 (comment) eliminates these and would be a good followup for a future PR.
This just makes an additional simplification after bitcoin#21366 replaced util::Ref with std::any. It was originally suggested bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin#21572.
937fd4a Fix wrong wallet RPC context set after #21366 (Russell Yanofsky) Pull request description: This bug doesn't have any effects currently because it only affects external signer RPCs which aren't currently using the wallet context, but it does cause an appveyor failure in a upcoming PR: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882 This bug is subtle and could have been avoided if JSONRPCRequest didn't have constructors that were so loose with type checking. Suggested change bitcoin/bitcoin#21366 (comment) eliminates these and would be a good followup for a future PR. This PR just implements the simplest possible fix. ACKs for top commit: theStack: Code-review ACK 937fd4a meshcollider: Code review ACK 937fd4a Tree-SHA512: 53e6265ed6c7abb47d2b3e77d1604edfeb993c3a2440f0c19679cfeb23516965e6707ff486196a0acfbeff21c79a9a08b5cd33bae9a232d33d0134bca1bd0ff3
This just makes an additional simplification after bitcoin#21366 replaced util::Ref with std::any. It was originally suggested bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin#21572.
This just makes an additional simplification after bitcoin#21366 replaced util::Ref with std::any. It was originally suggested bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin#21572.
…1366 937fd4a Fix wrong wallet RPC context set after bitcoin#21366 (Russell Yanofsky) Pull request description: This bug doesn't have any effects currently because it only affects external signer RPCs which aren't currently using the wallet context, but it does cause an appveyor failure in a upcoming PR: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882 This bug is subtle and could have been avoided if JSONRPCRequest didn't have constructors that were so loose with type checking. Suggested change bitcoin#21366 (comment) eliminates these and would be a good followup for a future PR. This PR just implements the simplest possible fix. ACKs for top commit: theStack: Code-review ACK 937fd4a meshcollider: Code review ACK 937fd4a Tree-SHA512: 53e6265ed6c7abb47d2b3e77d1604edfeb993c3a2440f0c19679cfeb23516965e6707ff486196a0acfbeff21c79a9a08b5cd33bae9a232d33d0134bca1bd0ff3
9044522 Drop JSONRPCRequest constructors after #21366 (Russell Yanofsky) Pull request description: This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested bitcoin/bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin/bitcoin#21572. ACKs for top commit: promag: ACK 9044522, fixed conflict in src/wallet/interfaces.cpp. Tree-SHA512: e909411b8f75013620b94e1a609296befb832fdcb574cd2e6689bfe3c636b03cd4ac1ccb2b32b532daf0f2131bb043464024966310fffc7e3cad77713d4bd0ef
…1366 9044522 Drop JSONRPCRequest constructors after bitcoin#21366 (Russell Yanofsky) Pull request description: This just makes an additional simplification after bitcoin#21366 replaced util::Ref with std::any. It was originally suggested bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin#21572. ACKs for top commit: promag: ACK 9044522, fixed conflict in src/wallet/interfaces.cpp. Tree-SHA512: e909411b8f75013620b94e1a609296befb832fdcb574cd2e6689bfe3c636b03cd4ac1ccb2b32b532daf0f2131bb043464024966310fffc7e3cad77713d4bd0ef
This bug doesn't have any effects currently because it only affects external signer RPCs which aren't currently using the wallet context, but it does cause an appveyor failure in a upcoming PR: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882 This bug is subtle and could have been avoided if JSONRPCRequest didn't have constructors that were so loose with type checking. Suggested change bitcoin#21366 (comment) eliminates these and would be a good followup for a future PR.
This just makes an additional simplification after bitcoin#21366 replaced util::Ref with std::any. It was originally suggested bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin#21572.
This change is required since bitcoin/bitcoin#21366 was merged.
As described in
util/ref.h
: "This implements a small subset of the functionality in C++17's std::any class, and can be dropped when the project updates to C++17". For accessing the contained object of astd::any
instance, a helper template functionAnyPtr
is introduced (thanks to ryanofsky).