-
Notifications
You must be signed in to change notification settings - Fork 226
Parachain multiplexing #295
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
Conversation
dvdplm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this will be fairly painless. :)
backend/src/node/connector.rs
Outdated
| backlog: Vec<NodeMessage>, | ||
| backlogs: BTreeMap<ConnId, Vec<NodeMessage>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR, but is it ok that the backlog is unbounded? Maybe we could re-work this into a bounded queue?
EDIT: I see it is bounded to 10 below. Let's document that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's bounded. I'm more worried about the number of ConnId being unbounded atm.
| backlog.remove(0); | ||
| } | ||
|
|
||
| backlog.push(msg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is the fix here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue was that I had a separate backlog and Addr<Chain> per ConnId, but was using a single NodeId. Now the NodeId is tied to ConnId, everything else (using the enum for ConnMultiplex) is a refactor to make the intent clearer, and to avoid having 3 separate BTreeMaps for no good reason (one for chain address, one for backlog, one for node id).
Companion PR to paritytech/substrate#7463
Expects
idfield on messages (defaults to0if absent), messages sent with differentids are treated as separate connections and can push the node to different chains.Unrelated: fixed continuation frames that sometimes errors on current build.