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

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

Multiple time range options (from 5 minutes to 28 days)
Data persistence between sessions
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:

Using the slider (or arrow keys) to select different time ranges
Restarting the application to verify data persistence
Clicking on the graph to toggle between linear and logarithmic scales
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.

Apr 09 '25 23:04 rebroad

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	RandyMcMillan

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#bitcoin/bitcoin/32862 (rpc: use CScheduler for relocking wallet and remove RPCTimer by pinheadmz)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

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.]

Apr 09 '25 23:04 DrahtBot

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

Hints

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.

Apr 09 '25 23:04 DrahtBot

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.

Apr 15 '25 23:04 rebroad

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.

May 01 '25 23:05 rebroad

🚧 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.}

Hints

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.

May 01 '25 23:05 DrahtBot

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?

May 02 '25 06:05 rebroad

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.

May 03 '25 19:05 rebroad

🚧 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.}

Hints

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.

May 04 '25 21:05 DrahtBot

🚧 At least one of the CI tasks failed. _{Task tidy: https://github.com/bitcoin-core/gui/runs/41904414250} _{LLM reason (✨ experimental): The failure is caused by compilation errors due to ignoring the return value of 'fclose()' functions marked as 'nodiscard', leading to build failure.}

Hints

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.

Jul 05 '25 16:07 DrahtBot

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

Jul 08 '25 01:07 DrahtBot

concept ACK

Sep 24 '25 15:09 RandyMcMillan