fix: refetch duties after metdata update (main)
#1429
Conversation
.gitlab-ci.yml
Outdated
| only: | ||
| - stage | ||
| - fix/refetch-duties-after-metadata-main |
network/discovery/options.go
Outdated
| @@ -80,10 +80,13 @@ func (opts *DiscV5Options) IPs() (net.IP, net.IP, string) { | |||
| return ipAddr, bindIP, n | |||
| } | |||
|
|
|||
| var pid = [6]byte{'s', 's', 'v', 'd', 'v', '5'} | |||
There was a problem hiding this comment.
need to make sure this is also deployed on bootnode
network/discovery/options.go
Outdated
| // DiscV5Cfg creates discv5 config from the options | ||
| func (opts *DiscV5Options) DiscV5Cfg(logger *zap.Logger) (*discover.Config, error) { | ||
| dv5Cfg := discover.Config{ | ||
| PrivateKey: opts.NetworkKey, | ||
| PrivateKey: opts.NetworkKey, | ||
| V5ProtocolID: &pid, // TODO: REVERT! |
There was a problem hiding this comment.
you want to revert or leave this?
There was a problem hiding this comment.
will revert, its just to be able to test on stage env
networkconfig/holesky-stage.go
Outdated
| @@ -16,7 +16,7 @@ var HoleskyStage = NetworkConfig{ | |||
| RegistrySyncOffset: new(big.Int).SetInt64(84599), | |||
| RegistryContractAddr: "0x0d33801785340072C452b994496B19f196b7eE15", | |||
| Bootnodes: []string{ | |||
| "enr:-Li4QPnPGESWx2wnu3s2qeu6keFbkaV2M0ZiGHgxxGI9ThP4XSgSaFzl6zYsF1zAdni3Mh04iA6BEZqoC6LZ52UFnwKGAYxEgLqeh2F0dG5ldHOIAAAAAAAAAACEZXRoMpD1pf1CAAAAAP__________gmlkgnY0gmlwhDQiKqmJc2VjcDI1NmsxoQP2e508AoA0B-KH-IaAd3nVCfI9q16lNztV-oTpcH72tIN0Y3CCE4mDdWRwgg-h", | |||
| "enr:-Ja4QDYHVgUs9NvlMqq93ot6VNqbmrIlMrwKnq4X3DPRgyUNB4ospDp8ubMvsf-KsgqY8rzpZKy4GbE1DLphabpRBc-GAY_diLjngmlkgnY0gmlwhDQrLYqJc2VjcDI1NmsxoQKnAiuSlgSR8asjCH0aYoVKM8uPbi4noFuFHZHaAHqknYNzc3YBg3RjcIITiYN1ZHCCD6E", | |||
There was a problem hiding this comment.
if you deploy the protocolID for prod/mainnet its best to restart there and maybe update the ENR for other networks.
There was a problem hiding this comment.
it will be reverted
| // Fetch metadata now if there is none. Otherwise, UpdateValidatorsMetadataLoop will handle it. | ||
| var hasMetadata bool | ||
| for _, share := range shares { | ||
| if !share.Liquidated && share.HasBeaconMetadata() { |
There was a problem hiding this comment.
oh because metadata might say it doesn't need to attest but still valid metadata
There was a problem hiding this comment.
oh because metadata might say it doesn't need to attest but still valid metadata
yeah we just want to know that its not a freshly synced database with 0 metadatas fetched, in which case its not crucial to fetch everything immediately and we can rely on the UpdateValidatorsMetadataLoop to fetch/update metadatas more strategically (with prioritization) in the background
registry/storage/shares.go
Outdated
| @@ -77,6 +82,7 @@ func (s *sharesStorage) load() error { | |||
| if err := val.Decode(obj.Value); err != nil { | |||
| return fmt.Errorf("failed to deserialize share: %w", err) | |||
| } | |||
| val.DomainType = types.GetDefaultDomain() | |||
There was a problem hiding this comment.
should probably be passed from NetworkConfig to the storage in next version, best to leave a TODO
There was a problem hiding this comment.
should probably be passed from
NetworkConfigto the storage in next version, best to leave a TODO
yeah this will be reverted its just an issue that we have on stage bc we modified domaintype of one of the existing networks which overwrote some shares in the DB :D
| if share.Liquidated { | ||
| return true | ||
| } | ||
| if share.BeaconMetadata == nil && share.MetadataLastUpdated().IsZero() { |
There was a problem hiding this comment.
could it even be that only one of these is true?
There was a problem hiding this comment.
yes if we already fetched metadata for an undeposited validator then MetadataLastUpdated will be set, but BeaconMetadata will be nil
if both MetadataLastUpdated and BeaconMetadata are unset then we dont know yet if its an undeposited validator or just a new validator waiting for first metadata fetch (thats because MetadataLastUpdated is not persistent)
This PR fixes the same bug as #1411 except it's based for main.
This bug occurs when a validator in an ongoing sync committee is added to the network and the nodes don't fetch their sync committee until the next period, causing incorrect message rejections.