Revert state copy PR #4811 (#4825)

* Revert "Add mempool feature flag (#4824)"

This reverts commit b3f2a33.
* Revert "Optimize Copying of Fields (#4811)"

This reverts commit 4f654d3.

beacon-chain/blockchain/process_attestation_helpers.go

@@ -1,7 +1,6 @@
 package blockchain
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 
@@ -29,14 +28,6 @@ func (s *Service) getAttPreState(ctx context.Context, c *ethpb.Checkpoint) (*sta
 		return cachedState, nil
 	}
 
-	headRoot, err := s.HeadRoot(ctx)
-	if err != nil {
-		return nil, errors.Wrapf(err, "could not get head root")
-	}
-	if bytes.Equal(headRoot, c.Root) {
-		return s.HeadState(ctx)
-	}
-
 	baseState, err := s.beaconDB.State(ctx, bytesutil.ToBytes32(c.Root))
 	if err != nil {
 		return nil, errors.Wrapf(err, "could not get pre state for slot %d", helpers.StartSlot(c.Epoch))

beacon-chain/blockchain/process_block_helpers.go

@@ -73,13 +73,7 @@ func (s *Service) verifyBlkPreState(ctx context.Context, b *ethpb.BeaconBlock) (
 		}
 		return preState.Copy(), nil
 	}
-	headRoot, err := s.HeadRoot(ctx)
-	if err != nil {
-		return nil, errors.Wrapf(err, "could not get head root")
-	}
-	if bytes.Equal(headRoot, b.ParentRoot) {
-		return s.HeadState(ctx)
-	}
+
 	preState, err := s.beaconDB.State(ctx, bytesutil.ToBytes32(b.ParentRoot))
 	if err != nil {
 		return nil, errors.Wrapf(err, "could not get pre state for slot %d", b.Slot)

beacon-chain/state/BUILD.bazel

@@ -19,7 +19,6 @@ go_library(
         "//proto/beacon/p2p/v1:go_default_library",
         "//shared/bytesutil:go_default_library",
         "//shared/hashutil:go_default_library",
-        "//shared/memorypool:go_default_library",
         "//shared/params:go_default_library",
         "//shared/stateutil:go_default_library",
         "@com_github_gogo_protobuf//proto:go_default_library",

beacon-chain/state/setters.go

@@ -9,7 +9,6 @@ import (
 	"github.com/prysmaticlabs/go-bitfield"
 	pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
-	"github.com/prysmaticlabs/prysm/shared/memorypool"
 )
 
 type fieldIndex int
@@ -40,9 +39,6 @@ const (
 	previousJustifiedCheckpoint
 	currentJustifiedCheckpoint
 	finalizedCheckpoint
-	// validatorIdxMap is not part of the state, but is used so as to be able to keep
-	// track of references to it to allow for efficient copy on write.
-	validatorIdxMap
 )
 
 // SetGenesisTime for the beacon state.
@@ -312,21 +308,14 @@ func (b *BeaconState) UpdateValidatorAtIndex(idx uint64, val *ethpb.Validator) e
 // SetValidatorIndexByPubkey updates the validator index mapping maintained internally to
 // a given input 48-byte, public key.
 func (b *BeaconState) SetValidatorIndexByPubkey(pubKey [48]byte, validatorIdx uint64) {
-	idxMap := b.valIdxMap
-	b.lock.RLock()
-	if b.sharedFieldReferences[validatorIdxMap].refs > 1 {
-		// copy-on-write for idx map
-		idxMap = b.validatorIndexMap()
-		b.sharedFieldReferences[validatorIdxMap].refs--
-		b.sharedFieldReferences[validatorIdxMap] = &reference{refs: 1}
-	}
-	b.lock.RUnlock()
+	// Copy on write since this is a shared map.
+	m := b.validatorIndexMap()
 
 	b.lock.Lock()
 	defer b.lock.Unlock()
 
-	idxMap[pubKey] = validatorIdx
-	b.valIdxMap = idxMap
+	m[pubKey] = validatorIdx
+	b.valIdxMap = m
 }
 
 // SetBalances for the beacon state. This PR updates the entire
@@ -392,9 +381,7 @@ func (b *BeaconState) UpdateRandaoMixesAtIndex(val []byte, idx uint64) error {
 	b.lock.RLock()
 	mixes := b.state.RandaoMixes
 	if refs := b.sharedFieldReferences[randaoMixes].refs; refs > 1 {
-		newMixes := memorypool.GetDoubleByteSlice(len(mixes))
-		copy(newMixes, mixes)
-		mixes = newMixes
+		mixes = b.RandaoMixes()
 		b.sharedFieldReferences[randaoMixes].refs--
 		b.sharedFieldReferences[randaoMixes] = &reference{refs: 1}
 	}
@@ -505,9 +492,7 @@ func (b *BeaconState) AppendCurrentEpochAttestations(val *pbp2p.PendingAttestati
 
 	atts := b.state.CurrentEpochAttestations
 	if b.sharedFieldReferences[currentEpochAttestations].refs > 1 {
-		copiedAtts := make([]*pbp2p.PendingAttestation, len(atts), len(atts)+1)
-		copy(copiedAtts, atts)
-		atts = copiedAtts
+		atts = b.CurrentEpochAttestations()
 		b.sharedFieldReferences[currentEpochAttestations].refs--
 		b.sharedFieldReferences[currentEpochAttestations] = &reference{refs: 1}
 	}
@@ -527,9 +512,7 @@ func (b *BeaconState) AppendPreviousEpochAttestations(val *pbp2p.PendingAttestat
 	b.lock.RLock()
 	atts := b.state.PreviousEpochAttestations
 	if b.sharedFieldReferences[previousEpochAttestations].refs > 1 {
-		copiedAtts := make([]*pbp2p.PendingAttestation, len(atts), len(atts)+1)
-		copy(copiedAtts, atts)
-		atts = copiedAtts
+		atts = b.PreviousEpochAttestations()
 		b.sharedFieldReferences[previousEpochAttestations].refs--
 		b.sharedFieldReferences[previousEpochAttestations] = &reference{refs: 1}
 	}
@@ -549,9 +532,7 @@ func (b *BeaconState) AppendValidator(val *ethpb.Validator) error {
 	b.lock.RLock()
 	vals := b.state.Validators
 	if b.sharedFieldReferences[validators].refs > 1 {
-		copiedVals := make([]*ethpb.Validator, len(b.state.Validators), len(b.state.Validators)+1)
-		copy(copiedVals, b.state.Validators)
-		vals = copiedVals
+		vals = b.Validators()
 		b.sharedFieldReferences[validators].refs--
 		b.sharedFieldReferences[validators] = &reference{refs: 1}
 	}

beacon-chain/state/types.go

@@ -12,7 +12,6 @@ import (
 	pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
-	"github.com/prysmaticlabs/prysm/shared/memorypool"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/shared/stateutil"
 )
@@ -74,7 +73,6 @@ func InitializeFromProtoUnsafe(st *pbp2p.BeaconState) (*BeaconState, error) {
 	b.sharedFieldReferences[validators] = &reference{refs: 1}
 	b.sharedFieldReferences[balances] = &reference{refs: 1}
 	b.sharedFieldReferences[historicalRoots] = &reference{refs: 1}
-	b.sharedFieldReferences[validatorIdxMap] = &reference{refs: 1}
 
 	return b, nil
 }
@@ -143,11 +141,8 @@ func (b *BeaconState) Copy() *BeaconState {
 
 	// Finalizer runs when dst is being destroyed in garbage collection.
 	runtime.SetFinalizer(dst, func(b *BeaconState) {
-		for i, v := range b.sharedFieldReferences {
+		for _, v := range b.sharedFieldReferences {
 			v.refs--
-			if i == randaoMixes && v.refs == 0 {
-				memorypool.PutDoubleByteSlice(b.state.RandaoMixes)
-			}
 		}
 	})
 
@@ -171,12 +166,6 @@ func (b *BeaconState) HashTreeRoot() ([32]byte, error) {
 	}
 
 	for field := range b.dirtyFields {
-		// do not compute root for field
-		// thats not part of the state.
-		if field == validatorIdxMap {
-			delete(b.dirtyFields, field)
-			continue
-		}
 		root, err := b.rootSelector(field)
 		if err != nil {
 			return [32]byte{}, err

shared/featureconfig/config.go

@@ -42,7 +42,6 @@ type Flags struct {
 	ProtectAttester                            bool   // ProtectAttester prevents the validator client from signing any attestations that would be considered a slashable offense.
 	DisableStrictAttestationPubsubVerification bool   // DisableStrictAttestationPubsubVerification will disabling strict signature verification in pubsub.
 	DisableUpdateHeadPerAttestation            bool   // DisableUpdateHeadPerAttestation will disabling update head on per attestation basis.
-	EnableByteMempool                          bool   // EnaableByteMempool memory management.
 
 	// DisableForkChoice disables using LMD-GHOST fork choice to update
 	// the head of the chain based on attestations and instead accepts any valid received block
@@ -149,10 +148,6 @@ func ConfigureBeaconChain(ctx *cli.Context) {
 		log.Warn("Disabled update head on per attestation basis")
 		cfg.DisableUpdateHeadPerAttestation = true
 	}
-	if ctx.GlobalBool(enableByteMempool.Name) {
-		log.Warn("Enabling experimental memory management for beacon state")
-		cfg.EnableByteMempool = true
-	}
 
 	Init(cfg)
 }

shared/featureconfig/flags.go

@@ -98,10 +98,6 @@ var (
 		Name:  "disable-update-head-attestation",
 		Usage: "Disable update fork choice head on per attestation. See PR 4802 for details.",
 	}
-	enableByteMempool = cli.BoolFlag{
-		Name: "enable-byte-mempool",
-		Usage: "Enable use of sync.Pool for certain byte arrays in the beacon state",
-	}
 )
 
 // Deprecated flags list.
@@ -260,7 +256,6 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
 	cacheFilteredBlockTreeFlag,
 	disableStrictAttestationPubsubVerificationFlag,
 	disableUpdateHeadPerAttestation,
-	enableByteMempool,
 }...)
 
 // E2EBeaconChainFlags contains a list of the beacon chain feature flags to be tested in E2E.
@@ -273,5 +268,4 @@ var E2EBeaconChainFlags = []string{
 	"--enable-eth1-data-vote-cache",
 	"--initial-sync-cache-state",
 	"--proto-array-forkchoice",
-	"--enable-byte-mempool",
 }