From 9a853e65bce9e32fd8a900ce27ba1406ac1ab175 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Sat, 2 Aug 2025 00:08:55 +0800 Subject: [PATCH 1/4] fix: improve performance of ApplyPowerTableDiffs in ImportSnapshotToDatastore --- certs/certs.go | 38 +++++++++++++++++++++++++++----------- certstore/snapshot.go | 5 +++-- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/certs/certs.go b/certs/certs.go index 760c2d10..b068b8ad 100644 --- a/certs/certs.go +++ b/certs/certs.go @@ -236,15 +236,38 @@ func MakePowerTableDiff(oldPowerTable, newPowerTable gpbft.PowerEntries) PowerTa return diff } +func PowerTableArrayToMap(pt gpbft.PowerEntries) map[gpbft.ActorID]gpbft.PowerEntry { + ptm := make(map[gpbft.ActorID]gpbft.PowerEntry, len(pt)) + for _, pe := range pt { + ptm[pe.ID] = pe + } + return ptm +} + +func PowerTableMapToArray(ptm map[gpbft.ActorID]gpbft.PowerEntry) gpbft.PowerEntries { + pt := make(gpbft.PowerEntries, 0, len(ptm)) + for _, pe := range ptm { + pt = append(pt, pe) + } + sort.Sort(pt) + return pt +} + // Apply a set of power table diffs to the passed power table. // // - The delta must be sorted by participant ID, ascending. // - The returned power table is sorted by power, descending. func ApplyPowerTableDiffs(prevPowerTable gpbft.PowerEntries, diffs ...PowerTableDiff) (gpbft.PowerEntries, error) { - powerTableMap := make(map[gpbft.ActorID]gpbft.PowerEntry, len(prevPowerTable)) - for _, pe := range prevPowerTable { - powerTableMap[pe.ID] = pe + powerTableMap := PowerTableArrayToMap(prevPowerTable) + powerTableMap, err := ApplyPowerTableDiffsToMap(powerTableMap, diffs...) + if err != nil { + return nil, err } + newPowerTable := PowerTableMapToArray(powerTableMap) + return newPowerTable, nil +} + +func ApplyPowerTableDiffsToMap(powerTableMap map[gpbft.ActorID]gpbft.PowerEntry, diffs ...PowerTableDiff) (map[gpbft.ActorID]gpbft.PowerEntry, error) { for j, diff := range diffs { var lastActorId gpbft.ActorID for i, d := range diff { @@ -298,17 +321,10 @@ func ApplyPowerTableDiffs(prevPowerTable gpbft.PowerEntries, diffs ...PowerTable default: // if the power becomes negative, something went wrong return nil, fmt.Errorf("diff %d resulted in negative power for participant %d", j, pe.ID) } - } } - newPowerTable := make(gpbft.PowerEntries, 0, len(powerTableMap)) - for _, pe := range powerTableMap { - newPowerTable = append(newPowerTable, pe) - } - - sort.Sort(newPowerTable) - return newPowerTable, nil + return powerTableMap, nil } // MakePowerTableCID returns the DagCBOR-blake2b256 CID of the given power entries. This method does diff --git a/certstore/snapshot.go b/certstore/snapshot.go index ab2d4f19..c78a4fcb 100644 --- a/certstore/snapshot.go +++ b/certstore/snapshot.go @@ -110,7 +110,7 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context if err != nil { return err } - pt := header.InitialPowerTable + ptm := certs.PowerTableArrayToMap(header.InitialPowerTable) for { certBytes, err := readSnapshotBlockBytes(snapshot) if err == io.EOF { @@ -123,10 +123,11 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context if err = cs.Put(ctx, &cert); err != nil { return err } - if pt, err = certs.ApplyPowerTableDiffs(pt, cert.PowerTableDelta); err != nil { + if ptm, err = certs.ApplyPowerTableDiffsToMap(ptm, cert.PowerTableDelta); err != nil { return err } if (cert.GPBFTInstance+1)%cs.powerTableFrequency == 0 { + pt := certs.PowerTableMapToArray(ptm) if err := cs.putPowerTable(ctx, cert.GPBFTInstance+1, pt); err != nil { return err } From af0db51de313ca87bc77e7c452e9d5bfaebaf1fb Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Mon, 4 Aug 2025 17:28:26 +0800 Subject: [PATCH 2/4] avoid sorting in CertStore.Put --- certstore/snapshot.go | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/certstore/snapshot.go b/certstore/snapshot.go index c78a4fcb..0e423e27 100644 --- a/certstore/snapshot.go +++ b/certstore/snapshot.go @@ -104,28 +104,42 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context dsb := autobatch.NewAutoBatching(ds, 1000) defer dsb.Flush(ctx) cs, err := OpenOrCreateStore(ctx, dsb, header.FirstInstance, header.InitialPowerTable) - if testingPowerTableFrequency > 0 { - cs.powerTableFrequency = testingPowerTableFrequency - } if err != nil { return err } + if testingPowerTableFrequency > 0 { + cs.powerTableFrequency = testingPowerTableFrequency + } ptm := certs.PowerTableArrayToMap(header.InitialPowerTable) - for { + for i := header.FirstInstance; ; i += 1 { certBytes, err := readSnapshotBlockBytes(snapshot) if err == io.EOF { break } else if err != nil { return fmt.Errorf("failed to decode finality certificate: %w", err) } + var cert certs.FinalityCertificate - cert.UnmarshalCBOR(bytes.NewReader(certBytes)) - if err = cs.Put(ctx, &cert); err != nil { + if err = cert.UnmarshalCBOR(bytes.NewReader(certBytes)); err != nil { return err } + + if i != cert.GPBFTInstance { + return fmt.Errorf("the certificate of instance %d is missing", i) + } + + if i > header.LatestInstance { + return fmt.Errorf("certificate of instance %d is found, expected latest instance %d", i, header.LatestInstance) + } + + if err := cs.ds.Put(ctx, cs.keyForCert(cert.GPBFTInstance), certBytes); err != nil { + return err + } + if ptm, err = certs.ApplyPowerTableDiffsToMap(ptm, cert.PowerTableDelta); err != nil { return err } + if (cert.GPBFTInstance+1)%cs.powerTableFrequency == 0 { pt := certs.PowerTableMapToArray(ptm) if err := cs.putPowerTable(ctx, cert.GPBFTInstance+1, pt); err != nil { @@ -133,7 +147,8 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context } } } - return nil + + return cs.writeInstanceNumber(ctx, certStoreLatestKey, header.LatestInstance) } type SnapshotHeader struct { From 96be7655affca51c9dfa0fa2ace7d393622a7209 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Mon, 4 Aug 2025 17:52:40 +0800 Subject: [PATCH 3/4] validate power tables --- certstore/snapshot.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/certstore/snapshot.go b/certstore/snapshot.go index 0e423e27..b1983455 100644 --- a/certstore/snapshot.go +++ b/certstore/snapshot.go @@ -110,6 +110,7 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context if testingPowerTableFrequency > 0 { cs.powerTableFrequency = testingPowerTableFrequency } + var latestCert *certs.FinalityCertificate ptm := certs.PowerTableArrayToMap(header.InitialPowerTable) for i := header.FirstInstance; ; i += 1 { certBytes, err := readSnapshotBlockBytes(snapshot) @@ -123,6 +124,7 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context if err = cert.UnmarshalCBOR(bytes.NewReader(certBytes)); err != nil { return err } + latestCert = &cert if i != cert.GPBFTInstance { return fmt.Errorf("the certificate of instance %d is missing", i) @@ -142,15 +144,34 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context if (cert.GPBFTInstance+1)%cs.powerTableFrequency == 0 { pt := certs.PowerTableMapToArray(ptm) + if err = checkPowerTable(pt, cert.SupplementalData.PowerTable); err != nil { + return err + } if err := cs.putPowerTable(ctx, cert.GPBFTInstance+1, pt); err != nil { return err } } } + pt := certs.PowerTableMapToArray(ptm) + if err = checkPowerTable(pt, latestCert.SupplementalData.PowerTable); err != nil { + return err + } + return cs.writeInstanceNumber(ctx, certStoreLatestKey, header.LatestInstance) } +func checkPowerTable(pt gpbft.PowerEntries, expectedCid cid.Cid) error { + ptCid, err := certs.MakePowerTableCID(pt) + if err != nil { + return err + } + if ptCid != expectedCid { + return fmt.Errorf("new power table differs from expected power table: %s != %s", ptCid, expectedCid) + } + return nil +} + type SnapshotHeader struct { Version uint64 FirstInstance uint64 From c04cb358d76fc1e6fd5f2bf506e8fe54818604b3 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Tue, 12 Aug 2025 22:22:19 +0800 Subject: [PATCH 4/4] Resolve AI comments --- certstore/snapshot.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/certstore/snapshot.go b/certstore/snapshot.go index b1983455..28a01fff 100644 --- a/certstore/snapshot.go +++ b/certstore/snapshot.go @@ -19,7 +19,10 @@ import ( "golang.org/x/crypto/blake2b" ) -var ErrUnknownLatestCertificate = errors.New("latest certificate is not known") +var ( + ErrUnknownLatestCertificate = errors.New("latest certificate is not known") + ErrNoCertificateExtracted = errors.New("no certificate is found in the snapshot") +) // ExportLatestSnapshot exports an F3 snapshot that includes the finality certificate chain until the current `latestCertificate`. // @@ -153,6 +156,14 @@ func importSnapshotToDatastoreWithTestingPowerTableFrequency(ctx context.Context } } + if latestCert == nil { + return ErrNoCertificateExtracted + } + + if latestCert.GPBFTInstance != header.LatestInstance { + return fmt.Errorf("extracted latest instance %d, but %d is expected", latestCert.GPBFTInstance, header.LatestInstance) + } + pt := certs.PowerTableMapToArray(ptm) if err = checkPowerTable(pt, latestCert.SupplementalData.PowerTable); err != nil { return err