Merge branch 'master' into invert-flag2

.github/ISSUE_TEMPLATE/feature_flag.md

@@ -0,0 +1,40 @@
+---
+name: "\U0001F984Feature Flag Tracking"
+about: Track a new feature, in development, in Prysm. This issue template should only be used by 
+developers or contributors!
+labels: Tracking
+
+---
+<!--💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎
+
+Hellooo! 😄 
+
+Thanks for taking the time to file a tracking issue for your new feature. These issues really help
+us track progress of features as they work their way through development. Be sure to review 
+shared/featureconfig/README.md for the latest documentation around feature flags.
+
+💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎💎-->
+
+# 🦄 Feature Tracking
+
+**Current status** (opt-in or opt-out): 
+
+**Opt-in feature flag name**:
+
+**Opt-out feature flag name**: 
+
+### Feature Description
+
+<!-- ✍️--> A clear and concise description of the new feature. Provide links to your technical
+design document or other supporting information.
+
+### Tasks
+
+- [ ] Feature flag created as opt-in
+- [ ] Feature implemented / code complete
+- [ ] Feature tested in production for adequate amount of time
+- [ ] Feature flag is inverted to be opt-out and the opt-in flag is deprecated
+- [ ] Feature has made it to a tagged production release as an opt-out flag
+- [ ] Opt-out feature flag is deprecated and old code paths are cleaned up
+
+This issue should be closed after all of the above tasks are complete.

beacon-chain/state/references_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/prysmaticlabs/go-bitfield"
 	p2ppb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
+	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 )
 
 func TestStateReferenceSharing_Finalizer(t *testing.T) {
@@ -49,6 +50,9 @@ func TestStateReferenceSharing_Finalizer(t *testing.T) {
 }
 
 func TestStateReferenceCopy_NoUnexpectedRootsMutation(t *testing.T) {
+	resetCfg := featureconfig.InitWithReset(&featureconfig.Flags{EnableStateRefCopy: true})
+	defer resetCfg()
+
 	root1, root2 := bytesutil.ToBytes32([]byte("foo")), bytesutil.ToBytes32([]byte("bar"))
 	a, err := InitializeFromProtoUnsafe(&p2ppb.BeaconState{
 		BlockRoots: [][]byte{
@@ -137,6 +141,9 @@ func TestStateReferenceCopy_NoUnexpectedRootsMutation(t *testing.T) {
 }
 
 func TestStateReferenceCopy_NoUnexpectedRandaoMutation(t *testing.T) {
+	resetCfg := featureconfig.InitWithReset(&featureconfig.Flags{EnableStateRefCopy: true})
+	defer resetCfg()
+
 	val1, val2 := []byte("foo"), []byte("bar")
 	a, err := InitializeFromProtoUnsafe(&p2ppb.BeaconState{
 		RandaoMixes: [][]byte{
@@ -194,6 +201,9 @@ func TestStateReferenceCopy_NoUnexpectedRandaoMutation(t *testing.T) {
 }
 
 func TestStateReferenceCopy_NoUnexpectedAttestationsMutation(t *testing.T) {
+	resetCfg := featureconfig.InitWithReset(&featureconfig.Flags{EnableStateRefCopy: true})
+	defer resetCfg()
+
 	assertAttFound := func(vals []*p2ppb.PendingAttestation, val uint64) {
 		for i := range vals {
 			if reflect.DeepEqual(vals[i].AggregationBits, bitfield.NewBitlist(val)) {

beacon-chain/state/setters.go

@@ -9,6 +9,7 @@ import (
 	"github.com/prysmaticlabs/go-bitfield"
 	coreutils "github.com/prysmaticlabs/prysm/beacon-chain/core/state/stateutils"
 	pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
+	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
 )
 
@@ -106,8 +107,12 @@ func (b *BeaconState) UpdateBlockRootAtIndex(idx uint64, blockRoot [32]byte) err
 	r := b.state.BlockRoots
 	if ref := b.sharedFieldReferences[blockRoots]; ref.refs > 1 {
 		// Copy on write since this is a shared array.
-		r = make([][]byte, len(b.state.BlockRoots))
-		copy(r, b.state.BlockRoots)
+		if featureconfig.Get().EnableStateRefCopy {
+			r = make([][]byte, len(b.state.BlockRoots))
+			copy(r, b.state.BlockRoots)
+		} else {
+			r = b.BlockRoots()
+		}
 
 		ref.MinusRef()
 		b.sharedFieldReferences[blockRoots] = &reference{refs: 1}
@@ -159,8 +164,12 @@ func (b *BeaconState) UpdateStateRootAtIndex(idx uint64, stateRoot [32]byte) err
 	r := b.state.StateRoots
 	if ref := b.sharedFieldReferences[stateRoots]; ref.refs > 1 {
 		// Perform a copy since this is a shared reference and we don't want to mutate others.
-		r = make([][]byte, len(b.state.StateRoots))
-		copy(r, b.state.StateRoots)
+		if featureconfig.Get().EnableStateRefCopy {
+			r = make([][]byte, len(b.state.StateRoots))
+			copy(r, b.state.StateRoots)
+		} else {
+			r = b.StateRoots()
+		}
 
 		ref.MinusRef()
 		b.sharedFieldReferences[stateRoots] = &reference{refs: 1}
@@ -236,9 +245,12 @@ func (b *BeaconState) AppendEth1DataVotes(val *ethpb.Eth1Data) error {
 	b.lock.RLock()
 	votes := b.state.Eth1DataVotes
 	if b.sharedFieldReferences[eth1DataVotes].refs > 1 {
-		votes = make([]*ethpb.Eth1Data, len(b.state.Eth1DataVotes))
-		copy(votes, b.state.Eth1DataVotes)
-
+		if featureconfig.Get().EnableStateRefCopy {
+			votes = make([]*ethpb.Eth1Data, len(b.state.Eth1DataVotes))
+			copy(votes, b.state.Eth1DataVotes)
+		} else {
+			votes = b.Eth1DataVotes()
+		}
 		b.sharedFieldReferences[eth1DataVotes].MinusRef()
 		b.sharedFieldReferences[eth1DataVotes] = &reference{refs: 1}
 	}
@@ -442,9 +454,12 @@ func (b *BeaconState) UpdateRandaoMixesAtIndex(idx uint64, val []byte) error {
 	b.lock.RLock()
 	mixes := b.state.RandaoMixes
 	if refs := b.sharedFieldReferences[randaoMixes].refs; refs > 1 {
-		mixes = make([][]byte, len(b.state.RandaoMixes))
-		copy(mixes, b.state.RandaoMixes)
-
+		if featureconfig.Get().EnableStateRefCopy {
+			mixes = make([][]byte, len(b.state.RandaoMixes))
+			copy(mixes, b.state.RandaoMixes)
+		} else {
+			mixes = b.RandaoMixes()
+		}
 		b.sharedFieldReferences[randaoMixes].MinusRef()
 		b.sharedFieldReferences[randaoMixes] = &reference{refs: 1}
 	}
@@ -553,9 +568,12 @@ func (b *BeaconState) AppendHistoricalRoots(root [32]byte) error {
 	b.lock.RLock()
 	roots := b.state.HistoricalRoots
 	if b.sharedFieldReferences[historicalRoots].refs > 1 {
-		roots = make([][]byte, len(b.state.HistoricalRoots))
-		copy(roots, b.state.HistoricalRoots)
-
+		if featureconfig.Get().EnableStateRefCopy {
+			roots = make([][]byte, len(b.state.HistoricalRoots))
+			copy(roots, b.state.HistoricalRoots)
+		} else {
+			roots = b.HistoricalRoots()
+		}
 		b.sharedFieldReferences[historicalRoots].MinusRef()
 		b.sharedFieldReferences[historicalRoots] = &reference{refs: 1}
 	}
@@ -579,9 +597,12 @@ func (b *BeaconState) AppendCurrentEpochAttestations(val *pbp2p.PendingAttestati
 
 	atts := b.state.CurrentEpochAttestations
 	if b.sharedFieldReferences[currentEpochAttestations].refs > 1 {
-		atts = make([]*pbp2p.PendingAttestation, len(b.state.CurrentEpochAttestations))
-		copy(atts, b.state.CurrentEpochAttestations)
-
+		if featureconfig.Get().EnableStateRefCopy {
+			atts = make([]*pbp2p.PendingAttestation, len(b.state.CurrentEpochAttestations))
+			copy(atts, b.state.CurrentEpochAttestations)
+		} else {
+			atts = b.CurrentEpochAttestations()
+		}
 		b.sharedFieldReferences[currentEpochAttestations].MinusRef()
 		b.sharedFieldReferences[currentEpochAttestations] = &reference{refs: 1}
 	}
@@ -605,9 +626,12 @@ func (b *BeaconState) AppendPreviousEpochAttestations(val *pbp2p.PendingAttestat
 	b.lock.RLock()
 	atts := b.state.PreviousEpochAttestations
 	if b.sharedFieldReferences[previousEpochAttestations].refs > 1 {
-		atts = make([]*pbp2p.PendingAttestation, len(b.state.PreviousEpochAttestations))
-		copy(atts, b.state.PreviousEpochAttestations)
-
+		if featureconfig.Get().EnableStateRefCopy {
+			atts = make([]*pbp2p.PendingAttestation, len(b.state.PreviousEpochAttestations))
+			copy(atts, b.state.PreviousEpochAttestations)
+		} else {
+			atts = b.PreviousEpochAttestations()
+		}
 		b.sharedFieldReferences[previousEpochAttestations].MinusRef()
 		b.sharedFieldReferences[previousEpochAttestations] = &reference{refs: 1}
 	}

shared/featureconfig/README.md

@@ -0,0 +1,61 @@
+# Prysm Feature Flags
+
+Part of Prysm's feature development often involves use of feature flags which serve as a way to
+toggle new features as they are introduced. Using this methodology, you are assured that your
+feature can be safely tested in production with a fall back option if any regression were to occur.
+This reduces the likelihood of an emergency release or rollback of a given feature due to
+unforeseen issues.
+
+## When to use a feature flag?
+
+It is best to use a feature flag any time you are adding or removing functionality in an existing
+critical application path. 
+
+Examples of when to use a feature flag:
+
+- Adding a caching layer to an expensive RPC call.
+- Optimizing a particular algorithm or routine.
+- Introducing a temporary public testnet configuration.
+
+Examples of when not to use a feature flag:
+
+- Adding a new gRPC endpoint. (Unless dangerous or expensive to expose).
+- Adding new logging statements.
+- Removing dead code.
+- Adding any trivial feature with no risk of regressions to existing functionality.
+
+## How to use a feature flag?
+
+Once it has been decided that you should use a feature flag. Follow these steps to safely
+releasing your feature. In general, try to create a single PR for each step of this process.
+
+1. Add your feature flag to shared/featureconfig/flags.go, use the flag to toggle a boolean in the
+feature config in shared/featureconfig/config.go. It is a good idea to use the `enable` prefix for
+your flag since you're going to invert the flag in a later step. i.e you will use `disable` prefix
+later. For example, `--enable-my-feature`. Additionally, [create a feature flag tracking issue](https://github.com/prysmaticlabs/prysm/issues/new?template=feature_flag.md) 
+for your feature using the appropriate issue template.
+2. Use the feature throughout the application to enable your new functionality and be sure to write
+tests carefully and thoughtfully to ensure you have tested all of your new funcitonality without losing
+coverage on the existing functionality. This is considered an opt-in feature flag. Example usage:
+```go
+func someExistingMethod(ctx context.Context) error {
+    if featureconfig.Get().MyNewFeature {
+       return newMethod(ctx)
+    }
+    // Otherwise continue with the existing code path.
+}
+``` 
+3. Add the flag to the end to end tests. This set of flags can also be found in shared/featureconfig/flags.go. 
+4. Test the functionality locally and safely in production. Once you have enough confidence that
+your new function works and is safe to release then move onto the next step.
+5. Move your existing flag to the deprecated section of shared/featureconfig/flags.go. It is
+important NOT to delete your existing flag outright. Deleting a flag can be extremely frustrating
+to users as it may break their existing workflow! Marking a flag as deprecated gives users time to
+adjust their start scripts and workflow. Add another feature flag to represent the inverse of your
+flag from step 1. For example `--disable-my-feature`. Read the value of this flag to appropriately
+the config value in shared/featureconfig/config.go.
+6. After your feature has been included in a release as "opt-out" and there are no issues,
+deprecate the opt-out feature flag, delete the config field from shared/featureconfig/config.go,
+delete any deprecated / obsolete code paths.
+
+Deprecated flags are deleted upon each major semver point release. Ex: v1, v2, v3.

shared/featureconfig/config.go

@@ -54,6 +54,7 @@ type Flags struct {
 	NewStateMgmt                               bool // NewStateMgmt enables the new state mgmt service.
 	EnableFieldTrie                            bool // EnableFieldTrie enables the state from using field specific tries when computing the root.
 	NoInitSyncBatchSaveBlocks                  bool // NoInitSyncBatchSaveBlocks disables batch save blocks mode during initial syncing.
+	EnableStateRefCopy                         bool // EnableStateRefCopy copies the references to objects instead of the objects themselves when copying state fields.
 	WaitForSynced                              bool // WaitForSynced uses WaitForSynced in validator startup to ensure it can communicate with the beacon node as soon as possible.
 	SkipRegenHistoricalStates                  bool // SkipRegenHistoricalState skips regenerating historical states from genesis to last finalized. This enables a quick switch over to using new-state-mgmt.
 	EnableInitSyncWeightedRoundRobin           bool // EnableInitSyncWeightedRoundRobin enables weighted round robin fetching optimization in initial syncing.
@@ -209,6 +210,11 @@ func ConfigureBeaconChain(ctx *cli.Context) {
 		log.Warn("Enabling weighted round robin in initial syncing")
 		cfg.EnableInitSyncWeightedRoundRobin = true
 	}
+	cfg.EnableStateRefCopy = true
+	if ctx.Bool(disableStateRefCopy.Name) {
+		log.Warn("Disabling state reference copy")
+		cfg.EnableStateRefCopy = false
+	}
 	Init(cfg)
 }
 

shared/featureconfig/flags.go

@@ -133,6 +133,10 @@ var (
 		Name:  "disable-init-sync-batch-save-blocks",
 		Usage: "Instead of saving batch blocks to the DB during initial syncing, this disables batch saving of blocks",
 	}
+	disableStateRefCopy = &cli.BoolFlag{
+		Name:  "disable-state-ref-copy",
+		Usage: "Disables the usage of a new copying method for our state fields.",
+	}
 	waitForSyncedFlag = &cli.BoolFlag{
 		Name:  "wait-for-synced",
 		Usage: "Uses WaitForSynced for validator startup, to ensure a validator is able to communicate with the beacon node as quick as possible",
@@ -476,6 +480,7 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
 	skipRegenHistoricalStates,
 	enableInitSyncWeightedRoundRobin,
 	disableFieldTrie,
+	disableStateRefCopy,
 }...)
 
 // E2EBeaconChainFlags contains a list of the beacon chain feature flags to be tested in E2E.