fix(gui): one answer for a peer's ident state, and never a blank field - #728

Summary

Fixes #727.

CUpDownClient had two accessors disagreeing about what a missing credits object means. GetCurrentIdentState() treats it as IS_NOTAVAILABLE; the five boolean predicates tested credits && instead, so with credits still NULL every one of them returned false — including SUINotSupported(), which is exactly the state the accessor was reporting. credits is NULL from construction until the peer's user hash resolves, so this is reachable rather than theoretical.

The visible effect was in the Client Details dialog. CClientRef::GetSecureIdentTextStatus() is an if / else if chain over those five predicates with no final else, and its result is assigned straight into the label. All five false left the string empty, blanking the Secure ident: field and overwriting the _("N/A") placeholder it was built with. The same dialog reads "Not supported" in amulegui, whose predicates compare m_identState directly and whose credits object is always allocated — so the monolithic GUI was the odd one out, disagreeing with both amulegui and the EC / Web API view, all of which go through GetCurrentIdentState().

The fix defines the predicates in terms of that accessor. The contradiction then cannot recur: they are the accessor's answer rather than a second opinion about it.

The chain also gets a final else. It is unreachable while the five predicates cover every EIdentState, but a sixth state would otherwise silently blank the field again instead of failing visibly.

Blast radius is smaller than it looks

Only SUINotSupported() changes behaviour, and it has exactly one caller in the tree — the chain being fixed. For the other four the guard was redundant: a NULL credits yields IS_NOTAVAILABLE, which already compares unequal to the state each of them asks about, so their 6 / 4 / 1 / 1 call sites are unaffected.

On the catalogs

No new translatable string — the fallback reuses the _("N/A") the field is already built with. The catalogs still move, because since #673 they record locations by file and that msgid's reference list gains src/ClientRef.cpp. Existing translations are preserved.

Test plan

Built on macOS 15 ARM64 with MONOLITHIC + DAEMON + REMOTEGUI + AMULECMD + AMULEAPI + TESTING: 0 errors, ctest 29/29. Both divergent implementations compile — the monolithic aMule.app and aMuleGUI.app now report the same thing for the same peer.

git clang-format origin/master reports nothing to format. Diff-scoped clang-tidy is clean against both .clang-tidy-new-code and .clang-tidy, each analysing 5 TUs with 0 truncated ASTs.

Not unit-tested, deliberately. Nothing currently links BaseClient / ClientRef; CUpDownClient's construction pulls a 49-include closure with five app-singleton headers, and the chain needs theApp for CryptoAvailable(). A harness for that would dwarf the fix. Routing the predicates through the accessor makes the disagreement unrepresentable instead, which is a stronger guarantee than a test could give — a test could only sample states, whereas the definition covers all of them.

The reproduction in the issue (a source added from a server response, opened before it completes the eD2k handshake) has a short window and depends on live peers, so it has not been reproduced end to end here; the fix is derived from the code paths rather than from a captured repro.

Left out

The issue's "Related, same area" note — that GenericClientListCtrl draws an overlay for IsIdentified() and IsBadGuy() but nothing for SUIFailed(), so a peer whose signature actually failed looks identical to one that never supported SecIdent. That is a real gap, but it needs a decision on which icon to use, so it belongs in its own change.