[DO NOT MERGE] Bug in Gloas specs due to BUILDER_INDEX_FLAG

[leolara]

Bug description:

process_withdrawals (gloas) calls update_next_withdrawal_validator_index (capella), that in some situations reads a ValidatorIndex that since gloas could contain a builder index by BUILDER_INDEX_FLAG in some situations. But update_next_withdrawal_validator_index does not handle BUILDER_INDEX_FLAG and hence assumes a inexistent validator index.

My personal opinion:

BUILDER_INDEX_FLAG will cause bugs, not only in the specs but also on the clients, it requires that any method that handles a ValidatorIndex should be rewritten in order to avoid bugs. We should evaluate if adding more fields to the state could be a more robust solution.

[jtraglia]

@leolara you are confusing two separate things. This was a known issue & my mistake. Fixed here:

• Reserve space for one validator sweep withdrawal #4832

[leolara]

process_withdrawals (gloas) calls update_next_withdrawal_validator_index (capella), that in some situations reads a ValidatorIndex that since gloas could contain a builder index by BUILDER_INDEX_FLAG in some situations. But update_next_withdrawal_validator_index does not handle BUILDER_INDEX_FLAG and hence assumes a inexistent validator index.

and #4832

Is your opinion about how this specific bug caused by BUILDER_INDEX_FLAG should be fixed.

@jtraglia is this incorrect?

[jtraglia]

Is your opinion about how this specific bug caused by BUILDER_INDEX_FLAG should be fixed.

@jtraglia is this incorrect?

I'm saying BUILDER_INDEX_FLAG isn't the reason for the bug. Let's assume builders were validators like before. If the withdrawals were full (the first condition in update_next_withdrawal_validator_index), the result would still be incorrect as it would set the next sweep index to some random validator index.

I've already explained to you during our meeting why we went this direction, but feel free to propose an alternative to BUILDER_INDEX_FLAG. This PR is not the appropriate place for it though. Start a thread on discord.

[leolara]

I'm saying BUILDER_INDEX_FLAG isn't the reason for the bug. Let's assume builders were validators like before. If the withdrawals were full (the first condition in update_next_withdrawal_validator_index), the result would still be incorrect as it would set the next sweep index to some random validator index.

It would be a different problem and different behaviour.

I've already explained to you during our meeting why we went this direction, but feel free to propose an alternative to BUILDER_INDEX_FLAG. This PR is not the appropriate place for it though. Start a thread on discord.

My intention is not to start a discussion here, but documenting the bug.

[leolara]

Closing as it is included here: #4830