Skip to content

Commit

Permalink
Unregister validator - fix behind feature flag (#12316)
Browse files Browse the repository at this point in the history
* adding changes to blocks

* trying out expiration

* adding implementation, have WIP for tests

* adding unit tests for cache

* fixing bazel complaints

* fix linting

* adding safe check for unint type

* changing approach to safety check

* adding cache to bazel to test fixing build

* reverting bazel change and adding flag to usage

* implementing interface on mock to fix build error

* fixing unit tests

* fixing unit test

* fixing unit tests

* fixing linting

* fixing more unit tests

* fixing produce blinded block tests

* Update beacon-chain/cache/registration.go

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

* resolving review comments

* fixing cache

* Update beacon-chain/cache/registration.go

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

* Update beacon-chain/cache/registration.go

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

* fixing time logic

* adding context to trace

* fix bazel lint

* fixing context dependency

* fix linting

* Update cmd/beacon-chain/flags/base.go

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

* addressing review comments

* fixing deepsource issues

* improving the default settings

* fixing bazel

* removing irrelevant unit test

* updating name

---------

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
  • Loading branch information
james-prysm and prestonvanloon authored Apr 28, 2023
1 parent d382abe commit 83416f3
Show file tree
Hide file tree
Showing 21 changed files with 294 additions and 43 deletions.
1 change: 1 addition & 0 deletions beacon-chain/builder/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ go_library(
deps = [
"//api/client/builder:go_default_library",
"//beacon-chain/blockchain:go_default_library",
"//beacon-chain/cache:go_default_library",
"//beacon-chain/db:go_default_library",
"//cmd/beacon-chain/flags:go_default_library",
"//consensus-types/interfaces:go_default_library",
Expand Down
9 changes: 9 additions & 0 deletions beacon-chain/builder/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package builder
import (
"github.com/prysmaticlabs/prysm/v4/api/client/builder"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/blockchain"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/cache"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/db"
"github.com/prysmaticlabs/prysm/v4/cmd/beacon-chain/flags"
"github.com/urfave/cli/v2"
Expand Down Expand Up @@ -50,3 +51,11 @@ func WithDatabase(beaconDB db.HeadAccessDatabase) Option {
return nil
}
}

// WithRegistrationCache uses a cache for the validator registrations instead of a persistent db.
func WithRegistrationCache() Option {
return func(s *Service) error {
s.registrationCache = cache.NewRegistrationCache()
return nil
}
}
38 changes: 33 additions & 5 deletions beacon-chain/builder/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v4/api/client/builder"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/blockchain"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/cache"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/db"
"github.com/prysmaticlabs/prysm/v4/consensus-types/interfaces"
"github.com/prysmaticlabs/prysm/v4/consensus-types/primitives"
Expand All @@ -26,6 +27,7 @@ type BlockBuilder interface {
SubmitBlindedBlock(ctx context.Context, block interfaces.ReadOnlySignedBeaconBlock) (interfaces.ExecutionData, error)
GetHeader(ctx context.Context, slot primitives.Slot, parentHash [32]byte, pubKey [48]byte) (builder.SignedBid, error)
RegisterValidator(ctx context.Context, reg []*ethpb.SignedValidatorRegistrationV1) error
RegistrationByValidatorID(ctx context.Context, id primitives.ValidatorIndex) (*ethpb.ValidatorRegistrationV1, error)
Configured() bool
}

Expand All @@ -38,10 +40,11 @@ type config struct {

// Service defines a service that provides a client for interacting with the beacon chain and MEV relay network.
type Service struct {
cfg *config
c builder.BuilderClient
ctx context.Context
cancel context.CancelFunc
cfg *config
c builder.BuilderClient
ctx context.Context
cancel context.CancelFunc
registrationCache *cache.RegistrationCache
}

// NewService instantiates a new service.
Expand Down Expand Up @@ -139,8 +142,12 @@ func (s *Service) RegisterValidator(ctx context.Context, reg []*ethpb.SignedVali
return ErrNoBuilder
}

// should be removed if db is removed
idxs := make([]primitives.ValidatorIndex, 0)
msgs := make([]*ethpb.ValidatorRegistrationV1, 0)

indexToRegistration := make(map[primitives.ValidatorIndex]*ethpb.ValidatorRegistrationV1)

valid := make([]*ethpb.SignedValidatorRegistrationV1, 0)
for i := 0; i < len(reg); i++ {
r := reg[i]
Expand All @@ -154,12 +161,33 @@ func (s *Service) RegisterValidator(ctx context.Context, reg []*ethpb.SignedVali
idxs = append(idxs, nx)
msgs = append(msgs, r.Message)
valid = append(valid, r)
indexToRegistration[nx] = r.Message
}
if err := s.c.RegisterValidator(ctx, valid); err != nil {
return errors.Wrap(err, "could not register validator(s)")
}

return s.cfg.beaconDB.SaveRegistrationsByValidatorIDs(ctx, idxs, msgs)
if len(indexToRegistration) != len(msgs) {
return errors.New("ids and registrations must be the same length")
}
if s.registrationCache != nil {
s.registrationCache.UpdateIndexToRegisteredMap(ctx, indexToRegistration)
return nil
} else {
return s.cfg.beaconDB.SaveRegistrationsByValidatorIDs(ctx, idxs, msgs)
}
}

// RegistrationByValidatorID returns either the values from the cache or db.
func (s *Service) RegistrationByValidatorID(ctx context.Context, id primitives.ValidatorIndex) (*ethpb.ValidatorRegistrationV1, error) {
if s.registrationCache != nil {
return s.registrationCache.RegistrationByIndex(id)
} else {
if s.cfg == nil || s.cfg.beaconDB == nil {
return nil, errors.New("nil beacon db")
}
return s.cfg.beaconDB.RegistrationByValidatorID(ctx, id)
}
}

// Configured returns true if the user has configured a builder client.
Expand Down
2 changes: 2 additions & 0 deletions beacon-chain/builder/testing/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//api/client/builder:go_default_library",
"//beacon-chain/cache:go_default_library",
"//beacon-chain/db:go_default_library",
"//config/params:go_default_library",
"//consensus-types/blocks:go_default_library",
"//consensus-types/interfaces:go_default_library",
Expand Down
22 changes: 21 additions & 1 deletion beacon-chain/builder/testing/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (

"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v4/api/client/builder"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/cache"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/db"
"github.com/prysmaticlabs/prysm/v4/config/params"
"github.com/prysmaticlabs/prysm/v4/consensus-types/blocks"
"github.com/prysmaticlabs/prysm/v4/consensus-types/interfaces"
Expand All @@ -14,6 +16,11 @@ import (
"github.com/prysmaticlabs/prysm/v4/time/slots"
)

// Config defines a config struct for dependencies into the service.
type Config struct {
BeaconDB db.HeadAccessDatabase
}

// MockBuilderService to mock builder.
type MockBuilderService struct {
HasConfigured bool
Expand All @@ -22,8 +29,10 @@ type MockBuilderService struct {
ErrSubmitBlindedBlock error
Bid *ethpb.SignedBuilderBid
BidCapella *ethpb.SignedBuilderBidCapella
RegistrationCache *cache.RegistrationCache
ErrGetHeader error
ErrRegisterValidator error
Cfg *Config
}

// Configured for mocking.
Expand All @@ -48,7 +57,7 @@ func (s *MockBuilderService) SubmitBlindedBlock(_ context.Context, _ interfaces.
}

// GetHeader for mocking.
func (s *MockBuilderService) GetHeader(ctx context.Context, slot primitives.Slot, hr [32]byte, pb [48]byte) (builder.SignedBid, error) {
func (s *MockBuilderService) GetHeader(_ context.Context, slot primitives.Slot, _ [32]byte, _ [48]byte) (builder.SignedBid, error) {
if slots.ToEpoch(slot) >= params.BeaconConfig().CapellaForkEpoch {
return builder.WrappedSignedBuilderBidCapella(s.BidCapella)
}
Expand All @@ -59,6 +68,17 @@ func (s *MockBuilderService) GetHeader(ctx context.Context, slot primitives.Slot
return w, s.ErrGetHeader
}

// RegistrationByValidatorID returns either the values from the cache or db.
func (s *MockBuilderService) RegistrationByValidatorID(ctx context.Context, id primitives.ValidatorIndex) (*ethpb.ValidatorRegistrationV1, error) {
if s.RegistrationCache != nil {
return s.RegistrationCache.RegistrationByIndex(id)
}
if s.Cfg.BeaconDB != nil {
return s.Cfg.BeaconDB.RegistrationByValidatorID(ctx, id)
}
return nil, cache.ErrNotFoundRegistration
}

// RegisterValidator for mocking.
func (s *MockBuilderService) RegisterValidator(context.Context, []*ethpb.SignedValidatorRegistrationV1) error {
return s.ErrRegisterValidator
Expand Down
4 changes: 4 additions & 0 deletions beacon-chain/cache/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ go_library(
"proposer_indices.go",
"proposer_indices_disabled.go", # keep
"proposer_indices_type.go",
"registration.go",
"skip_slot_cache.go",
"subnet_ids.go",
"sync_committee.go",
Expand Down Expand Up @@ -65,6 +66,7 @@ go_test(
"committee_test.go",
"payload_id_test.go",
"proposer_indices_test.go",
"registration_test.go",
"skip_slot_cache_test.go",
"subnet_ids_test.go",
"sync_committee_head_state_test.go",
Expand All @@ -83,7 +85,9 @@ go_test(
"//testing/assert:go_default_library",
"//testing/require:go_default_library",
"//testing/util:go_default_library",
"@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
"@com_github_google_gofuzz//:go_default_library",
"@com_github_sirupsen_logrus//hooks/test:go_default_library",
"@org_golang_google_protobuf//proto:go_default_library",
],
)
4 changes: 3 additions & 1 deletion beacon-chain/cache/error.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package cache

import "errors"
import "github.com/pkg/errors"

var (
// ErrNilValueProvided for when we try to put a nil value in a cache.
Expand All @@ -12,4 +12,6 @@ var (
// ErrNonExistingSyncCommitteeKey when sync committee key (root) does not exist in cache.
ErrNonExistingSyncCommitteeKey = errors.New("does not exist sync committee key")
errNotSyncCommitteeIndexPosition = errors.New("not syncCommitteeIndexPosition struct")
// ErrNotFoundRegistration when validator registration does not exist in cache.
ErrNotFoundRegistration = errors.Wrap(ErrNotFound, "no validator registered")
)
82 changes: 82 additions & 0 deletions beacon-chain/cache/registration.go
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
// 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] = &ethpb.ValidatorRegistrationV1{
Pubkey: bytesutil.SafeCopyBytes(value.Pubkey),
FeeRecipient: bytesutil.SafeCopyBytes(value.FeeRecipient),
GasLimit: value.GasLimit,
Timestamp: value.Timestamp,
}
}
}
82 changes: 82 additions & 0 deletions beacon-chain/cache/registration_test.go
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] = &ethpb.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] = &ethpb.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] = &ethpb.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)
})
}
Loading

0 comments on commit 83416f3

Please sign in to comment.