Feature Request: Allow Destination nodes for un-specified aliases

[carlaKC]

Describe desired feature

Right now, we allow nodes to specify a destination pubkey in their activity that is not one of the nodes defined in the nodes the simulator is configured with. This makes sense, because on public signets you might want to spray traffic out to nodes you don't control.

However, we don't currently allow specifying a destination alias for nodes that are not defined in our nodes. This makes some sense, because an alias isn't a unique identifier, and they're allowed as a convenience for people testing small networks.

Use case for feature

Aliases are more readable than pubkeys, so it could make sense to allow them in the same way as pubkeys. However, they're not unique so we'll need to treat them with some care.

I'd suggest:

• Allow aliases and look them up in our graph to find the corresponding pubkey
• If the alias is unique in the network, allow the simulation to run with the alias
• If the alias is not unique in the network, error our and print all the duplicate pubkeys so that the end user can pick the one that they intended to use

Would you like to contribute code for this feature?

No, this isn't very high priority for my use case - opening the issue for visibility / somebody else to pick up.

[chuksys]

I'd like to pick this up. Could you assign it to me?

[chuksys]

Hi @carlaKC , following your suggestion, I would like to share the approach I have in mind and would be glad to know your thoughts.

I think the core component for this feature is the network graph. I checked and it seems the graph isn't currently implemented for the real nodes (only SimNode). If this is so, I am thinking of adding a new function definition to the LightningNode trait and implementing it in the different real node implementations. The new function would fetch complete information regarding the channels a node is connected to (including NodeInfo of remote nodes) - I thought of doing this in list_channels (which currently only returns channel capacity) but I figured that might introduce breaking changes to parts of the codebase currently using that function. The information fetched in the new function would be used to create a complete network graph of the user's network which we can run a lookup against to see if the alias exists and if there's a duplicate. Please let me know what you think.

[chuksys]

    let (clients, clients_info) = get_clients(nodes).await?;
    // We need to be able to look up destination nodes in the graph, because we allow defined activities to send to
    // nodes that we do not control. To do this, we can just grab the first node in our map and perform the lookup.
    let get_node = async |pk: &PublicKey| -> Result<NodeInfo, LightningError> {
        if let Some(c) = clients.values().next() {
            return c.lock().await.get_node_info(pk).await;
        }

        Err(LightningError::GetNodeInfoError(
            "no nodes for query".to_string(),
        ))
    };

To me it seems get_node, in create_simulation in the parsing module, does this lookup using the provided destination public_key and the get_node_info function in the LightningNode trait CLN implementation, which filters all the nodes connected to the given node to see if a node with the public_key exists - I think the LND and Eclair implementations do something similar.

I could modify the get_node_info function to allow alias lookup as well or implement a new function in LightningNode that surfaces the graph so we can do the lookup in the parsing module. Would be glad to know your thoughts. Thanks.

[carlaKC]

I think the core component for this feature is the network graph.

This isn't the case. The issue is describing allowing something like this:

nodes [
   alice
  bob
]

activity [
   alice -> charlie
]

That's independent of the type of network we're running on - these could be real nodes or sim-node implementations.

[chuksys]

nodes [
alice
bob
]

activity [
alice -> charlie
]

I think I understand this part, the part I want some clarity on is the graph lookup.

Allow aliases and look them up in our graph to find the corresponding pubkey

It seems get_node_info is the function doing the graph lookup. If so, in SimNode, get_node_info calls a lookup_node fn which is part of SimGraph (which implements SimNetwork) but in the real node implementations, get_node_info does the lookup internally. The way it's done in SimNode seems to be cleaner - I would prefer replicating this in the other implementations, just wanted to know your thoughts about this just in case I am totally wrong about something here.

[carlaKC]

The way it's done in SimNode seems to be cleaner - I would prefer replicating this in the other implementations, just wanted to know your thoughts about this just in case I am totally wrong about something here.

We could be looking up any node in the network here, so to make this like SimNode we'd need to cache the entire graph in each of our lightning nodes, which definitley isn't something that we want to do.

AFAIK none of the implementation surface lookup by alias (for good reason), so you would need to add a get_graph method to the lightning node trait and then search through the graph for a node with the right alias. This is an expensive call, so we'd only want to make it once while we're validating our destinations.

If we're going to have to pull the whole graph anyway, we can probably look up our pubkey destinations here (rather than making multiple calls to get_node_info to fetch each).