Release enableCustomBlockHTR to all (#5742)

* Release deprecatedEnableCustomBlockHTR to all

* Release deprecatedEnableCustomBlockHTR to all

* Fix builds

* Treat nil body as empty

* Use custom HTR in signing root

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

beacon-chain/core/helpers/BUILD.bazel

@@ -68,7 +68,6 @@ go_test(
         "//proto/beacon/p2p/v1:go_default_library",
         "//shared/bls:go_default_library",
         "//shared/bytesutil:go_default_library",
-        "//shared/featureconfig:go_default_library",
         "//shared/hashutil:go_default_library",
         "//shared/params:go_default_library",
         "//shared/roughtime:go_default_library",

beacon-chain/core/helpers/signing_root.go

@@ -35,9 +35,14 @@ var ErrSigFailedToVerify = errors.New("signature did not verify")
 //    )
 //    return hash_tree_root(domain_wrapped_object)
 func ComputeSigningRoot(object interface{}, domain []byte) ([32]byte, error) {
-	// utilise generic ssz library
 	return signingRoot(func() ([32]byte, error) {
-		return ssz.HashTreeRoot(object)
+		switch object.(type) {
+		case *ethpb.BeaconBlock:
+			return stateutil.BlockRoot(object.(*ethpb.BeaconBlock))
+		default:
+			// utilise generic ssz library
+			return ssz.HashTreeRoot(object)
+		}
 	}, domain)
 }
 

beacon-chain/core/helpers/signing_root_test.go

@@ -8,7 +8,6 @@ import (
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"
 	ethereum_beacon_p2p_v1 "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
-	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
 
@@ -40,11 +39,6 @@ func TestComputeDomain_OK(t *testing.T) {
 }
 
 func TestSigningRoot_Compatibility(t *testing.T) {
-	featureFlags := new(featureconfig.Flags)
-	featureFlags.EnableBlockHTR = true
-	reset := featureconfig.InitWithReset(featureFlags)
-	defer reset()
-
 	parRoot := [32]byte{'A'}
 	stateRoot := [32]byte{'B'}
 	blk := &ethpb.BeaconBlock{

beacon-chain/state/stateutil/blocks.go

@@ -39,9 +39,6 @@ func BlockHeaderRoot(header *ethpb.BeaconBlockHeader) ([32]byte, error) {
 
 // BlockRoot returns the block hash tree root of the provided block.
 func BlockRoot(blk *ethpb.BeaconBlock) ([32]byte, error) {
-	if !featureconfig.Get().EnableBlockHTR {
-		return ssz.HashTreeRoot(blk)
-	}
 	fieldRoots := make([][32]byte, 5)
 	if blk != nil {
 		headerSlotBuf := make([]byte, 8)
@@ -67,14 +64,13 @@ func BlockRoot(blk *ethpb.BeaconBlock) ([32]byte, error) {
 
 // BlockBodyRoot returns the hash tree root of the block body.
 func BlockBodyRoot(body *ethpb.BeaconBlockBody) ([32]byte, error) {
-	if !featureconfig.Get().EnableBlockHTR {
-		return ssz.HashTreeRoot(body)
+	if body == nil {
+		// Treat nil body to be the same as empty. This is mostly for test setup purposes and would
+		// be very unlikely to happen in production workflow.
+		return [32]byte{117, 149, 118, 243, 29, 85, 147, 152, 201, 11, 234, 19, 146, 229, 35, 209, 93, 246, 109, 242, 141, 181, 176, 126, 79, 196, 1, 189, 124, 203, 199, 62}, nil
 	}
 	hasher := hashutil.CustomSHA256Hasher()
 	fieldRoots := make([][32]byte, 8)
-	if body == nil {
-		return [32]byte{}, errors.New("nil block body provided")
-	}
 	rawRandao := bytesutil.ToBytes96(body.RandaoReveal)
 	packedRandao, err := pack([][]byte{rawRandao[:]})
 	if err != nil {

beacon-chain/state/stateutil/blocks_test.go

@@ -4,13 +4,11 @@ import (
 	"testing"
 
 	"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"
-	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/testutil"
+	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 )
 
 func TestBlockRoot(t *testing.T) {
-	resetCfg := featureconfig.InitWithReset(&featureconfig.Flags{EnableBlockHTR: true})
-	defer resetCfg()
 	genState, keys := testutil.DeterministicGenesisState(t, 100)
 	blk, err := testutil.GenerateFullBlock(genState, keys, testutil.DefaultBlockGenConfig(), 10)
 	if err != nil {
@@ -43,3 +41,19 @@ func TestBlockRoot(t *testing.T) {
 		t.Fatalf("Wanted %#x but got %#x", expectedRoot, receivedRoot)
 	}
 }
+
+func TestBlockBodyRoot_NilIsSameAsEmpty(t *testing.T) {
+	a, err := stateutil.BlockBodyRoot(&ethpb.BeaconBlockBody{})
+	if err != nil {
+		t.Error(err)
+	}
+	b, err := stateutil.BlockBodyRoot(nil)
+	if err != nil {
+		t.Error(err)
+	}
+	if a != b {
+		t.Log(a)
+		t.Log(b)
+		t.Error("A nil and empty block body do not generate the same root")
+	}
+}

shared/featureconfig/config.go

@@ -50,7 +50,6 @@ type Flags struct {
 	NewStateMgmt                               bool // NewStateMgmt enables the new state mgmt service.
 	DisableInitSyncQueue                       bool // DisableInitSyncQueue disables the new initial sync implementation.
 	EnableFieldTrie                            bool // EnableFieldTrie enables the state from using field specific tries when computing the root.
-	EnableBlockHTR                             bool // EnableBlockHTR enables custom hashing of our beacon blocks.
 	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.
@@ -195,10 +194,6 @@ func ConfigureBeaconChain(ctx *cli.Context) {
 		log.Warn("Enabling state field trie")
 		cfg.EnableFieldTrie = true
 	}
-	if ctx.Bool(enableCustomBlockHTR.Name) {
-		log.Warn("Enabling custom block hashing")
-		cfg.EnableBlockHTR = true
-	}
 	if ctx.Bool(disableInitSyncBatchSaveBlocks.Name) {
 		log.Warn("Disabling init sync batch save blocks mode")
 		cfg.NoInitSyncBatchSaveBlocks = true

shared/featureconfig/flags.go

@@ -128,10 +128,6 @@ var (
 		Name:  "enable-state-field-trie",
 		Usage: "Enables the usage of state field tries to compute the state root",
 	}
-	enableCustomBlockHTR = &cli.BoolFlag{
-		Name:  "enable-custom-block-htr",
-		Usage: "Enables the usage of a custom hashing method for our block",
-	}
 	disableInitSyncBatchSaveBlocks = &cli.BoolFlag{
 		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",
@@ -152,7 +148,6 @@ var (
 
 // devModeFlags holds list of flags that are set when development mode is on.
 var devModeFlags = []cli.Flag{
-	enableCustomBlockHTR,
 	enableStateRefCopy,
 	enableFieldTrie,
 	enableNewStateMgmt,
@@ -323,6 +318,11 @@ var (
 		Usage:  deprecatedUsage,
 		Hidden: true,
 	}
+	deprecatedEnableCustomBlockHTR = &cli.BoolFlag{
+		Name:  "enable-custom-block-htr",
+		Usage:  deprecatedUsage,
+		Hidden: true,
+	}
 	deprecatedDisableInitSyncQueueFlag = &cli.BoolFlag{
 		Name:   "disable-init-sync-queue",
 		Usage:  deprecatedUsage,
@@ -364,6 +364,7 @@ var deprecatedFlags = []cli.Flag{
 	deprecatedDisableProtectProposerFlag,
 	deprecatedDisableProtectAttesterFlag,
 	deprecatedDisableInitSyncQueueFlag,
+	deprecatedEnableCustomBlockHTR,
 }
 
 // ValidatorFlags contains a list of all the feature flags that apply to the validator client.
@@ -414,7 +415,6 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
 	broadcastSlashingFlag,
 	enableNewStateMgmt,
 	enableFieldTrie,
-	enableCustomBlockHTR,
 	disableInitSyncBatchSaveBlocks,
 	enableStateRefCopy,
 	waitForSyncedFlag,
@@ -430,5 +430,4 @@ var E2EBeaconChainFlags = []string{
 	"--enable-state-field-trie",
 	"--enable-state-ref-copy",
 	"--enable-new-state-mgmt",
-	"--enable-custom-block-htr",
 }