Skip to content

Conversation

@potuz
Copy link
Contributor

@potuz potuz commented Jul 26, 2022

in the function withdraw_balance the variable index takes different meanings and types. This confused at least one member of our team.

@ralexstokes
Copy link
Member

fwiw the types do not change and I'd say this implies the meaning does not change but I'd support this PR if it clarifies things

@potuz
Copy link
Contributor Author

potuz commented Jul 27, 2022

fwiw the types do not change and I'd say this implies the meaning does not change but I'd support this PR if it clarifies things

I think the confusing thing is statements like

        index=state.next_withdrawal_index,
        address=ExecutionAddress(state.validators[index].withdrawal_credentials[12:]),

Where in one line index is assigned a WithdrawalIndex and in the very next line, index is used with the ValidatorIndex that was passed to the function.

Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

Agreed that index=state.next_withdrawal_index, address=ExecutionAddress(state.validators[index].withdrawal_credentials[12:]) is confusing. 👍

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

makes sense!

@djrtwo djrtwo merged commit fb71509 into ethereum:dev Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants