From 5489020aaf45487827081aa400056b91fc1f1682 Mon Sep 17 00:00:00 2001 From: rauljordan Date: Mon, 13 Apr 2020 09:34:51 -0500 Subject: [PATCH 1/8] dynamic --- beacon-chain/sync/subscriber.go | 16 ++++++++-------- endtoend/components/beacon_node.go | 1 + shared/featureconfig/flags.go | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/beacon-chain/sync/subscriber.go b/beacon-chain/sync/subscriber.go index 4c39db143170..c17c987015a7 100644 --- a/beacon-chain/sync/subscriber.go +++ b/beacon-chain/sync/subscriber.go @@ -288,14 +288,14 @@ func (r *Service) subscribeDynamic(topicFormat string, determineSubsLen func() i wantedSubs := determineSubsLen() // Resize as appropriate. if len(subscriptions) > wantedSubs { // Reduce topics - var cancelSubs []*pubsub.Subscription - subscriptions, cancelSubs = subscriptions[:wantedSubs-1], subscriptions[wantedSubs:] - for i, sub := range cancelSubs { - sub.Cancel() - if err := r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, i+wantedSubs)); err != nil { - log.WithError(err).Error("Failed to unregister topic validator") - } - } + //var cancelSubs []*pubsub.Subscription + //subscriptions, cancelSubs = subscriptions[:wantedSubs-1], subscriptions[wantedSubs:] + //for i, sub := range cancelSubs { + // sub.Cancel() + // if err := r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, i+wantedSubs)); err != nil { + // log.WithError(err).Error("Failed to unregister topic validator") + // } + //} } else if len(subscriptions) < wantedSubs { // Increase topics for i := len(subscriptions); i < wantedSubs; i++ { sub := r.subscribeWithBase(base, fmt.Sprintf(topicFormat, digest, i), validate, handle) diff --git a/endtoend/components/beacon_node.go b/endtoend/components/beacon_node.go index 445329e5c71a..25ad3799c696 100644 --- a/endtoend/components/beacon_node.go +++ b/endtoend/components/beacon_node.go @@ -54,6 +54,7 @@ func StartNewBeaconNode(t *testing.T, config *types.E2EConfig, index int, enr st fmt.Sprintf("--rpc-max-page-size=%d", params.BeaconConfig().MinGenesisActiveValidatorCount), "--force-clear-db", fmt.Sprintf("--bootstrap-node=%s", enr), + "--verbosity debug", } args = append(args, featureconfig.E2EBeaconChainFlags...) args = append(args, config.BeaconFlags...) diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index f9994200a574..367b9d7559ad 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -368,5 +368,5 @@ var E2EBeaconChainFlags = []string{ "--check-head-state", "--enable-state-field-trie", // TODO(5123): This flag currently fails E2E. Commenting until it's resolved. - //"--enable-dynamic-committee-subnets", + "--enable-dynamic-committee-subnets", } From 50697458e79bd85195aba34be61a84fad938da94 Mon Sep 17 00:00:00 2001 From: rauljordan Date: Mon, 13 Apr 2020 09:50:23 -0500 Subject: [PATCH 2/8] repro --- beacon-chain/sync/subscriber.go | 16 ++++++++-------- endtoend/components/beacon_node.go | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/beacon-chain/sync/subscriber.go b/beacon-chain/sync/subscriber.go index c17c987015a7..4c39db143170 100644 --- a/beacon-chain/sync/subscriber.go +++ b/beacon-chain/sync/subscriber.go @@ -288,14 +288,14 @@ func (r *Service) subscribeDynamic(topicFormat string, determineSubsLen func() i wantedSubs := determineSubsLen() // Resize as appropriate. if len(subscriptions) > wantedSubs { // Reduce topics - //var cancelSubs []*pubsub.Subscription - //subscriptions, cancelSubs = subscriptions[:wantedSubs-1], subscriptions[wantedSubs:] - //for i, sub := range cancelSubs { - // sub.Cancel() - // if err := r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, i+wantedSubs)); err != nil { - // log.WithError(err).Error("Failed to unregister topic validator") - // } - //} + var cancelSubs []*pubsub.Subscription + subscriptions, cancelSubs = subscriptions[:wantedSubs-1], subscriptions[wantedSubs:] + for i, sub := range cancelSubs { + sub.Cancel() + if err := r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, i+wantedSubs)); err != nil { + log.WithError(err).Error("Failed to unregister topic validator") + } + } } else if len(subscriptions) < wantedSubs { // Increase topics for i := len(subscriptions); i < wantedSubs; i++ { sub := r.subscribeWithBase(base, fmt.Sprintf(topicFormat, digest, i), validate, handle) diff --git a/endtoend/components/beacon_node.go b/endtoend/components/beacon_node.go index 25ad3799c696..826aa8f39100 100644 --- a/endtoend/components/beacon_node.go +++ b/endtoend/components/beacon_node.go @@ -54,7 +54,7 @@ func StartNewBeaconNode(t *testing.T, config *types.E2EConfig, index int, enr st fmt.Sprintf("--rpc-max-page-size=%d", params.BeaconConfig().MinGenesisActiveValidatorCount), "--force-clear-db", fmt.Sprintf("--bootstrap-node=%s", enr), - "--verbosity debug", + "--verbosity=debug", } args = append(args, featureconfig.E2EBeaconChainFlags...) args = append(args, config.BeaconFlags...) From e51ae3ec500490d3c4e5729db227fd559a1a24bb Mon Sep 17 00:00:00 2001 From: rauljordan Date: Mon, 13 Apr 2020 11:02:16 -0500 Subject: [PATCH 3/8] e2e pass --- beacon-chain/p2p/service.go | 5 ++--- beacon-chain/sync/subscriber.go | 32 ++++++++++++++++---------------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/beacon-chain/p2p/service.go b/beacon-chain/p2p/service.go index 40ae9768078c..543b0f0e1ed9 100644 --- a/beacon-chain/p2p/service.go +++ b/beacon-chain/p2p/service.go @@ -46,9 +46,8 @@ var _ = shared.Service(&Service{}) // Check local table every 5 seconds for newly added peers. var pollingPeriod = 5 * time.Second -// Refresh rate of ENR set at every quarter of an epoch. -var refreshRate = time.Duration((params.BeaconConfig().SecondsPerSlot* - params.BeaconConfig().SlotsPerEpoch)/4) * time.Second +// Refresh rate of ENR set at once per slot. +var refreshRate = time.Duration(params.BeaconConfig().SecondsPerSlot/2) * time.Second // search limit for number of peers in discovery v5. const searchLimit = 100 diff --git a/beacon-chain/sync/subscriber.go b/beacon-chain/sync/subscriber.go index 4c39db143170..88b6960a851b 100644 --- a/beacon-chain/sync/subscriber.go +++ b/beacon-chain/sync/subscriber.go @@ -12,16 +12,16 @@ import ( "github.com/libp2p/go-libp2p-core/peer" pubsub "github.com/libp2p/go-libp2p-pubsub" pb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" + "go.opencensus.io/trace" + "github.com/prysmaticlabs/prysm/beacon-chain/core/feed" statefeed "github.com/prysmaticlabs/prysm/beacon-chain/core/feed/state" "github.com/prysmaticlabs/prysm/beacon-chain/p2p" - "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/messagehandler" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/roughtime" "github.com/prysmaticlabs/prysm/shared/slotutil" "github.com/prysmaticlabs/prysm/shared/traceutil" - "go.opencensus.io/trace" ) const pubsubMessageTimeout = 30 * time.Second @@ -96,20 +96,20 @@ func (r *Service) registerSubscribers() { r.validateAttesterSlashing, r.attesterSlashingSubscriber, ) - if featureconfig.Get().EnableDynamicCommitteeSubnets { - r.subscribeDynamicWithSubnets( - "/eth2/%x/committee_index%d_beacon_attestation", - r.validateCommitteeIndexBeaconAttestation, /* validator */ - r.committeeIndexBeaconAttestationSubscriber, /* message handler */ - ) - } else { - r.subscribeDynamic( - "/eth2/%x/committee_index%d_beacon_attestation", - r.committeesCount, /* determineSubsLen */ - r.validateCommitteeIndexBeaconAttestation, /* validator */ - r.committeeIndexBeaconAttestationSubscriber, /* message handler */ - ) - } + //if featureconfig.Get().EnableDynamicCommitteeSubnets { + r.subscribeDynamicWithSubnets( + "/eth2/%x/committee_index%d_beacon_attestation", + r.validateCommitteeIndexBeaconAttestation, /* validator */ + r.committeeIndexBeaconAttestationSubscriber, /* message handler */ + ) + //} else { + // r.subscribeDynamic( + // "/eth2/%x/committee_index%d_beacon_attestation", + // r.committeesCount, /* determineSubsLen */ + // r.validateCommitteeIndexBeaconAttestation, /* validator */ + // r.committeeIndexBeaconAttestationSubscriber, /* message handler */ + // ) + //} } // subscribe to a given topic with a given validator and subscription handler. From 6fe4eb9e51aeba3c46882d11d6c06bb9306c3265 Mon Sep 17 00:00:00 2001 From: rauljordan Date: Mon, 13 Apr 2020 11:10:31 -0500 Subject: [PATCH 4/8] flag flip --- beacon-chain/sync/subscriber.go | 29 +++++++++++++++-------------- shared/featureconfig/config.go | 8 ++++---- shared/featureconfig/flags.go | 16 ++++++++++------ 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/beacon-chain/sync/subscriber.go b/beacon-chain/sync/subscriber.go index 88b6960a851b..b06ebbdd5f32 100644 --- a/beacon-chain/sync/subscriber.go +++ b/beacon-chain/sync/subscriber.go @@ -17,6 +17,7 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/core/feed" statefeed "github.com/prysmaticlabs/prysm/beacon-chain/core/feed/state" "github.com/prysmaticlabs/prysm/beacon-chain/p2p" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/messagehandler" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/roughtime" @@ -96,20 +97,20 @@ func (r *Service) registerSubscribers() { r.validateAttesterSlashing, r.attesterSlashingSubscriber, ) - //if featureconfig.Get().EnableDynamicCommitteeSubnets { - r.subscribeDynamicWithSubnets( - "/eth2/%x/committee_index%d_beacon_attestation", - r.validateCommitteeIndexBeaconAttestation, /* validator */ - r.committeeIndexBeaconAttestationSubscriber, /* message handler */ - ) - //} else { - // r.subscribeDynamic( - // "/eth2/%x/committee_index%d_beacon_attestation", - // r.committeesCount, /* determineSubsLen */ - // r.validateCommitteeIndexBeaconAttestation, /* validator */ - // r.committeeIndexBeaconAttestationSubscriber, /* message handler */ - // ) - //} + if featureconfig.Get().DisableDynamicCommitteeSubnets { + r.subscribeDynamic( + "/eth2/%x/committee_index%d_beacon_attestation", + r.committeesCount, /* determineSubsLen */ + r.validateCommitteeIndexBeaconAttestation, /* validator */ + r.committeeIndexBeaconAttestationSubscriber, /* message handler */ + ) + } else { + r.subscribeDynamicWithSubnets( + "/eth2/%x/committee_index%d_beacon_attestation", + r.validateCommitteeIndexBeaconAttestation, /* validator */ + r.committeeIndexBeaconAttestationSubscriber, /* message handler */ + ) + } } // subscribe to a given topic with a given validator and subscription handler. diff --git a/shared/featureconfig/config.go b/shared/featureconfig/config.go index e828d21e389b..82400cdc2d39 100644 --- a/shared/featureconfig/config.go +++ b/shared/featureconfig/config.go @@ -31,7 +31,7 @@ type Flags struct { MinimalConfig bool // MinimalConfig as defined in the spec. WriteSSZStateTransitions bool // WriteSSZStateTransitions to tmp directory. InitSyncNoVerify bool // InitSyncNoVerify when initial syncing w/o verifying block's contents. - EnableDynamicCommitteeSubnets bool // Enables dynamic attestation committee subnets via p2p. + DisableDynamicCommitteeSubnets bool // Disables dynamic attestation committee subnets via p2p. SkipBLSVerify bool // Skips BLS verification across the runtime. EnableBackupWebhook bool // EnableBackupWebhook to allow database backups to trigger from monitoring port /db/backup. PruneEpochBoundaryStates bool // PruneEpochBoundaryStates prunes the epoch boundary state before last finalized check point. @@ -104,9 +104,9 @@ func ConfigureBeaconChain(ctx *cli.Context) { log.Warn("UNSAFE: Disabled fork choice for updating chain head") cfg.DisableForkChoice = true } - if ctx.Bool(enableDynamicCommitteeSubnets.Name) { - log.Warn("Enabled dynamic attestation committee subnets") - cfg.EnableDynamicCommitteeSubnets = true + if ctx.Bool(disableDynamicCommitteeSubnets.Name) { + log.Warn("Disabled dynamic attestation committee subnets") + cfg.DisableDynamicCommitteeSubnets = true } cfg.EnableSSZCache = true if ctx.Bool(disableSSZCache.Name) { diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index 367b9d7559ad..d9e755e2c2ea 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -17,9 +17,9 @@ var ( Name: "interop-write-ssz-state-transitions", Usage: "Write ssz states to disk after attempted state transition", } - enableDynamicCommitteeSubnets = &cli.BoolFlag{ - Name: "enable-dynamic-committee-subnets", - Usage: "Enable dynamic committee attestation subnets.", + disableDynamicCommitteeSubnets = &cli.BoolFlag{ + Name: "disable-dynamic-committee-subnets", + Usage: "Disable dynamic committee attestation subnets.", } // disableForkChoiceUnsafeFlag disables using the LMD-GHOST fork choice to update // the head of the chain based on attestations and instead accepts any valid received block @@ -142,6 +142,11 @@ var ( const deprecatedUsage = "DEPRECATED. DO NOT USE." var ( + deprecatedEnableDynamicCommitteeSubnets = &cli.BoolFlag{ + Name: "enable-dynamic-committee-subnets", + Usage: deprecatedUsage, + Hidden: true, + } deprecatedNoCustomConfigFlag = &cli.BoolFlag{ Name: "no-custom-config", Usage: deprecatedUsage, @@ -286,6 +291,7 @@ var ( ) var deprecatedFlags = []cli.Flag{ + deprecatedEnableDynamicCommitteeSubnets, deprecatedNoCustomConfigFlag, deprecatedEnableInitSyncQueue, deprecatedEnableFinalizedBlockRootIndexFlag, @@ -335,7 +341,7 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{ minimalConfigFlag, writeSSZStateTransitionsFlag, disableForkChoiceUnsafeFlag, - enableDynamicCommitteeSubnets, + disableDynamicCommitteeSubnets, disableSSZCache, enableEth1DataVoteCacheFlag, initSyncVerifyEverythingFlag, @@ -367,6 +373,4 @@ var E2EBeaconChainFlags = []string{ "--enable-state-gen-sig-verify", "--check-head-state", "--enable-state-field-trie", - // TODO(5123): This flag currently fails E2E. Commenting until it's resolved. - "--enable-dynamic-committee-subnets", } From 7e491ed728aaa512f9ea2c44c80a8c5e48fa05f9 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 13 Apr 2020 11:12:54 -0500 Subject: [PATCH 5/8] Update beacon-chain/sync/subscriber.go --- beacon-chain/sync/subscriber.go | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon-chain/sync/subscriber.go b/beacon-chain/sync/subscriber.go index b06ebbdd5f32..d4be1dde036c 100644 --- a/beacon-chain/sync/subscriber.go +++ b/beacon-chain/sync/subscriber.go @@ -13,7 +13,6 @@ import ( pubsub "github.com/libp2p/go-libp2p-pubsub" pb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "go.opencensus.io/trace" - "github.com/prysmaticlabs/prysm/beacon-chain/core/feed" statefeed "github.com/prysmaticlabs/prysm/beacon-chain/core/feed/state" "github.com/prysmaticlabs/prysm/beacon-chain/p2p" From 97d361bfd1146616e5443783848362d1a8fb17ba Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 13 Apr 2020 11:13:02 -0500 Subject: [PATCH 6/8] Update beacon-chain/p2p/service.go --- beacon-chain/p2p/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/p2p/service.go b/beacon-chain/p2p/service.go index 543b0f0e1ed9..bb7d6c05e0a5 100644 --- a/beacon-chain/p2p/service.go +++ b/beacon-chain/p2p/service.go @@ -46,7 +46,7 @@ var _ = shared.Service(&Service{}) // Check local table every 5 seconds for newly added peers. var pollingPeriod = 5 * time.Second -// Refresh rate of ENR set at once per slot. +// Refresh rate of ENR set at twice per slot. var refreshRate = time.Duration(params.BeaconConfig().SecondsPerSlot/2) * time.Second // search limit for number of peers in discovery v5. From 52fff61026e22ba2d117ede6a9dcc09cfd3a41a0 Mon Sep 17 00:00:00 2001 From: rauljordan Date: Mon, 13 Apr 2020 11:34:34 -0500 Subject: [PATCH 7/8] fix broken test --- .../subscriber_committee_index_beacon_attestation_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go b/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go index bdab5f9e1128..d7740c9dbc18 100644 --- a/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go +++ b/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go @@ -17,11 +17,15 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/operations/attestations" p2ptest "github.com/prysmaticlabs/prysm/beacon-chain/p2p/testing" mockSync "github.com/prysmaticlabs/prysm/beacon-chain/sync/initial-sync/testing" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/testutil" ) func TestService_committeeIndexBeaconAttestationSubscriber_ValidMessage(t *testing.T) { p := p2ptest.NewTestP2P(t) + fc := featureconfig.Get() + fc.DisableDynamicCommitteeSubnets = true + featureconfig.Init(fc) ctx := context.Background() db := dbtest.SetupDB(t) From 4a303812361c8c93735b9b154a6b498914ee6d03 Mon Sep 17 00:00:00 2001 From: rauljordan Date: Mon, 13 Apr 2020 12:03:09 -0500 Subject: [PATCH 8/8] gaz --- beacon-chain/sync/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/sync/BUILD.bazel b/beacon-chain/sync/BUILD.bazel index 3fe9df374e27..ca6884522a51 100644 --- a/beacon-chain/sync/BUILD.bazel +++ b/beacon-chain/sync/BUILD.bazel @@ -136,6 +136,7 @@ go_test( "//shared/attestationutil:go_default_library", "//shared/bls:go_default_library", "//shared/bytesutil:go_default_library", + "//shared/featureconfig:go_default_library", "//shared/params:go_default_library", "//shared/roughtime:go_default_library", "//shared/testutil:go_default_library",