Why not try std::map<std::string, ConnectionType> m_added_nodes, then we can avoid to define AddedNodeEntry?

Hmm, I'm a bit undecided about that: I think avoiding a struct is not really a good argument for switching form vector to map: We would need to switch to a pair or struct if we'd ever need to add another member, plus there is a case for introducing a struct even if we'd switch to a map, as was recently done in PruneLocks example.
Performance-wise, this doesn't seem important at all since the container cannot have more than 8 entries (MAX_ADDNODE_CONNECTIONS).

Why not directly change IsBlockOnlyConn() directly as below, then can avoid to define new function IsManualBlockRelayConn()?

 bool IsBlockOnlyConn() const {

•    return m_conn_type == ConnectionType::BLOCK_RELAY;
  

•    return m_conn_type == ConnectionType::BLOCK_RELAY || m_conn_type == ConnectionType::MANUAL_BLOCK_RELAY;
  

  }

See #24170 (comment) - there are several placed that use IsBlockOnlyConn() with logic specific to automatic block-relay-only connections that have logic that does not apply to manual ones. So if I did that, we'd need additional logic in these places to distinguish between manual and automatic connections.

That seems preferable?

I have changed the logic now such that IsBlockRelayConn is used when both connection types are meant, where as IsManualBlockRelayConn and IsAutomaticBlockRelayConn are used in spots where only one is meant.

I know that this is maybe a bit of a stretch for this PR, but I really don't like the addnode/addednode terminology, and would much prefer this to be named something like ManualConnectionEntry, which much more precisely describes the nature of these connections.

The downside of ManualConnectionEntry is that it is not a connection, just a name of a node we'll try to connect to (which may succeed or not). I didn't really like the name AddedNodeEntry either though, but couldn't think of anything better. Maybe ManualNodeEntry?

Or maybe ManualConnectionTarget or DesiredManualConnection? I'm not sure they're great either.

Prefer =blocksonly to be clearer and match the existing -blocksonly option.

-blocksonly mode is different from block relay peers.

How so? Maybe it needs better docs on the difference...

Here you go: https://bitcoin.stackexchange.com/questions/112828/what-is-a-block-relay-only-connection-what-is-it-used-for

That doesn't contrast.

Some differences:

• -blocksonly is a global startup option (applying to all connections) -block-relay-only is connection-specific
• A node in -blocksonly mode participates in address relay, we never send out addresses on -block-relay-only connections (and ignore incoming ones).
• A node in -blocksonly signals not to receive any transactions (fRelay) and will disconnect if that is breached, but it may in special cases send out transactions itself (if submitted via RPC). We'll never send a tx over a block-relay-only connection.
• A node in -blocksonly mode still has two of their outbound connections as -block-relay-only (for which then the block-relay-only rules apply).

Supposed option modify case need not be handled?
If add node firstly with setting manual and then setting manual block relay.

No, a modify option is not supported, especially since it is not possible to change the connection connection type of an existing connection. A user could instead use the 'remove' option and then re-add with another connection type (which would not change existing connections though).

It might be an option to use std::any_of here if using standard library algorithms is preferable, but personally I think it might end up a little less readable.

Addconnection rpc should not support manual block relay, so seems here need not check?

It's true that it doesn't support this, just as it doesn't support INBOUND or MANUAL. But leaving it out her would result in a compiler warning for the switch, see comment below:
// no default case, so the compiler can warn about missing cases

What would be the difference between resolvedAddress and m_node_address?

m_node_address is a string, which is part of AddedNodeParams, which has just the unvalidated user input.

resolvedAddress is a CService built from m_node_address, which is generated in GetAddedNodeInfo and used in ThreadOpenConnection / rpc code.

perhaps there is a way to simplify it:

diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 6155af323..b9e082a63 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -304,22 +304,19 @@ static RPCHelpMan addnode()
         throw std::runtime_error(
             self.ToString());
     }
-    const std::string manual_name{ConnectionTypeAsString(ConnectionType::MANUAL)};
-    const std::string manual_block_relay_name{ConnectionTypeAsString(ConnectionType::MANUAL_BLOCK_RELAY)};
+
     if (strCommand == "remove" && !request.params[2].isNull()) {
         throw JSONRPCError(RPC_INVALID_PARAMS, "Cannot specify a connection type with 'remove'");
     }
-    std::string connection_type_name{manual_name};
+
+    const std::string manual_block_relay_name{ConnectionTypeAsString(ConnectionType::MANUAL_BLOCK_RELAY)};
+    ConnectionType connection_type{ConnectionType::MANUAL};
     if (!request.params[2].isNull()) {
-        connection_type_name = request.params[2].get_str();
-    }
-    ConnectionType connection_type;
-    if (connection_type_name == manual_block_relay_name) {
-        connection_type = ConnectionType::MANUAL_BLOCK_RELAY;
-    } else if (connection_type_name == manual_name) {
-        connection_type = ConnectionType::MANUAL;
-    }  else {
-        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Unsupported connection type: " + connection_type_name);
+        const std::string connection_type_name{request.params[2].get_str()};
+        if (connection_type_name == manual_block_relay_name)
+            connection_type = ConnectionType::MANUAL_BLOCK_RELAY;
+        else
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Unsupported connection type: " + connection_type_name);
     }
 
     NodeContext& node = EnsureAnyNodeContext(request.context);

Yes, thanks, that's a bit simpler and should be equivalent, so I took your suggestion!