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

nodebuilder/p2p | nodebuilder/header: Ensure bootstrappers / trusted peers cannot be blocked by the host #2702

Open
renaynay opened this issue Sep 14, 2023 · 7 comments

Comments

@renaynay
Copy link
Member

Ensure that bootstrappers and trusted peers cannot be blocked by the instance of the libp2p host (ensure they are protected).

@Wondertan
Copy link
Member

@Walldis, why do we need it now?

@walldiss
Copy link
Member

@Wondertan Answered in sync. It is not top priority right now for sure. All other top priority issues are under development by other people and I can't contribute to those at current prototyping stage. Also I was looking to what I can work on, while waiting for cache reviews. This one is relatively useful and fast to implement.

@walldiss
Copy link
Member

@renaynay Why do we want to protect trusted peers specifically? Seems like same could be achieved by specifying same peers as mutual peers. Those are protected from being trimmed. At the same time protecting peer has nothing to do with blacklisting in connection gater.

So I would suggest update an issue a bit. Modify it to require to never blacklist (conngater property) bootstrapers and mutual peers on top of protecting them (connmanager property). Would it satisfy functional requirements of this issue or you wanted to achieve something else?

@Wondertan
Copy link
Member

@Waldiss, My main issue with it, is that it's usually a sign of some other problem, like buggy verification on the go-header side.

The fix here won't fix the example bug and the node will likely go in an endless cycle of requests to bootstrappers with failed to verify errors.

So I see this issue as a preventive measure that doesn't even guarantee to work and thus I see this having the lowest priority.

@renaynay
Copy link
Member Author

Why do we want to protect trusted peers specifically? Seems like same could be achieved by specifying same peers as mutual peers. Those are protected from being trimmed.

When a user specifies trusted peers, or uses the ones provided (bootstrappers), the assumption is that those peers are acting honestly and therefore have no reason to be blacklisted. Nodes should try to maintain connections to those trusted peers that they've specified.

The fact that we ran into the issue that some trusted peers were blacklisted is a bug in go-header code (which I am working on resolving), but I think regardless, we should protect those peers from being trimmed / banned.

And yes @Wondertan is right that its a symptom and is lower prio

@walldiss
Copy link
Member

Just want to add, that Bootstrapper / mutual peers could also be blacklisted by peer-manager, which has no idea about what type of peer he is dealing with. Blacklisting is disabled by default in peer-manager, but it is still could be enabled by config. Still not a high priority, but implementing this issue would be required to enable blacklisting in peer-manager.

@Wondertan
Copy link
Member

Still not a high priority, but implementing this issue would be required to enable blacklisting in peer-manager.

Not really, if peer manager ends up blocking bootstrappers, it means peer man is buggy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants