rpc, cli: Add client ID field to cluster nodes

[leafaar]

Problem

The client ID field in validator version information is not exposed through the RPC interface or CLI, making it impossible to analyze validator client diversity on the network. Users cannot see which client implementation (Agave, Jito, Firedancer, etc.) validators are running.

Summary of Changes

• Add client_id field to RpcContactInfo in getClusterNodes RPC response
• Make Version::client field public to allow access from RPC layer
• Add CliClientId type for parsing and displaying client IDs in CLI
• Add client ID column to validator list display
• Support --sort=client-id option for sorting validators
• Add stake distribution breakdown by client ID

Fixes #

[mergify]

If this PR represents a change to the public RPC API:

1. Make sure it includes a complementary update to rpc-client/ (example)
2. Open a follow-up PR to update the JavaScript client @solana/kit (example)

Thank you for keeping the RPC clients in sync with the server API @leafaar.

[0xbrw]

@steviez would you know the right person to review this?

[steviez]

@steviez would you know the right person to review this?

Probably a generalist like myself; can try to take a look later this week.

Edit: I didn't get the chance to look but seems like Alan nominated himself

[alannza]

This looks great. I only found one minor issue with the formatting of the header.

Changing the RPC API response could break clients that depend on not receiving extra, unknown fields in the response but it looks like we've already done that so many times that it would be silly to draw the line here.

Thank you for the PR!

[joncinque]

Thanks for your contribution! Can you rebase your branch? There were some other CLI related changes recently. After that, we should be able to land this soon

[leafaar]

I removed the extra space @joncinque, but notice, this PR didn't introduced the extra space, it already existed, if you look you will see more extra spaces, do you want to me remove them in this PR? Maybe it would be better a new PR so it would be clear what the change is.

[joncinque]

Sorry for the late reply, I meant to remove only the extra spaces added in this PR, so your change looks good to me! I'll let @alannza shepherd the PR through CI and all that, there are some clippy errors to be fixed

[codecov-commenter]

Codecov Report

❌ Patch coverage is 9.90099% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.2%. Comparing base (01159e4) to head (0a97cb4).
⚠️ Report is 813 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #7817     +/-   ##
=========================================
- Coverage    83.2%    83.2%   -0.1%     
=========================================
  Files         845      846      +1     
  Lines      319902   319999     +97     
=========================================
+ Hits       266473   266500     +27     
- Misses      53429    53499     +70     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[joncinque]

@mcintyre94 can you take a look whenever you have a chance? After the previous approval, I rebased, updated a test for the new field, and added a changelog entry

[mcintyre94]

Looks good to me!

[t-nelson]

why did we put the discriminant on the wire instead of the string representation?

[joncinque]

why did we put the discriminant on the wire instead of the string representation?

No particular reason other than simplicity, but we can change that pretty easily -- for my own info, what's the reason to prefer a string for the field? For any enhancements in the future?

[t-nelson]

just can't imagine that people know what the number means and not particularly keen on exposing the crate to public api

[joncinque]

People have to interpret all sorts of bytes from frontends, so you'd be surprised 😅

Anyway, looks like Display isn't implemented on the enum -- do you want the exact strings that are there? SolanaLabs / JitoLabs / Frankendancer / Agave / AgavePaladin / Firedancer / AgaveBam / Sig / "Unknown(<INSERT_NUMBER_HERE>)"

[t-nelson]

i think we abuse Debug for that elsewhere

i already have a cleanup coming for that module tho. just need to land something that i want to bp first. we can fix this afterward

[joncinque]

Considering it's a new RPC response field, I want to fix it before it lands in a release. I can go with abusing Debug and keeping the changes to RPC