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

Add a 2 second timeout to fetching eth1data #5583

Merged
merged 10 commits into from
Apr 23, 2020
Merged

Add a 2 second timeout to fetching eth1data #5583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1b840f8
Add a 2 second timeout to fetching eth1data
prestonvanloon Apr 22, 2020
6ddab97
goimports
prestonvanloon Apr 22, 2020
e9e7911
fix
prestonvanloon Apr 22, 2020
1a201dc
use package const
prestonvanloon Apr 22, 2020
a6b4a1b
Merge refs/heads/master into graceful-failure-eth1data
prylabs-bulldozer[bot] Apr 22, 2020
8aa9117
fix test
prestonvanloon Apr 23, 2020
fc3542a
Merge refs/heads/master into graceful-failure-eth1data
prylabs-bulldozer[bot] Apr 23, 2020
d4b9050
Merge refs/heads/master into graceful-failure-eth1data
prylabs-bulldozer[bot] Apr 23, 2020
ea9cbc4
Merge refs/heads/master into graceful-failure-eth1data
prylabs-bulldozer[bot] Apr 23, 2020
e662696
Merge refs/heads/master into graceful-failure-eth1data
prylabs-bulldozer[bot] Apr 23, 2020
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
15 changes: 13 additions & 2 deletions beacon-chain/rpc/validator/proposer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"math/big"
"math/rand"
"time"

"github.com/pkg/errors"
ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
Expand All @@ -30,6 +31,8 @@ import (
// eth1DataNotification is a latch to stop flooding logs with the same warning.
var eth1DataNotification bool

const eth1dataTimeout = 2 * time.Second

// GetBlock is called by a proposer during its assigned slot to request a block to sign
// by passing in the slot and the signed randao reveal of the slot.
func (vs *Server) GetBlock(ctx context.Context, req *ethpb.BlockRequest) (*ethpb.BeaconBlock, error) {
Expand Down Expand Up @@ -154,6 +157,9 @@ func (vs *Server) ProposeBlock(ctx context.Context, blk *ethpb.SignedBeaconBlock
// - Subtract that eth1block.number by ETH1_FOLLOW_DISTANCE.
// - This is the eth1block to use for the block proposal.
func (vs *Server) eth1Data(ctx context.Context, slot uint64) (*ethpb.Eth1Data, error) {
ctx, cancel := context.WithTimeout(ctx, eth1dataTimeout)
defer cancel()

if vs.MockEth1Votes {
return vs.mockETH1DataVote(ctx, slot)
}
Expand All @@ -169,11 +175,13 @@ func (vs *Server) eth1Data(ctx context.Context, slot uint64) (*ethpb.Eth1Data, e
// Look up most recent block up to timestamp
blockNumber, err := vs.Eth1BlockFetcher.BlockNumberByTimestamp(ctx, eth1VotingPeriodStartTime)
if err != nil {
return nil, errors.Wrap(err, "could not get block number from timestamp")
log.WithError(err).Error("Failed to get block number from timestamp")
Copy link
Member

Choose a reason for hiding this comment

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

We also could consider warning for these, i dont feel strongly for either, up to you!

Copy link
Member Author

Choose a reason for hiding this comment

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

Its an error that should be monitored

return vs.randomETH1DataVote(ctx)
}
eth1Data, err := vs.defaultEth1DataResponse(ctx, blockNumber)
if err != nil {
return nil, errors.Wrap(err, "could not get eth1 data from block number")
log.WithError(err).Error("Failed to get eth1 data from block number")
return vs.randomETH1DataVote(ctx)
}

return eth1Data, nil
Expand Down Expand Up @@ -362,6 +370,9 @@ func (vs *Server) canonicalEth1Data(ctx context.Context, beaconState *stateTrie.
// hash of eth1 block hash that is FOLLOW_DISTANCE back from its
// latest block.
func (vs *Server) defaultEth1DataResponse(ctx context.Context, currentHeight *big.Int) (*ethpb.Eth1Data, error) {
if ctx.Err() != nil {
return nil, ctx.Err()
}
eth1FollowDistance := int64(params.BeaconConfig().Eth1FollowDistance)
ancestorHeight := big.NewInt(0).Sub(currentHeight, big.NewInt(eth1FollowDistance))
blockHash, err := vs.Eth1BlockFetcher.BlockHashByHeight(ctx, ancestorHeight)
Expand Down
6 changes: 2 additions & 4 deletions beacon-chain/rpc/validator/proposer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"context"
"math/big"
"reflect"
"strings"
"testing"

"github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -1130,9 +1129,8 @@ func TestEth1Data_EmptyVotesFetchBlockHashFailure(t *testing.T) {
BlockReceiver: &mock.ChainService{State: beaconState},
HeadFetcher: &mock.ChainService{State: beaconState},
}
want := "could not fetch ETH1_FOLLOW_DISTANCE ancestor"
if _, err := proposerServer.eth1Data(context.Background(), beaconState.Slot()+1); err == nil || !strings.Contains(err.Error(), want) {
t.Errorf("Expected error %v, received %v", want, err)
if _, err := proposerServer.eth1Data(context.Background(), beaconState.Slot()+1); err != nil {
t.Errorf("A failed request should not have returned an error, got %v", err)
}
}

Expand Down