berachain / beacon-kit

A modular framework for building EVM consensus clients ⛵️✨
https://berachain.com
Other
82 stars 46 forks source link

Block headers in blob sidecars do not carry block signatures #841

Open sbudella-gco opened 2 months ago

sbudella-gco commented 2 months ago

Description

After the Deneb upgrade, validators of the beacon chain must obtain from the execution clients supplementary data carried in new transactions named "blob transactions". The blobs in the transaction are opaque to the execution layer, and validators are in charge of handling them in an exceptional way: blobs must be only referenced in beacon blocks but not therein embedded. Indeed, the blobs are propagated in the consensus layer network after the validator wraps them in further envelopes named "sidecars". Blob sidecars must uniquely be linked to the block they were processed and validated with.

In Beacon-kit, the blobs are fetched and processed at the time of the block proposal by the builder service; first, a new block structure is instantiated with EmptyBeaconBlock, then the local builder returns the payload and the blobs bundle obtained from the execution engine by calling GetBestPayload, as seen in mod/runtime/services/builder/service.go:

func (s *Service) RequestBestBlock(
        ctx context.Context,
        st state.BeaconState,
        slot primitives.Slot,
) (beacontypes.BeaconBlock, *datypes.BlobSidecars, error) {
[..]
        // Create a new empty block from the current state.
        blk, err := beacontypes.EmptyBeaconBlock( 
                slot,
                proposerIndex,
                parentBlockRoot,
                stateRoot,
                s.BeaconCfg().ActiveForkVersionForSlot(slot),
                reveal,
        ) 
        if err != nil { 
                return nil, nil, err 
        } else if blk == nil {
                return nil, nil, beacontypes.ErrNilBlk
        }

        latestExecutionPayload, err := st.GetLatestExecutionPayload()
        if err != nil {
                return nil, nil, err
        }
        parentEth1BlockHash := latestExecutionPayload.GetBlockHash()

        // Get the payload for the block.
        payload, blobsBundle, overrideBuilder, err := s.localBuilder.GetBestPayload(
                ctx,
                st,
                slot,
                parentBlockRoot,
                parentEth1BlockHash,
        )
        if err != nil {
                return blk, nil, fmt.Errorf(
                        "failed to get block root at index: %w",
                        err,
                )
        }

Afterwards, the sidecars are built and returned to the invoking PrepareProposalHandler handler from mod/runtime/abci/proposal.go, which is the entrypoint to the CometBFT ABCI:

[..]
        blobSidecars, err := s.blobFactory.BuildSidecars(blk, blobsBundle)
        if err != nil {
                return nil, nil, err
        }

        s.Logger().Info("finished assembling beacon block 🛟",
                "slot", slot, "deposits", len(deposits))

        return blk, blobSidecars, nil
}

It is important to mention that at this stage of execution, the block has not been signed yet, contrary to the beacon specification; this is due to the fact that the ABCI expects the data structures needed for the block proposal and will sign the proposal just before propagating it to the Tendermint network, a process over which Cosmos SDK has no control, given that it can only hook into before block finalization. This has also been mentioned elsewhere as the root cause of another issue.

The specification mandates that the block associated with the blobs be signed, and that the blob sidecars reference the block by means of a signed block header. However, as seen above, Beacon-kit builds the sidecar by referencing the unsigned block; the blob sidecar instances returned by BuildSidecars below, under mod/da/factory.go, will carry the block header and not the signed block header:

func (f *SidecarFactory[BBB]) BuildSidecars(
        blk BeaconBlock[BBB],
        blobs *engineprimitives.BlobsBundleV1,
) (*datypes.BlobSidecars, error) {
        numBlobs := uint64(len(blobs.Blobs))
        sidecars := make([]*datypes.BlobSidecar, numBlobs)
        body := blk.GetBody()
        g := errgroup.Group{}
        for i := range numBlobs {
                g.Go(func() error {
                        inclusionProof, err := f.BuildKZGInclusionProof(
                                body, i,
                        )
                        if err != nil {
                                return err
                        }
                        blob := kzg.Blob(blobs.Blobs[i])
                        sidecars[i] = datypes.BuildBlobSidecar(
                                i,
                                blk.GetHeader(),
                                &blob,
                                kzg.Commitment(blobs.Commitments[i]),
                                kzg.Proof(blobs.Proofs[i]),
                                inclusionProof,
                        )
                        return nil
                })
        }

        return &datypes.BlobSidecars{Sidecars: sidecars}, g.Wait()
}

The reason why this is a concern lies in the fact that the blob sidecars are meant to be propagated in the consensus layer in a separate gossip channel and, as already mentioned, the sidecars do not embed the full block. Therefore, whomever requests the sidecar to access the blob needs to fetch the block it refers to separately but would have no means to verify that the validator who built the sidecar is the same as the legitimate proposer of the block associated with the blobs. The requestor of the blob could only verify the identity of the actor who propagated the sidecar, which may be not necessarily be the same as the block proposer. In other words, a valid blob could not lie about the block it belongs to, thanks to the inclusion proof, but could lie about the proposer. As equivocating blobs could lead to slashing, this issue negates accountability about which proposer is lying. In the beacon specification, both the inclusion proof and the signature of the associated block allow to leverage the validator apparatus of the blocks without re-implementing a second one for the blobs. As equivocating blobs could lead to slashing, this issue precludes accountability about which proposer is lying.

itsdevbear commented 2 months ago

Do you think it would be safe if we had a proposer sign both the CometBFT block as well as the embedded beacon block? And then we check the beacon block signature?

sbudella-gco commented 2 months ago

Do you plan to "emulate" attestations, committees, slashing or will you offload those to CometBFT's native facilities?