multi: Add getmixpoolinfo RPC.#3720
Conversation
davecgh
left a comment
There was a problem hiding this comment.
I haven't really looked at this much yet, but at a first glance, I noticed this removing a type from the public rpc/jsonrpc/types module. We can't just remove types from the module without a major API version version bump because that would break semver.
We could, of course, go through the whole process or creating a new major module version, but it doesn't seem worth it to me for now. I would just deprecate it to be removed whenever a new major version is done.
|
There is already a very clear warning that it will likely be removed in the next version which is as good as a deprecation notice IMO, it just doesn't use the exact word "deprecated". I actually intended to do the version bump in this PR but forgot. I'm still happy to add it in if you agree? Thinking more about this PR I'd actually like to change |
I'm fine with that. Please just make sure to follow the major version bumping process. Example for the rpc/jsonprc/types/v4 bump: #2910. You can do it in this same PR if you really want, but I'd really prefer doing it separately as the linked PR does and then rebase this one on top. The reason I prefer it separate is because it makes it much easier and cleaner to identify exactly when major changes happened in history instead of being hidden behind a PR with some other name. I have often had to go back and find such changes and it's kind of annoying when they're hidden.
I go back and forth on that topic a bit. In a perfect world, I would ideally prefer that all RPCs were 100% consistent in their usage of amounts, but they aren't, and we really can't change existing ones now because that would certainly break all kinds of callers, even with a major version bump, because it would be the type of nearly invisible change that has huge consequences. With consistency in mind, I looked through the code to make a quick summary of which RPCs work with atoms and which DCR. I wasn't being super thorough since the point was just to get a general idea. It looks like there are only 3 that return/accept atoms:
But, perhaps more importantly, (almost?) all of the RPCs that involve reporting transaction details (e.g. So, given that, I think reporting them in DCR for this RPC makes the most sense. As a bit of a related aside, I personally generally prefer atoms myself in RPCs that are intended to be used programmatically because they don't have any of the myriad floating point issues, however, I find consistency to be far more important, and most of the RPCs end up doubling as being human-readable output, so DCR makes sense. |
c274577 to
64b534d
Compare
|
Now includes proper go module bumps and changed atoms to DCR |
This bumps the major version of the JSON RPC server in anticipation of upcoming changes.
The GetMixPairRequests RPC, which was already explicitly flagged experimental/do-not-use, simply returned a raw hex-dump of pair requests, which was at odds with the rest of the RPC calls which typically return nicely structured JSON.
davecgh
left a comment
There was a problem hiding this comment.
This looks good to me overall. I appreciate the attention to detail and making sure the documentation and tests are updated as well. That is often something that gets missed.
I saw a minor typo in the PR description: "Ths removes".
Comments inline.
| if !ok { | ||
| group = &types.Pairing{ | ||
| MixAmount: dcrutil.Amount(pr.MixAmount).ToCoin(), | ||
| ScriptClass: pr.ScriptClass, |
There was a problem hiding this comment.
I would personally rather see this, at the very least, match types.Vin.ScriptSig so it has the actual script hex and decoded asm. Particularly because, and I hadn't noticed this until now, the string the wire message uses is inconsistent with the naming used by stdscript and isn't really descriptive enough either. P2PKH-secp256k1-v0 doesn't tell you one of the most important parts -- the signature scheme (ecdsa). Notice stdscript calls them p2pkh-ecdsa-secp256k1 and p2pkh-schnorr-secp256k1 specifically to differentiate.
More generally, I think maybe even the entire types.Vin struct would make sense for each utxo. There are some cases that would obviously not apply (e.g. coinbase, stakebase, treasurybase, treasuryspend), but it has a bunch of details about the referenced utxo like its transaction hash, output index, tree, amountin, blockheight, blockindex, scriptSig.asm, and scriptSig.hex.
That would also make it more consistent with getrawtransaction, decoderawtransaction, etc.
There was a problem hiding this comment.
This might be uncovering a wider bug because that string representation is not only used in the wire message but also in the compatibility matching (CompatiblePRs matches using Pairing).
Given that the Pairing.ScriptClass field of the new RPC response is intended to describe the values used for compatibility matching, and not the individual UTXOs necessarily, I believe it is correct as-is.
In order to give more detail to the returned UTXOs we could include the values from wire.MixPairReqUTXO quite easily?
This adds a new RPC named GetMixpoolInfo which returns the current state of the mixpool including timing of the next mix epoch and pending pair requests. PRs are grouped by mixing compatibility, i.e. if they share a common MixAmount, ScriptClass, TxVersion, LockTime, and PairingFlags. Available to limited users because it only retrieves public information and performs no sensitive actions.
Rebased on #3726
This removes the
GetMixPairRequestsRPC, which was already explicitly flagged experimental/do-not-use and simply returned a raw hex-dump of pair requests. This was at odds with the rest of the RPC calls which typically return nicely structured JSON.In its place a new RPC named
GetMixpoolInfois added. It returns the current state of the mixpool including timing of the next mix epoch and pending pair requests. PRs are grouped by mixing compatibility, i.e. if they share a common MixAmount, ScriptClass, TxVersion, LockTime, and PairingFlags.The new RPC is available to limited users because it only retrieves public information and performs no sensitive actions.
Here is an example result.
{ "epoch": 600, "nextepoch": 1783066800, "pairings": [ { "mixamount": 26599170196, "scriptclass": "P2PKH-secp256k1-v0", "txversion": 1, "locktime": 0, "pairingflags": 1, "pairrequests": [ { "identity": "03037f2bc0ae3764191be26f8cfb674724a5a743aaa67223e339999d29db517e87", "messagecount": 1, "inputvalue": 27229421568, "utxos": [ "7a7c50f5fdff3f82dc71313011d167c5db0af9387d19a82bd28f745426ba5de0:9", "7a7c50f5fdff3f82dc71313011d167c5db0af9387d19a82bd28f745426ba5de0:4", "7a7c50f5fdff3f82dc71313011d167c5db0af9387d19a82bd28f745426ba5de0:8", "e2c0936308c74732b7248248450fa28712ccf2fa33e844f4e394c7e63522331c:10", "02079bbad50bb313fcf520a4d6ada4444dd43b2d9bac6aad68e96ddd0a753877:24", "1643f406860f748501a3c788df580d27870cfa233ac1b9ff252115c15121b511:12", "7a7c50f5fdff3f82dc71313011d167c5db0af9387d19a82bd28f745426ba5de0:11", "9aead787dd5b54332e38d6ae6c852dc83462b5dabd58c14ba5cc020f4f008fef:8", "b2a7b68a57319d9c2da9c9ec4705eeca3ff25ff4e370dcfbf5b5cf4f398ca909:6", "9aead787dd5b54332e38d6ae6c852dc83462b5dabd58c14ba5cc020f4f008fef:4", "7a7c50f5fdff3f82dc71313011d167c5db0af9387d19a82bd28f745426ba5de0:12", "7a7c50f5fdff3f82dc71313011d167c5db0af9387d19a82bd28f745426ba5de0:7" ], "expiry": 1094127, "hash": "bd37f484dbf5b3566bd22806f5de486623faf4c31f5db1103286a4b03d8b1352" }, { "identity": "03780f4652c2669b5ea38a24ef7335a887f825b788c044d31207e25602239d49a6", "messagecount": 1, "inputvalue": 36325294080, "utxos": [ "7ad3ca85f88fc8f85066ad240f5d210366e3056f94ac8f4b9e6538b24297c95a:22", "c2b0a446e1fd6bc2394ed66cbf0456b42dfc95def1cb9c7e22ae21ce0034cb62:7", "679c24e8c8d8b794071c06a884d30f5c74e3bea4693f844104a7e71cb08e9c40:3", "829ca5376b94e8074b47789487be0e525dedb94b89c2b43edf55d2a9610c04ce:6", "829ca5376b94e8074b47789487be0e525dedb94b89c2b43edf55d2a9610c04ce:14", "ba24758a4b1fb6eab6c6e39ea10e9b062dd2931247d02bbc2836536cfb5d9313:16", "93c085c4f91bb245d6a23fff7f9504b17b23d14775091685cf05c92cec4ac20a:21", "0a3ae022430afccf46a7f3cf85e4035edfa469407aa18330071851de69dce8ab:7", "787a0171b10877df1fd80601670f7c8b9ec9a2f77ce00d32cbfe9a92be5fb4ea:4", "7ad3ca85f88fc8f85066ad240f5d210366e3056f94ac8f4b9e6538b24297c95a:19", "9aead787dd5b54332e38d6ae6c852dc83462b5dabd58c14ba5cc020f4f008fef:5", "e518ea6b2687233db21469ad885ba7e4fd747a4cd030b9e45e2f89694b4ca29c:6", "7ad3ca85f88fc8f85066ad240f5d210366e3056f94ac8f4b9e6538b24297c95a:7", "7ad3ca85f88fc8f85066ad240f5d210366e3056f94ac8f4b9e6538b24297c95a:14", "787a0171b10877df1fd80601670f7c8b9ec9a2f77ce00d32cbfe9a92be5fb4ea:6" ], "expiry": 1094124, "hash": "6a3e564e54d832bc59fb025055956ca359e122be4368b003ab63cb46e72b8c96" } ] }, { "mixamount": 16777216, "scriptclass": "P2PKH-secp256k1-v0", "txversion": 1, "locktime": 0, "pairingflags": 2, "pairrequests": [ { "identity": "03c293e4df897b83df611c4115b4ad479ff66672c4634f08052791502f1b17f77c", "messagecount": 2, "inputvalue": 47962348, "utxos": [ "410a3340232bc50bf147a51c52fa1506a9046aae633ee66033e1e951271e9bc3:1" ], "expiry": 1094130, "hash": "851ea5d33bb875c5a8d973541c6598e00dd2b1802ad774248000a641e5544e71" } ] } ] }