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.

Sign up for GitHub

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

Do not HTR the state when checking for optimistic mode #12143

Merged
merged 35 commits into from
Mar 20, 2023
Merged

Do not HTR the state when checking for optimistic mode #12143

Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
de952c9
initial impl
rkapka Mar 16, 2023
b4af356
review feedback
rkapka Mar 17, 2023
22b4427
fix tests
rkapka Mar 17, 2023
fd5da7f
review feedback
rkapka Mar 17, 2023
32d3506
Merge branch 'develop' into is-optimistic-htr
rkapka Mar 17, 2023
af3798b
some improvements
rkapka Mar 17, 2023
bf2245a
tests and small improvements
rkapka Mar 17, 2023
adb6333
gzl
rkapka Mar 17, 2023
1a4b3ef
one more review
rkapka Mar 17, 2023
85570af
fix test
potuz Mar 17, 2023
d453acc
fix other test
potuz Mar 17, 2023
2387d9a
get the roots instead of hashing them
potuz Mar 17, 2023
0676266
fix comment
potuz Mar 17, 2023
08d5109
fix justified case
potuz Mar 17, 2023
1587a3d
fix all tests
potuz Mar 17, 2023
52315ea
misc
rkapka Mar 17, 2023
5b988d2
gzl
rkapka Mar 17, 2023
1c58968
Merge branch '__develop' into is-optimistic-htr
rkapka Mar 17, 2023
566dbeb
Merge branch 'develop' into is-optimistic-htr
nisdas Mar 18, 2023
71e4115
fix broken tests
rauljordan Mar 18, 2023
8751e76
use isOptimisticForRoot once we have the blockroot
potuz Mar 18, 2023
b0b7776
Fix is_not_finalized_when_head_is_optimistic but reviewing the logic …
terencechain Mar 18, 2023
f83de8c
Fix is_not_finalized_when_head_is_optimistic
terencechain Mar 18, 2023
1090deb
better root tests
rkapka Mar 18, 2023
f6fd393
move optimistic check before parsing root
rkapka Mar 18, 2023
a0feab7
check for last validated checkpoint
potuz Mar 18, 2023
6355c69
add right check for finalized
potuz Mar 18, 2023
63d3bc9
fix finalized tests
rkapka Mar 20, 2023
316edd6
Merge branch 'develop' into is-optimistic-htr
rkapka Mar 20, 2023
a63fde4
removed impossible condition
rkapka Mar 20, 2023
7ffae21
fix TestGetSyncCommitteeDuties
rkapka Mar 20, 2023
d51958d
Merge remote-tracking branch 'origin/develop' into is-optimistic-htr
potuz Mar 20, 2023
b4cfa55
Use Ancestor from chaininfo
potuz Mar 20, 2023
7711657
fix test
potuz Mar 20, 2023
c8c47bf
Merge branch 'develop' into is-optimistic-htr
potuz Mar 20, 2023
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
3 changes: 2 additions & 1 deletion beacon-chain/blockchain/testing/mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type ChainService struct {
ReceiveBlockMockErr error
OptimisticCheckRootReceived [32]byte
FinalizedRoots map[[32]byte]bool
OptimisticRoots map[[32]byte]bool
}

// ForkChoicer mocks the same method in the chain service
Expand Down Expand Up @@ -457,7 +458,7 @@ func (s *ChainService) InForkchoice(_ [32]byte) bool {
// IsOptimisticForRoot mocks the same method in the chain service.
func (s *ChainService) IsOptimisticForRoot(_ context.Context, root [32]byte) (bool, error) {
s.OptimisticCheckRootReceived = root
return s.Optimistic, nil
return s.OptimisticRoots[root], nil
}

// UpdateHead mocks the same method in the chain service.
Expand Down
8 changes: 6 additions & 2 deletions beacon-chain/rpc/eth/beacon/blinded_blocks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,8 +520,10 @@ func TestServer_GetBlindedBlock(t *testing.T) {
Block: chainBlk,
Root: headBlock.BlockRoot,
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(headBlock.BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down Expand Up @@ -734,8 +736,10 @@ func TestServer_GetBlindedBlockSSZ(t *testing.T) {
Block: chainBlk,
Root: headBlock.BlockRoot,
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(headBlock.BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down
18 changes: 18 additions & 0 deletions beacon-chain/rpc/eth/beacon/blocks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,9 @@ func TestServer_GetBlockHeader(t *testing.T) {
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(headBlock.BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down Expand Up @@ -634,6 +637,9 @@ func TestServer_ListBlockHeaders(t *testing.T) {
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(blkContainers[30].BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down Expand Up @@ -1666,6 +1672,9 @@ func TestServer_GetBlockV2(t *testing.T) {
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(headBlock.BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down Expand Up @@ -1927,6 +1936,9 @@ func TestServer_GetBlockSSZV2(t *testing.T) {
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(headBlock.BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down Expand Up @@ -2098,6 +2110,9 @@ func TestServer_GetBlockRoot(t *testing.T) {
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(headBlock.BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down Expand Up @@ -2494,6 +2509,9 @@ func TestServer_ListBlockAttestations(t *testing.T) {
FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blkContainers[64].BlockRoot},
Optimistic: true,
FinalizedRoots: map[[32]byte]bool{},
OptimisticRoots: map[[32]byte]bool{
bytesutil.ToBytes32(headBlock.BlockRoot): true,
},
}
bs := &Server{
BeaconDB: beaconDB,
Expand Down
8 changes: 4 additions & 4 deletions beacon-chain/rpc/eth/beacon/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (bs *Server) GetStateRoot(ctx context.Context, req *ethpb.StateRequest) (*e
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not get state: %v", err)
}
isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down Expand Up @@ -100,7 +100,7 @@ func (bs *Server) GetStateFork(ctx context.Context, req *ethpb.StateRequest) (*e
return nil, helpers.PrepareStateFetchGRPCError(err)
}
fork := st.Fork()
isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down Expand Up @@ -131,7 +131,7 @@ func (bs *Server) GetFinalityCheckpoints(ctx context.Context, req *ethpb.StateRe
if err != nil {
return nil, helpers.PrepareStateFetchGRPCError(err)
}
isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down Expand Up @@ -186,7 +186,7 @@ func (bs *Server) GetRandao(ctx context.Context, req *eth2.RandaoRequest) (*eth2
return nil, status.Errorf(codes.Internal, "Could not get randao mix at index %d", idx)
}

isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down
28 changes: 14 additions & 14 deletions beacon-chain/rpc/eth/beacon/state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestGetStateRoot(t *testing.T) {
}

resp, err := server.GetStateRoot(context.Background(), &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand All @@ -121,7 +121,7 @@ func TestGetStateRoot(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetStateRoot(context.Background(), &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -155,7 +155,7 @@ func TestGetStateRoot(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetStateRoot(context.Background(), &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -189,7 +189,7 @@ func TestGetStateFork(t *testing.T) {
}

resp, err := server.GetStateFork(ctx, &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -218,7 +218,7 @@ func TestGetStateFork(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetStateFork(context.Background(), &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -251,7 +251,7 @@ func TestGetStateFork(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetStateFork(context.Background(), &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -292,7 +292,7 @@ func TestGetFinalityCheckpoints(t *testing.T) {
}

resp, err := server.GetFinalityCheckpoints(ctx, &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -323,7 +323,7 @@ func TestGetFinalityCheckpoints(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetFinalityCheckpoints(context.Background(), &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -356,7 +356,7 @@ func TestGetFinalityCheckpoints(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetFinalityCheckpoints(context.Background(), &eth.StateRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -398,17 +398,17 @@ func TestGetRandao(t *testing.T) {
}

t.Run("no epoch requested", func(t *testing.T) {
resp, err := server.GetRandao(ctx, &eth2.RandaoRequest{StateId: make([]byte, 0)})
resp, err := server.GetRandao(ctx, &eth2.RandaoRequest{StateId: []byte("head")})
require.NoError(t, err)
assert.DeepEqual(t, mixCurrent, resp.Data.Randao)
})
t.Run("current epoch requested", func(t *testing.T) {
resp, err := server.GetRandao(ctx, &eth2.RandaoRequest{StateId: make([]byte, 0), Epoch: &epochCurrent})
resp, err := server.GetRandao(ctx, &eth2.RandaoRequest{StateId: []byte("head"), Epoch: &epochCurrent})
require.NoError(t, err)
assert.DeepEqual(t, mixCurrent, resp.Data.Randao)
})
t.Run("old epoch requested", func(t *testing.T) {
resp, err := server.GetRandao(ctx, &eth2.RandaoRequest{StateId: make([]byte, 0), Epoch: &epochOld})
resp, err := server.GetRandao(ctx, &eth2.RandaoRequest{StateId: []byte("head"), Epoch: &epochOld})
require.NoError(t, err)
assert.DeepEqual(t, mixOld, resp.Data.Randao)
})
Expand Down Expand Up @@ -450,7 +450,7 @@ func TestGetRandao(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetRandao(context.Background(), &eth2.RandaoRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down Expand Up @@ -482,7 +482,7 @@ func TestGetRandao(t *testing.T) {
BeaconDB: db,
}
resp, err := server.GetRandao(context.Background(), &eth2.RandaoRequest{
StateId: make([]byte, 0),
StateId: []byte("head"),
})
require.NoError(t, err)
assert.NotNil(t, resp)
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/rpc/eth/beacon/sync_committee.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (bs *Server) ListSyncCommittees(ctx context.Context, req *ethpbv2.StateSync
return nil, status.Errorf(codes.Internal, "Could not extract sync subcommittees: %v", err)
}

isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down
13 changes: 10 additions & 3 deletions beacon-chain/rpc/eth/beacon/sync_committee_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ func TestListSyncCommittees(t *testing.T) {
require.NoError(t, err)
db := dbTest.SetupDB(t)

chainService := &mock.ChainService{}
stSlot := st.Slot()
chainService := &mock.ChainService{Slot: &stSlot}
s := &Server{
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
Expand All @@ -173,6 +174,7 @@ func TestListSyncCommittees(t *testing.T) {
OptimisticModeFetcher: chainService,
FinalizationFetcher: chainService,
BeaconDB: db,
ChainInfoFetcher: chainService,
}
req := &ethpbv2.StateSyncCommitteesRequest{StateId: stRoot[:]}
resp, err := s.ListSyncCommittees(ctx, req)
Expand Down Expand Up @@ -205,7 +207,8 @@ func TestListSyncCommittees(t *testing.T) {
util.SaveBlock(t, ctx, db, blk)
require.NoError(t, db.SaveGenesisBlockRoot(ctx, root))

chainService := &mock.ChainService{Optimistic: true}
stSlot := st.Slot()
chainService := &mock.ChainService{Optimistic: true, Slot: &stSlot}
s := &Server{
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
Expand All @@ -217,6 +220,7 @@ func TestListSyncCommittees(t *testing.T) {
OptimisticModeFetcher: chainService,
FinalizationFetcher: chainService,
BeaconDB: db,
ChainInfoFetcher: chainService,
}
resp, err := s.ListSyncCommittees(ctx, req)
require.NoError(t, err)
Expand All @@ -234,10 +238,12 @@ func TestListSyncCommittees(t *testing.T) {

headerRoot, err := st.LatestBlockHeader().HashTreeRoot()
require.NoError(t, err)
stSlot := st.Slot()
chainService := &mock.ChainService{
FinalizedRoots: map[[32]byte]bool{
headerRoot: true,
},
Slot: &stSlot,
}
s := &Server{
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Expand All @@ -250,6 +256,7 @@ func TestListSyncCommittees(t *testing.T) {
OptimisticModeFetcher: chainService,
FinalizationFetcher: chainService,
BeaconDB: db,
ChainInfoFetcher: chainService,
}
resp, err := s.ListSyncCommittees(ctx, req)
require.NoError(t, err)
Expand Down Expand Up @@ -310,7 +317,7 @@ func TestListSyncCommitteesFuture(t *testing.T) {
FinalizationFetcher: chainService,
BeaconDB: db,
}
req := &ethpbv2.StateSyncCommitteesRequest{}
req := &ethpbv2.StateSyncCommitteesRequest{StateId: []byte("head")}
epoch := 2 * params.BeaconConfig().EpochsPerSyncCommitteePeriod
req.Epoch = &epoch
_, err := s.ListSyncCommittees(ctx, req)
Expand Down
8 changes: 4 additions & 4 deletions beacon-chain/rpc/eth/beacon/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (bs *Server) GetValidator(ctx context.Context, req *ethpb.StateValidatorReq
return nil, status.Error(codes.NotFound, "Could not find validator")
}

isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down Expand Up @@ -86,7 +86,7 @@ func (bs *Server) ListValidators(ctx context.Context, req *ethpb.StateValidators
return nil, handleValContainerErr(err)
}

isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down Expand Up @@ -155,7 +155,7 @@ func (bs *Server) ListValidatorBalances(ctx context.Context, req *ethpb.Validato
}
}

isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down Expand Up @@ -220,7 +220,7 @@ func (bs *Server) ListCommittees(ctx context.Context, req *ethpb.StateCommittees
}
}

isOptimistic, err := helpers.IsOptimistic(ctx, st, bs.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, bs.OptimisticModeFetcher, bs.StateFetcher, bs.ChainInfoFetcher, bs.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/rpc/eth/debug/debug.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (ds *Server) GetBeaconStateV2(ctx context.Context, req *ethpbv2.BeaconState
if err != nil {
return nil, helpers.PrepareStateFetchGRPCError(err)
}
isOptimistic, err := helpers.IsOptimistic(ctx, beaconSt, ds.OptimisticModeFetcher)
isOptimistic, err := helpers.IsOptimistic(ctx, req.StateId, ds.OptimisticModeFetcher, ds.StateFetcher, ds.ChainInfoFetcher, ds.BeaconDB)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not check if slot's block is optimistic: %v", err)
}
Expand Down
Loading