Unregister validator - fix behind feature flag #12316
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package cache | ||
|
|
||
| import ( | ||
| "context" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "github.com/pkg/errors" | ||
| "github.com/prysmaticlabs/prysm/v4/config/params" | ||
| "github.com/prysmaticlabs/prysm/v4/consensus-types/primitives" | ||
| "github.com/prysmaticlabs/prysm/v4/encoding/bytesutil" | ||
| "github.com/prysmaticlabs/prysm/v4/math" | ||
| ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1" | ||
| log "github.com/sirupsen/logrus" | ||
| "go.opencensus.io/trace" | ||
| ) | ||
|
|
||
| // RegistrationCache is used to store the cached results of an Validator Registration request. | ||
| // beacon api /eth/v1/validator/register_validator | ||
| type RegistrationCache struct { | ||
| indexToRegistration map[primitives.ValidatorIndex]*ethpb.ValidatorRegistrationV1 | ||
| lock sync.RWMutex | ||
| } | ||
|
|
||
| // NewRegistrationCache initializes the map and underlying cache. | ||
| func NewRegistrationCache() *RegistrationCache { | ||
| return &RegistrationCache{ | ||
| indexToRegistration: make(map[primitives.ValidatorIndex]*ethpb.ValidatorRegistrationV1), | ||
| lock: sync.RWMutex{}, | ||
| } | ||
| } | ||
|
|
||
| // GetRegistrationByIndex returns the registration by index in the cache and also removes items in the cache if expired. | ||
| func (regCache *RegistrationCache) GetRegistrationByIndex(id primitives.ValidatorIndex) (*ethpb.ValidatorRegistrationV1, error) { | ||
prestonvanloon marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Optional: effective go guidelines suggest that you don't use the prefix "Get" for a getter and simply call it the thing like |
||
| regCache.lock.RLock() | ||
| v, ok := regCache.indexToRegistration[id] | ||
| if !ok { | ||
| regCache.lock.RUnlock() | ||
| return nil, errors.Wrapf(ErrNotFoundRegistration, "validator id %d", id) | ||
| } | ||
| isExpired, err := RegistrationTimeStampExpired(v.Timestamp) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "failed to check registration expiration") | ||
| } | ||
| if isExpired { | ||
| regCache.lock.RUnlock() | ||
| regCache.lock.Lock() | ||
| defer regCache.lock.Unlock() | ||
| delete(regCache.indexToRegistration, id) | ||
| log.Warnf("registration for validator index %d expired at unix time %d", id, v.Timestamp) | ||
| return nil, errors.Wrapf(ErrNotFoundRegistration, "validator id %d", id) | ||
| } | ||
| regCache.lock.RUnlock() | ||
| return v, nil | ||
| } | ||
|
|
||
| func RegistrationTimeStampExpired(ts uint64) (bool, error) { | ||
| // safely convert unint64 to int64 | ||
| i, err := math.Int(ts) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| expiryDuration := params.BeaconConfig().RegistrationDuration | ||
|
There was a problem hiding this comment. I hope this is a good way of doing it... |
||
| // registered time + expiration duration < current time = expired | ||
| return time.Unix(int64(i), 0).Add(expiryDuration).Before(time.Now()), nil | ||
| } | ||
|
|
||
| // UpdateIndexToRegisteredMap adds or updates values in the cache based on the argument. | ||
| func (regCache *RegistrationCache) UpdateIndexToRegisteredMap(ctx context.Context, m map[primitives.ValidatorIndex]*ethpb.ValidatorRegistrationV1) { | ||
| _, span := trace.StartSpan(ctx, "RegistrationCache.UpdateIndexToRegisteredMap") | ||
| defer span.End() | ||
| regCache.lock.Lock() | ||
| defer regCache.lock.Unlock() | ||
| for key, value := range m { | ||
| regCache.indexToRegistration[key] = ðpb.ValidatorRegistrationV1{ | ||
| Pubkey: bytesutil.SafeCopyBytes(value.Pubkey), | ||
| FeeRecipient: bytesutil.SafeCopyBytes(value.FeeRecipient), | ||
| GasLimit: value.GasLimit, | ||
| Timestamp: value.Timestamp, | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package cache | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/ethereum/go-ethereum/common/hexutil" | ||
| "github.com/prysmaticlabs/prysm/v4/config/params" | ||
| "github.com/prysmaticlabs/prysm/v4/consensus-types/primitives" | ||
| ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1" | ||
| "github.com/prysmaticlabs/prysm/v4/testing/require" | ||
| logTest "github.com/sirupsen/logrus/hooks/test" | ||
| ) | ||
|
|
||
| func TestRegistrationCache(t *testing.T) { | ||
| hook := logTest.NewGlobal() | ||
| pubkey, err := hexutil.Decode("0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a") | ||
| require.NoError(t, err) | ||
| validatorIndex := primitives.ValidatorIndex(1) | ||
| cache := NewRegistrationCache() | ||
| m := make(map[primitives.ValidatorIndex]*ethpb.ValidatorRegistrationV1) | ||
|
|
||
| m[validatorIndex] = ðpb.ValidatorRegistrationV1{ | ||
| FeeRecipient: []byte{}, | ||
| GasLimit: 100, | ||
| Timestamp: uint64(time.Now().Unix()), | ||
| Pubkey: pubkey, | ||
| } | ||
| cache.UpdateIndexToRegisteredMap(context.Background(), m) | ||
| reg, err := cache.GetRegistrationByIndex(validatorIndex) | ||
| require.NoError(t, err) | ||
| require.Equal(t, string(reg.Pubkey), string(pubkey)) | ||
| t.Run("Registration expired", func(t *testing.T) { | ||
| validatorIndex2 := primitives.ValidatorIndex(2) | ||
| overExpirationPadTime := time.Second * time.Duration(params.BeaconConfig().SecondsPerSlot*uint64(params.BeaconConfig().SlotsPerEpoch)*4) // 4 epochs | ||
| m[validatorIndex2] = ðpb.ValidatorRegistrationV1{ | ||
| FeeRecipient: []byte{}, | ||
| GasLimit: 100, | ||
| Timestamp: uint64(time.Now().Add(-1 * overExpirationPadTime).Unix()), | ||
| Pubkey: pubkey, | ||
| } | ||
| cache.UpdateIndexToRegisteredMap(context.Background(), m) | ||
| _, err := cache.GetRegistrationByIndex(validatorIndex2) | ||
| require.ErrorContains(t, "no validator registered", err) | ||
| require.LogsContain(t, hook, "expired") | ||
| }) | ||
| t.Run("Registration close to expiration still passes", func(t *testing.T) { | ||
| pubkey, err := hexutil.Decode("0x88247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a") | ||
| require.NoError(t, err) | ||
| validatorIndex2 := primitives.ValidatorIndex(2) | ||
| overExpirationPadTime := time.Second * time.Duration((params.BeaconConfig().SecondsPerSlot*uint64(params.BeaconConfig().SlotsPerEpoch)*3)-5) // 3 epochs - 5 seconds | ||
| m[validatorIndex2] = ðpb.ValidatorRegistrationV1{ | ||
| FeeRecipient: []byte{}, | ||
| GasLimit: 100, | ||
| Timestamp: uint64(time.Now().Add(-1 * overExpirationPadTime).Unix()), | ||
| Pubkey: pubkey, | ||
| } | ||
| cache.UpdateIndexToRegisteredMap(context.Background(), m) | ||
| reg, err := cache.GetRegistrationByIndex(validatorIndex2) | ||
| require.NoError(t, err) | ||
| require.Equal(t, string(reg.Pubkey), string(pubkey)) | ||
| }) | ||
| } | ||
|
|
||
| func Test_RegistrationTimeStampExpired(t *testing.T) { | ||
| // expiration set at 3 epochs | ||
| t.Run("expired registration", func(t *testing.T) { | ||
| overExpirationPadTime := time.Second * time.Duration(params.BeaconConfig().SecondsPerSlot*uint64(params.BeaconConfig().SlotsPerEpoch)*4) // 4 epochs | ||
| ts := uint64(time.Now().Add(-1 * overExpirationPadTime).Unix()) | ||
| isExpired, err := RegistrationTimeStampExpired(ts) | ||
| require.NoError(t, err) | ||
| require.Equal(t, true, isExpired) | ||
| }) | ||
| t.Run("is not expired registration", func(t *testing.T) { | ||
| overExpirationPadTime := time.Second * time.Duration((params.BeaconConfig().SecondsPerSlot*uint64(params.BeaconConfig().SlotsPerEpoch)*3)-5) // 3 epochs -5 seconds | ||
| ts := uint64(time.Now().Add(-1 * overExpirationPadTime).Unix()) | ||
| isExpired, err := RegistrationTimeStampExpired(ts) | ||
| require.NoError(t, err) | ||
| require.Equal(t, false, isExpired) | ||
| }) | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deepsource complained the lock was open on the struct so now i added it internally here, can revert to align with other caches.