Block deposit: refactor batch verify and validation activation #8698
Conversation
beacon-chain/core/blocks/deposit.go
Outdated
| @@ -55,7 +64,7 @@ func ProcessPreGenesisDeposits( | |||
| return nil, err | |||
| } | |||
| } | |||
| return beaconState, nil | |||
| return nil, nil | |||
There was a problem hiding this comment.
This function should not return a beacon state at all.
There was a problem hiding this comment.
I think you meant it should
This is just a clean refactor. There's no logic changes. The main goal here is to avoid duplicated code across forks |
…sm into block-deposit-refactor
beacon-chain/core/blocks/deposit.go
Outdated
| return false, err | ||
| } | ||
|
|
||
| var verified bool |
There was a problem hiding this comment.
This var name is pretty confusing for its actual use case, as on a first read it would look like the signature is verified when in truth it means the opposite
There was a problem hiding this comment.
It's more standard than verifySignature which signals that the variable is actually a function.
Verified doesn't always imply true, it reads more as the act of verifying. ie. the state of being verified
In our code base, we do use verified as boolean through out
One way to improve is to make it verified := false
What type of PR is this?
What does this PR do? Why is it needed?
Refactor functions
activateValidatorWithEffectiveBalanceandbatchVerifyDepositsSignaturesso they can be reused for Altair