[Qt] Add interactive mempool graph #8550

pull jonasschnelli wants to merge 2 commits into bitcoin:master from jonasschnelli:2016/08/stats_qt changing 19 files +883 −2
  1. jonasschnelli commented at 7:30 PM on August 19, 2016: contributor

    Includes #8501

    At the moment, the mempool graph is not very prominently placed (next to the debug window). <img width="351" alt="bildschirmfoto 2016-08-19 um 21 20 48" src="https://cloud.githubusercontent.com/assets/178464/17821624/e225da0e-6652-11e6-9322-55c918f47012.png">

    Features:

    • interactive graph with options for tx count, dynamic memory usage and minRelayFee/KB
    • collects stats in the background, changing the timespan will directly redraw (unlike our bandwidth graph)
    • Dynamic size drawing, window can be resized
    • The mempool graph is a QWidget which means, it could be placed together with other graphs in a combine multi-graph view (screensaver approach)

    <img width="592" alt="bildschirmfoto 2016-08-19 um 21 28 09" src="https://cloud.githubusercontent.com/assets/178464/17821824/ecb39b72-6653-11e6-87ab-b445556b306a.png">

  2. jonasschnelli added the label GUI on Aug 19, 2016
  3. jonasschnelli force-pushed on Aug 19, 2016
  4. jonasschnelli force-pushed on Aug 19, 2016
  5. MarcoFalke commented at 8:05 PM on August 19, 2016: member

    Concept ACK. Will test later.

  6. jonasschnelli force-pushed on Aug 19, 2016
  7. jonasschnelli commented at 8:39 PM on August 19, 2016: contributor

    Added the missing button PNGs now to git. This should fix the compile issue.

  8. luke-jr commented at 8:44 PM on August 19, 2016: member

    Nice. Some people might want to see actual byte-size, weights, etc; probably will overflow the one-line-per-item paradigm quickly - maybe have a dropdown box to add stuff?

  9. jonasschnelli commented at 8:49 PM on August 19, 2016: contributor

    Yes. Dropdown box could make sense. Adding a second graph below or on the right that just plots different data would probably also look good.

  10. MarcoFalke commented at 9:17 PM on August 19, 2016: member

    Nit: Fonts appear larger on my system. Otherwise this looks great!

    screenshot from 2016-08-19 23-15-51

  11. in src/qt/mempoolstats.cpp:None in 7650b315cc outdated 
     181 | +    // set the values into the overview labels
 182 | +    if (vSamples.size())
 183 | +    {
 184 | +        dynMemUsageValueItem->setPlainText(GUIUtil::formatBytes(vSamples.back().dynMemUsage));
 185 | +        txCountValueItem->setPlainText(QString::number(vSamples.back().txCount));
 186 | +        minFeeValueItem->setPlainText(QString::number(vSamples.back().minFeePerK));
    MarcoFalke commented at 9:19 PM on August 19, 2016:

    Nit: Missing value. I think this is in satoshis. What about using BTC?

  12. in src/qt/bitcoingui.cpp:None in 7650b315cc outdated 
     353 | @@ -351,6 +354,11 @@ void BitcoinGUI::createActions()
 354 |      // initially disable the debug window menu item
 355 |      openRPCConsoleAction->setEnabled(false);
 356 |  
 357 | +    showMempoolStatsAction = new QAction(platformStyle->TextColorIcon(":/icons/mempoolstats"), tr("&Mempool Statistics"), this);
    MarcoFalke commented at 7:18 AM on August 20, 2016:

    I can't find this icon

    jonasschnelli commented at 7:19 AM on August 20, 2016:

    Indeed,... this does not exists yet. Will create one.

    MarcoFalke commented at 7:20 AM on August 20, 2016:

    http://typicons.com/ has some chart icons ;)

    luke-jr commented at 8:33 AM on August 20, 2016:

    Please remember to keep in mind licensing. typicons are CC-BY-3.0, which is a non-free license.

    MarcoFalke commented at 8:41 AM on August 20, 2016:

    Huh, I recall he gave you permission to dual-license as MIT?

    dae0a89d4b66f08c83ccc8c20cf37521084b6257

    jonasschnelli commented at 8:41 AM on August 20, 2016:

    @luke-jr: keep in mind that we have permission from the author to distribute them under MIT: #6367 (comment)

    luke-jr commented at 7:00 PM on August 20, 2016:

    Ah, forgot about that. Thanks for the reminder.

  13. jonasschnelli force-pushed on Aug 20, 2016
  14. jonasschnelli commented at 8:20 AM on August 20, 2016: contributor

    Switched to "Arial" as font and added a auto-size-adjustment. Also added the "charts" icon for the menu.

  15. MarcoFalke commented at 8:27 AM on August 20, 2016: member

    Nit: run optimize-pngs and mention the icons in assets_attribution?

  16. jonasschnelli force-pushed on Aug 20, 2016
  17. jonasschnelli force-pushed on Aug 20, 2016
  18. jonasschnelli force-pushed on Aug 20, 2016
  19. jonasschnelli commented at 10:06 AM on August 20, 2016: contributor

    Optimized the pngs and added them to the assets attribution file.

  20. ghost commented at 11:26 AM on August 20, 2016: none

    Concept ACK.

  21. isle2983 commented at 11:21 PM on August 21, 2016: contributor

    Nit1: The graph window is always drawn on top of the main window, even when the main window is active. It makes it annoying to use the rest of the GUI while this is open.

    Nit2: The quantity labels for the Dynamic Memory Usage values on the left hand side of the graph are cramped and cut off some of the characters. See "500.00 KB" in the screenshot. It is like that for larger values once it gets into the 100MBs too.

    mempoolstats

  22. rebroad commented at 9:40 AM on August 24, 2016: contributor

    I'm not sure what I'm doing wrong but I've merged this and the graph is empty, and has been for an hour now... :-s i.e. Dynamic Memory Usage is stuck on 0 bytes, Amount of Txs also 0, MinRelayFee also zero.

  23. jonasschnelli commented at 9:41 AM on August 24, 2016: contributor

    @rebroad: what does getmempoolinfo tells you? Are you in sync on mainnet (or testnet)?

  24. rebroad commented at 3:46 PM on August 25, 2016: contributor

    @jonasschnelli problem solved - I had blocksonly=1 in the config file!! It's working really nicely now - great job!! I love how it can be resized and the period of time changed without the graph resetting (unlike the network traffic graph!).

    Also, ACK

  25. fanquake commented at 1:08 PM on August 26, 2016: member

    OS X screenshots: init more-than-10

  26. luke-jr commented at 4:23 AM on August 27, 2016: member

    Using images for basically-a-checkbox gives a non-native look on every system. It looks like we can just colour a QCheckBox?

  27. paveljanik commented at 10:13 AM on September 7, 2016: contributor

    Rebase needed.

    Concept ACK

    Agree about checkboxes...

  28. jonasschnelli force-pushed on Sep 9, 2016
  29. jonasschnelli commented at 7:04 AM on September 9, 2016: contributor

    Rebased on top of #8501

  30. in src/Makefile.qt.include:None in d0d5c7d60f outdated 
     236 | @@ -234,6 +237,11 @@ RES_ICONS = \
 237 |    qt/res/icons/bitcoin.ico \
 238 |    qt/res/icons/bitcoin_testnet.ico \
 239 |    qt/res/icons/bitcoin.png \
 240 | +  qt/res/icons/button_off.png \
 241 | +  qt/res/icons/button_on_blue.png \
    MarcoFalke commented at 11:04 AM on September 9, 2016:

    @jonasschnelli What do you think about using QCheckBox with setPallette() as suggested by @luke-jr ?

    jonasschnelli commented at 7:36 AM on September 15, 2016:

    I think Checkboxes are appropriate for now.

    But we should consider using non-standard UI elements to improve the look and feel. I know, developers hate this. :-)

    But have a look at #5896. I think people liked it because the look and feel was different then just "Arial 12 and standard UI elements".

    luke-jr commented at 9:16 AM on November 24, 2016:

    Non-standard UI elements ruin the look and feel...

    jonasschnelli commented at 10:23 AM on November 24, 2016:

    Not sure. That's probably a matter of taste. The thing why I'd like to keep the non-standard UI elements is, that I'm using a QGraphicScene at this point. Adding a QWidget (checkbox) at this point breaks the flexibility of reusing the QGraphicScene and draw it in any scale.

    I think the checkbox could be different then the one thats currently used. And it probably should be draw internally and not use a PNG.

    luke-jr commented at 9:28 PM on November 24, 2016:

    Because it's a matter of taste is precisely why it should use standard UI elements. The standard element should do all the drawing. If the user doesn't like their standard widgets, they can change them.

  31. in src/main.cpp:None in d0d5c7d60f outdated 
    2843 | @@ -2837,6 +2844,9 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams,
2844 |      for(unsigned int i=0; i < pblock->vtx.size(); i++)
2845 |          txChanged.push_back(std::make_tuple(pblock->vtx[i], pindexNew, i));
2846 |  
2847 | +    // update mempool stats
2848 | +    CStats::DefaultStats()->addMempoolSample(mempool.size(), mempool.DynamicMemoryUsage(), mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK());
    paveljanik commented at 7:33 AM on September 15, 2016:

    Can you please do something with this long line? It is there ~3 times and does a simple thing - worth a better method to add a sample?

    jonasschnelli commented at 7:37 AM on September 15, 2016:

    Hmm.. I think this line is reasonable? How would you improve it?

    paveljanik commented at 7:38 AM on September 15, 2016:

    Pass const ref. mempool only and do the rest in the method?

    jonasschnelli commented at 7:42 AM on September 15, 2016:

    I like the current layering concept. IMO its preferable that the stats.cpp module has no knowledge over the txmempool.h's classes, etc., I prefer to keep it as "dump" as possible.

  32. in src/qt/mempoolstats.cpp:None in d0d5c7d60f outdated 
     346 | +        dynMemUsageGridPath.lineTo(GRAPH_PADDING_LEFT+lX, bottom-maxheightG);
 347 | +
 348 | +        QGraphicsTextItem *item = scene->addText(drawTime.toString("HH:mm"), gridFont);
 349 | +        item->setPos(GRAPH_PADDING_LEFT+lX-(item->boundingRect().width()/2), bottom);
 350 | +        redrawItems.append(item);
 351 | +        qint64 step = secsTotal/amountOfLinesV;
    paveljanik commented at 7:46 AM on September 15, 2016:

    -Wshadow warning:

    qt/mempoolstats.cpp:351:16: warning: declaration shadows a local variable [-Wshadow]
        qint64 step = secsTotal/amountOfLinesV;
               ^
qt/mempoolstats.cpp:240:11: note: previous declaration is here
    qreal step = maxwidth/(double)vSamples.size();
          ^
1 warning generated.
  33. in src/stats/rpc_stats.cpp:None in d0d5c7d60f outdated 
      60 | +    //  --------------------- ------------------------  -----------------------  ----------
  61 | +        {"stats",             "getmempoolstats",        &getmempoolstats,        true},
  62 | +
  63 | +};
  64 | +
  65 | +void RegisterStatsRPCCommands(CRPCTable& tableRPC)
    paveljanik commented at 7:47 AM on September 15, 2016:

    -Wshadow warning:

    stats/rpc_stats.cpp:65:42: warning: declaration shadows a variable in the global namespace [-Wshadow]
void RegisterStatsRPCCommands(CRPCTable& tableRPC)
                                         ^
./rpc/server.h:172:18: note: previous declaration is here
extern CRPCTable tableRPC;
                 ^
1 warning generated.
  34. paveljanik commented at 9:12 AM on September 15, 2016: contributor

    I really like this!

    Suggestions:

    1. the menu item should read Mempool statistics (small s) to match other items in the menu.
    2. the window title should be the same (both title bar and top header - I think it is there only to balance the "legenda" on the right...) ;-)
    3. the graph itself in the window is not centered - it is a bit on the left side here.
    4. Esc should close the window.
  35. paveljanik commented at 10:22 AM on September 25, 2016: contributor

    Rebase needed.

    Hmm, github shows "Conflicting files"...

  36. rebroad commented at 7:11 AM on October 16, 2016: contributor

    I've been testing this under various window managers. On mwm the title of the mempool window says "bitcoin-qt". Also, it's not its own window like the debug window is, but a child of the main bitcoin-qt window, so it cannot be minimised on its own, and is minimised whenever the main window is minimised.

  37. jonasschnelli commented at 6:45 AM on October 17, 2016: contributor

    Thanks @rebroad. I have plans to completely overhaul this PR during this week.

  38. rebroad commented at 3:41 AM on October 22, 2016: contributor

    @jonasschnelli Needed a little rebasing... d7857e11b67db6342466b06016971a6343b29ddd

  39. laanwj commented at 11:54 AM on October 26, 2016: member

    Concept ACK, looks nice!

  40. fanquake commented at 7:12 AM on November 7, 2016: member

    ping @jonasschnelli Did you get to overhauling this PR?

  41. jonasschnelli force-pushed on Nov 11, 2016
  42. Victorsueca commented at 8:23 PM on November 13, 2016: none

    ACK 7a7d631 Tested on Windows x64 Nit: Some elements overlay with my system font size and default window size Maybe we should add this as a Debug window tab? GUI Screenshot

  43. fanquake commented at 9:12 AM on November 24, 2016: member

    Post overhaul OS X screenshots: screen shot screen shot 3 screen shot 5

  44. jonasschnelli force-pushed on Nov 25, 2016
  45. jonasschnelli commented at 10:36 AM on November 25, 2016: contributor

    Removed the custom checkbox icons. Thanks to QGraphicsProxyWidget, it's still flexible in Size.

  46. jonasschnelli force-pushed on Nov 25, 2016
  47. jonasschnelli commented at 10:40 AM on November 25, 2016: contributor

    This is how it looks now on OSX: <img width="592" alt="bildschirmfoto 2016-11-25 um 11 38 32" src="https://cloud.githubusercontent.com/assets/178464/20622607/f7ad273a-b303-11e6-8941-1ccd11cd604d.png">

  48. in contrib/debian/copyright:None in 251ee281a3 outdated 
      79 | @@ -79,6 +80,7 @@ Comment: Modifications of Stephan Hutchings Typicons
  80 |  
  81 |  Files: src/qt/res/icons/about.png
  82 |         src/qt/res/icons/bitcoin.*
  83 | +       src/qt/res/icons/button_*
    paveljanik commented at 1:53 PM on November 25, 2016:

    No button_* files now.

  49. paveljanik commented at 4:20 PM on November 25, 2016: contributor

    @jonasschnelli Please address my comments here: #8550 (comment)

  51. luke-jr commented at 7:33 AM on November 27, 2016: member

    I think the click-the-human-readable-timeframe was better, but I replaced it with a slider to match our bandwidth graph. Probably they should both be the same, whatever we go with (ie, replace the sliders together, in a subsequent PR).

  52. gmaxwell commented at 10:04 PM on December 4, 2016: contributor

    More graph things are cool. But I think this data is basically worthless to users, if someone goes and makes and gives you a zillion minrelayfee transactions that will never get confirmed ... the graph will read maximum, but nothing has really changed for the user or the network.

    A stacked chart of mempool bytes at different feerates would be much more interesting and useful to users like how sipa's chart displays the utxo set http://bitcoin.sipa.be/utxo_size.png as it would make it possible to tell things that will get confirmed (bottom stacks of the graph) from noise (higher stacks).

  53. rebroad commented at 1:06 AM on December 5, 2016: contributor

    @gmaxwell Showing memory utilization, especially in relation to minfeerelay is useful for anyone wanting to refine the algorithm for setting minfeerelay.

    I have set maxmempool to 144, i.e. 24 hours worth of 1MB blocks - in order to give me an indication of the minimum fee needed to keep a tx in the mempool for at least 24 hours - although not an indication for how much the fee needs to be to get it confirmed within 24 hours, admittedly.

  54. jonasschnelli commented at 1:10 PM on December 5, 2016: contributor

    I like the idea of the stacked chart. However I wanted to start with a simple graph to get in the fundamental stuff (stats core classes, drawing stuff). At the moment, the only insight into the mempool from the GUI you can get is the amount of transactions. IMO step after step.

  55. laanwj commented at 12:16 PM on December 8, 2016: member

    However I wanted to start with a simple graph to get in the fundamental stuff (stats core classes, drawing stuff).

    I tend to agree - one step at a time, scope creep before the initial merge will likely mean that this will never happen at all. (needs rebase)

  56. rebroad commented at 3:50 AM on December 11, 2016: contributor

    screenshot at 2016-12-11 10 34 09 @laanwj spot on re scope creep. In this sense, I'd say ACK to merging it now.

    The screenshot attached is a typical example of what it looks like after running for a week or so. One thing that becomes apparent is that the horizontal graph label points become useless - i.e. once it goes over 24 hours of duration, it would make sense to switch to date instead of time.

  57. in src/qt/optionsmodel.cpp:None in 251ee281a3 outdated 
     147 | @@ -148,6 +148,9 @@ void OptionsModel::Init(bool resetSettings)
 148 |      if (!SoftSetArg("-lang", settings.value("language").toString().toStdString()))
 149 |          addOverriddenOption("-lang");
 150 |  
 151 | +    // Enable statistics by default
 152 | +    SoftSetBoolArg("-statsenable", true);
    luke-jr commented at 9:07 AM on December 21, 2016:

    Doing it this way doesn't update -help...

  58. luke-jr referenced this in commit 71bfbf01d4 on Dec 21, 2016
  59. luke-jr referenced this in commit 67c1eb806e on Dec 21, 2016
  60. paveljanik commented at 9:42 PM on February 22, 2017: contributor

    Rebase needed.

    I really really would like to see this included in the GUI in the next version!

  61. luke-jr commented at 12:45 AM on February 23, 2017: member

    FWIW, rebased (at least part of the way) in my stats_rpc-0.14 and stats_qt-0.14 branches.

  62. jonasschnelli commented at 7:42 AM on February 23, 2017: contributor

    This is based on #8501 (core change with additional RPC command) which has to get right first. Another solution would be to not depend on a core statistics "module" and therefore collect the stats in a GUI container only. Maybe this would be the better first step.

  63. jtimon commented at 11:45 PM on February 23, 2017: contributor

    Concept ACK needs rebase

  64. ghost commented at 6:14 PM on March 8, 2017: none

    The graphics are good, the design does not forget Here you can see the different designs of graphs http://www.bittbox.com/showcase/30-flat-analytics-ui-dashboards

    I like this one

    https://dribbble.com/shots/1719845-Statistics-General-trends

  65. laanwj commented at 3:00 PM on June 6, 2017: member

    Seems the discussion here bled out.

    I'd say please review the code, and then get it merged. Visual details etc can be changed later. Real-time statistics is one reason why people like to run their own nodes.

  66. laanwj commented at 1:33 PM on June 28, 2017: member

    Needs rebase.

  67. Sjors commented at 2:47 PM on November 9, 2017: member

    Concept ACK.

    I can't compile this (log gist). I'll try again after the rebase.

    A stacked chart of mempool bytes at different feerates would be much more interesting and useful

    Agreed.

    It would be nice to have a little chart icon in the bottom right corner of the wallet (in a future PR).

    That said, I also agree with:

    Visual details etc can be changed later.

  68. jonasschnelli commented at 6:54 PM on November 9, 2017: contributor

    This is based on #8501 and the question is, should it be Qt only (no Qt free stats collector) or should we try to get #8501 done? (will rebase both PRs soon)

  69. jonasschnelli force-pushed on Nov 22, 2017
  70. jonasschnelli force-pushed on Nov 22, 2017
  71. Add mempool statistics collector 9e83a882ec
  72. [Qt] Add interactive mempool graph 63fb11652f
  73. jonasschnelli force-pushed on Nov 22, 2017
  74. jonasschnelli commented at 10:33 PM on November 22, 2017: contributor

    Rebased. It's currently not Qt4 compatible due to a lambda signal connect (travis reported / working on it).

  75. laanwj commented at 3:31 PM on November 23, 2017: member

    It's currently not Qt4 compatible due to a lambda signal connect (travis reported / working on it).

    If it's a lot of work, feel free to leave "nice but not critical" features such as this out when compiling with Qt4.

  76. PierreRochard commented at 12:53 AM on December 15, 2017: contributor

    ACK with Qt5

  77. jb55 changes_requested
  78. jb55 commented at 6:23 PM on December 29, 2017: member

    Hmm for some reason I'm getting this error on my system:

    qt/mempoolstats.cpp: In member function ‘void MempoolStats::drawChart()’:
qt/mempoolstats.cpp:183:42: error: ‘ceil’ was not declared in this scope
     maxValueSize = ceil(maxValueSize*0.11)*10; //use size steps of 10dip
                                          ^
qt/mempoolstats.cpp:256:108: error: ‘log10’ was not declared in this scope
     int64_t dynMemUsagelog10Val = pow(10.0f, floor(log10(maxDynMemUsage*paddingTopSizeFactor-minDynMemUsage)));
                                                                                                            ^
qt/mempoolstats.cpp:256:109: error: ‘floor’ was not declared in this scope
     int64_t dynMemUsagelog10Val = pow(10.0f, floor(log10(maxDynMemUsage*paddingTopSizeFactor-minDynMemUsage)));
                                                                                                             ^
qt/mempoolstats.cpp:256:110: error: ‘pow’ was not declared in this scope
     int64_t dynMemUsagelog10Val = pow(10.0f, floor(log10(maxDynMemUsage*paddingTopSizeFactor-minDynMemUsage)));
                                                                                                              ^

    I fixed it with this patch:

    diff --git a/src/qt/mempoolstats.cpp b/src/qt/mempoolstats.cpp
index 139efa1a9..af33a655b 100644
--- a/src/qt/mempoolstats.cpp
+++ b/src/qt/mempoolstats.cpp
@@ -8,6 +8,7 @@
 #include <qt/clientmodel.h>
 #include <qt/guiutil.h>
 #include <stats/stats.h>
+#include <math.h>
 
 static const char *LABEL_FONT = "Arial";
 static int LABEL_TITLE_SIZE = 22;
  79. in src/qt/mempoolstats.cpp:1 in 63fb11652f 
       0 | @@ -0,0 +1,423 @@
   1 | +// Copyright (c) 2016 The Bitcoin Core developers
    Sjors commented at 11:29 AM on January 8, 2018:

    Nit: bump to 2018?

  80. in src/qt/mempoolstats.cpp:320 in 63fb11652f 
     315 | +        redrawItems.append(itemDynSize);
 316 | +        redrawItems.append(itemTxCount);
 317 | +    }
 318 | +
 319 | +    // draw vertical grid
 320 | +    int amountOfLinesV = 4;
    Sjors commented at 12:15 PM on January 8, 2018:

    Nit: numberOfLinesV

  81. in src/qt/mempoolstats.cpp:297 in 63fb11652f 
     292 | +    dynMemUsagePathFill.lineTo(GRAPH_PADDING_LEFT+maxwidth, bottom);
 293 | +    dynMemUsagePathFill.lineTo(GRAPH_PADDING_LEFT, bottom);
 294 | +
 295 | +    QPainterPath dynMemUsageGridPath(QPointF(currentX, bottom));
 296 | +
 297 | +    // draw horizontal grid
    Sjors commented at 12:16 PM on January 8, 2018:

    Move grid drawing to a function?

  82. in src/qt/mempoolstats.cpp:88 in 63fb11652f 
      83 | +    if (!isVisible())
  84 | +        return;
  85 | +
  86 | +    if (!titleItem)
  87 | +    {
  88 | +        // create labels (only once)
    Sjors commented at 12:22 PM on January 8, 2018:

    Move the once-only stuff to another function?

  83. in src/qt/mempoolstats.cpp:98 in 63fb11652f 
      93 | +        
  94 | +        
  95 | +        QCheckBox *cb0 = new QCheckBox("Dynamic Memory Usage");
  96 | +        cb0->setStyleSheet("background-color: rgb(255,255,255);");
  97 | +        dynMemUsageSwitch = scene->addWidget(cb0);
  98 | +        connect(cb0, &QCheckBox::stateChanged, [cb0, this](){ drawDynMemUsage = cb0->isChecked(); drawChart(); });
    Sjors commented at 12:23 PM on January 8, 2018:

    Move the user interacting bits to another function?

    Also, do these really need to get redrawn every time? I'm not worried about performance, just code complexity. Adding and removing UI elements where it's not needed might also make using the interface builder more difficult.

  84. in src/qt/mempoolstats.cpp:81 in 63fb11652f 
      76 | +
  77 | +    if (model)
  78 | +        connect(model, SIGNAL(mempoolStatsDidUpdate()), this, SLOT(drawChart()));
  79 | +}
  80 | +
  81 | +void MempoolStats::drawChart()
    Sjors commented at 12:31 PM on January 8, 2018:

    Rename to drawChartFrame()? It draws a lot more than the chart.

  85. in src/qt/mempoolstats.cpp:212 in 63fb11652f 
     207 | +    lastHourLabel->setPos((width()-totalWidth)/2.0+last10MinLabel->boundingRect().width()+10,height()-filterBottomPadding);
 208 | +    lastDayLabel->setPos((width()-totalWidth)/2.0+last10MinLabel->boundingRect().width()+lastHourLabel->boundingRect().width()+20,height()-filterBottomPadding);
 209 | +    allDataLabel->setPos((width()-totalWidth)/2.0+last10MinLabel->boundingRect().width()+lastHourLabel->boundingRect().width()+lastDayLabel->boundingRect().width()+30,height()-filterBottomPadding);
 210 | +
 211 | +    // don't paint the grind/graph if there are no or only a signle sample
 212 | +    if (vSamples.size() < 2)
    Sjors commented at 12:32 PM on January 8, 2018:

    drawEmptyChart()

  86. in src/qt/mempoolstats.cpp:216 in 63fb11652f 
     211 | +    // don't paint the grind/graph if there are no or only a signle sample
 212 | +    if (vSamples.size() < 2)
 213 | +    {
 214 | +        noDataItem->setVisible(true);
 215 | +        return;
 216 | +    }
    Sjors commented at 12:33 PM on January 8, 2018: 
    else {
  drawChart(vSamples, ...)
}
  87. Sjors commented at 12:37 PM on January 8, 2018: member

    I did not need @jb55's patch on MacOS 10.13.2.

    Although it can wait for a followup commit, I would prefer if more of the UI was built using interface builder (see #11950). That would it easier for other to improve the looks.

    This should be easy for the header section above the chart and the duration toggles below. It's probably not realistic for the grid and X, Y axis and colored lines, although it would be nice if they can be styled using visual tools.

    I left some inline comments, but didn't see any must-fix stuff.

    The drawChart() function is rather large. Some stuff needs to be drawn only once, other stuff maybe only when there's a resize. Maybe that can be split? It might be safer to move some of the calculations into their own functions and test those against boundary values.

  88. laanwj commented at 9:37 AM on May 14, 2018: member

    This has been lingering for a long time. I would prefer if it makes it in, of course, it has tons of concept ACKs (the most of any open PR, according to bitcoinacks). But otherwise should we close and put this up for grabs?

  89. MarcoFalke added the label Needs rebase on Jun 6, 2018
  90. laanwj added the label Up for grabs on Aug 31, 2018
  91. laanwj commented at 10:46 AM on August 31, 2018: member

    Although it would be really nice to have, this has been inactive for very long. Closing and adding "up for grabs".

  92. laanwj closed this on Aug 31, 2018
  93. laanwj removed the label Needs rebase on Oct 24, 2019
  94. rebroad commented at 4:53 AM on August 20, 2020: contributor

    @laanwj A rebased version of this seems to be in Bitcoin Knots. Should I open a new pull request using this rebased version?

  95. jonatack commented at 1:30 PM on August 20, 2020: member

    This looks very nice. @rebroad I would review/test it if you pick it up.

  96. luke-jr commented at 1:45 PM on August 20, 2020: member

    Note that #8501 was updated after this, and in a conflicting way. In Knots, I'm using an older version of it.

    So there is some work to be done, but it sounds like a good idea since @jonasschnelli seems to have abandoned it.

    (If you're not up to merging #8501 with this, maybe it'd be fine to use the older version in Knots and the code can be updated later when the RPC interface is added)

  97. jarolrod commented at 11:04 PM on March 4, 2021: member

    @hebasto I guess this isn't up for grabs anymore since it's been picked up again by jonasschnelli over on the GUI repo

  98. hebasto removed the label Up for grabs on Mar 4, 2021
  99. rebroad commented at 4:18 PM on March 26, 2021: contributor

    @hebasto This could still be up for grabs given that it was superior in various ways to the newer implementation - e.g. it could run in a separate window to the debug window - the timescales were changeable, and it also shows memory utilization, and fee filter.

  100. hebasto commented at 4:29 PM on March 26, 2021: member

    @rebroad

    @hebasto This could still be up for grabs given that it was superior in various ways to the newer implementation - e.g. it could run in a separate window to the debug window - the timescales were changeable, and it also shows memory utilization, and fee filter.

    The "Up for grabs" label means "nobody's working on it", isn't it?

  101. laanwj commented at 8:36 AM on March 29, 2021: member

    The "Up for grabs" label means "nobody's working on it", isn't it?

    Yes. If you wanted to handle it in a different way, you should have picked it up yourself.

  102. MarcoFalke commented at 8:56 AM on March 29, 2021: member

    Or leave a suggestion on the pull that picked it up ;)

  103. rebroad commented at 8:22 AM on May 12, 2021: contributor

    @rebroad

    @hebasto This could still be up for grabs given that it was superior in various ways to the newer implementation - e.g. it could run in a separate window to the debug window - the timescales were changeable, and it also shows memory utilization, and fee filter.

    The "Up for grabs" label means "nobody's working on it", isn't it?

    Yes, it appears that no one is working on this - the fee graph is a different feature - related, but not the same. I think the UpForGrabs label ought to remain until someone is working on this particular graph. @luke-jr are you still rebasing this?

  104. rebroad commented at 8:09 AM on May 16, 2021: contributor

    A current rebase is here - https://github.com/rebroad/bitcoin/commit/3ecfd95bde4f649b345d3d15c9ffc0da15156bca

  105. DrahtBot locked this on Aug 16, 2022
Contributors
jonasschnelliMarcoFalkeluke-jrghostisle2983rebroadfanquakepaveljaniklaanwjVictorsuecagmaxwelljtimonSjorsPierreRochardjb55jonatackjarolrodhebasto
Labels
GUI
Linked (view graph)
#8501 Add mempool statistics collector
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 18:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me