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.

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

Update KZG library according to latest consensus spec changes #49

Merged
merged 10 commits into from
Nov 15, 2022
Merged
13 changes: 3 additions & 10 deletions core/beacon/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ type payloadAttributesMarshaling struct {

// BlobsBundleV1 holds the blobs of an execution payload, to be retrieved separately
type BlobsBundleV1 struct {
BlockHash common.Hash `json:"blockHash" gencodec:"required"`
KZGs []types.KZGCommitment `json:"kzgs" gencodec:"required"`
Blobs []types.Blob `json:"blobs" gencodec:"required"`
AggregatedProof types.KZGProof `json:"aggregatedProof" gencodec:"required"`
BlockHash common.Hash `json:"blockHash" gencodec:"required"`
KZGs []types.KZGCommitment `json:"kzgs" gencodec:"required"`
Blobs []types.Blob `json:"blobs" gencodec:"required"`
}

//go:generate go run github.com/fjl/gencodec -type ExecutableDataV1 -field-override executableDataMarshaling -out gen_ed.go
Expand Down Expand Up @@ -236,11 +235,5 @@ func BlockToBlobData(block *types.Block) (*BlobsBundleV1, error) {
blobsBundle.KZGs = append(blobsBundle.KZGs, kzgs...)
}
}

_, _, aggregatedProof, err := types.Blobs(blobsBundle.Blobs).ComputeCommitmentsAndAggregatedProof()
if err != nil {
return nil, err
}
blobsBundle.AggregatedProof = aggregatedProof
return blobsBundle, nil
}
197 changes: 53 additions & 144 deletions core/types/data_blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/crypto/kzg"
"github.com/ethereum/go-ethereum/params"
"github.com/protolambda/go-kzg/bls"
"github.com/protolambda/ztyp/codec"
"github.com/protolambda/ztyp/tree"
)
Expand Down Expand Up @@ -103,6 +102,7 @@ func (p *KZGProof) UnmarshalText(text []byte) error {
return hexutil.UnmarshalFixedText("KZGProof", text, p[:])
}

// BLSFieldElement is the raw bytes representation of a field element
type BLSFieldElement [32]byte

func (p BLSFieldElement) MarshalText() ([]byte, error) {
Expand All @@ -120,6 +120,16 @@ func (p *BLSFieldElement) UnmarshalText(text []byte) error {
// Blob data
type Blob [params.FieldElementsPerBlob]BLSFieldElement
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the specs, each Blob is defined as a flat sequence of bytes, so something like:

type Blob [BYTES_PER_FIELD_ELEMENT * FIELD_ELEMENTS_PER_BLOB]byte

Which concretely is:

type Blob [32 * 4096]byte

You could probably do type Blob []byte but i'm not familiar enough with how we will manage different blob sizes to suggest this with convinction.

In either case, you could then remove BLSFieldElement and it would be upto the crypto to partition each blob into 32 byte segments. You would then have an extra check which fails if the number of bytes is not divisible by 32 in the cryptography code, since the cryptography API would take a []byte slice since we ideally want it to be generic over the blob sizes

Copy link
Collaborator Author

@roberto-bayardo roberto-bayardo Nov 14, 2022

Choose a reason for hiding this comment

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

Updating this would be a bit out of scope for this change, which was to get the implementation of the shared library (kzg_bytes.go) into shape to support multiple clients which may choose different representations that do not necessarily match the spec. For example the eip-4844 devnet Prysm fork uses [][]byte on the client side for its Blob type.

I am not 100% sure why these types were chosen the way they are to be honest. We could update them in a later change if this seems better. Since kzg_bytes.go now expects an interface for Blob it allows us to pretty easily change the client side representation if we decide to do so. All one needs to do is provide the simple interface adapter.

Copy link
Collaborator Author

@roberto-bayardo roberto-bayardo Nov 14, 2022

Choose a reason for hiding this comment

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

Oh I should have added, I did try to use the flat byte array Blob type from the specification in the kzg lib side but Golang doesn't make it easy to convert among such types without a lot of cumbersome & costly copying, so opted for that interface approach instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see, yeah it makes sense to use an interface for now then.

As to why []byte, the rationale was that we abstracted away as much detail as possible from client devs, so just passing a stream of bytes for a blob seemed like the most opaque thing to do, because then clients didn't even need to know that the blobs should be segmented into 32 bytes each.

If the copies are costly compared to, I guess using protobuf(?) to do it, then maybe we need to revisit this

Copy link
Collaborator Author

@roberto-bayardo roberto-bayardo Nov 14, 2022

Choose a reason for hiding this comment

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

Oh yes I get why the spec uses a single opaque byte slice. I wasn't sure why the geth implementation decided to use type Blob [params.FieldElementsPerBlob]BLSFieldElement instead. Perhaps that was what was used in an earlier revision of the spec? I think Prysm uses [][]byte because that was convenient with their protobuf based parsing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@roberto-bayardo fyi we'll be changing the blob representation in prysm to a flat byte slice (we've already done this in upstream prysm).

Copy link
Collaborator

Choose a reason for hiding this comment

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

but yeah i agree that doing this is out of scope. We can follow up later


// kzg.Blob interface
func (blob Blob) Len() int {
return len(blob)
}

// kzg.Blob interface
func (blob Blob) At(i int) [32]byte {
return [32]byte(blob[i])
}

func (blob *Blob) Deserialize(dr *codec.DecodingReader) error {
if blob == nil {
return errors.New("cannot decode ssz into nil Blob")
Expand Down Expand Up @@ -156,17 +166,6 @@ func (blob *Blob) HashTreeRoot(hFn tree.HashFn) tree.Root {
}, params.FieldElementsPerBlob)
}

// Convert a blob to kzg.Blob
func (blob *Blob) ToKZGBlob() (kzg.Blob, bool) {
frs := make([]bls.Fr, len(blob))
for i, elem := range blob {
if !bls.FrFrom32(&frs[i], elem) {
return []bls.Fr{}, false
}
}
return kzg.Blob(frs), true
}

func (blob *Blob) MarshalText() ([]byte, error) {
out := make([]byte, 2+params.FieldElementsPerBlob*32*2)
copy(out[:2], "0x")
Expand Down Expand Up @@ -209,6 +208,15 @@ func (blob *Blob) UnmarshalText(text []byte) error {

type BlobKzgs []KZGCommitment

// kzg.KZGCommitmentSequence interface
func (bk BlobKzgs) Len() int {
return len(bk)
}

func (bk BlobKzgs) At(i int) kzg.KZGCommitment {
return kzg.KZGCommitment(bk[i])
}

func (li *BlobKzgs) Deserialize(dr *codec.DecodingReader) error {
return dr.List(func() codec.Deserializable {
i := len(*li)
Expand All @@ -227,7 +235,7 @@ func (li BlobKzgs) ByteLength() uint64 {
return uint64(len(li)) * 48
}

func (li *BlobKzgs) FixedLength() uint64 {
func (li BlobKzgs) FixedLength() uint64 {
return 0
}

Expand All @@ -245,17 +253,14 @@ func (li BlobKzgs) copy() BlobKzgs {

type Blobs []Blob

// Extract the crypto material underlying these blobs
func (blobs Blobs) toKZGBlobSequence() ([][]bls.Fr, bool) {
out := make([][]bls.Fr, len(blobs))
for i, b := range blobs {
blob, ok := b.ToKZGBlob()
if !ok {
return nil, false
}
out[i] = blob
}
return out, true
// kzg.BlobSequence interface
func (blobs Blobs) Len() int {
return len(blobs)
}

// kzg.BlobSequence interface
func (blobs Blobs) At(i int) kzg.Blob {
return blobs[i]
}

func (a *Blobs) Deserialize(dr *codec.DecodingReader) error {
Expand Down Expand Up @@ -301,42 +306,21 @@ func (blobs Blobs) ComputeCommitmentsAndAggregatedProof() (commitments []KZGComm
commitments = make([]KZGCommitment, len(blobs))
versionedHashes = make([]common.Hash, len(blobs))
for i, blob := range blobs {
frs, ok := blob.ToKZGBlob()
c, ok := kzg.BlobToKZGCommitment(blob)
if !ok {
return nil, nil, KZGProof{}, errors.New("invalid blob for commitment")
return nil, nil, KZGProof{}, errors.New("could not convert blob to commitment")
}
commitments[i] = KZGCommitment(kzg.BlobToKZGCommitment(frs))
versionedHashes[i] = common.Hash(kzg.KZGToVersionedHash(kzg.KZGCommitment(commitments[i])))
commitments[i] = KZGCommitment(c)
versionedHashes[i] = common.Hash(kzg.KZGToVersionedHash(c))
}

var kzgProof KZGProof
if len(blobs) != 0 {
aggregatePoly, aggregateCommitmentG1, err := computeAggregateKzgCommitment(blobs, commitments)
proof, err := kzg.ComputeAggregateKZGProof(blobs)
if err != nil {
return nil, nil, KZGProof{}, err
}

var aggregateCommitment KZGCommitment
copy(aggregateCommitment[:], bls.ToCompressedG1(aggregateCommitmentG1))

var aggregateBlob Blob
for i := range aggregatePoly {
aggregateBlob[i] = bls.FrTo32(&aggregatePoly[i])
}
sum, err := sszHash(&PolynomialAndCommitment{aggregateBlob, aggregateCommitment})
if err != nil {
return nil, nil, KZGProof{}, err
}
z := kzg.BytesToBLSField(sum)

var y bls.Fr
kzg.EvaluatePolyInEvaluationForm(&y, aggregatePoly[:], z)

aggProofG1, err := kzg.ComputeProof(aggregatePoly, z)
if err != nil {
return nil, nil, KZGProof{}, err
}
copy(kzgProof[:], bls.ToCompressedG1(aggProofG1))
kzgProof = KZGProof(proof)
}

return commitments, versionedHashes, kzgProof, nil
Expand All @@ -363,27 +347,6 @@ func (b *BlobsAndCommitments) FixedLength() uint64 {
return 0
}

type PolynomialAndCommitment struct {
b Blob
c KZGCommitment
}

func (p *PolynomialAndCommitment) HashTreeRoot(hFn tree.HashFn) tree.Root {
return hFn.HashTreeRoot(&p.b, &p.c)
}

func (p *PolynomialAndCommitment) Serialize(w *codec.EncodingWriter) error {
return w.Container(&p.b, &p.c)
}

func (p *PolynomialAndCommitment) ByteLength() uint64 {
return codec.ContainerLength(&p.b, &p.c)
}

func (p *PolynomialAndCommitment) FixedLength() uint64 {
return 0
}

type BlobTxWrapper struct {
Tx SignedBlobTx
BlobKzgs BlobKzgs
Expand Down Expand Up @@ -421,19 +384,30 @@ func (b *BlobTxWrapData) sizeWrapData() common.StorageSize {
return common.StorageSize(4 + 4 + b.BlobKzgs.ByteLength() + b.Blobs.ByteLength() + b.KzgAggregatedProof.ByteLength())
}

func (b *BlobTxWrapData) verifyVersionedHash(inner TxData) error {
// validateBlobTransactionWrapper implements validate_blob_transaction_wrapper from EIP-4844
func (b *BlobTxWrapData) validateBlobTransactionWrapper(inner TxData) error {
blobTx, ok := inner.(*SignedBlobTx)
if !ok {
return fmt.Errorf("expected signed blob tx, got %T", inner)
}
if a, b := len(blobTx.Message.BlobVersionedHashes), params.MaxBlobsPerBlock; a > b {
return fmt.Errorf("too many blobs in blob tx, got %d, expected no more than %d", a, b)
l1 := len(b.BlobKzgs)
l2 := len(blobTx.Message.BlobVersionedHashes)
l3 := len(b.Blobs)
if l1 != l2 || l2 != l3 {
return fmt.Errorf("lengths don't match %v %v %v", l1, l2, l3)
}
if a, b := len(b.BlobKzgs), len(b.Blobs); a != b {
return fmt.Errorf("expected equal amount but got %d kzgs and %d blobs", a, b)
// the following check isn't strictly necessary as it would be caught by data gas processing
// (and hence it is not explicitly in the spec for this function), but it doesn't hurt to fail
// early in case we are getting spammed with too many blobs or there is a bug somewhere:
if l1 > params.MaxBlobsPerBlock {
return fmt.Errorf("number of blobs exceeds max: %v", l1)
}
if a, b := len(b.BlobKzgs), len(blobTx.Message.BlobVersionedHashes); a != b {
return fmt.Errorf("expected equal amount but got %d kzgs and %d versioned hashes", a, b)
ok, err := kzg.VerifyAggregateKZGProof(b.Blobs, b.BlobKzgs, kzg.KZGProof(b.KzgAggregatedProof))
if err != nil {
return fmt.Errorf("error during proof verification: %v", err)
}
if !ok {
return errors.New("failed to verify kzg")
}
for i, h := range blobTx.Message.BlobVersionedHashes {
if computed := b.BlobKzgs[i].ComputeVersionedHash(); computed != h {
Expand All @@ -443,41 +417,6 @@ func (b *BlobTxWrapData) verifyVersionedHash(inner TxData) error {
return nil
}

// Blob verification using KZG proofs
func (b *BlobTxWrapData) verifyBlobs(inner TxData) error {
if err := b.verifyVersionedHash(inner); err != nil {
return err
}

aggregatePoly, aggregateCommitmentG1, err := computeAggregateKzgCommitment(b.Blobs, b.BlobKzgs)
if err != nil {
return fmt.Errorf("failed to compute aggregate commitment: %v", err)
}
var aggregateBlob Blob
for i := range aggregatePoly {
aggregateBlob[i] = bls.FrTo32(&aggregatePoly[i])
}
var aggregateCommitment KZGCommitment
copy(aggregateCommitment[:], bls.ToCompressedG1(aggregateCommitmentG1))
sum, err := sszHash(&PolynomialAndCommitment{aggregateBlob, aggregateCommitment})
if err != nil {
return err
}
z := kzg.BytesToBLSField(sum)

var y bls.Fr
kzg.EvaluatePolyInEvaluationForm(&y, aggregatePoly[:], z)

aggregateProofG1, err := bls.FromCompressedG1(b.KzgAggregatedProof[:])
if err != nil {
return fmt.Errorf("aggregate proof parse error: %v", err)
}
if !kzg.VerifyKZGProofFromPoints(aggregateCommitmentG1, z, &y, aggregateProofG1) {
return errors.New("failed to verify kzg")
}
return nil
}

func (b *BlobTxWrapData) copy() TxWrapData {
return &BlobTxWrapData{
BlobKzgs: b.BlobKzgs.copy(),
Expand Down Expand Up @@ -514,33 +453,3 @@ func (b *BlobTxWrapData) encodeTyped(w io.Writer, txdata TxData) error {
}
return EncodeSSZ(w, &wrapped)
}

func computeAggregateKzgCommitment(blobs Blobs, commitments []KZGCommitment) ([]bls.Fr, *bls.G1Point, error) {
// create challenges
sum, err := sszHash(&BlobsAndCommitments{blobs, commitments})
if err != nil {
return nil, nil, err
}
r := kzg.BytesToBLSField(sum)

powers := kzg.ComputePowers(r, len(blobs))

commitmentsG1 := make([]bls.G1Point, len(commitments))
for i := 0; i < len(commitmentsG1); i++ {
p, _ := bls.FromCompressedG1(commitments[i][:])
bls.CopyG1(&commitmentsG1[i], p)
}
aggregateCommitmentG1 := bls.LinCombG1(commitmentsG1, powers)
var aggregateCommitment KZGCommitment
copy(aggregateCommitment[:], bls.ToCompressedG1(aggregateCommitmentG1))

polys, ok := blobs.toKZGBlobSequence()
if !ok {
return nil, nil, err
}
aggregatePoly, err := bls.PolyLinComb(polys, powers)
if err != nil {
return nil, nil, err
}
return aggregatePoly, aggregateCommitmentG1, nil
}
4 changes: 2 additions & 2 deletions core/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ type TxWrapData interface {
aggregatedProof() KZGProof
encodeTyped(w io.Writer, txdata TxData) error
sizeWrapData() common.StorageSize
verifyBlobs(inner TxData) error
validateBlobTransactionWrapper(inner TxData) error
}

// TxData is the underlying data of a transaction.
Expand Down Expand Up @@ -543,7 +543,7 @@ func (tx *Transaction) IsIncomplete() bool {
// VerifyBlobs verifies the blob transaction
func (tx *Transaction) VerifyBlobs() error {
if tx.wrapData != nil {
return tx.wrapData.verifyBlobs(tx.inner)
return tx.wrapData.validateBlobTransactionWrapper(tx.inner)
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion core/types/transaction_marshalling.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error {
KzgAggregatedProof: dec.KzgAggregatedProof,
}
// Verify that versioned hashes match kzgs, and kzgs match blobs.
if err := tx.wrapData.verifyBlobs(&itx); err != nil {
if err := tx.wrapData.validateBlobTransactionWrapper(&itx); err != nil {
return fmt.Errorf("blob wrapping data is invalid: %v", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/types/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ func TestTransactionCoding(t *testing.T) {
GasTipCap: view.Uint256View(*uint256.NewInt(42)),
GasFeeCap: view.Uint256View(*uint256.NewInt(10)),
AccessList: AccessListView(accesses),
BlobVersionedHashes: VersionedHashesView{common.HexToHash("0x01624652859a6e98ffc1608e2af0147ca4e86e1ce27672d8d3f3c9d4ffd6ef7e")},
BlobVersionedHashes: VersionedHashesView{common.HexToHash("0x010657f37554c781402a22917dee2f75def7ab966d7b770905398eba3c444014")},
},
}
var kzgProof KZGProof
Expand Down
2 changes: 1 addition & 1 deletion core/vm/testdata/precompiles/pointEvaluation.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[
{
"Input": "01342233e6ebb423c766d3a0f8d183e84c453865b392f5ab1f8a8218506e89d842000000000000000000000000000000000000000000000000000000000000002b2f0b0a19cbe19b4c9dbc32af755539fec08bae3eeecbe0ec625037fe3f0a6fa3cfbde6cf9875270479e0e2290726d150412591e07b4fad36472fa1ad38c19eb232cd2ebd3738ea1d9a0a3be07764a8b2faf3776cf5fb7bea8263ab92181326b898c4dc5da95e76e6977c4e204a94f1a3fe5033e19435fa51a8c70b272c06ac",
"Input": "01d0db71b458e8955efa3ef62f1b6b45a2d9c8633dc59ed0b995cecf8b7bb48442000000000000000000000000000000000000000000000000000000000000003b11ebd59d5d12d6ef8e5e48f71770515e032595a0e55eaf80e906b65b2a625282bead0f31f58ee4fd81e93f796bb57acd6f8f6e4def04182c8e949c71ea00d85a44c12102bd817bc97696a6b8fd75618fcd8c2030080c9602e08e935cdf6f779d0d89e7764d855edfbaa730eddfff836fc324957db4f74d1565503bcfcaf157",
"Expected": "",
"Name": "pointEvaluation1",
"Gas": 50000,
Expand Down
Loading