Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework iroh-net address book #2646

Open
rklaehn opened this issue Aug 19, 2024 · 3 comments
Open

Rework iroh-net address book #2646

rklaehn opened this issue Aug 19, 2024 · 3 comments
Labels

Comments

@rklaehn
Copy link
Contributor

rklaehn commented Aug 19, 2024

I think #2612 is just one out of many examples that you need some more information about nodes in the address book.

We should have a database containing for each node id some information about the provenance (where we learned about it), and maybe also simple statistics.

While doing this, we should also rework persistence. Not sure how to best do it, maybe a redb database that is in mem by default and can be pointed to a file optionally?

@divagant-martian
Copy link
Contributor

we already have a provenance that we use for logging, but we don't store. So in a way we are halfway there

@flub
Copy link
Contributor

flub commented Aug 20, 2024

@rklaehn what information do you need other than what will need to be added for #2612?

We decided a while ago to explicitly move persistence out of iroh-net.

iroh-net internally keeps a whole bunch of information in the NodeState, one major important component of that is in the PathState. This is essentially the information that's made public in some way via Endpoint::remote_info (was Endpoint::connection_info) which I assume is what you consider to be the "address book".

I think it will be natural that as our understanding, implementation and requirements evolve that this structs will evolve. I think that's fine. I don't see a particular reason why involving a database here will make life easier, on the contrary.

So I don't think I can see benefits here for a major rework along the lines you are suggesting.


For context what I'm currently do want to work on is cleaning up the NodeState and PathState because right now they contain a bunch of conflicting information, and claim to provide information which they can't. Some fields are updated and used in the wrong ways and other such minor disasters. That's a quite delicate cleanup because obviously things work good enough to mostly work right now, but also there are many bugs because of this.

The major change I would like this to lead to is that whenever the NodeState/PathState is updated the new send address is computed and stored. Currently this computation happens on the hot-path when sending a datagram. I believe getting things cleaned up enough to get to that stage will have significant benefits.

@flub flub added the c-iroh label Aug 20, 2024
@rklaehn
Copy link
Contributor Author

rklaehn commented Oct 10, 2024

We don't have to make this overly complex provided that we have a way for an external entity to get every single thing that is related to discovery as an event.

Then for those cases that need something more complex than the logic we need for realtime, we can combine an event listener and a discovery mechanism to have the persistent storage external.

Note that this requires that the discovery trait allows a discovery subsystem to emit arbitrary provenances, which is currently the case. https://docs.rs/iroh-net/latest/iroh_net/discovery/struct.DiscoveryItem.html

@ramfox ramfox moved this to 📋 Backlog in iroh Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

3 participants