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

Correct swap direction when permuting a list #774

Closed
mkalinin opened this issue Mar 14, 2019 · 2 comments
Closed

Correct swap direction when permuting a list #774

mkalinin opened this issue Mar 14, 2019 · 2 comments
Labels
general:enhancement New feature or request

Comments

@mkalinin
Copy link
Collaborator

Problem

There is a difference between @protolambda's optimised list shuffling and get_shuffling method in the spec. This difference is in the way permuted index is applied in order to get a shuffled list.

# spec:
output[i] = input[permuted(i)]
# protolamba:
output[permuted(i)] = input[i]

Both approaches seem to work well. But we need a consensus here.
As for me output[permuted(i)] = input[i] looks like a right direction cause then permute(index, value) can be written as (permute(index), value).

What do you guys think about that?

@protolambda
Copy link
Collaborator

Note that it's shuffling the input list in-place, see: https://github.com/protolambda/eth2-shuffle/blob/fd840f1036c1f8f6d7625ffe6ff4d9c60f942876/shuffle_test.go#L103

Admittedly, it's a big source of confusion in testing, but it the algorithm is still correct.
I.e. the input list is shuffled to become the output list.
So when you're reading input[i] after executing the shuffle, it's actually original_input[unpermuted(i)].

The thing is, I am applying and testing it with my semantic interpretation: "permute-index returns the permuted, i.e. the new position, for any given original list position", and then it places this validator at this new position.

The spec seems to intepret it differently: "permute index returns the original input position for any index in the output list", and then it fetches the validator from the original input.

I think my interpretation makes more sense when you're talking about "permuting". It's a change of the list.
But I like the approach of the spec better, because it makes it easy to reason about the subset of indices you're interested in, e.g. if you're looking for 1 particular validator or committee, you just query these indices, and get the corresponding validators.

vbuterin added a commit that referenced this issue Mar 15, 2019
See #729 and #774 

The behavior now is that the first committee will consist of `get_permuted_index(0..n-1)`, the second committee `get_permuted_index(n....2n-1)`, etc.
@hwwhww hwwhww added the general:enhancement New feature or request label Mar 19, 2019
@JustinDrake
Copy link
Collaborator

I think this is addressed in #776. Please reopen if not :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants