'getmininginfo' method #721

pull luke-jr wants to merge 2 commits into bitcoin:master from luke-jr:getmininginfo changing 3 files +42 −3

luke-jr commented at 4:25 AM on December 21, 2011: member

This supercedes the first half of pull #556 (the second half was already merged in pull #695), moving the new information (along with other mining-related information) to a new 'getmininginfo' JSON-RPC method (as requested by Gavin). The old information is still in 'getinfo' for compatibility, but is noted as deprecated.
in src/main.h:None in ddb2de4175 outdated
```
  62 | @@ -63,7 +63,10 @@
  63 |  extern CBigNum bnBestInvalidWork;
  64 |  extern uint256 hashBestChain;
  65 |  extern CBlockIndex* pindexBest;
  66 | +extern uint64 nPooledTx;
```
gavinandresen commented at 7:46 PM on December 21, 2011:

I think we should start moving away from the Just Add Another Global Variable style of coding.

How about: nPooled as a static member of CTransaction, with a GetNumPooled() public method.

And instead of nLastBlockTx/Size, maybe hoist vNewBlock up out of the getwork() method and return vNewBlock.last()->ntransactions/size...

luke-jr commented at 7:59 PM on December 21, 2011:

Doesn't the fact that we even have the transaction (and it's not in a block) imply that it's pooled? Correct me if I'm wrong, but I suspect your ideas for alternatives are inherently much slower, aren't they? If we want to move away from global variables, I think the "proper" route is to make an object responsible for managing the mining bits. But right now, all the mining stuff is global...
gavinandresen commented at 4:57 PM on December 23, 2011: contributor

Is this really necessary?

All of the extra information (and more, I believe) is returned in some form or another from the existing "getmemorypool" command.
luke-jr commented at 4:59 PM on December 23, 2011: member

Maybe if one wants to do the math externally, making a number of assumptions on data not included... "getmemorypool" also has a lot of overhead, while this has basically none.
jgarzik commented at 4:44 PM on January 6, 2012: contributor

Initial reaction: IMO not needed / overkill. It's not the end of the world if we just add a few new fields to 'getinfo', if it is simply additional information you desire. It is not as if getinfo is somehow resource constrained.

However, after thinking a bit more, I think it would be nice to (1) add this getmininginfo, while (2) deleting (not simply deprecating) the mining info in getinfo.

While odd people (alt/test chain users often) continue to find some utility in the internal reference miner, it should be safe to simply move that stuff out of getinfo. If nothing else, that will cut down on people confused about why "hashespersec" or "generate" shows zero when they are mining. Let's clean it up, and not worry about the odd duck who would see breakage.
sipa commented at 9:16 PM on January 12, 2012: member

Agree with @jgarzik here: let's clean up getinfo, and create a specific getmininginfo call for those who are interested.

Over what time frame can we remove fields from getinfo?
jgarzik commented at 12:43 AM on January 13, 2012: contributor

The commit should remove gen/gpl/hps from getinfo, and create getmininginfo, at the same time IMO.

I would ACK that for immediate timeframe.
Add new "getmininginfo" JSON-RPC method, with mining-only fields moved out of "getinfo" 6950bb6200
collect more info on tx pooling and block finding for getmininginfo 340f0876ea
luke-jr commented at 12:52 AM on January 13, 2012: member

Like this?
gavinandresen merged this on Jan 13, 2012
gavinandresen closed this on Jan 13, 2012
jgarzik commented at 3:50 PM on January 13, 2012: contributor

looks great, thanks
destenson referenced this in commit 18afe88401 on Jun 26, 2016
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#556 "txinfo"#695 Add block_hash and block_index to transaction info