Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and verify weak-subjectivity-checkpoint flag #7256

Merged
merged 10 commits into from
Sep 17, 2020
7 changes: 7 additions & 0 deletions beacon-chain/flags/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,11 @@ var (
Name: "network-id",
Usage: "Sets the network id of the beacon chain.",
}
// WeakSubjectivityCheckpt defines the weak subjectivity checkpoint the node must sync through to defend against long range attacks.
WeakSubjectivityCheckpt = &cli.StringFlag{
Name: "weak-subjectivity-checkpoint",
Usage: "Input in `block_root:epoch_number` format. This guarantee that syncing leads to the given Weak Subjectivity Checkpoint being in the canonical chain. " +
"If such a sync is not possible, the node will treat it a critical and irrecoverable failure",
Value: "",
}
)
10 changes: 8 additions & 2 deletions beacon-chain/node/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test")

go_library(
name = "go_default_library",
srcs = ["node.go"],
srcs = [
"helper.go",
"node.go",
],
importpath = "github.com/prysmaticlabs/prysm/beacon-chain/node",
visibility = ["//beacon-chain:__subpackages__"],
deps = [
Expand Down Expand Up @@ -46,7 +49,10 @@ go_library(
go_test(
name = "go_default_test",
size = "small",
srcs = ["node_test.go"],
srcs = [
"helper_test.go",
"node_test.go",
],
embed = [":go_default_library"],
deps = [
"//beacon-chain/core/feed/state:go_default_library",
Expand Down
49 changes: 49 additions & 0 deletions beacon-chain/node/helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package node

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

// Given input string `block_root:epoch_number`, this verifies the input string is valid, and
// returns the block root as bytes and epoch number as unsigned integers.
func convertWspInput(wsp string) ([]byte, uint64, error) {
if wsp == "" {
return nil, 0, nil
}

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

// Strip prefix "0x" if it's part of the input string.
if strings.HasPrefix(wsp, "0x") {
wsp = wsp[2:]
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe output why it is not well formatted, showing them the expected format. Perhaps that should also be part of the flag usage if not specified already

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's part of the flag usage. I'll reiterate in the error again


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

// Get the epoch number from input string.
epoch, err := strconv.ParseUint(s[1], 10, 64)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if len is < 2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll add a check

if err != nil {
return nil, 0, err
}

return bRoot, epoch, nil
}
67 changes: 67 additions & 0 deletions beacon-chain/node/helper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package node

import (
"reflect"
"testing"

"github.com/prysmaticlabs/prysm/shared/testutil/require"
)

func TestConvertWspInput(t *testing.T) {
tests := []struct {
name string
input string
bRoot []byte
epoch uint64
wantErr bool
errStr string
}{
{
name: "No column in string",
input: "0x111111;123",
wantErr: true,
errStr: "did not contain column",
},
{
name: "Too many columns in string",
input: "0x010203:123:456",
wantErr: false,
errStr: "weak subjectivity checkpoint input should be in `block_root:epoch_number` format",
},
{
name: "Incorrect block root length",
input: "0x010203:987",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt the block root be 32 bytes. Lets add a check for it.

wantErr: false,
errStr: "block root is not length of 32",
},
{
name: "Correct input",
input: "0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:123456789",
bRoot: []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: 123456789,
wantErr: false,
},
{
name: "Correct input without 0x",
input: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:123456789",
bRoot: []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: 123456789,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
bRoot, epoch, err := convertWspInput(tt.input)
if (err != nil) != tt.wantErr {
require.ErrorContains(t, tt.errStr, err)
return
}
if !reflect.DeepEqual(bRoot, tt.bRoot) {
t.Errorf("convertWspInput() block root = %v, want %v", bRoot, tt.bRoot)
}
if epoch != tt.epoch {
t.Errorf("convertWspInput() epoch = %v, want %v", epoch, tt.epoch)
}
})
}
}
8 changes: 8 additions & 0 deletions beacon-chain/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,12 @@ func (b *BeaconNode) registerSyncService() error {
}

func (b *BeaconNode) registerInitialSyncService() error {
wsp := b.cliCtx.String(flags.WeakSubjectivityCheckpt.Name)
bRoot, epoch, err := convertWspInput(wsp)
if err != nil {
return err
}

var chainService *blockchain.Service
if err := b.services.FetchService(&chainService); err != nil {
return err
Expand All @@ -550,6 +556,8 @@ func (b *BeaconNode) registerInitialSyncService() error {
P2P: b.fetchP2P(),
StateNotifier: b,
BlockNotifier: b,
WspBlockRoot: bRoot,
WspEpoch: epoch,
})
return b.services.RegisterService(is)
}
Expand Down
2 changes: 2 additions & 0 deletions beacon-chain/sync/initial-sync/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type Config struct {
Chain blockchainService
StateNotifier statefeed.Notifier
BlockNotifier blockfeed.Notifier
WspBlockRoot []byte
WspEpoch uint64
}

// Service service.
Expand Down