Skip to content
175 changes: 175 additions & 0 deletions certstore/cbor_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

90 changes: 90 additions & 0 deletions certstore/snapshot.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package certstore

import (
"bytes"
"context"
"encoding/binary"
"errors"
"io"

"github.com/filecoin-project/go-f3/gpbft"
"github.com/ipfs/go-datastore"
xerrors "golang.org/x/xerrors"
Copy link
Member

Choose a reason for hiding this comment

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

This repo avoids using xerrors in favour of standard Go SDK packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

)

var ErrlatestCertificateNil = errors.New("latest certificate is not available")

// Exports an F3 snapshot that includes the finality certificate chain until the current `latestCertificate`.
Copy link
Member

Choose a reason for hiding this comment

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

By convention start godoc with the function name. Ditto in other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

func (cs *Store) ExportLatestSnapshot(ctx context.Context, writer io.Writer) error {
Copy link
Member

@masih masih Jun 27, 2025

Choose a reason for hiding this comment

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

The term Snapshot doesn't carry enough of a weight within the context of go-f3. What we really mean by it at this stage is simply: export certificates.

You could make it mean something: define a type Snapshot that is either produced by the Store, or takes the Store, and implements io.WriterTo, io.ReaderFrom etc.

Or avoid the term entirely and instead make the store simply an Iterator of certs and separate IO ops elsewhere.

I would probably go with the first approach but there is not enough implementation in this PR for me to fully wrap my head around how you are thinking of approaching the problem.

So please feel free to ignore the recommendations if they happen to not make sense as the work progresses forward.

Copy link
Contributor Author

@hanabi1224 hanabi1224 Jul 14, 2025

Choose a reason for hiding this comment

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

Added FRC link to the comment to define Snapshot. As I understand, a Snapshot struct is an in-memory representation of the snapshot which is not ideal for implementing a streaming export with a low RAM usage. Implementing io.WriterTo would not allow cancellation with context.Context.

if cs.latestCertificate == nil {
return ErrlatestCertificateNil
}
return cs.ExportSnapshot(ctx, cs.latestCertificate.GPBFTInstance, writer)

Check warning on line 22 in certstore/snapshot.go

View check run for this annotation

Codecov / codecov/patch

certstore/snapshot.go#L18-L22

Added lines #L18 - L22 were not covered by tests
}

// Exports an F3 snapshot that includes the finality certificate chain until the specified `lastInstance`.
Copy link
Member

Choose a reason for hiding this comment

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

Clarify what the from instance is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

func (cs *Store) ExportSnapshot(ctx context.Context, lastInstance uint64, writer io.Writer) error {
Copy link
Member

Choose a reason for hiding this comment

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

Simply call latestInstance, to? There is nothing in the logic that mandates "latest instance".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

Copy link
Member

@masih masih Jun 27, 2025

Choose a reason for hiding this comment

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

Maybe makes sense to define an Exporter type that implements io.WriterTo instead, which takes Store and any other to-from instance params? also see other comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

io.WriterTo does not take context.Context for cancellation given the export/import could take some time

initialPowerTable, err := cs.GetPowerTable(ctx, cs.firstInstance)
if err != nil {
return xerrors.Errorf("failed to get initial power table at instance %d: %w", cs.firstInstance, err)
}
header := SnapshotHeader{1, cs.firstInstance, lastInstance, initialPowerTable}
if err := header.WriteToSnapshot(writer); err != nil {
return xerrors.Errorf("failed to write snapshot header: %w", err)
}
for i := cs.firstInstance; i <= lastInstance; i++ {
cert, err := cs.ds.Get(ctx, cs.keyForCert(i))
if err != nil {
return xerrors.Errorf("failed to get certificate at instance %d:: %w", i, err)
}
buffer := bytes.NewBuffer(cert)
if err := writeSnapshotBlockBytes(writer, buffer); err != nil {
return err
}

Check warning on line 43 in certstore/snapshot.go

View check run for this annotation

Codecov / codecov/patch

certstore/snapshot.go#L26-L43

Added lines #L26 - L43 were not covered by tests
}
return nil

Check warning on line 45 in certstore/snapshot.go

View check run for this annotation

Codecov / codecov/patch

certstore/snapshot.go#L45

Added line #L45 was not covered by tests
}

// Imports an F3 snapshot and opens the certificate store.
//
// The passed Datastore has to be thread safe.
func ImportSnapshotAndOpenStore(ctx context.Context, ds datastore.Datastore) error {
return xerrors.New("to be implemented")

Check warning on line 52 in certstore/snapshot.go

View check run for this annotation

Codecov / codecov/patch

certstore/snapshot.go#L51-L52

Added lines #L51 - L52 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

  • Implementation missing to review fully.
  • The name of the function alone tends to indicate that this is planning to do too many things. I don't know if it is actually going to, but at least its name should be something simple: e.g. ImportSnapshotTo.

}

type SnapshotHeader struct {
Version uint64
FirstInstance uint64
LatestInstance uint64
InitialPowerTable gpbft.PowerEntries
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to fix the upper limit length for this just to be on the safe side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current limit works fine on mainnet

}

func (h *SnapshotHeader) WriteToSnapshot(writer io.Writer) error {
Copy link
Member

Choose a reason for hiding this comment

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

I recommend adhering to the existing SDK interfaces like io.WriterTo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

return writeSnapshotCborEncodedBlock(writer, h)

Check warning on line 63 in certstore/snapshot.go

View check run for this annotation

Codecov / codecov/patch

certstore/snapshot.go#L62-L63

Added lines #L62 - L63 were not covered by tests
}

// Writes CBOR-encoded header or data block with a varint-encoded length prefix
func writeSnapshotCborEncodedBlock(writer io.Writer, block MarshalCBOR) error {
Copy link
Member

Choose a reason for hiding this comment

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

This seems over refactored, i.e. called only from one place.

var buffer bytes.Buffer
if err := block.MarshalCBOR(&buffer); err != nil {
return err
}
return writeSnapshotBlockBytes(writer, &buffer)

Check warning on line 72 in certstore/snapshot.go

View check run for this annotation

Codecov / codecov/patch

certstore/snapshot.go#L67-L72

Added lines #L67 - L72 were not covered by tests
}

// Writes header or data block with a varint-encoded length prefix
func writeSnapshotBlockBytes(writer io.Writer, buffer *bytes.Buffer) error {
buf := make([]byte, 8)
n := binary.PutUvarint(buf, uint64(buffer.Len()))
if _, err := writer.Write(buf[:n]); err != nil {
return err
}
if _, err := buffer.WriteTo(writer); err != nil {
return err
}
return nil

Check warning on line 85 in certstore/snapshot.go

View check run for this annotation

Codecov / codecov/patch

certstore/snapshot.go#L76-L85

Added lines #L76 - L85 were not covered by tests
}

type MarshalCBOR interface {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this type defined? Consider using existing types from cborgen?

Copy link
Contributor Author

@hanabi1224 hanabi1224 Jul 14, 2025

Choose a reason for hiding this comment

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

Fixed. (Only found Marshaler in github.com/filecoin-project/go-state-types/cbor)

MarshalCBOR(w io.Writer) error
}
6 changes: 6 additions & 0 deletions gen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/filecoin-project/go-f3/certexchange"
"github.com/filecoin-project/go-f3/certs"
"github.com/filecoin-project/go-f3/certstore"
"github.com/filecoin-project/go-f3/chainexchange"
"github.com/filecoin-project/go-f3/gpbft"
gen "github.com/whyrusleeping/cbor-gen"
Expand Down Expand Up @@ -47,6 +48,11 @@ func main() {
chainexchange.Message{},
)
})
eg.Go(func() error {
return gen.WriteTupleEncodersToFile("../certstore/cbor_gen.go", "certstore",
certstore.SnapshotHeader{},
)
})
if err := eg.Wait(); err != nil {
fmt.Printf("Failed to complete cborg_gen: %v\n", err)
os.Exit(1)
Expand Down