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

swarm: remove duplication between SwarmEvent and FromSwarm #4268

Open
thomaseizinger opened this issue Jul 29, 2023 · 2 comments
Open

swarm: remove duplication between SwarmEvent and FromSwarm #4268

thomaseizinger opened this issue Jul 29, 2023 · 2 comments

Comments

@thomaseizinger
Copy link
Contributor

Description

Today, SwarmEvent is essentially a super-set of FromSwarm. This super-set is however not expressed in the type-system which leads to inconsistencies as described in #4251.

We can fix this by expressing SwarmEvent as a composition of FromSwarm and ToSwarm (a type parameter that will be filled in by NetworkBehaviour::ToSwarm.

As far as I know, the only thing that is blocking this is #3046 which is "just" waiting for the next round of breaking changes.

@thomaseizinger
Copy link
Contributor Author

There is another thing we need to figure out: FromSwarm currently has a lifetime to make it cheap to the event to the behaviour tree. But all the data in the current SwarmEvent variants is owned!

If we want to reuse FromSwarm within SwarmEvent, I think we'll have to have some kind of Bow (BorrowedOrOwned) type that doesn't impose ToOwned because some of the error types in FromSwarm don't implement that.

Generally, it does seem to be a bit of a mis-match. Within NetworkBehaviour, we will always have borrowed data and within SwarmEvent, it will always be owned ...

@thomaseizinger
Copy link
Contributor Author

Perhaps, what we need is to move the references from within the types like ConnectionEstablished to the enum itself:

pub enum FromSwarm {
    /// Informs the behaviour about a newly established connection to a peer.
    ConnectionEstablished(ConnectionEstablished),
}

And the signature will change to:

fn on_swarm_event(&mut self, event: &FromSwarm);

That is a bit annoying because it means that some data which could be owned because it is Copy will also be borrowed.

@thomaseizinger thomaseizinger removed this from the v0.53.0 milestone Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant