Implement Protocol v1.6 #305
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…oind responses Added a new method, mempoool.get_info which is just the result of `getmempoolinfo` on the bitcoind side. We cache this response for 5 seconds. Added a generic mechanism to cache bitcoind responses for calls that do not need to be super up-to-date and which can be cached for a time. The cache is owned by BitcoinDMgr and is cleared when re-connecting to bticoind. It auto-cleans itself when entries get stale.
Optional second arg, is a string. Gets forwarded to bitcoind verbatim so it's whatever bitcoind supports (e.g. "conservative" or "economical"). Also in this commit: Redid some of the auto-detection logic to determine what RPCs the remote daemon supports.
Let's do what ElectrumX is doing and not return the full dictionary from `getmempoolinfo` and instead restrict it to a subset of useful keys to preserve privacy.
cculianu
force-pushed
the
protocol_1_6
branch
from
861ff74 to
12156e7
Compare
Having it be too large may confuse clients especially after a block is confirmed. 250msec should be enough to rate-limit spam but also provide reasonable responsiveness to wallets that care about this stat.
…coinDMgr Also added detection for the new `submitpackages` RPC (bitcoin core >= 28.0.0). Not yet used but the flag is there.
Only change between Core 27 and Core 28 is that 28 has 2 new optional args... otherwise nearly identical semantics. Allow submitpackage as early as on Core 27.
Apparently Core 27.0.0 has it but it's largely not useful. According to ElectrumX devs. See: https://github.com/spesmilo/electrum-protocol/pull/6/files#r2459860616
BTC Core v28.0.0 or above only. As per Protocol v1.6 spec.
Only if the server supports it (currently only Bitcoin Core >= v28.0.0)
And does the unexpected thing if appending a list to a list.. :/
Paranoia just in case an exception is somehow thrown before we hand the job off to the threadpool.
We validate args in an asynch thread since that is potentially costly (we must hash the first txn, etc). When that's done, we forward request to bitcoind. This required us allowing generic_do_async to accept an optional "completion" function. In order to make things cleaner and faster we templatized ServerBase::generic_do_async, and redid its internals slightly. Hopefully this is cleaner and also hopefully redundant copies of e.g. std::function are avoided with this template implementation. Also in this commit: Various nits and fixups in Util.h, added ThrowInternalErrorIf() macro, and other misc. cleanup.
…t txn, not the first
…submitpackage Also in this commit: - Updated README.md to talk about v1.6 protocol compatibility and need for v28.0.0 or above of Core/Knots - Updated RPC error message saying Bitcoin Core or Bitcoin Knots (not just Bitcoin Core) >= v28.0.0 is ok for submitpackage.
…ersion They just end up in the ether but to do to debug log if debug logging is enabled.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implementation of protocol v1.6 as discussed here: spesmilo/electrum-protocol#6 .
Summary of protocol changes:
blockchain.estimatefeechanges (added optionalmode2nd parameter)mempool.get_infoblockchain.transaction.broadcast_packageserver.versionallow extra args, but ignore themblockchain.block.headerschanged to return a list, rather than a concatenated hex blobAlso did some internal changes:
struct BitcoinDInfo