reject blocks with more commitments than spec (#12863)

* reject blocks with more commitments than spec

* add details of excess commitments to error

* good catch, linter

---------

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>
This commit is contained in:
kasey 2023-09-07 15:21:03 -05:00 committed by GitHub
parent f17898f658
commit 809a67ebcc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 41 additions and 1 deletions

View File

@ -16,12 +16,14 @@ import (
"github.com/prysmaticlabs/prysm/v4/beacon-chain/core/transition"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v4/config/features"
fieldparams "github.com/prysmaticlabs/prysm/v4/config/fieldparams"
"github.com/prysmaticlabs/prysm/v4/config/params"
consensusblocks "github.com/prysmaticlabs/prysm/v4/consensus-types/blocks"
"github.com/prysmaticlabs/prysm/v4/consensus-types/interfaces"
"github.com/prysmaticlabs/prysm/v4/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/v4/monitoring/tracing"
"github.com/prysmaticlabs/prysm/v4/runtime/version"
prysmTime "github.com/prysmaticlabs/prysm/v4/time"
"github.com/prysmaticlabs/prysm/v4/time/slots"
"github.com/sirupsen/logrus"
@ -29,7 +31,8 @@ import (
)
var (
ErrOptimisticParent = errors.New("parent of the block is optimistic")
ErrOptimisticParent = errors.New("parent of the block is optimistic")
errRejectCommitmentLen = errors.New("[REJECT] The length of KZG commitments is less than or equal to the limitation defined in Consensus Layer")
)
// validateBeaconBlockPubSub checks that the incoming block has a valid BLS signature.
@ -91,6 +94,10 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms
}()
}
if err := validateDenebBeaconBlock(blk.Block()); err != nil {
return pubsub.ValidationReject, err
}
// Verify the block is the first block received for the proposer for the slot.
if s.hasSeenBlockIndexSlot(blk.Block().Slot(), blk.Block().ProposerIndex()) {
return pubsub.ValidationIgnore, nil
@ -221,6 +228,10 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.ReadOn
ctx, span := trace.StartSpan(ctx, "sync.validateBeaconBlock")
defer span.End()
if err := validateDenebBeaconBlock(blk.Block()); err != nil {
return err
}
if !s.cfg.chain.InForkchoice(blk.Block().ParentRoot()) {
s.setBadBlock(ctx, blockRoot)
return blockchain.ErrNotDescendantOfFinalized
@ -262,6 +273,22 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.ReadOn
return nil
}
func validateDenebBeaconBlock(blk interfaces.ReadOnlyBeaconBlock) error {
if blk.Version() < version.Deneb {
return nil
}
commits, err := blk.Body().BlobKzgCommitments()
if err != nil {
return errors.New("unable to read commitments from deneb block")
}
// [REJECT] The length of KZG commitments is less than or equal to the limitation defined in Consensus Layer
// -- i.e. validate that len(body.signed_beacon_block.message.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK
if len(commits) > fieldparams.MaxBlobsPerBlock {
return errors.Wrapf(errRejectCommitmentLen, "%d > %d", len(commits), fieldparams.MaxBlobsPerBlock)
}
return nil
}
// validateBellatrixBeaconBlock validates the block for the Bellatrix fork.
// spec code:
//

View File

@ -1415,3 +1415,16 @@ func Test_getBlockFields(t *testing.T) {
require.LogsContain(t, hook, "nil block")
require.LogsContain(t, hook, "bad block")
}
func Test_validateDenebBeaconBlock(t *testing.T) {
bb := util.NewBeaconBlockBellatrix()
b, err := blocks.NewSignedBeaconBlock(bb)
require.NoError(t, err)
require.NoError(t, validateDenebBeaconBlock(b.Block()))
bd := util.NewBeaconBlockDeneb()
bd.Block.Body.BlobKzgCommitments = make([][]byte, 7)
bdb, err := blocks.NewSignedBeaconBlock(bd)
require.NoError(t, err)
require.ErrorIs(t, validateDenebBeaconBlock(bdb.Block()), errRejectCommitmentLen)
}