-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unregister validator - fix behind feature flag #12316
Merged
Merged
Changes from all commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
7e2936c
adding changes to blocks
james-prysm f7e0b21
trying out expiration
james-prysm 627dd71
adding implementation, have WIP for tests
james-prysm e84a94b
adding unit tests for cache
james-prysm d7d1406
fixing bazel complaints
james-prysm d06c80b
Merge branch 'develop' into unregister-validator
james-prysm 5db494b
fix linting
james-prysm 5050a02
adding safe check for unint type
james-prysm ab7ea94
changing approach to safety check
james-prysm 63e5521
adding cache to bazel to test fixing build
james-prysm 1857d58
reverting bazel change and adding flag to usage
james-prysm 785936c
implementing interface on mock to fix build error
james-prysm 2cf8e4a
fixing unit tests
james-prysm baa8f10
fixing unit test
james-prysm 96cb86f
Merge branch 'develop' into unregister-validator
james-prysm 723babc
fixing unit tests
james-prysm 67b8631
fixing linting
james-prysm 52bffd8
fixing more unit tests
james-prysm 76a6413
fixing produce blinded block tests
james-prysm 374e992
Merge branch 'develop' into unregister-validator
james-prysm 9f15eac
Update beacon-chain/cache/registration.go
james-prysm 1a6ecc0
resolving review comments
james-prysm 0c8f838
fixing cache
james-prysm e4b4cc5
Update beacon-chain/cache/registration.go
james-prysm 03636bf
Update beacon-chain/cache/registration.go
james-prysm 56b5b95
Merge branch 'develop' into unregister-validator
james-prysm 5c7e7ef
Merge branch 'develop' into unregister-validator
james-prysm a941512
fixing time logic
james-prysm ca13004
adding context to trace
james-prysm a25a123
Merge branch 'develop' into unregister-validator
james-prysm dc9fd00
fix bazel lint
james-prysm 5ba32bf
fixing context dependency
james-prysm 7b36b5c
fix linting
james-prysm 0c0a9e4
Merge branch 'develop' into unregister-validator
james-prysm cc289f4
Merge branch 'develop' into unregister-validator
james-prysm ea5a80f
Merge branch 'develop' into unregister-validator
james-prysm e6a4ff1
Merge branch 'develop' into unregister-validator
james-prysm fae8acf
Update cmd/beacon-chain/flags/base.go
james-prysm 2e717d9
addressing review comments
james-prysm ce566ee
fixing deepsource issues
james-prysm 7e32cac
improving the default settings
james-prysm a51fdce
fixing bazel
james-prysm 3dc794d
removing irrelevant unit test
james-prysm 851a3e2
updating name
james-prysm de3dd5b
Merge branch 'develop' into unregister-validator
james-prysm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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{}, | ||
} | ||
} | ||
|
||
// RegistrationByIndex returns the registration by index in the cache and also removes items in the cache if expired. | ||
func (regCache *RegistrationCache) RegistrationByIndex(id primitives.ValidatorIndex) (*ethpb.ValidatorRegistrationV1, error) { | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.RegistrationByIndex(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.RegistrationByIndex(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.RegistrationByIndex(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.