Skip to content

Commit

Permalink
Reject future blocks (#1509)
Browse files Browse the repository at this point in the history
* Additional header validations

* blocks from the future forbidden

* lint fix

* log improved

* ut fixed

* ut fixed

* UT fixes

* Added block time drift in the genesis command

* CR changes

---------

Co-authored-by: Stefan Negovanović <[email protected]>
Co-authored-by: Marko Jelaca <[email protected]>
  • Loading branch information
3 people authored May 26, 2023
1 parent 4f2a67e commit 687dacc
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 23 deletions.
7 changes: 7 additions & 0 deletions command/genesis/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,13 @@ func setFlags(cmd *cobra.Command) {
"",
"configuration of reward wallet in format <address:amount>",
)

cmd.Flags().Uint64Var(
&params.blockTimeDrift,
blockTimeDriftFlag,
defaultBlockTimeDrift,
"configuration for block time drift value (in seconds)",
)
}

// Access Control Lists
Expand Down
1 change: 1 addition & 0 deletions command/genesis/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ type genesisParams struct {
sprintSize uint64
blockTime time.Duration
epochReward uint64
blockTimeDrift uint64

initialStateRoot string

Expand Down
4 changes: 4 additions & 0 deletions command/genesis/polybft_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ const (
blockTimeFlag = "block-time"
trieRootFlag = "trieroot"

blockTimeDriftFlag = "block-time-drift"

defaultEpochSize = uint64(10)
defaultSprintSize = uint64(5)
defaultValidatorSetSize = 100
defaultBlockTime = 2 * time.Second
defaultEpochReward = 1
defaultBlockTimeDrift = uint64(10)

contractDeployerAllowListAdminFlag = "contract-deployer-allow-list-admin"
contractDeployerAllowListEnabledFlag = "contract-deployer-allow-list-enabled"
Expand Down Expand Up @@ -139,6 +142,7 @@ func (p *genesisParams) generatePolyBftChainConfig(o command.OutputFormatter) er
WalletAddress: walletPremineInfo.address,
WalletAmount: walletPremineInfo.amount,
},
BlockTimeDrift: p.blockTimeDrift,
}

// Disable london hardfork if burn contract address is not provided
Expand Down
10 changes: 8 additions & 2 deletions consensus/polybft/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"math/big"
"time"

"github.com/0xPolygon/go-ibft/messages"
"github.com/0xPolygon/go-ibft/messages/proto"
Expand Down Expand Up @@ -293,7 +294,7 @@ func (f *fsm) Validate(proposal []byte) error {
}

// validate header fields
if err := validateHeaderFields(f.parent, block.Header); err != nil {
if err := validateHeaderFields(f.parent, block.Header, f.config.BlockTimeDrift); err != nil {
return fmt.Errorf(
"failed to validate header (parent header# %d, current header#%d): %w",
f.parent.Number,
Expand Down Expand Up @@ -631,7 +632,7 @@ func (f *fsm) verifyDistributeRewardsTx(distributeRewardsTx *types.Transaction)
return errDistributeRewardsTxNotExpected
}

func validateHeaderFields(parent *types.Header, header *types.Header) error {
func validateHeaderFields(parent *types.Header, header *types.Header, blockTimeDrift uint64) error {
// header extra data must be higher or equal to ExtraVanity = 32 in order to be compliant with Ethereum blocks
if len(header.ExtraData) < ExtraVanity {
return fmt.Errorf("extra-data shorter than %d bytes (%d)", ExtraVanity, len(header.ExtraData))
Expand All @@ -644,6 +645,11 @@ func validateHeaderFields(parent *types.Header, header *types.Header) error {
if header.Number != parent.Number+1 {
return fmt.Errorf("invalid number")
}
// verify time is from the future
if header.Timestamp > (uint64(time.Now().UTC().Unix()) + blockTimeDrift) {
return fmt.Errorf("block from the future. block timestamp: %s, configured block time drift %d seconds",
time.Unix(int64(header.Timestamp), 0).Format(time.RFC3339), blockTimeDrift)
}
// verify header nonce is zero
if header.Nonce != types.ZeroNonce {
return fmt.Errorf("invalid nonce")
Expand Down
67 changes: 50 additions & 17 deletions consensus/polybft/fsm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,52 +27,58 @@ import (
func TestFSM_ValidateHeader(t *testing.T) {
t.Parallel()

blockTimeDrift := uint64(1)
extra := createTestExtra(validator.AccountSet{}, validator.AccountSet{}, 0, 0, 0)
parent := &types.Header{Number: 0, Hash: types.BytesToHash([]byte{1, 2, 3})}
header := &types.Header{Number: 0}

// parent extra data
require.ErrorContains(t, validateHeaderFields(parent, header), "extra-data shorter than")
// extra data
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "extra-data shorter than")
header.ExtraData = extra

// parent hash
require.ErrorContains(t, validateHeaderFields(parent, header), "incorrect header parent hash")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "incorrect header parent hash")
header.ParentHash = parent.Hash

// sequence number
require.ErrorContains(t, validateHeaderFields(parent, header), "invalid number")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "invalid number")
header.Number = 1

// failed timestamp
require.ErrorContains(t, validateHeaderFields(parent, header), "timestamp older than parent")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "timestamp older than parent")
header.Timestamp = 10

// failed nonce
header.SetNonce(1)
require.ErrorContains(t, validateHeaderFields(parent, header), "invalid nonce")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "invalid nonce")

header.SetNonce(0)

// failed gas
header.GasLimit = 10
header.GasUsed = 11
require.ErrorContains(t, validateHeaderFields(parent, header), "invalid gas limit")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "invalid gas limit")
header.GasLimit = 10
header.GasUsed = 10

// mix digest
require.ErrorContains(t, validateHeaderFields(parent, header), "mix digest is not correct")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "mix digest is not correct")
header.MixHash = PolyBFTMixDigest

// difficulty
header.Difficulty = 0
require.ErrorContains(t, validateHeaderFields(parent, header), "difficulty should be greater than zero")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "difficulty should be greater than zero")

header.Difficulty = 1
header.Hash = types.BytesToHash([]byte{11, 22, 33})
require.ErrorContains(t, validateHeaderFields(parent, header), "invalid header hash")
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "invalid header hash")
header.Timestamp = uint64(time.Now().UTC().Unix() + 150)
require.ErrorContains(t, validateHeaderFields(parent, header, blockTimeDrift), "block from the future")

header.Timestamp = uint64(time.Now().UTC().Unix())

header.ComputeHash()
require.NoError(t, validateHeaderFields(parent, header))
require.NoError(t, validateHeaderFields(parent, header, blockTimeDrift))
}

func TestFSM_verifyCommitEpochTx(t *testing.T) {
Expand Down Expand Up @@ -784,6 +790,7 @@ func TestFSM_Validate_EpochEndingBlock_MismatchInDeltas(t *testing.T) {
distributeRewardsInput: distributeRewards,
polybftBackend: polybftBackendMock,
newValidatorsDelta: newValidatorDelta,
config: &PolyBFTConfig{BlockTimeDrift: 1},
}

err = fsm.Validate(proposal)
Expand Down Expand Up @@ -857,6 +864,7 @@ func TestFSM_Validate_EpochEndingBlock_UpdatingValidatorSetInNonEpochEndingBlock
validators: validators.ToValidatorSet(),
logger: hclog.NewNullLogger(),
polybftBackend: polybftBackendMock,
config: &PolyBFTConfig{BlockTimeDrift: 1},
}

err = fsm.Validate(proposal)
Expand All @@ -882,8 +890,15 @@ func TestFSM_Validate_IncorrectHeaderParentHash(t *testing.T) {
}
parent.ComputeHash()

fsm := &fsm{parent: parent, backend: &blockchainMock{},
validators: validators.ToValidatorSet(), logger: hclog.NewNullLogger()}
fsm := &fsm{
parent: parent,
backend: &blockchainMock{},
validators: validators.ToValidatorSet(),
logger: hclog.NewNullLogger(),
config: &PolyBFTConfig{
BlockTimeDrift: 1,
},
}

stateBlock := createDummyStateBlock(parent.Number+1, types.Hash{100, 15}, parent.ExtraData)

Expand Down Expand Up @@ -917,8 +932,14 @@ func TestFSM_Validate_InvalidNumber(t *testing.T) {
for _, blockNum := range []uint64{parentBlockNumber - 1, parentBlockNumber, parentBlockNumber + 2} {
stateBlock := createDummyStateBlock(blockNum, parent.Hash, parent.ExtraData)
mBlockBuilder := newBlockBuilderMock(stateBlock)
fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, backend: &blockchainMock{},
validators: validators.ToValidatorSet(), logger: hclog.NewNullLogger()}
fsm := &fsm{
parent: parent,
blockBuilder: mBlockBuilder,
backend: &blockchainMock{},
validators: validators.ToValidatorSet(),
logger: hclog.NewNullLogger(),
config: &PolyBFTConfig{BlockTimeDrift: 1},
}

proposalHash, err := new(CheckpointData).Hash(fsm.backend.GetChainID(), stateBlock.Block.Number(), stateBlock.Block.Hash())
require.NoError(t, err)
Expand Down Expand Up @@ -953,8 +974,14 @@ func TestFSM_Validate_TimestampOlder(t *testing.T) {
ExtraData: parent.ExtraData,
}
stateBlock := &types.FullBlock{Block: consensus.BuildBlock(consensus.BuildBlockParams{Header: header})}
fsm := &fsm{parent: parent, backend: &blockchainMock{},
validators: validators.ToValidatorSet(), logger: hclog.NewNullLogger()}
fsm := &fsm{
parent: parent,
backend: &blockchainMock{},
validators: validators.ToValidatorSet(),
logger: hclog.NewNullLogger(),
config: &PolyBFTConfig{
BlockTimeDrift: 1,
}}

checkpointHash, err := new(CheckpointData).Hash(fsm.backend.GetChainID(), header.Number, header.Hash)
require.NoError(t, err)
Expand Down Expand Up @@ -995,6 +1022,9 @@ func TestFSM_Validate_IncorrectMixHash(t *testing.T) {
backend: &blockchainMock{},
validators: validators.ToValidatorSet(),
logger: hclog.NewNullLogger(),
config: &PolyBFTConfig{
BlockTimeDrift: 1,
},
}
rlpBlock := buildBlock.Block.MarshalRLP()

Expand Down Expand Up @@ -1405,6 +1435,9 @@ func TestFSM_Validate_FailToVerifySignatures(t *testing.T) {
polybftBackend: polybftBackendMock,
validators: validatorSet,
logger: hclog.NewNullLogger(),
config: &PolyBFTConfig{
BlockTimeDrift: 1,
},
}

finalBlock := consensus.BuildBlock(consensus.BuildBlockParams{
Expand Down
6 changes: 3 additions & 3 deletions consensus/polybft/polybft.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,12 +546,12 @@ func (p *Polybft) VerifyHeader(header *types.Header) error {
)
}

return p.verifyHeaderImpl(parent, header, nil)
return p.verifyHeaderImpl(parent, header, p.consensusConfig.BlockTimeDrift, nil)
}

func (p *Polybft) verifyHeaderImpl(parent, header *types.Header, parents []*types.Header) error {
func (p *Polybft) verifyHeaderImpl(parent, header *types.Header, blockTimeDrift uint64, parents []*types.Header) error {
// validate header fields
if err := validateHeaderFields(parent, header); err != nil {
if err := validateHeaderFields(parent, header, blockTimeDrift); err != nil {
return fmt.Errorf("failed to validate header for block %d. error = %w", header.Number, err)
}

Expand Down
3 changes: 3 additions & 0 deletions consensus/polybft/polybft_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ type PolyBFTConfig struct {

// RewardConfig defines rewards configuration
RewardConfig *RewardsConfig `json:"rewardConfig"`

// BlockTimeDrift defines the time slot in which a new block can be created
BlockTimeDrift uint64 `json:"blockTimeDrift"`
}

// LoadPolyBFTConfig loads chain config from provided path and unmarshals PolyBFTConfig
Expand Down
3 changes: 2 additions & 1 deletion consensus/polybft/polybft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func TestPolybft_VerifyHeader(t *testing.T) {
InitialValidatorSet: validators.GetParamValidators(),
EpochSize: fixedEpochSize,
SprintSize: 5,
BlockTimeDrift: 10,
}

validatorSet := validators.GetPublicIdentities()
Expand Down Expand Up @@ -127,7 +128,7 @@ func TestPolybft_VerifyHeader(t *testing.T) {

parentHeader := &types.Header{
Number: polyBftConfig.EpochSize,
Timestamp: uint64(time.Now().UTC().UnixMilli()),
Timestamp: uint64(time.Now().UTC().Unix()),
}
parentCommitment := updateHeaderExtra(parentHeader, parentDelta, nil, &CheckpointData{EpochNumber: 1}, accountSetParent)

Expand Down

0 comments on commit 687dacc

Please sign in to comment.