Skip to content

Commit

Permalink
Weak subjectivity minor refactoring (#8715)
Browse files Browse the repository at this point in the history 
* Add IsWithinWeakSubjectivityPeriod helper method

* Add LatestWeakSubjectivityEpoch method

* ParseWeakSubjectivityInputString helper

* switch to Checkpoint

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
  • Loading branch information
@farazdagi @rauljordan
farazdagi and rauljordan authored Apr 6, 2021
1 parent 91df011 commit 74d0134
Show file tree
Hide file tree
Showing 10 changed files with 159 additions and 171 deletions.
29 changes: 14 additions & 15 deletions beacon-chain/blockchain/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,20 @@ type Service struct {

// Config options for the service.
type Config struct {
BeaconBlockBuf int
ChainStartFetcher powchain.ChainStartFetcher
BeaconDB db.HeadAccessDatabase
DepositCache *depositcache.DepositCache
AttPool attestations.Pool
ExitPool voluntaryexits.PoolManager
SlashingPool slashings.PoolManager
P2p p2p.Broadcaster
MaxRoutines int
StateNotifier statefeed.Notifier
ForkChoiceStore f.ForkChoicer
OpsService *attestations.Service
StateGen *stategen.State
WspBlockRoot []byte
WspEpoch types.Epoch
BeaconBlockBuf int
ChainStartFetcher powchain.ChainStartFetcher
BeaconDB db.HeadAccessDatabase
DepositCache *depositcache.DepositCache
AttPool attestations.Pool
ExitPool voluntaryexits.PoolManager
SlashingPool slashings.PoolManager
P2p p2p.Broadcaster
MaxRoutines int
StateNotifier statefeed.Notifier
ForkChoiceStore f.ForkChoicer
OpsService *attestations.Service
StateGen *stategen.State
WeakSubjectivityCheckpt *ethpb.Checkpoint
}

// NewService instantiates a new block service instance that will
Expand Down
12 changes: 6 additions & 6 deletions beacon-chain/blockchain/weak_subjectivity_checks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
// Reference design: https://github.com/ethereum/eth2.0-specs/blob/master/specs/phase0/weak-subjectivity.md#weak-subjectivity-sync-procedure
func (s *Service) VerifyWeakSubjectivityRoot(ctx context.Context) error {
// TODO(7342): Remove the following to fully use weak subjectivity in production.
if len(s.cfg.WspBlockRoot) == 0 || s.cfg.WspEpoch == 0 {
if s.cfg.WeakSubjectivityCheckpt == nil || len(s.cfg.WeakSubjectivityCheckpt.Root) == 0 || s.cfg.WeakSubjectivityCheckpt.Epoch == 0 {
return nil
}

Expand All @@ -23,12 +23,12 @@ func (s *Service) VerifyWeakSubjectivityRoot(ctx context.Context) error {
if s.wsVerified {
return nil
}
if s.cfg.WspEpoch > s.finalizedCheckpt.Epoch {
if s.cfg.WeakSubjectivityCheckpt.Epoch > s.finalizedCheckpt.Epoch {
return nil
}

r := bytesutil.ToBytes32(s.cfg.WspBlockRoot)
log.Infof("Performing weak subjectivity check for root %#x in epoch %d", r, s.cfg.WspEpoch)
r := bytesutil.ToBytes32(s.cfg.WeakSubjectivityCheckpt.Root)
log.Infof("Performing weak subjectivity check for root %#x in epoch %d", r, s.cfg.WeakSubjectivityCheckpt.Epoch)
// Save initial sync cached blocks to DB.
if err := s.cfg.BeaconDB.SaveBlocks(ctx, s.getInitSyncBlocks()); err != nil {
return err
Expand All @@ -38,7 +38,7 @@ func (s *Service) VerifyWeakSubjectivityRoot(ctx context.Context) error {
return fmt.Errorf("node does not have root in DB: %#x", r)
}

startSlot, err := helpers.StartSlot(s.cfg.WspEpoch)
startSlot, err := helpers.StartSlot(s.cfg.WeakSubjectivityCheckpt.Epoch)
if err != nil {
return err
}
Expand All @@ -56,5 +56,5 @@ func (s *Service) VerifyWeakSubjectivityRoot(ctx context.Context) error {
}
}

return fmt.Errorf("node does not have root in db corresponding to epoch: %#x %d", r, s.cfg.WspEpoch)
return fmt.Errorf("node does not have root in db corresponding to epoch: %#x %d", r, s.cfg.WeakSubjectivityCheckpt.Epoch)
}
19 changes: 8 additions & 11 deletions beacon-chain/blockchain/weak_subjectivity_checks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
types "github.com/prysmaticlabs/eth2-types"
ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
testDB "github.com/prysmaticlabs/prysm/beacon-chain/db/testing"
"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/testutil"
"github.com/prysmaticlabs/prysm/shared/testutil/require"
)
Expand All @@ -22,8 +23,7 @@ func TestService_VerifyWeakSubjectivityRoot(t *testing.T) {
tests := []struct {
wsVerified bool
wantErr bool
wsRoot [32]byte
wsEpoch types.Epoch
checkpt *ethpb.Checkpoint
finalizedEpoch types.Epoch
errString string
name string
Expand All @@ -34,45 +34,42 @@ func TestService_VerifyWeakSubjectivityRoot(t *testing.T) {
},
{
name: "already verified",
wsEpoch: 2,
checkpt: &ethpb.Checkpoint{Epoch: 2},
finalizedEpoch: 2,
wsVerified: true,
wantErr: false,
},
{
name: "not yet to verify, ws epoch higher than finalized epoch",
wsEpoch: 2,
checkpt: &ethpb.Checkpoint{Epoch: 2},
finalizedEpoch: 1,
wantErr: false,
},
{
name: "can't find the block in DB",
wsEpoch: 1,
wsRoot: [32]byte{'a'},
checkpt: &ethpb.Checkpoint{Root: bytesutil.PadTo([]byte{'a'}, 32), Epoch: 1},
finalizedEpoch: 3,
wantErr: true,
errString: "node does not have root in DB",
},
{
name: "can't find the block corresponds to ws epoch in DB",
wsEpoch: 2,
wsRoot: r, // Root belongs in epoch 1.
checkpt: &ethpb.Checkpoint{Root: r[:], Epoch: 2}, // Root belongs in epoch 1.
finalizedEpoch: 3,
wantErr: true,
errString: "node does not have root in db corresponding to epoch",
},
{
name: "can verify and pass",
wsEpoch: 1,
wsRoot: r,
checkpt: &ethpb.Checkpoint{Root: r[:], Epoch: 1},
finalizedEpoch: 3,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := &Service{
cfg: &Config{BeaconDB: beaconDB, WspBlockRoot: tt.wsRoot[:], WspEpoch: tt.wsEpoch},
cfg: &Config{BeaconDB: beaconDB, WeakSubjectivityCheckpt: tt.checkpt},
wsVerified: tt.wsVerified,
finalizedCheckpt: &ethpb.Checkpoint{Epoch: tt.finalizedEpoch},
}
Expand Down
43 changes: 43 additions & 0 deletions beacon-chain/core/helpers/weak_subjectivity.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package helpers

import (
"bytes"
"encoding/hex"
"errors"
"fmt"
"strconv"
"strings"

types "github.com/prysmaticlabs/eth2-types"
eth "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
Expand Down Expand Up @@ -156,3 +159,43 @@ func LatestWeakSubjectivityEpoch(st iface.ReadOnlyBeaconState) (types.Epoch, err
finalizedEpoch := st.FinalizedCheckpointEpoch()
return finalizedEpoch - (finalizedEpoch % wsPeriod), nil
}

// ParseWeakSubjectivityInputString parses "blocks_root:epoch_number" string into a checkpoint.
func ParseWeakSubjectivityInputString(wsCheckpointString string) (*eth.Checkpoint, error) {
if wsCheckpointString == "" {
return nil, nil
}

// Weak subjectivity input string must contain ":" to separate epoch and block root.
if !strings.Contains(wsCheckpointString, ":") {
return nil, fmt.Errorf("%s did not contain column", wsCheckpointString)
}

// Strip prefix "0x" if it's part of the input string.
wsCheckpointString = strings.TrimPrefix(wsCheckpointString, "0x")

// Get the hexadecimal block root from input string.
s := strings.Split(wsCheckpointString, ":")
if len(s) != 2 {
return nil, errors.New("weak subjectivity checkpoint input should be in `block_root:epoch_number` format")
}

bRoot, err := hex.DecodeString(s[0])
if err != nil {
return nil, err
}
if len(bRoot) != 32 {
return nil, errors.New("block root is not length of 32")
}

// Get the epoch number from input string.
epoch, err := strconv.ParseUint(s[1], 10, 64)
if err != nil {
return nil, err
}

return &eth.Checkpoint{
Epoch: types.Epoch(epoch),
Root: bRoot,
}, nil
}
70 changes: 70 additions & 0 deletions beacon-chain/core/helpers/weak_subjectivity_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,76 @@ func TestWeakSubjectivity_IsWithinWeakSubjectivityPeriod(t *testing.T) {
}
}

func TestWeakSubjectivity_ParseWeakSubjectivityInputString(t *testing.T) {
tests := []struct {
name string
input string
checkpt *ethpb.Checkpoint
wantedErr string
}{
{
name: "No column in string",
input: "0x111111;123",
wantedErr: "did not contain column",
},
{
name: "Too many columns in string",
input: "0x010203:123:456",
wantedErr: "weak subjectivity checkpoint input should be in `block_root:epoch_number` format",
},
{
name: "Incorrect block root length",
input: "0x010203:987",
wantedErr: "block root is not length of 32",
},
{
name: "Correct input",
input: "0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:123456789",
checkpt: &ethpb.Checkpoint{
Root: []byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255},
Epoch: types.Epoch(123456789),
},
},
{
name: "Correct input without 0x",
input: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:123456789",
checkpt: &ethpb.Checkpoint{
Root: []byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255},
Epoch: types.Epoch(123456789),
},
},
{
name: "Correct input",
input: "0xF0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:123456789",
checkpt: &ethpb.Checkpoint{
Root: []byte{0xf0, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255},
Epoch: types.Epoch(123456789),
},
},
{
name: "Correct input without 0x",
input: "F0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:123456789",
checkpt: &ethpb.Checkpoint{
Root: []byte{0xf0, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255},
Epoch: types.Epoch(123456789),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
wsCheckpt, err := helpers.ParseWeakSubjectivityInputString(tt.input)
if tt.wantedErr != "" {
require.ErrorContains(t, tt.wantedErr, err)
return
}
require.NoError(t, err)
require.NotNil(t, wsCheckpt)
require.DeepEqual(t, tt.checkpt.Root, wsCheckpt.Root, "Roots do not match")
require.Equal(t, tt.checkpt.Epoch, wsCheckpt.Epoch, "Epochs do not match")
})
}
}

func genState(t *testing.T, valCount uint64, avgBalance uint64) iface.BeaconState {
beaconState, err := testutil.NewBeaconState()
require.NoError(t, err)
Expand Down
8 changes: 2 additions & 6 deletions beacon-chain/node/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test")
go_library(
name = "go_default_library",
srcs = [
"helper.go",
"log.go",
"node.go",
],
Expand All @@ -16,6 +15,7 @@ go_library(
deps = [
"//beacon-chain/blockchain:go_default_library",
"//beacon-chain/cache/depositcache:go_default_library",
"//beacon-chain/core/helpers:go_default_library",
"//beacon-chain/db:go_default_library",
"//beacon-chain/db/kv:go_default_library",
"//beacon-chain/forkchoice:go_default_library",
Expand Down Expand Up @@ -56,17 +56,13 @@ go_library(
go_test(
name = "go_default_test",
size = "small",
srcs = [
"helper_test.go",
"node_test.go",
],
srcs = ["node_test.go"],
embed = [":go_default_library"],
deps = [
"//beacon-chain/core/feed/state:go_default_library",
"//shared/cmd:go_default_library",
"//shared/testutil/assert:go_default_library",
"//shared/testutil/require:go_default_library",
"@com_github_prysmaticlabs_eth2_types//:go_default_library",
"@com_github_sirupsen_logrus//hooks/test:go_default_library",
"@com_github_urfave_cli_v2//:go_default_library",
],
Expand Down
49 changes: 0 additions & 49 deletions beacon-chain/node/helper.go
View file

This file was deleted.

Loading

0 comments on commit 74d0134

Please sign in to comment.