Skip to content

feat: certstore snapshot export #1032

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

)

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

Choose a reason for hiding this comment

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

Use consistent CamelCasing (i.e. ErrLatestCertificateNil), or better yet follow the existing naming convention based on which I would name this error ErrUnknownLatestCertificate

Suggested change
var ErrlatestCertificateNil = errors.New("latest certificate is not available")
var ErrUnknownLatestCertificate = errors.New("latest certificate is not known")


// 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.

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.

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.

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
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.

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?

}

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.

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?

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