[interop] Epoch processing - SSZ hash of active_index_roots issue in final updates

[mratsim]

See #372 (comment)

Impacted line: https://github.com/status-im/nim-beacon-chain/blob/297e9079d47f6070e06f425e8999eb46546029c8/beacon_chain/spec/state_transition_epoch.nim#L488-L489

unit test: https://github.com/status-im/nim-beacon-chain/blob/297e9079d47f6070e06f425e8999eb46546029c8/tests/official/test_fixture_state_transition_epoch.nim#L83-L88

[zah]

Have we ruled out the possibility that get_active_validator_indices just returns a different set of validators compared to the Python spec in this particular case?

[mratsim]

If I skip the line active_index_roots: https://github.com/status-im/nim-beacon-chain/blob/297e9079d47f6070e06f425e8999eb46546029c8/beacon_chain/spec/state_transition_epoch.nim#L485-L494

The final state has the correct value (off-by-one processing maybe?) but we have the wrong committees root (skipped or not). It also needs to get the sszList treatment:
https://github.com/status-im/nim-beacon-chain/blob/297e9079d47f6070e06f425e8999eb46546029c8/beacon_chain/spec/beaconstate.nim#L176-L201

Even with sszList we still have the wrong compact_committees_roots though.

And in general, I think all our seq/arrays need to be revisited to make sure they use sszList. Note that at the moment sszList doesn't work with plain arrays due to maxChunksCount not supporting them, they need to be converted to seq first.

Sidenote: int literals are not automatically converted to static int64 and return a confusing error (type mismatch with no highlight that it was the static failing)

[zah]

I've tried to review all usages of hash_tree_root and didn't find any additional places where the sszList treatment is necessary. In general, it should be necessary only when you are passing a sequence to hash_tree_root. I can try to turn this into a compile-time error instead.

The fixed-size array merkleization code seems to have an issue though - I'll try to push a fix tomorrow.

Can you give me some pointers regarding how you run the same code in Python in order to compare the results?

[mratsim]

The first thing to do is extracting the python code from the markdown spec with: https://github.com/ethereum/eth2.0-specs/tree/dev/test_libs/pyspec#manual

I suggest venv but make might work.

If needed the mocking proc are hidden in the utils folder: https://github.com/ethereum/eth2.0-specs/tree/dev/test_libs/pyspec/eth2spec/test/helpers

The test generators for final updates is here: https://github.com/ethereum/eth2.0-specs/blob/dev/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_final_updates.py

The dumps are generated here: https://github.com/ethereum/eth2.0-specs/blob/dev/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/run_epoch_process_base.py#L36
Basically, it runs epoch processing until final_updates, snapshots the state into pre.ssz/pre.yaml, run final_updates, snapshots the state into post.ssz/post.yaml

[mratsim]

I've edited the original python code with debug prints on preset minimal.

def process_final_updates(state: BeaconState) -> None:
    current_epoch = get_current_epoch(state)
    next_epoch = Epoch(current_epoch + 1)
    # Reset eth1 data votes
    if (state.slot + 1) % SLOTS_PER_ETH1_VOTING_PERIOD == 0:
        state.eth1_data_votes = []
    # Update effective balances with hysteresis
    for index, validator in enumerate(state.validators):
        balance = state.balances[index]
        HALF_INCREMENT = EFFECTIVE_BALANCE_INCREMENT // 2
        if balance < validator.effective_balance or validator.effective_balance + 3 * HALF_INCREMENT < balance:
            validator.effective_balance = min(balance - balance % EFFECTIVE_BALANCE_INCREMENT, MAX_EFFECTIVE_BALANCE)
    # Set active index root
    index_epoch = Epoch(next_epoch + ACTIVATION_EXIT_DELAY)
    index_root_position = index_epoch % EPOCHS_PER_HISTORICAL_VECTOR
    indices_list = List[ValidatorIndex, VALIDATOR_REGISTRY_LIMIT](get_active_validator_indices(state, index_epoch))

    print(f'\nstate hash_tree_root before: {hash_tree_root(state).hex()}')
    print(f'indices_list: {indices_list}')
    print(f'indices_list hash: {hash_tree_root(indices_list).hex()}')
    state.active_index_roots[index_root_position] = hash_tree_root(indices_list)
    # Set committees root
    committee_root_position = next_epoch % EPOCHS_PER_HISTORICAL_VECTOR
    state.compact_committees_roots[committee_root_position] = get_compact_committees_root(state, next_epoch)
    # Reset slashings
    state.slashings[next_epoch % EPOCHS_PER_SLASHINGS_VECTOR] = Gwei(0)
    # Set randao mix
    state.randao_mixes[next_epoch % EPOCHS_PER_HISTORICAL_VECTOR] = get_randao_mix(state, current_epoch)
    ...

Here is the output:

collecting ... Running test_eth1_vote_no_reset
Running test_eth1_vote_reset
Running test_effective_balance_hysteresis
Running test_historical_root_accumulator
Running test_compact_committees_root
collected 5 items                                                                                                           

eth2spec/test/phase_0/epoch_processing/test_process_final_updates.py 
state hash_tree_root before: 9e781c7d6d991e1b66bf4a084010a2cd219995ce082050a8650e42ba59cc9d8e
indices_list: List[ValidatorIndex, 1099511627776](0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63)
indices_list hash: ba1031ba1a5daab0d49597cfa8664ce2b4c9b4db6ca69fbef51e0a9a325a3b63
.
state hash_tree_root before: e3d5626df47a0438d8e5469be4a5919978a3eaff171b0b497d3555132991de6e
indices_list: List[ValidatorIndex, 1099511627776](0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63)
indices_list hash: ba1031ba1a5daab0d49597cfa8664ce2b4c9b4db6ca69fbef51e0a9a325a3b63
.
state hash_tree_root before: f7821679723095c46efefefb5d15959e9be48a551190f06c51eec54953ede8c0
indices_list: List[ValidatorIndex, 1099511627776](0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63)
indices_list hash: ba1031ba1a5daab0d49597cfa8664ce2b4c9b4db6ca69fbef51e0a9a325a3b63
.
state hash_tree_root before: 5e65e1d465eb5826552aedc6b8306d6748b7623963487937b7f4008c301e4d04
indices_list: List[ValidatorIndex, 1099511627776](0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63)
indices_list hash: ba1031ba1a5daab0d49597cfa8664ce2b4c9b4db6ca69fbef51e0a9a325a3b63
.
state hash_tree_root before: e219017c5d2f6b6bcccdc71f6583c38640c7275218dec0ec586bed43ec5bc6c0
indices_list: List[ValidatorIndex, 1099511627776](0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63)
indices_list hash: ba1031ba1a5daab0d49597cfa8664ce2b4c9b4db6ca69fbef51e0a9a325a3b63
.

And in our case (order is not kept and the OK is because I reused the test suite)

[Suite] Official - Epoch Processing - Final updates [Preset:  [Preset: minimal]
state hash_tree_root before: 9E781C7D6D991E1B66BF4A084010A2CD219995CE082050A8650E42BA59CC9D8E
indices_list: @[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
indices_list hash: 16E7DA2912B78AE1AA7558D8927DF8D3B35E2B039AF9E7C80E6B7269403C0E4B
  [OK] Final updates - eth1_vote_no_reset [Preset: minimal]
state hash_tree_root before: F7821679723095C46EFEFEFB5D15959E9BE48A551190F06C51EEC54953EDE8C0
indices_list: @[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
indices_list hash: 16E7DA2912B78AE1AA7558D8927DF8D3B35E2B039AF9E7C80E6B7269403C0E4B
  [OK] Final updates - effective_balance_hysteresis [Preset: minimal]
state hash_tree_root before: 5E65E1D465EB5826552AEDC6B8306D6748B7623963487937B7F4008C301E4D04
indices_list: @[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
indices_list hash: 16E7DA2912B78AE1AA7558D8927DF8D3B35E2B039AF9E7C80E6B7269403C0E4B
  [OK] Final updates - historical_root_accumulator [Preset: minimal]
state hash_tree_root before: E3D5626DF47A0438D8E5469BE4A5919978A3EAFF171B0B497D3555132991DE6E
indices_list: @[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
indices_list hash: 16E7DA2912B78AE1AA7558D8927DF8D3B35E2B039AF9E7C80E6B7269403C0E4B
  [OK] Final updates - eth1_vote_reset [Preset: minimal]
state hash_tree_root before: E219017C5D2F6B6BCCCDC71F6583C38640C7275218DEC0EC586BED43EC5BC6C0
indices_list: @[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63]
indices_list hash: 16E7DA2912B78AE1AA7558D8927DF8D3B35E2B039AF9E7C80E6B7269403C0E4B
  [OK] Final updates - compact_committees_root [Preset: minimal]

VALIDATOR_REGISTRY_LIMIT is correctly set at 1099511627776 so it seems like a ssz hashing bug of the 0 ..63 list

[mratsim]

As get_compact_committees_root has an issue beyond SSZ, I've opened a new issue here - #378, and will edit delete my comments to reduce thread pollution.

The new threads also has detailed reproduction instructions that you can use for this active_indices_root

[zah]

BTW, I had to resort to this patter of editing the Python spec myself while I was working on the SSZ suite. I think it would be valuable if we push our modifications to Status's own eth2.0-specs repo in order to make it easier for everyone to replicate the process. We can try to keep the individual testing modifications and scripts in separate branches that can be rebased and mixed in arbitrary ways. Here is my branch for SSZ tracing for example:

https://github.com/status-im/eth2.0-specs/commits/ssz-tracing