Skip to content

Commit

Permalink
Fix duplicate relay registrations in cache (ethereum#15)
Browse files Browse the repository at this point in the history
* Fix duplicate relay registrations in cache

* Remove Timestamp from builder validation data as it should be ignored

* Adjust cfg.SecondaryRemoteRelayEndpoints for empty list

Co-authored-by: Mateusz Morusiewicz <[email protected]>
  • Loading branch information
avalonche and Ruteri committed Mar 9, 2023
1 parent b359394 commit 9292b34
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 24 deletions.
7 changes: 3 additions & 4 deletions builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ type PubkeyHex string

type ValidatorData struct {
Pubkey PubkeyHex
FeeRecipient boostTypes.Address `json:"feeRecipient"`
GasLimit uint64 `json:"gasLimit"`
Timestamp uint64 `json:"timestamp"`
FeeRecipient boostTypes.Address
GasLimit uint64
}

type IBeaconClient interface {
Expand Down Expand Up @@ -137,7 +136,7 @@ func (b *Builder) onSealedBlock(block *types.Block, ordersClosedAt time.Time, se
}

if b.dryRun {
err = b.validator.ValidateBuilderSubmissionV1(&blockvalidation.BuilderBlockValidationRequest{blockSubmitReq, vd.GasLimit})
err = b.validator.ValidateBuilderSubmissionV1(&blockvalidation.BuilderBlockValidationRequest{BuilderSubmitBlockRequest: blockSubmitReq, RegisteredGasLimit: vd.GasLimit})
if err != nil {
log.Error("could not validate block", "err", err)
}
Expand Down
1 change: 0 additions & 1 deletion builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func TestOnPayloadAttributes(t *testing.T) {
Pubkey: PubkeyHex(testBeacon.validator.Pk.String()),
FeeRecipient: feeRecipient,
GasLimit: 10,
Timestamp: 15,
},
}

Expand Down
23 changes: 15 additions & 8 deletions builder/local_relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ type ForkData struct {
GenesisValidatorsRoot string
}

type FullValidatorData struct {
ValidatorData
Timestamp uint64
}

type LocalRelay struct {
beaconClient IBeaconClient

Expand All @@ -36,7 +41,7 @@ type LocalRelay struct {
proposerSigningDomain boostTypes.Domain

validatorsLock sync.RWMutex
validators map[PubkeyHex]ValidatorData
validators map[PubkeyHex]FullValidatorData

enableBeaconChecks bool

Expand Down Expand Up @@ -70,7 +75,7 @@ func NewLocalRelay(sk *bls.SecretKey, beaconClient IBeaconClient, builderSigning
proposerSigningDomain: proposerSigningDomain,
serializedRelayPubkey: pkBytes,

validators: make(map[PubkeyHex]ValidatorData),
validators: make(map[PubkeyHex]FullValidatorData),

enableBeaconChecks: enableBeaconChecks,

Expand Down Expand Up @@ -161,11 +166,13 @@ func (r *LocalRelay) handleRegisterValidator(w http.ResponseWriter, req *http.Re

for _, registerRequest := range payload {
pubkeyHex := PubkeyHex(strings.ToLower(registerRequest.Message.Pubkey.String()))
r.validators[pubkeyHex] = ValidatorData{
Pubkey: pubkeyHex,
FeeRecipient: registerRequest.Message.FeeRecipient,
GasLimit: registerRequest.Message.GasLimit,
Timestamp: registerRequest.Message.Timestamp,
r.validators[pubkeyHex] = FullValidatorData{
ValidatorData: ValidatorData{
Pubkey: pubkeyHex,
FeeRecipient: registerRequest.Message.FeeRecipient,
GasLimit: registerRequest.Message.GasLimit,
},
Timestamp: registerRequest.Message.Timestamp,
}

log.Info("registered validator", "pubkey", pubkeyHex, "data", r.validators[pubkeyHex])
Expand All @@ -183,7 +190,7 @@ func (r *LocalRelay) GetValidatorForSlot(nextSlot uint64) (ValidatorData, error)
r.validatorsLock.RLock()
if vd, ok := r.validators[pubkeyHex]; ok {
r.validatorsLock.RUnlock()
return vd, nil
return vd.ValidatorData, nil
}
r.validatorsLock.RUnlock()
log.Info("no local entry for validator", "validator", pubkeyHex)
Expand Down
4 changes: 2 additions & 2 deletions builder/local_relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestValidatorRegistration(t *testing.T) {
rr := testRequest(t, relay, "POST", "/eth/v1/builder/validators", payload)
require.Equal(t, http.StatusOK, rr.Code)
require.Contains(t, relay.validators, PubkeyHex(v.Pk.String()))
require.Equal(t, ValidatorData{Pubkey: PubkeyHex(v.Pk.String()), FeeRecipient: payload[0].Message.FeeRecipient, GasLimit: payload[0].Message.GasLimit, Timestamp: payload[0].Message.Timestamp}, relay.validators[PubkeyHex(v.Pk.String())])
require.Equal(t, FullValidatorData{ValidatorData: ValidatorData{Pubkey: PubkeyHex(v.Pk.String()), FeeRecipient: payload[0].Message.FeeRecipient, GasLimit: payload[0].Message.GasLimit}, Timestamp: payload[0].Message.Timestamp}, relay.validators[PubkeyHex(v.Pk.String())])

rr = testRequest(t, relay, "POST", "/eth/v1/builder/validators", payload)
require.Equal(t, http.StatusOK, rr.Code)
Expand Down Expand Up @@ -113,7 +113,7 @@ func registerValidator(t *testing.T, v *ValidatorPrivateData, relay *LocalRelay)
rr := testRequest(t, relay, "POST", "/eth/v1/builder/validators", payload)
require.Equal(t, http.StatusOK, rr.Code)
require.Contains(t, relay.validators, PubkeyHex(v.Pk.String()))
require.Equal(t, ValidatorData{Pubkey: PubkeyHex(v.Pk.String()), FeeRecipient: payload[0].Message.FeeRecipient, GasLimit: payload[0].Message.GasLimit, Timestamp: payload[0].Message.Timestamp}, relay.validators[PubkeyHex(v.Pk.String())])
require.Equal(t, FullValidatorData{ValidatorData: ValidatorData{Pubkey: PubkeyHex(v.Pk.String()), FeeRecipient: payload[0].Message.FeeRecipient, GasLimit: payload[0].Message.GasLimit}, Timestamp: payload[0].Message.Timestamp}, relay.validators[PubkeyHex(v.Pk.String())])
}

func TestGetHeader(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion builder/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ func (r *RemoteRelay) getSlotValidatorMapFromRelay() (map[uint64]ValidatorData,
Pubkey: pubkeyHex,
FeeRecipient: feeRecipient,
GasLimit: data.Entry.Message.GasLimit,
Timestamp: data.Entry.Message.Timestamp,
}
}

Expand Down
4 changes: 4 additions & 0 deletions builder/relay_aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ func (r *RemoteRelayAggregator) updateRelayRegistrations(nextSlot uint64, regist
topRegistrationCh <- registrations[bestRelayRegistrationIndex].vd
}

if nextSlot == r.registrationsCacheSlot {
return
}

if nextSlot > r.registrationsCacheSlot {
// clear the cache
r.registrationsCache = make(map[ValidatorData][]IRelay)
Expand Down
7 changes: 3 additions & 4 deletions builder/relay_aggregator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ type testRelay struct {
gvsVd ValidatorData
gvsErr error

requestedSlot uint64
submittedMsg *boostTypes.BuilderSubmitBlockRequest
submittedMsgCh chan *boostTypes.BuilderSubmitBlockRequest
submittedRegistration ValidatorData
requestedSlot uint64
submittedMsg *boostTypes.BuilderSubmitBlockRequest
submittedMsgCh chan *boostTypes.BuilderSubmitBlockRequest
}

type testRelayAggBackend struct {
Expand Down
3 changes: 0 additions & 3 deletions builder/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func TestRemoteRelay(t *testing.T) {
Pubkey: "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a",
FeeRecipient: boostTypes.Address{0xab, 0xcf, 0x8e, 0xd, 0x4e, 0x95, 0x87, 0x36, 0x9b, 0x23, 0x1, 0xd0, 0x79, 0x3, 0x47, 0x32, 0x3, 0x2, 0xcc, 0x9},
GasLimit: uint64(1),
Timestamp: uint64(1),
}
require.Equal(t, expectedValidator_123, vd)

Expand Down Expand Up @@ -98,14 +97,12 @@ func TestRemoteRelay(t *testing.T) {
Pubkey: "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a",
FeeRecipient: boostTypes.Address{0xab, 0xcf, 0x8e, 0xd, 0x4e, 0x95, 0x87, 0x36, 0x9b, 0x23, 0x1, 0xd0, 0x79, 0x3, 0x47, 0x32, 0x3, 0x2, 0xcc, 0x10},
GasLimit: uint64(1),
Timestamp: uint64(1),
}

expectedValidator_156 := ValidatorData{
Pubkey: "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a",
FeeRecipient: boostTypes.Address{0xab, 0xcf, 0x8e, 0xd, 0x4e, 0x95, 0x87, 0x36, 0x9b, 0x23, 0x1, 0xd0, 0x79, 0x3, 0x47, 0x32, 0x3, 0x2, 0xcc, 0x11},
GasLimit: uint64(1),
Timestamp: uint64(1),
}

vd, err = relay.GetValidatorForSlot(155)
Expand Down
2 changes: 1 addition & 1 deletion builder/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg *Config) error {
return errors.New("neither local nor remote relay specified")
}

if len(cfg.SecondaryRemoteRelayEndpoints) > 0 {
if len(cfg.SecondaryRemoteRelayEndpoints) > 0 && !(len(cfg.SecondaryRemoteRelayEndpoints) == 1 && cfg.SecondaryRemoteRelayEndpoints[0] == "") {
secondaryRelays := make([]IRelay, len(cfg.SecondaryRemoteRelayEndpoints))
for i, endpoint := range cfg.SecondaryRemoteRelayEndpoints {
secondaryRelays[i] = NewRemoteRelay(endpoint, nil)
Expand Down

0 comments on commit 9292b34

Please sign in to comment.