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

Implement optimistic sync spectests #11391

Merged
merged 4 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions beacon-chain/execution/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ go_library(
"//beacon-chain:__subpackages__",
"//cmd/beacon-chain:__subpackages__",
"//contracts:__subpackages__",
"//testing/spectest:__subpackages__",
],
deps = [
"//beacon-chain/cache/depositcache:go_default_library",
Expand Down
1 change: 1 addition & 0 deletions testing/spectest/shared/common/forkchoice/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ go_library(
"//beacon-chain/core/time:go_default_library",
"//beacon-chain/core/transition:go_default_library",
"//beacon-chain/db/testing:go_default_library",
"//beacon-chain/execution:go_default_library",
"//beacon-chain/forkchoice/protoarray:go_default_library",
"//beacon-chain/operations/attestations:go_default_library",
"//beacon-chain/state:go_default_library",
Expand Down
28 changes: 28 additions & 0 deletions testing/spectest/shared/common/forkchoice/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package forkchoice

import (
"context"
"errors"
"fmt"
"testing"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/blockchain"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/execution"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v3/config/params"
"github.com/prysmaticlabs/prysm/v3/consensus-types/interfaces"
Expand Down Expand Up @@ -50,6 +52,32 @@ func (bb *Builder) Tick(t testing.TB, tick int64) {
bb.lastTick = tick
}

// SetPayloadStatus sets the payload status that the engine will return
func (bb *Builder) SetPayloadStatus(resp *MockEngineResp) error {
if resp == nil {
return errors.New("invalid nil payload status")
}
if resp.LatestValidHash == nil {
bb.execMock.latestValidHash = common.FromHex("0x0000000000000000000000000000000000000000000000000000000000000000")
} else {
bb.execMock.latestValidHash = common.FromHex(*resp.LatestValidHash)
}
if resp.Status == nil {
return errors.New("invalid nil status")
}
switch *resp.Status {
case "SYNCING":
bb.execMock.payloadStatus = execution.ErrAcceptedSyncingPayloadStatus
case "VALID":
bb.execMock.payloadStatus = nil
case "INVALID":
bb.execMock.payloadStatus = execution.ErrInvalidPayloadStatus
default:
return errors.New("unknown payload status")
}
return nil
}

// block returns the block root.
func (bb *Builder) block(t testing.TB, b interfaces.SignedBeaconBlock) [32]byte {
r, err := b.Block().HashTreeRoot()
Expand Down
18 changes: 15 additions & 3 deletions testing/spectest/shared/common/forkchoice/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,21 @@ func init() {
transition.SkipSlotCache.Disable()
}

// Run executes "forkchoice" test.
// Run executes "forkchoice" and "sync" test.
func Run(t *testing.T, config string, fork int) {
runTest(t, config, fork, "fork_choice")
runTest(t, config, fork, "sync")
}

func runTest(t *testing.T, config string, fork int, basePath string) {
require.NoError(t, utils.SetConfig(t, config))
testFolders, _ := utils.TestFolders(t, config, version.String(fork), "fork_choice")
testFolders, _ := utils.TestFolders(t, config, version.String(fork), basePath)
if testFolders == nil {
return
Comment on lines +36 to +38
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if t.Fatal is better than return here to eliminate false positives

}

for _, folder := range testFolders {
folderPath := path.Join("fork_choice", folder.Name(), "pyspec_tests")
folderPath := path.Join(basePath, folder.Name(), "pyspec_tests")
testFolders, testsFolderPath := utils.TestFolders(t, config, version.String(fork), folderPath)

for _, folder := range testFolders {
Expand Down Expand Up @@ -113,6 +121,10 @@ func Run(t *testing.T, config string, fork int) {
require.NoError(t, att.UnmarshalSSZ(attSSZ), "Failed to unmarshal")
builder.Attestation(t, att)
}
if step.PayloadStatus != nil {
require.NoError(t, builder.SetPayloadStatus(step.PayloadStatus))

}
if step.PowBlock != nil {
powBlockFile, err := util.BazelFileBytes(testsFolderPath, folder.Name(), fmt.Sprint(*step.PowBlock, ".ssz_snappy"))
require.NoError(t, err)
Expand Down
12 changes: 9 additions & 3 deletions testing/spectest/shared/common/forkchoice/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ import (
"github.com/prysmaticlabs/prysm/v3/testing/require"
)

func startChainService(t testing.TB, st state.BeaconState, block interfaces.SignedBeaconBlock, engineMock *engineMock) *blockchain.Service {
func startChainService(t testing.TB,
st state.BeaconState,
block interfaces.SignedBeaconBlock,
engineMock *engineMock,
) *blockchain.Service {
db := testDB.SetupDB(t)
ctx := context.Background()
require.NoError(t, db.SaveBlock(ctx, block))
Expand Down Expand Up @@ -67,7 +71,9 @@ func startChainService(t testing.TB, st state.BeaconState, block interfaces.Sign
}

type engineMock struct {
powBlocks map[[32]byte]*ethpb.PowBlock
powBlocks map[[32]byte]*ethpb.PowBlock
latestValidHash []byte
payloadStatus error
}

func (m *engineMock) GetPayload(context.Context, [8]byte) (*pb.ExecutionPayload, error) {
Expand All @@ -77,7 +83,7 @@ func (m *engineMock) ForkchoiceUpdated(context.Context, *pb.ForkchoiceState, *pb
return nil, nil, nil
}
func (m *engineMock) NewPayload(context.Context, interfaces.ExecutionData) ([]byte, error) {
return nil, nil
return m.latestValidHash, m.payloadStatus
}

func (m *engineMock) LatestExecutionBlock(context.Context) (*pb.ExecutionBlock, error) {
Expand Down
21 changes: 14 additions & 7 deletions testing/spectest/shared/common/forkchoice/type.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package forkchoice

type Step struct {
Tick *int `json:"tick"`
Block *string `json:"block"`
Valid *bool `json:"valid"`
Attestation *string `json:"attestation"`
AttesterSlashing *string `json:"attester_slashing"`
PowBlock *string `json:"pow_block"`
Check *Check `json:"checks"`
Tick *int `json:"tick"`
Block *string `json:"block"`
Valid *bool `json:"valid"`
Attestation *string `json:"attestation"`
AttesterSlashing *string `json:"attester_slashing"`
PayloadStatus *MockEngineResp `json:"payload_status"`
PowBlock *string `json:"pow_block"`
Check *Check `json:"checks"`
}

type Check struct {
Expand All @@ -29,3 +30,9 @@ type EpochRoot struct {
Epoch int `json:"epoch"`
Root string `json:"root"`
}

type MockEngineResp struct {
Status *string `json:"status"`
LatestValidHash *string `json:"latest_valid_hash"`
ValidationError *string `json:"validation_error"`
}
4 changes: 3 additions & 1 deletion testing/spectest/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ func UnmarshalYaml(y []byte, dest interface{}) error {
func TestFolders(t testing.TB, config, forkOrPhase, folderPath string) ([]os.DirEntry, string) {
testsFolderPath := path.Join("tests", config, forkOrPhase, folderPath)
filepath, err := bazel.Runfile(testsFolderPath)
require.NoError(t, err)
if err != nil {
return nil, ""
}
Comment on lines +36 to +38
Copy link
Member

@terencechain terencechain Sep 13, 2022

Choose a reason for hiding this comment

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

Can you explain this change? or even better add a comment

Copy link
Member

Choose a reason for hiding this comment

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

Addressed offline, this is due to folder changes

testFolders, err := os.ReadDir(filepath)
require.NoError(t, err)

Expand Down