From b00bbfeec77308c9f6e33d8a3a22b259425e629e Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Fri, 8 May 2020 13:50:39 -0700 Subject: [PATCH] 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 | 1 - beacon-chain/core/helpers/signing_root.go | 9 +++++++-- .../core/helpers/signing_root_test.go | 6 ------ beacon-chain/state/stateutil/blocks.go | 12 ++++------- beacon-chain/state/stateutil/blocks_test.go | 20 ++++++++++++++++--- shared/featureconfig/config.go | 5 ----- shared/featureconfig/flags.go | 13 ++++++------ 7 files changed, 34 insertions(+), 32 deletions(-) diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index b283bc451186..00c9210f3270 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/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", diff --git a/beacon-chain/core/helpers/signing_root.go b/beacon-chain/core/helpers/signing_root.go index b151af734110..d37d3aa69e78 100644 --- a/beacon-chain/core/helpers/signing_root.go +++ b/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) } diff --git a/beacon-chain/core/helpers/signing_root_test.go b/beacon-chain/core/helpers/signing_root_test.go index 7164843dd508..a8104e1f269e 100644 --- a/beacon-chain/core/helpers/signing_root_test.go +++ b/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 := ðpb.BeaconBlock{ diff --git a/beacon-chain/state/stateutil/blocks.go b/beacon-chain/state/stateutil/blocks.go index 5dfc348fea33..48297673968f 100644 --- a/beacon-chain/state/stateutil/blocks.go +++ b/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 { diff --git a/beacon-chain/state/stateutil/blocks_test.go b/beacon-chain/state/stateutil/blocks_test.go index db63cf0a05b1..38291540b752 100644 --- a/beacon-chain/state/stateutil/blocks_test.go +++ b/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(ðpb.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") + } +} \ No newline at end of file diff --git a/shared/featureconfig/config.go b/shared/featureconfig/config.go index 0ca2d5b396d3..dcedb43b82e6 100644 --- a/shared/featureconfig/config.go +++ b/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 diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index 794437821f3d..db3ffe31bd52 100644 --- a/shared/featureconfig/flags.go +++ b/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", }