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

Add SWIP-19: Neighbourhood hopping #48

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Add SWIP-19: Neighbourhood hopping #48

wants to merge 13 commits into from

Conversation

dysordys
Copy link
Collaborator

This SWIP describes how to support neighbourhood hopping; i.e., moving a node to another neighbourhood without the need to pay the stake each time.

@dysordys dysordys self-assigned this May 13, 2024
@dysordys dysordys added the improvement enhancement of an existing protocol/strategy/convention label May 13, 2024
@zelig zelig changed the title Neighbourhood hopping SWIP fully specified SWIP-19: Neighbourhood hopping May 13, 2024
@n00b21337 n00b21337 changed the title SWIP-19: Neighbourhood hopping Add SWIP-19: Neighbourhood hopping May 28, 2024

## Context

Neighbourhood hopping is currently supported by the Bee client. The overlay address, which specifies which neighbourhood a node falls into, is derived by hashing the node key together with an arbitrary overlay nonce chosen by the user (refer to the figure below).
Copy link
Member

@janos janos Jun 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neighbourhood hopping is currently supported by the Bee client. is a questionable statement. As mentioned in this parameter, it is possible to mine an overlay address that falls into a specific neighbourhood, but neighbourhood hopping has to be defined in the terms of the feature requirements.

As the overlay address changes, syncing indexes and intervals in the database need to be adjusted, which will create a noticeable impact on the usability and performance of the node. I am not aware of an automated way to reindex syncing, either at the runtime or before the node is started. This has to be implemented on regards when neighbourhood hopping can happen, at the runtime or on start. Alternative is to remove all node storage, but is not ideal.

The frequency of overlay address changes is also important as it makes the network topology changes more frequent with a consequence of the higher syncing activity, or even p2p disconnections. The implementation may open a door for a potential disruptions of the network by very frequently changing the overlay address. The benefit is that the node needs to be resilient to that behaviour resulting a more stable node, but this aspect has to be taken into account for the implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you janos for raising these points.

Indeed how the node is supposed to do neighbourhood hopping should be discussed and written.
But I am not sure the details should part of this SWIP, rather they belong to documention.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yes, the node must nuke their db and sync from scratch in the new neighbourhood.
To optimise, the old node's reserve and cache should be the new node's cache.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good point about syncing and frequent hoppers.
unfortunately the frequent hopping attack can be played the same without stake, so this swip made no difference in that.
Mitigating against this attack must be part of the planned pull-sync upgrade SWIP. That proposal aspires to optimise the sync protocol by eliminating predictable redundancies that are the cause of excessive bandwidth.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zelig thanks for addressing these concerns. I agree that the documentation is the right place for details about the neighbourhood hopping procedure on the UX and internals. It is the case that current state of the implementation does not have a clear interface to perform the hopping from the UX perspective, by my opinion.

I support the effort to harden the syncing protocol in regards of the frequent hopping attack. That is indeed possible now, but I believe it would be easier to accomplish (or just be more obvious) when neighbourhood hopping is implemented and advertised.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feature is being supported on client start specifying target-neighborhood. corresponding PR and discussion

Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good proposal, I am just raising awareness of the impact from the Bee node implementation aspect.

Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it was strange for me first to have 2 events for the same change but for gas saving purposes it is fine.
The Bee code could change towards better UX at any time, than that may be rolled out first wit nuking. Would that be enough if initial implementation is referenced in the doc and other subsequent PRs just referenced on GitHub?

SWIPs/swip-19.md Outdated Show resolved Hide resolved
SWIPs/swip-19.md Outdated Show resolved Hide resolved

## Context

Neighbourhood hopping is currently supported by the Bee client. The overlay address, which specifies which neighbourhood a node falls into, is derived by hashing the node key together with an arbitrary overlay nonce chosen by the user (refer to the figure below).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feature is being supported on client start specifying target-neighborhood. corresponding PR and discussion

zelig and others added 8 commits June 6, 2024 15:22
Co-authored-by: nugaon <toth.viktor.levente@gmail.com>
Co-authored-by: nugaon <toth.viktor.levente@gmail.com>
remove obsolete function and write proper explanation
Update stake  endpoint
fix typos and wrong params
empty space
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement enhancement of an existing protocol/strategy/convention pull-request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants