Get node ILP Address from parent

[emschwartz]

Right now the node needs to be configured with an ILP address. If you are running a node as a "child" of another, it would be easier and more reliable to automatically query the parent node for the ILP address. (This would also mean that with #186, the ILP address of each account on the node will be correct without any configuration, whereas right now you can mess it up by using an un-routable address)

• Make ilp_address optional in the node configuration
• If ilp_address is not provided, make an ILDCP query to get the address
• Figure out which account to send the ILDCP query to (Should there be an index of accounts listed as Parents? If there are multiple, which one should we use?)
• What happens when the node is run with no accounts configured? Should there be a hook on the account creation API that would recognize when a "parent" is added that would send the ILDCP query and update the node's address?

[gakonst]

A few things (many are CCP related):

---

From the ILDCP Spec:

if the node has a parent in the network hierarchy, the ILP address of the child node can be allocated to it by the parent. This is done using ILDCP.

Based on that and your comment:

Figure out which account to send the ILDCP query to (Should there be an index of accounts listed as Parents? If there are multiple, which one should we use?)

This makes me believe that it only makes sense for a node to ever have 1 parent. Is that the case, @emschwartz?

---

example topology (for my reference)

--------------------  g.a.a    g.a.b     g.a.c <- children of `a` all must have the same parent and cannot have another one
|                       \       |        /     
|                         ----- g.a -----
|           --------------------| 
| p       /                    
| e     g        `a` and `b` are both children of `g`
| e       \
| r         --------------------|
|                         ----- g.b -----
|                       /      |        \
|------------------   g.b.a    g.b.b     g.b.c
                                        / \ 
                                        ... 
                                          ^children of `b.c                             

Assuming only child/parent relations, a payment such as g.b.a -> g.a.c:
g.b.a -> g.b -> g -> g.a -> g.a.c.
With the peer connection between g.b.a and g.a.a this becomes g.b.a -> g.a.a -> g.a -> g.a.c which is once less hop. Does the use case for peer-relationships end up being only for nodes which are "far" from each other in the network, who opt to peer directly as Peers to improve the network overhead?

Based on that, shouldn't Peer relations have priority over Child/Parent ones? https://github.com/interledger-rs/interledger-rs/blob/master/crates/interledger-ccp/src/server.rs#L798-L799

---

Currently, we have two booleans, send_routes, and receive_routes which are set during account creation. Can it ever be the case that an account is inserted to a node as Child, and we'd want to set receive_routes = true? I would expect that anytime a Child node tries to broadcast its routes to us, we should reject them. Similarly, whenever we set a node as a Parent, we should definitely receive routes from them.

Does this mean we can remove receive_routes and send_routes and from CcpRoutingAccount and just use the routing_relation to judge if we should drop broadcast packets or not [1] [2]? I guess their main use is when 2 nodes are connected together with a Peer relationship, but wouldn't they both be true in that case?

---

Should we also add a blacklist on the CCP Service which is loaded in memory/from the store so that filter_routes can remove any routes to such calls (e.g. known nodes which a connector should avoid doing interacting with)

---

Could we remove the spawn_tasks variable and use cfg(test) in code-blocks like this? As per the comment in spawn_tasks' definition, this seems to be a variable related to tests.

---

Do we also want an account's address to dynamically adjust if any of their parents changes their parent connection or thier own ILP Address?

e.g. A: (g.a) is the parent of B: (g.a.test). For whatever reason, B removes A from being its Parent and replaces A with C as the new Parent: (g.c). Should B become g.c.test? What happens to all of B's accounts for the time that it has no parent accounts?

[sappenin]

This makes me believe that it only makes sense for a node to ever have 1 parent. Is that the case,

My understanding is that a node could have multiple parents (though I would be open to restricting that) as long as it only performs IL-DCP with one of them.

In Java I just choose the first account that is a parent, but we should probably have a smarter way to indicate which parent to use? Or else maybe restrict the number of parent nodes?

[sappenin]

Can it ever be the case that an account is inserted to a node as Child, and we'd want to set receive_routes = true?

I would say, "no" because one of the primary purposes of the "child" classification is to indicate that the parent node doesn't really trust the routing tables of that child peer. If a parent node does trust a child, it should classify that account as a peer instead.

I would expect that anytime a Child node tries to broadcast its routes to us, we should reject them. Similarly, whenever we set a node as a Parent, we should definitely receive routes from them.

That's my understanding. JS and Java implementations do this -- although in fairness, Java is a copy of JS, and just because JS did it doesn't mean it's correct.

[sappenin]

Does this mean we can remove receive_routes and send_routes and from CcpRoutingAccount and just use the routing_relation to judge if we should drop broadcast packets or not [1] [2]? I guess their main use is when 2 nodes are connected together with a Peer relationship, but wouldn't they both be true in that case?

Curious to hear more opinions here, especially from @emschwartz. I suppose it could be the case that there is some other reason to accept routing updates from a child peer?

[emschwartz]

I opened separate issues to discuss the questions @gakonst brought up.

Could we remove the spawn_tasks variable and use cfg(test) in code-blocks like this? As per the comment in spawn_tasks' definition, this seems to be a variable related to tests.

Sounds reasonable to me.