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

Layer selection #664

Merged
merged 12 commits into from
Jul 1, 2021
Merged

Layer selection #664

merged 12 commits into from
Jul 1, 2021

Conversation

neacsu
Copy link
Contributor

@neacsu neacsu commented Jun 30, 2021

No description provided.

@jstuczyn
Copy link
Contributor

I see you interpreted the ticket slightly differently than what I initially had in mind. I'm not saying it's good or wrong. I guess we would need an external pair of eyes on it, @durch or @futurechimp . What I had in mind was to completely get rid of layer argument from the mixnode side (i.e. remove it from the data you sent to the contract) - in the long run mixnode should have no say in what layer it's gonna reside on because it might change dynamically.

So basically when mixnode bonds, contract says "hey, thanks for bonding, you're now on layer X"

@neacsu
Copy link
Contributor Author

neacsu commented Jun 30, 2021

What I had in mind was to completely get rid of layer argument from the mixnode side (i.e. remove it from the data you sent to the contract)

Yep, I thought of that originally too. I kind of changed my mind on the way though, as it seemed to offer more flexibility for the mixnode operator.

in the long run mixnode should have no say in what layer it's gonna reside on because it might change dynamically.

If that is the case, then I can remove it completely now and be done with it

Copy link
Contributor

@jstuczyn jstuczyn left a comment

Choose a reason for hiding this comment

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

One additional comment: we could now remove LayerDistribution.invalid field as it will be impossible to set it. Perhaps it might be worth to have a choose_with_fewest method on it to more easily find appropriate layer?

mixnode/src/commands/run.rs Outdated Show resolved Hide resolved
mixnode/src/commands/upgrade.rs Outdated Show resolved Hide resolved
mixnode/src/config/mod.rs Outdated Show resolved Hide resolved
@neacsu neacsu requested a review from jstuczyn July 1, 2021 13:29
neacsu added 11 commits July 1, 2021 16:36
The layer is currently computed locally. This should be moved to
the validator, and the mixnode should find this information via
a query. Until the query is actually processed, we keep the layer
field as None.

Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Predictably compute the best layer of a bonding mixnode by putting it on
the layer with the fewest mixnodes and, in case of equality, with the
smallest index e.g. layer 1 is better then layer 3 if both have
the fewest mixnodes.

The layer received from the client app is regarded as a preferred option
and is only accepted if the number of nodes on the preferred layer is not
bigger then the one on the best layer.

Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
The config file is changed so that it's only containing the layer if
the layer exists.

Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Remove the option of a client app to propose a preferred
layer.

Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
... because layer choice is now present only in the contract.

Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
pub sphinx_key: SphinxKey,
/// Base58 encoded ed25519 EdDSA public key.
pub identity_key: IdentityKey,
pub version: String,
}

#[derive(Copy, Clone, Debug, Deserialize, PartialEq, Serialize, JsonSchema)]
pub enum Layer {
Copy link
Contributor

Choose a reason for hiding this comment

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

More type-safety - I like it

Signed-off-by: Bogdan-Ștefan Neacșu <bogdan@nymtech.net>
@neacsu neacsu merged commit 57df77a into develop Jul 1, 2021
@neacsu neacsu deleted the layer_selection branch July 1, 2021 14:56
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

Successfully merging this pull request may close these issues.

None yet

2 participants