GUI: Show warnings on all tabs

[kwsantiago]

Fixes #300.

The status-bar warning from ClientModel::getStatusBarWarnings() (e.g. unknown soft-fork activation, "large-work invalid chain", clock-skew warnings, pre-release build notices) was only ever rendered on the Overview tab. Its alert label lived inside OverviewPage, so every other tab showed nothing. Because the Send/Receive/Transactions/Pairing pages each live in their own widget in the stack, switching to any of them hid the warning.

This moves the warning label out of OverviewPage and up into WalletFrame, as a sibling of m_global_stack rather than a child of any single page. The WalletFrame layout changes from an QHBoxLayout to a QVBoxLayout so the label sits above the stack and stays visible on every tab. OverviewPage::labelAlerts and its updateAlerts slot are removed so there is a single source for the warning instead of a per-page duplicate.

Result: the warning shows on all five tabs when a wallet is loaded (Overview/Send/Receive/Transactions/Pairing) and on both no-wallet tabs (Overview/Pairing). The RPCConsole path used under -disablewallet is untouched.

How to reproduce

getStatusBarWarnings() only returns a non-empty string when the node actually raises a warning (unknown version-bit activation, invalid-chain detection, system-clock skew, etc.), which is awkward to trigger on demand. The simplest way to see the bug is to inject a fixed warning, build, and click through the tabs.

1. Hardcode a warning in src/qt/clientmodel.cpp:

   QString ClientModel::getStatusBarWarnings() const
   {
       return "TEST WARNING";
       // return QString::fromStdString(m_node.getWarnings().translated);
   }

2. Build the GUI and start on regtest with no wallet:

   ./src/qt/bitcoin-qt -regtest -noconnect
   

3. Click through the tabs.

Before this PR: the yellow warning bar appears only on the Overview tab. Switching to Pairing (or, with a wallet loaded, to Send/Receive/Transactions) hides it.

With this PR: the warning bar stays visible on every accessible tab. With no wallet that is Overview and Pairing (Send/Receive/Transactions are greyed out); with a wallet loaded it is all five tabs.

Remember to revert the hardcoded warning before testing anything else.

[chrisguida]

After looking into this briefly, I'm still confused as to the bug here... is the problem that the Pairing tab doesn't display warnings? Because that's the only non-wallet tab besides Overview, and Overview already displays warnings properly?

[kwsantiago]

@chrisguida Overview already shows it (fixed in 81d5172 last July); the actual gap is the Pairing tab, which lives in a parallel stack and so the existing alert label gets hidden when you switch to it. This PR fixes that.

[chrisguida]

@luke-jr says the bug is already fixed, do we still need this PR?

[kwsantiago]

@chrisguida , @luke-jr fixed the Overview tab bug, this PR fixes the Pairing tab bug, per my comment above.

[luke-jr]

The Send/Receive/Transactions tabs also don't show the warning. I don't think it would be consistent to put it on Pairing without putting it everywhere...?

[kwsantiago]

@luke-jr Good point. Updated to drop OverviewPage::labelAlerts so WalletFrame's alert label is the single source above every tab (Overview/Send/Receive/Transactions/Pairing).

[chrisguida]

I've tested the new version of this PR with a hardcoded warning in ClientModel::getStatusBarWarnings(), confirmed that the warning is now displayed prominently on all 5 tabs when a wallet is loaded, and on the 2 non-wallet tabs (Overview and Pairing) when no wallet is loaded (the other 3 wallet tabs are inaccessible when no wallet is loaded). Also confirmed that prior to this change, only the Overview tab displays warnings (implemented in 81d5172), in both wallet mode and no-wallet mode.

Technically this PR is larger in scope than the original issue, which only mentions no-wallet tabs, but there are only 2 no-wallet tabs: Overview and Pairing, and Overview was already working properly. The first version of this PR fixed the Pairing tab, which was the exact scope originally outlined in #300 (and still referred to in this PR's title), but fixing warnings on all 5 tabs (regardless of whether a wallet is loaded) strikes me as the correct solution, and this new version is much cleaner than the old version anyway.

Other than the PR's title and top comment needing an update for this new version:

ACK a7685c4

[pdath]

I don't clearly understand the problem here and, consequently, how to test it. I don't have Tor enabled. Is that critical to this bug?

What I can offer is that when I run the PR301 code on regtest on Windows, with no wallet, this is what I see:

The send/review/transaction tabs are greyed out. This is my peering tab.

[chrisguida]

@pdath I tested it by hardcoding a warning in the UI like so:

QString ClientModel::getStatusBarWarnings() const
{  
    return QStringLiteral("TEST WARNING: should be visible on ALL tabs");
    return QString::fromStdString(m_node.getWarnings().translated);
}

I think I explained what's going on pretty well in my comment.

[pdath]

@pdath I tested it by hardcoding a warning in the UI like so:

QString ClientModel::getStatusBarWarnings() const
{  
    return QStringLiteral("TEST WARNING: should be visible on ALL tabs");
    return QString::fromStdString(m_node.getWarnings().translated);
}

I think I explained what's going on pretty well in my comment.

This is not working for me on Windows.
Which platform did you test on?

[chrisguida]

@pdath Linux x86, this is what it looks like for me:

When you say "not working", do you mean not building? Or you're just not seeing the warning banner?

[pdath]

@pdath Linux x86, this is what it looks like for me:

When you say "not working", do you mean not building? Or you're just not seeing the warning banner?

The warning bar in your screenshot - I don't get that on Windows. Everything is building as expected. No errors.

[pdath]

I know what I am doing wrong now.
When using guix, I found that the script "make_release_tarball.sh" runs "git archive-- prefix="${DISTNAME}/" HEAD", which means it uses whatever is in the most recent commit - ignoring any changes you have made locally (such as to display an error).

So I have done a dummy commit locally, and it now works!!!

I tested the code using this approach.
git remote add upstream https://github.com/bitcoinknots/bitcoin.git
git fetch upstream pull/301/head:pr301
git switch pr301

I built the code using these commands:
sudo apt install -y guix
env HOSTS='x86_64-w64-mingw32' ./contrib/guix/guix-build

I tested on Windows 11 using the "How to reproduce" documentation.
I have not reviewed the source node. I don't know anything about QT.

I did an AI code review. I think the summary comment is the only one worth mentioning.
"This is a small, focused, and correct refactoring."

I used a default configuration, and started the test with:
bitcoin-qt.exe -regtest -noconnect

On the basis of the test results only (no human code review), tACK a7685c4.

[luke-jr]

How about adding 1ex of padding to m_label_alerts since we're losing its margin?