Update Shuffle Function to Swap-or-not #1643
Conversation
|
Many tests failing, can you please take a look? |
There was a problem hiding this comment.
@cyberbono3 This doesnt match the spec, its hard to follow what is going on. This is the current one in the spec
https://github.com/ethereum/eth2.0-specs/blob/dev/specs/core/0_beacon-chain.md#get_permuted_index
beacon-chain/utils/shuffle.go
Outdated
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/prysmaticlabs/prysm/shared/hashutil" | ||
| "github.com/prysmaticlabs/prysm/shared/params" | ||
| //"github.com/prysmaticlabs/prysm/shared/params" | ||
| ) | ||
|
|
||
| // ShuffleIndices returns a list of pseudorandomly sampled | ||
| // indices. This is used to shuffle validators on ETH2.0 beacon chain. | ||
| func ShuffleIndices(seed common.Hash, indicesList []uint64) ([]uint64, error) { |
There was a problem hiding this comment.
This is confusing this doesnt match the spec at all , this is the function in the spec
def get_permuted_index(index: int, list_size: int, seed: Bytes32) -> int:There also no assertions from the spec
assert index < list_size
assert list_size <= 2**40
beacon-chain/utils/shuffle.go
Outdated
| bs4 := make([]byte, 8) | ||
| listSize := len(indicesList) | ||
| num := int(math.Floor(float64((listSize + 255) / 256))) | ||
| powersOfTwo := []uint64{1, 2, 4, 8, 16, 32, 64, 128} |
There was a problem hiding this comment.
Please use the PowerOf2function in shared/mathutils for this
beacon-chain/utils/shuffle_test.go
Outdated
| t.Errorf("list 2 was incorrectly shuffled") | ||
| } | ||
|
|
||
| // if !reflect.DeepEqual(list1, []uint64{3,3,0,3,0,3,0,2,0,3}) { |
There was a problem hiding this comment.
dont comment out code, either check it in or remove it.
|
This implementation might be a helpful reference or library to use: https://github.com/protolambda/eth2-shuffle |
|
thanks for your feedback, guys ! |
|
Please fix tests |
beacon-chain/utils/shuffle.go
Outdated
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/prysmaticlabs/prysm/shared/hashutil" | ||
| "github.com/prysmaticlabs/prysm/shared/params" | ||
| //"github.com/prysmaticlabs/prysm/shared/params" |
There was a problem hiding this comment.
Please remove this, if you are not using. dont comment it out
|
Taking a look at this to see what I can do to help |
…ain test failed: shuffle result error: expected [], actual []
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
prestonvanloon
added
Blocked
|
Blocked by testnet feature freeze |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
|
This pull request has been closed due to inactivity. Please reopen this pull request if you would like to continue working on it. |
[Resolves] #1529
Description
I have implemented shuffle algorithm according to spec
I ran bazel test and majority of tests failed with timeout error.
I think with high probability my implementation is not correct.
I would appreciate your review and feedback.