Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence #866

pull rebroad wants to merge 3 commits into bitcoin-core:master from rebroad:trafficgraphwidget-rebased changing 9 files +717 −134
  1. rebroad commented at 11:00 pm on April 9, 2025: contributor

    This PR improves the network traffic graph widget in the Debug window to provide:

    1. Multiple time range options (from 5 minutes to 28 days)
    2. Data persistence between sessions
    3. Interactive visualization features including tooltips and logarithmic scaling

    Motivation

    The existing network traffic graph has limited utility with its fixed time range and lack of historical data preservation. This enhancement allows users to:

    • Analyze network traffic patterns over different timeframes
    • Preserve historical traffic data between session restarts
    • Interact with the graph to get specific data points
    • Better visualize varying traffic volumes with logarithmic scaling

    These improvements are valuable for:

    • Network debugging and troubleshooting
    • Understanding Bitcoin Core’s network behaviour
    • Monitoring traffic patterns for optimization
    • Research purposes

    Implementation

    The implementation preserves all existing functionality while adding new features:

    • Added a pre-configured timeframe selection (13 different timeframes)
    • Implemented traffic data serialization and deserialization
    • Enhanced the visualization with interactive features
    • Improved tooltip information with precise timestamps and traffic rates

    Supporting changes:

    • Added formatBytesps function for a prettier display of traffic rates
    • Added FormatISO8601Time for better time display

    Testing

    Tested on Linux with various network conditions. The new functionality can be exercised by:

    1. Using the slider (or arrow keys) to select different time ranges
    2. Restarting the application to verify data persistence
    3. Clicking on the graph to toggle between linear and logarithmic scales
    4. Hovering over data points to view detailed information

    Documentation

    The changes are mostly self-documenting through the UI and are constrained to the Qt interface without affecting core functionality.

    Compatibility

    This PR maintains compatibility with existing functionality. The data persistence file uses proper serialization versioning to allow for future format changes if needed.

  2. DrahtBot commented at 11:00 pm on April 9, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    No conflicts as of last run.

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • if (m_tt_point && m_tt_point <= DESIRED_SAMPLES) -> if (m_tt_point > 0 && m_tt_point <= m_time_stamp[m_value].size()) [Index check should use the actual queue size, not a constant]
    • if (m_tt_point > DESIRED_SAMPLES) m_tt_point = 0; -> if (m_tt_point > m_time_stamp[m_value].size()) m_tt_point = 0; [Index check should use the actual queue size, not a constant]
    • while (m_time_stamp[i].size() > DESIRED_SAMPLES) -> while (m_time_stamp[i].size() > static_cast<int>(DESIRED_SAMPLES * static_cast<double>(m_values[i]) / m_values[0])) [The maximum number of samples stored per range should scale with the range duration, not a fixed constant, making the queue management logic unclear.]
    • static uint64_t last_time_ms; -> uint64_t last_time_ms; [Static variable within a loop across ranges leads to incorrect state persistence, corrupting data loading.]
    • if (!dst_point || (!is_peak && !is_dip)) return dst_point; -> if (dst_point == 0 || (!is_peak && !is_dip)) return 0; [Inconsistent indexing (0-based vs 1-based) and potential out-of-bounds access (accessing at dst_point - 1 when dst_point could be 0) makes the function logic unclear and potentially buggy.]
  3. DrahtBot added the label CI failed on Apr 9, 2025
  4. DrahtBot commented at 11:49 pm on April 9, 2025: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin-core/gui/runs/40286962281

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  5. rebroad force-pushed on Apr 10, 2025
  6. DrahtBot removed the label CI failed on Apr 10, 2025
  7. rebroad marked this as ready for review on Apr 10, 2025
  8. rebroad renamed this:
    qt: Improve TrafficGraphWidget functionality
    qt: Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence
    on Apr 11, 2025
  9. in src/qt/trafficgraphwidget.cpp:42 in 31b1ffc0ff outdated
    51+
    52+            // Restore the saved total bytes counts to the node if they exist
    53+            if (m_total_bytes_recv > 0 || m_total_bytes_sent > 0) {
    54+                model->node().setTotalBytesRecv(m_total_bytes_recv);
    55+                model->node().setTotalBytesSent(m_total_bytes_sent);
    56+            }
    


    luke-jr commented at 10:10 pm on April 14, 2025:

    The GUI should not be changing low-level internals.

    If for some reason the clientModel is changed, its data should be taken as authoritative, even if it contradicts the history so far (if that’s an issue, wipe the history).

    Alternatively, maybe the history should be stored with the node, and loaded into the GUI (or RPC, in another hypothetical PR).


    rebroad commented at 11:01 pm on April 15, 2025:
    The idea is that the data is not persistent - but yes, I see your point - it should probably not be something that only happens when the GUI is running, and should perhaps also happen in bitcoind also.

    rebroad commented at 8:52 pm on April 30, 2025:

    The GUI should not be changing low-level internals.

    If for some reason the clientModel is changed, its data should be taken as authoritative, even if it contradicts the history so far (if that’s an issue, wipe the history).

    Alternatively, maybe the history should be stored with the node, and loaded into the GUI (or RPC, in another hypothetical PR). @luke-jr I’ve removed the setTotalBytes functions as I agree with your reasoning. The GUI keeps a local track of the totals from the last session and simply adds them to the totals provided by the node now. I did think about storing the data on the node, but I am not sure it makes much sense given the data is currently stored on the GUI and so it makes more sense to keep the totals local with the traffic history for now. Perhaps someone thinks it’s worth storing this on the node in the future, but it doesn’t seem like an urgent need for now.

  10. in src/qt/forms/debugwindow.ui:698 in 31b1ffc0ff outdated
    702@@ -694,16 +703,6 @@
    703              </property>
    704             </widget>
    705            </item>
    706-           <item>
    707-            <widget class="QPushButton" name="btnClearTrafficGraph">
    


    luke-jr commented at 10:12 pm on April 14, 2025:

    If we’re deleting this, maybe there should be a way for the user to insert a reference line?

    For now, I’d move removal of anything to a separate PR.


    rebroad commented at 10:59 pm on April 15, 2025:
    The user can effectively remove the data by deleting the .dat file - is there any basis for being able to do this while the client is running?
  11. in src/qt/rpcconsole.cpp:581 in 31b1ffc0ff outdated
    575@@ -578,7 +576,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty
    576     // based timer interface
    577     m_node.rpcSetTimerInterfaceIfUnset(rpcTimerInterface);
    578 
    579-    setTrafficGraphRange(INITIAL_TRAFFIC_GRAPH_MINS);
    


    luke-jr commented at 10:12 pm on April 14, 2025:
    Why are we losing this default constant?

    rebroad commented at 10:57 pm on April 15, 2025:
    because it isn’t needed - on startup it “defaults” to the first non-full range
  12. in src/util/time.cpp:78 in 31b1ffc0ff outdated
    74@@ -75,6 +75,13 @@ void MockableSteadyClock::ClearMockTime()
    75 
    76 int64_t GetTime() { return GetTime<std::chrono::seconds>().count(); }
    77 
    78+int64_t GetTimeMillis()
    


    luke-jr commented at 10:14 pm on April 14, 2025:
    I doubt we’ll be restoring this. It was removed intentionally.

    rebroad commented at 10:56 pm on April 15, 2025:
    yes, I’m going to remove this, will push an update shortly
  13. luke-jr changes_requested
  14. luke-jr commented at 10:14 pm on April 14, 2025: member
    Merging it all together likes this makes review very annoying. It was better as separate PRs.
  15. rebroad commented at 11:02 pm on April 15, 2025: contributor

    Merging it all together likes this makes review very annoying. It was better as separate PRs.

    I vaguely remember people complaining that I was raising several PRs that could be combined when I first raised the PRs years ago…. I guess one cannot please everyone all of the time. @luke-jr As a middle-ground, I can move the distinct functional changes into different commits.

  16. rebroad requested review from luke-jr on Apr 15, 2025
  17. rebroad force-pushed on Apr 16, 2025
  18. laanwj renamed this:
    qt: Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence
    Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence
    on Apr 24, 2025
  19. laanwj added the label Feature on Apr 24, 2025
  20. rebroad force-pushed on Apr 30, 2025
  21. rebroad force-pushed on Apr 30, 2025
  22. rebroad force-pushed on Apr 30, 2025
  23. rebroad force-pushed on Apr 30, 2025
  24. rebroad force-pushed on Apr 30, 2025
  25. rebroad force-pushed on May 1, 2025
  26. rebroad commented at 11:11 pm on May 1, 2025: contributor
    I’ve removed the code that changed the totals on the node - the GUI just keeps track now without writing values to the node. Also, added some checks for corrupt save files so that it doesn’t load bad data.
  27. rebroad force-pushed on May 1, 2025
  28. rebroad force-pushed on May 1, 2025
  29. DrahtBot added the label CI failed on May 1, 2025
  30. DrahtBot commented at 11:15 pm on May 1, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin-core/gui/runs/41512420338 LLM reason (✨ experimental): The CI failure is caused by a lint check failure due to the use of tabs instead of spaces in the codebase.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  31. rebroad force-pushed on May 2, 2025
  32. rebroad commented at 6:36 am on May 2, 2025: contributor
    I realise this PR is starting to look a bit messy with all the updates. I also realise I ought to separate various functionality changes into separate commits. Is it better to raise a new PR once this is done, force push to this one?
  33. rebroad force-pushed on May 2, 2025
  34. DrahtBot removed the label CI failed on May 2, 2025
  35. rebroad force-pushed on May 2, 2025
  36. rebroad force-pushed on May 2, 2025
  37. rebroad force-pushed on May 2, 2025
  38. rebroad force-pushed on May 2, 2025
  39. rebroad force-pushed on May 2, 2025
  40. rebroad marked this as a draft on May 2, 2025
  41. rebroad force-pushed on May 3, 2025
  42. rebroad commented at 7:46 pm on May 3, 2025: contributor

    Have just added some more GUI improvements - the text on the graph now has a black outline around it making it easier to read (and is printed AFTER the graph rather than before) .Also, the grey line at the bottom of the graph is now printed after the graph also, causing the graph not to spill onto that line.

    Also, the bright green and red lines no longer surround the sides of the graph but only draw an outline along actual graph values. This makes it easier to distinguish about the graph going off the edge of the display areas, vs a sudden drop in traffic (which previously looked the same). The now graph also fills the area fully to the edges at the side now (which it did not before due to rounding issues).

    It would be useful to know whether I should split all these changes into individual pull requests, or individual commits, or better all together like this.

  43. rebroad force-pushed on May 4, 2025
  44. DrahtBot added the label CI failed on May 4, 2025
  45. DrahtBot commented at 9:26 pm on May 4, 2025: contributor

    🚧 At least one of the CI tasks failed. Task previous releases, depends DEBUG: https://github.com/bitcoin-core/gui/runs/41616791301 LLM reason (✨ experimental): The CI failed due to a compilation error caused by treating warnings as errors, specifically a sign comparison warning in trafficgraphwidget.cpp.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  46. rebroad force-pushed on May 5, 2025
  47. rebroad force-pushed on May 5, 2025
  48. rebroad force-pushed on May 5, 2025
  49. rebroad force-pushed on May 5, 2025
  50. rebroad force-pushed on May 6, 2025
  51. rebroad force-pushed on May 6, 2025
  52. rebroad force-pushed on May 6, 2025
  53. rebroad force-pushed on May 6, 2025
  54. rebroad force-pushed on May 6, 2025
  55. DrahtBot removed the label CI failed on May 6, 2025
  56. rebroad force-pushed on May 6, 2025
  57. qt: Add formatBytesps function for bytes per second display
    Add a new GUI utility function to format bytes per second values with appropriate
    units (B/s, kB/s, MB/s, GB/s) and precision, ensuring consistent and readable
    display of network traffic rates in the traffic graph widget.
    34794f80a2
  58. util: Add FormatISO8601Time function for time-only ISO format
    Add a new time formatting function that matches the existing date and datetime
    ISO8601 formatting functions, providing consistent time display for the
    traffic graph widget tooltips and other UI elements that need time-only display.
    2158ab7dc9
  59. rebroad force-pushed on May 8, 2025
  60. qt: Enhance TrafficGraphWidget with multi-timeframe support and data persistence
    This commit significantly improves the network traffic graph widget with:
    
    1. Multiple timeframe support - View traffic data across different time
       periods (5 minutes to 28 days) using an enhanced slider interface
    
    2. Traffic data persistence - Save and restore traffic information
       between sessions, preserving historical traffic patterns
    
    3. Interactive visualization features:
       - Logarithmic scale toggle (mouse click) for better visualization of
         varying traffic volumes
       - Interactive tooltips showing detailed traffic information at specific points
       - Yellow highlight indicators for selected data points
    
    4. Smooth transitions between different time ranges with animated scaling
    
    These improvements allow users to better monitor and analyze network
    traffic patterns over time, which is especially useful for debugging
    connectivity issues or understanding network behavior under different
    conditions.
    
    The implementation includes proper thread-safety considerations and
    handles edge cases like time jumps or missing data appropriately.
    bc13d01983
  61. rebroad force-pushed on May 8, 2025


rebroad DrahtBot luke-jr


luke-jr

Labels
Feature


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-05-09 10:20 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me