Also in this commit various nits and subtle bugs prevented.
Refactored the 'Stats' mechanism to return a generic QVariant rather
than QVariantMap, which is more flexible.
Misc. other nits.
The strings that act as jsonrpc keys are used a lot in the app -- using
eg varmap["error"] is slower than pre-creating a static QString, s_error = "error",
and then referring to it as varmap[s_error]. So we do that and it
should shave a few percentage points of cpu usage during heavy json rpc
use.
It's less code to maintain to do that, plus it works with all compilers
(I had trouble with std::variant on non-Apple-clang).
On top of that the QVariant approach likely is slightly faster as no
conversion needs to be done from our Message::id -> QVariant when it
*is* a QVariant already.
We sacrifice a tiny bit of type safety. We just have to be sure in the
code to never allow fractional doubles to go out as an id (this is
unlikely to happen anyway in practice).
And added some test code in Controller to use the scheme.
It's very asynchronous. Needs some tuning long term to see how it deals
with load and perhaps throttle the requests if we know bitcoind is under
load.
But for right now, for testing, it should work ok.
Also in this commit small nits, plus bugfix to RPC::Message factory
methods (I forgot to populate some instance properties when constructing
the objects).
Additionally, we were registering the qMetaType wrong for RPC::Message
and RPC::Message::Id (this has been fixed).
Forward delcared private classes whose implementation is opaque to
outside code confuse unique_ptr's auto-generated deleter function.
If you specify your own deleter, you can totally use unique_ptr with opaque
types.
Basically it's not called on app exit. We made it so it's not so crucial
it be called for cleanup.
Also added some preliminary stats to the http /stats endpoint for the
BitcoinD instances.
This is to prevent memory exhaustion attacks from peers and enforce the
MAX_BUFFER we already defined for the clss.
Also added the EXPECT macro for clang's __builtin_expect for some branch
prediction to make some unlikely checks we make not be terribly costly
on the processor.
It works! I still haven't figured out how to provoke the dreaded
"Workdepth exceeded" error .. but we pretty much drop the connection
anyway on anything too unexpected from bitcoind.
When the bitcoind manager class is written it will back off and try
again if too many disconnects happen.
For now though, the code we have is fast and works well against
bitcoind.
This is enabled with the 'v1' flag to various functions that operate on
the json data and validate it. Default is jsonrpc 2.0 but v1 flag can
be used to interop with e.g. bitcoind.
This allows it to talk to things like bitcoind. It still wants a tcp
socket as its transport -- but it can at least now be wrapped in HTTP
for bitcoin as well as re-use of the same code for ElectronX style
linefeed based JSON.
Our RPC lib is nowhere near 100% generic and re-usable but it's a good
first step here with this refactor job.
This should hopefully fully complete our implementation to support
JSON-RPC 2.0 without batching. We still are missing batching but at
least now we implement the non-batching portion of the spec more
completely.
It turns out server should always ignore notifications it wasn't
expecting and never respond to them.
What I was doing by returning errors was incorrect.
Instead we add to the error tally for unknown notifications and
eventually disconnect the client.
Also added preliminary support for param maps . . .
I restricted the possible underlying types it can contain down to
qint64, QString, and nullptr_t. This should make it more type safe since
we don't want it to inadvertently end up holding lists or maps
(previously it was a straight up QVariant).
It has a conversion to QVariant so it should work nicely.
- It made the code potentially slower
- Made modifying/adding methods too complex
- Wasn't very beneficial.
- Instead, we just verify JSON-RPC 2.0 spec in the core Connection
class, then we pass on the method call to the subclass if it's found in
the methodlist, etc.
- It's still a little more restrictive than the spec allows -- doesn't
like non-positive-int 'id'
- We not made the shuffle.spec status report back 'notifications' to the
client rather than repeated 'result' messages (as those would be out of
spec). The first response to a 'shuffle.spec' request is either
result="pending", result="accepted", or an error response
- Subsequent shuffle.spec status changes come as a method notification
for "shuffle.spec" with params=["accepted"] or params=["some error
message"]
Overall I think our bare bones JSON-RPC will at least not break now as
interop with other impls.
Now we can re-use the RPC method<->result code in the TcpServer side
which will face wallets. Phew! Took me long enough!
It's a minimal JSON-RPC protocol impl.. but it'll do.
The 1 nice bit is the Schema spec I came up with which more-or-less
works well enough as a first-pass validatior.
Further passes are needed in interested client code, but the initial
pass validation can be done in a thread so as to not waste the main
thread's time validating json or dict key presence/absense. Yay.
