EIP6110: add validator index cache

[terencechain]

This PR introduces a new validator index that complies with EIP-6110, specifically adhering to the validator index invariant constraint detailed in this section. Below are the design considerations:

• The cache is located within the native-state package adjacent to its usage at (b *BeaconState) ValidatorIndexByPubkey.

• The cache is shared among all BeaconState instances. When a state is copied, only the reference to the cache is copied.

• The cache maintains a map of public key to index and includes a lock mechanism.

• A feature flag eip6110-validator-cache has been introduced to enable risk-free merging into the develop branch and facilitate testing, allowing easy toggling on and off as needed.

• A new beacon state helper getter, validatorsReadOnlySinceIndex(index int), has been implemented. This helper enables the state to return an array of read only validator pointers after the specified index. After reviewing all beacon state helpers for retrieving validators, none proved sufficient for the following reasons:

  • ValidatorsReadOnly() initializes every validator, significantly increasing heap usage.
  • PublicKeys() copies all public keys across the validator set.
  • PubkeyAtIndex(idx primitives.ValidatorIndex) requires frequent locking and unlocking, which is suboptimal in the validator loop.
  • ReadFromEveryValidator(f func(idx int, val ReadOnlyValidator) error) also necessitates frequent lock changes for similar reasons as above.
  • PublicKeySlice is added for read only validator to return public key slice without copying to 48 bytes

• For the read usage side, nothing needs to be done. For the write usage side, we need to call SaveValidatorIndices() when a new finalized state is emitted and when the node starts up. This part is TBD

[kasey]

I was going to say it would be better for this to return a slice of validators, which would be super cheap if we had normal slices, but I suppose mv slice messes that up. Can mv slice return a go slice without making a bunch of copies?

Even it if has to copy validator pointers, that's still going to be a lot less than copying all the keys.

Another option is to design this as a method like findValidator that takes a callback ("query") and iterates through the validators until the callback returns true, then return the matching validator. Or similar idea but it iterates until the callback tells it to stop, and then the callback grabs the validator it wants via enclosure.

[terencechain]

Re: pointer, there is ValidatorsReadOnly() []state.ReadOnlyValidator which returns a list of

type readOnlyValidator struct {
	validator *ethpb.Validator
}

Even then, 1M read-only validators (pointers) seem to be a lot for every deposit. I think it'd still make sense to implement ValidatorsSinceIndex(index int). Do you agree with that?

[kasey]

Yes I thought the point about only iterating validators since the index was implied - ValidatorsSinceIndex(index int) is actually what I had in mind :)

[terencechain]

Thanks. Updated the code to do that : )
Unfortunately, the read-only validator public key getter also requires a copy. I added a slice version

func (v readOnlyValidator) PublicKey() [fieldparams.BLSPubkeyLength]byte {
	var pubkey [fieldparams.BLSPubkeyLength]byte
	copy(pubkey[:], v.validator.PublicKey)
	return pubkey
}

[rkapka]

removing the field name will allow you to write validatorIndexCache.Lock() directly

[rkapka]

This will also return the last finalized validator, which is not a bug but also not exactly what we want. The "correct" thing to do is to call the function with finalizedIndex + 1 and later write index := primitives.ValidatorIndex(finalizedIndex + i + 1). Similarly for saveValidatorIndices.

[terencechain]

I think it's better to update publicKeysSinceIndexReadOnly to return i+1. Example:
[0,1,2,3,4], where input 2 should return [3,4], not [2,3,4]

I ended up removing the index and just used len(indexMap) to serve the same purpose. It's cleaner

[rkapka]

This should track only finalized validators, so I would rename it to finalizedValidatorIndexCache

[rkapka]

This function can be unexported

[rkapka]

Suggested change

	// We should get able to retrieve these validators by public key even they are not in the cache
	// We should get able to retrieve these validators by public key even when they are not in the cache

[rkapka]

Suggested change

	// We should get able to retrieve these validators by public key even they are not in the cache
	// We should be able to retrieve these validators by public key even when they are not in the cache

[rkapka]

Suggested change

	// We should get able to retrieve these validators by public key even they are not in the cache
	// We should be able to retrieve these validators by public key even when they are not in the cache

[rkapka]

One additional thing you might want to check is retrieving a saved index

[rkapka]

Suggested change

	// State is finalized. We flush [0, 1, 2 ] to the cache.
	// State is finalized. We save [0, 1, 2] to the cache.

This is nitpicky, but technically "flush" would mean these indices are no longer present in the state. Someone reading the test might get the wrong impression of what the code does.

[rkapka]

Suggested change

	// State is finalized. We flush [4, 5] to the cache.
	// State is finalized. We save [4, 5] to the cache.