Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Use little endian #361

Merged
merged 4 commits into from
Mar 4, 2019
Merged

Use little endian #361

merged 4 commits into from
Mar 4, 2019

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Mar 3, 2019

What was wrong?

Fix #303

How was it fixed?

Replace big endian conversions with little endian

TODO:

Cute Animal Picture

tiger-2196128_640

11, 20, 50, 30, 34, 49, 33, 59, 79, 62,
67, 96, 78, 8, 10, 99, 87, 27, 32, 23,
),
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the testcase since we will have test suite.

17: 0,
18: 0,
19: 75, # 3 * (100 // 4)
19: 125, # 5 * (100 // 4)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if I updated it correctly or not... ping @NIC619

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's correct.
The inclusion_slots parameters above is updated to:

                2: 31,  # proposer index for inclusion slot 31: 6
                3: 31,
                4: 32,  # proposer index for inclusion slot 32: 12
                5: 32,
                6: 32,
                9: 35,  # proposer index for inclusion slot 35: 19
                10: 35,
                11: 35,
                12: 35,
                13: 35,
                15: 38,  # proposer index for inclusion slot 38: 8
                16: 38,
                17: 38,

state.slot,
CommitteeConfig(config),
)
slashing_proposer_index = (whistleblower_index + 1) % len(state.validator_registry)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was interesting to find that after the endianness changes shuffling result, whistleblower_index is as same as the slashing_proposer_index!
Change it so that the slashing_proposer_index won't be equal to whistleblower_index.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm probably mistaken but why was whistleblower tied to shuffling instead of anyone can be whistleblower?

Copy link
Contributor Author

@hwwhww hwwhww Mar 4, 2019

Choose a reason for hiding this comment

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

We take the block proposer who includes the ProposerSlashing operation as the whistleblower. see https://github.com/ethereum/eth2.0-specs/blob/dev/specs/core/0_beacon-chain.md#slash_validator

 whistleblower_index = get_beacon_proposer_index(state, state.slot)

So changing shuffling would change the whistleblower in this test.

@hwwhww hwwhww changed the title [WIP] Use little endian Use little endian Mar 4, 2019
@hwwhww hwwhww requested a review from jannikluhn March 4, 2019 05:23
@hwwhww hwwhww requested a review from NIC619 March 4, 2019 05:24
Copy link
Contributor

@NIC619 NIC619 left a comment

Choose a reason for hiding this comment

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

LGTM

state.slot,
CommitteeConfig(config),
)
slashing_proposer_index = (whistleblower_index + 1) % len(state.validator_registry)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm probably mistaken but why was whistleblower tied to shuffling instead of anyone can be whistleblower?

@hwwhww hwwhww merged commit 8a7814d into ethereum:master Mar 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Little endian
2 participants