Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: mempool stats chart #108

Closed

Conversation

jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Oct 23, 2020

This PR adds a mempool statistics to the "debug" window.
The diagram is heavy inspired by Jochen Hoenicke's webversion (Thanks @jhoenicke for that).

macOS:
Bildschirmfoto 2020-10-23 um 16 30 51

Ubuntu:
Bildschirmfoto 2020-10-24 um 11 34 19

The approach is it to collect the fees also when the mempool chart or the node window is not open (collect it from the start of the application).
Clicking on the fee-range color square highlights the fee group over time.

To keep the scope minimal:

  • flexible timeframe is out of scope (can be added later)

See also bitcoin/bitcoin#8550 (an initial attempt, different concept).

TODO:

  • Make it non HiDPI compatible (linux/win/etc.).
  • Analyze memory consumption (should not be too bad)
  • Optimize performance
  • Try to make fee ranges dynamic

@jonatack
Copy link
Member

Concept ACK -- nice!

@jonasschnelli jonasschnelli force-pushed the 2020/03/mempool_graph branch 3 times, most recently from 96ee92a to 52267ab Compare October 24, 2020 09:41
@RandyMcMillan
Copy link
Contributor

Should the background color of the Network Traffic display be changed to white as well? - to create a consistent UX within the different tabs? If so - should they share a common theme - so they can be toggled together?

@RandyMcMillan
Copy link
Contributor

Concept ACK :)

It seems that the parent window should trigger a repaint/resize of the view simular to the peer table data model...

void PeerTableModel::refresh()

Screen Shot 2020-10-25 at 6 33 17 PM


  • A similar implementation may be useful to actually hide the tab until the node is up and running and some initial values have been populated
  • Should flushing the mempool prior to shut down - as an option - be added to the tab as a check box? default no? This would allow the window to check cached data to speed up the initial paint.

void BitcoinGUI::showDebugWindowActivateConsole()

We can attempt to use similar settings to make a more consistent UX between the tabs by using the same dimensions for the data presentation view and the margin underneath...

PR #90

@promag
Copy link
Contributor

promag commented Oct 26, 2020

Concept ACK.

@Sjors
Copy link
Member

Sjors commented Oct 26, 2020

Very cool, concept ACK.

@jonasschnelli
Copy link
Contributor Author

@RandyMcMillan
Thanks for the feedback.
I'd like to avoid visual alignment of both of the graphs in this PR. Off course this should happen but my takeaway from bitcoin/bitcoin#8550 is that scope creep must be avoided if we want to get this in master.

I recommend to do the visual alignment later on.

Copy link
Member

@Sjors Sjors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The peers tab is dropped from the menu, try expanding std::vector<TabTypes> tabs() in rpcconsole.h

I managed to get a segfault by clicking on the chart (macOS), though only once. Also when clicking on an item in the legend.

You could split this into two PR's, one where the node collects all the histogram data and exposes it via RPC (with a functional test). And a GUI PR on top.

ClickableRectItem *fee_rect = new ClickableRectItem();
fee_rect->setRect(4, c_y, c_w, c_h);

QColor brush_color = colors[(i < static_cast<int>(colors.size()) ? i : static_cast<int>(colors.size())-1)];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comparison of integers of different signs: 'int' and 'std::__1::vector<QColor, std::__1::allocator<QColor> >::size_type' (aka 'unsigned long') [-Werror,-Wsign-compare]

(a bunch of other warnings too)

@hebasto
Copy link
Member

hebasto commented Oct 30, 2020

Concept ACK.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

One Note:
On macOS 11.1, Qt 5.15.2 - I can reliably cause a segmentation fault by clicking on any of the text on the left side. I can click on the corresponding box, but clicking on the text will cause a segmentation fault. Here is a pastebin link to the error report: https://pastebin.com/E7my5X8V

@ghost
Copy link

ghost commented Mar 4, 2021

IMO the bar graph used by https://btc.bitaps.com is better than https://jochen-hoenicke.de/queue/#BTC%20(default%20mempool),24h

Reason: Its easy to look at it and use the information while broadcasting a bitcoin transaction

image

@hebasto
Copy link
Member

hebasto commented Mar 19, 2021

IMO the bar graph used by https://btc.bitaps.com is better than https://jochen-hoenicke.de/queue/#BTC%20(default%20mempool),24h

Reason: Its easy to look at it and use the information while broadcasting a bitcoin transaction

I like it. It uses the widget area more effective.

@rebroad
Copy link
Contributor

rebroad commented Mar 29, 2021

image
It would be ideal if the functionality of the previous graph and the new could be combined. e.g. allow different durations to be shown (from 3 hours, to 2 weeks), have one line for tx count, but show the fee rates within the memory usage graph. The scale (tx count and MB usage) could both be shown on the right, with the fee-rates shown on the left. Change the colours slightly so that the minfeerate and tx count are still clearly visible if plotted over the memusage/feerate. Oh, and ideally the window could be a separate window, rather than as a tab on the debug window.

At least, this would be perfection IMHO :)

@rebroad
Copy link
Contributor

rebroad commented Mar 29, 2021

image
The functionality of clicking on a fee range and it displaying the number of transactions seems a bit off. In this example it's saying there are over 1,000,000 transactions in the 30-40 satoshi range, yet the vertical axis only goes up to 20,000 tx.

@RandyMcMillan
Copy link
Contributor

#320 is a rebase of this PR.

@hebasto
Copy link
Member

hebasto commented May 29, 2021

The "22.0" milestone seems unrealistic here. Removing it.

@hebasto hebasto removed this from the 22.0 milestone May 29, 2021
@rebroad
Copy link
Contributor

rebroad commented Jul 3, 2021

image
Thinking it might be nice if the vertical axis scaled like it did on the old graph so that more detail can be seen...

@Rspigler
Copy link
Contributor

@jonasschnelli Do you still have plans to work on this? Thanks!

@laanwj
Copy link
Member

laanwj commented Aug 22, 2021

Concept ACK, this looks really neat. I think it's good for people running the GUI to have some introspection into what is happening on their node, and visualizations like this are useful for that.

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@DrahtBot
Copy link
Contributor

There hasn't been much activity lately and the patch still needs rebase. What is the status here?
  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@hebasto
Copy link
Member

hebasto commented Apr 13, 2022

Closing due to inactivity. Feel free to reopen.

@hebasto hebasto closed this Apr 13, 2022
@Rspigler
Copy link
Contributor

Mark up for grabs?

@hebasto
Copy link
Member

hebasto commented Apr 27, 2022

Mark up for grabs?

Done.

@bitcoin-core bitcoin-core locked and limited conversation to collaborators Apr 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.