From 70d4c7a30eb2652bc4f48904d2053d88e85d743f Mon Sep 17 00:00:00 2001 From: Martin Hutchinson Date: Wed, 23 Oct 2024 13:43:19 +0000 Subject: [PATCH] More removal of compact range wiring --- internal/http/server_test.go | 27 +------ internal/persistence/inmemory/inmemory.go | 12 ++- .../persistence/inmemory/inmemory_test.go | 4 +- internal/persistence/persistence.go | 4 +- internal/persistence/sql/sql.go | 19 +++-- internal/persistence/testonly/persistence.go | 4 +- internal/witness/witness.go | 78 ++----------------- internal/witness/witness_test.go | 34 +------- 8 files changed, 29 insertions(+), 153 deletions(-) diff --git a/internal/http/server_test.go b/internal/http/server_test.go index 065210a..ea90b8b 100644 --- a/internal/http/server_test.go +++ b/internal/http/server_test.go @@ -282,8 +282,6 @@ func TestUpdate(t *testing.T) { desc string initC []byte initSize uint64 - useCR bool - initCR [][]byte body api.UpdateRequest wantStatus int wantCP []byte @@ -318,27 +316,6 @@ func TestUpdate(t *testing.T) { initSize: 5, body: api.UpdateRequest{Checkpoint: []byte("aaa"), Proof: consProof}, wantStatus: http.StatusBadRequest, - }, { - desc: "compact range happy path", - initC: crInit, - initSize: 10, - useCR: true, - initCR: crInitRange, - body: api.UpdateRequest{Checkpoint: crNext, Proof: crProof}, - wantStatus: http.StatusOK, - }, { - desc: "compact range garbage proof", - initC: crInit, - initSize: 10, - body: api.UpdateRequest{Checkpoint: crNext, Proof: [][]byte{ - dh("aaaa", 2), - dh("bbbb", 2), - dh("cccc", 2), - dh("dddd", 2), - }}, - useCR: true, - initCR: crInitRange, - wantStatus: http.StatusBadRequest, }, } { t.Run(test.desc, func(t *testing.T) { @@ -349,9 +326,9 @@ func TestUpdate(t *testing.T) { ID: logID, origin: logOrigin, PK: mPK, - useCompact: test.useCR}}) + useCompact: false}}) // Set an initial checkpoint for the log. - if _, err := w.Update(ctx, logID, test.initC, test.initCR); err != nil { + if _, err := w.Update(ctx, logID, test.initC, nil); err != nil { t.Errorf("failed to set checkpoint: %v", err) } // Now set up the http server. diff --git a/internal/persistence/inmemory/inmemory.go b/internal/persistence/inmemory/inmemory.go index d6daf56..8c8e776 100644 --- a/internal/persistence/inmemory/inmemory.go +++ b/internal/persistence/inmemory/inmemory.go @@ -33,8 +33,7 @@ func NewPersistence() persistence.LogStatePersistence { } type checkpointState struct { - rawChkpt []byte - compactRange []byte + rawChkpt []byte } type inMemoryPersistence struct { @@ -115,17 +114,16 @@ type readWriter struct { read, toStore *checkpointState } -func (rw *readWriter) GetLatest() ([]byte, []byte, error) { +func (rw *readWriter) GetLatest() ([]byte, error) { if rw.read == nil { - return nil, nil, status.Error(codes.NotFound, "no checkpoint found") + return nil, status.Error(codes.NotFound, "no checkpoint found") } - return rw.read.rawChkpt, rw.read.compactRange, nil + return rw.read.rawChkpt, nil } func (rw *readWriter) Set(c []byte, rng []byte) error { rw.toStore = &checkpointState{ - rawChkpt: c, - compactRange: rng, + rawChkpt: c, } return rw.write(rw.read, *rw.toStore) diff --git a/internal/persistence/inmemory/inmemory_test.go b/internal/persistence/inmemory/inmemory_test.go index 0d29ce3..51cff61 100644 --- a/internal/persistence/inmemory/inmemory_test.go +++ b/internal/persistence/inmemory/inmemory_test.go @@ -53,7 +53,7 @@ func TestWriteOpsConcurrent(t *testing.T) { return fmt.Errorf("WriteOps %d: %v", i, err) } defer w.Close() - if _, _, err := w.GetLatest(); err != nil { + if _, err := w.GetLatest(); err != nil { if status.Code(err) != codes.NotFound { return fmt.Errorf("GetLatest %d: %v", i, err) } @@ -72,7 +72,7 @@ func TestWriteOpsConcurrent(t *testing.T) { if err != nil { t.Fatal(err) } - cp, _, err := r.GetLatest() + cp, err := r.GetLatest() if err != nil { t.Fatal(err) } diff --git a/internal/persistence/persistence.go b/internal/persistence/persistence.go index 5d43750..899ca13 100644 --- a/internal/persistence/persistence.go +++ b/internal/persistence/persistence.go @@ -42,9 +42,9 @@ type LogStatePersistence interface { // LogStateReadOps allows the data about a single log to be read. type LogStateReadOps interface { - // GetLatest returns the latest checkpoint and its compact range (if applicable). + // GetLatest returns the latest checkpoint. // If no checkpoint exists, it must return codes.NotFound. - GetLatest() ([]byte, []byte, error) + GetLatest() ([]byte, error) } // LogStateWriteOps allows data about a single log to be read and written diff --git a/internal/persistence/sql/sql.go b/internal/persistence/sql/sql.go index 017d57d..c59fe44 100644 --- a/internal/persistence/sql/sql.go +++ b/internal/persistence/sql/sql.go @@ -89,7 +89,7 @@ type reader struct { db *sql.DB } -func (r *reader) GetLatest() ([]byte, []byte, error) { +func (r *reader) GetLatest() ([]byte, error) { return getLatestCheckpoint(r.db.QueryRow, r.logID) } @@ -98,7 +98,7 @@ type writer struct { tx *sql.Tx } -func (w *writer) GetLatest() ([]byte, []byte, error) { +func (w *writer) GetLatest() ([]byte, error) { return getLatestCheckpoint(w.tx.QueryRow, w.logID) } @@ -114,18 +114,17 @@ func (w *writer) Close() error { return w.tx.Rollback() } -func getLatestCheckpoint(queryRow func(query string, args ...interface{}) *sql.Row, logID string) ([]byte, []byte, error) { - row := queryRow("SELECT chkpt, range FROM chkpts WHERE logID = ?", logID) +func getLatestCheckpoint(queryRow func(query string, args ...interface{}) *sql.Row, logID string) ([]byte, error) { + row := queryRow("SELECT chkpt FROM chkpts WHERE logID = ?", logID) if err := row.Err(); err != nil { - return nil, nil, err + return nil, err } var chkpt []byte - var pf []byte - if err := row.Scan(&chkpt, &pf); err != nil { + if err := row.Scan(&chkpt); err != nil { if err == sql.ErrNoRows { - return nil, nil, status.Errorf(codes.NotFound, "no checkpoint for log %q", logID) + return nil, status.Errorf(codes.NotFound, "no checkpoint for log %q", logID) } - return nil, nil, err + return nil, err } - return chkpt, pf, nil + return chkpt, nil } diff --git a/internal/persistence/testonly/persistence.go b/internal/persistence/testonly/persistence.go index 0eed7e4..9896009 100644 --- a/internal/persistence/testonly/persistence.go +++ b/internal/persistence/testonly/persistence.go @@ -69,7 +69,7 @@ func TestWriteOps(t *testing.T, lspFactory func() (persistence.LogStatePersisten if err != nil { t.Fatalf("ReadOps(): %v", err) } - _, _, err = read.GetLatest() + _, err = read.GetLatest() if got, want := status.Code(err), codes.NotFound; got != want { t.Fatalf("error code got != want (%s, %s): %v", got, want, err) } @@ -83,7 +83,7 @@ func TestWriteOps(t *testing.T, lspFactory func() (persistence.LogStatePersisten t.Fatalf("ReadOps(): %v", err) } var cpRaw []byte - if cpRaw, _, err = read.GetLatest(); err != nil { + if cpRaw, err = read.GetLatest(); err != nil { t.Fatalf("GetLatest(): %v", err) } if got, want := cpRaw, []byte("foo cp"); !bytes.Equal(got, want) { diff --git a/internal/witness/witness.go b/internal/witness/witness.go index d818ae3..5b880c4 100644 --- a/internal/witness/witness.go +++ b/internal/witness/witness.go @@ -25,7 +25,6 @@ import ( "github.com/transparency-dev/formats/log" "github.com/transparency-dev/merkle" - "github.com/transparency-dev/merkle/compact" "github.com/transparency-dev/merkle/proof" "github.com/transparency-dev/witness/internal/persistence" "github.com/transparency-dev/witness/monitoring" @@ -123,7 +122,7 @@ func (w *Witness) GetCheckpoint(logID string) ([]byte, error) { if err != nil { return nil, fmt.Errorf("ReadOps(): %v", err) } - chkpt, _, err := read.GetLatest() + chkpt, err := read.GetLatest() if err != nil { return nil, err } @@ -164,8 +163,8 @@ func (w *Witness) Update(ctx context.Context, logID string, nextRaw []byte, cPro // The WriteOps contract is that Close must always be called. defer write.Close() - // Get the latest checkpoint (if one exists) and compact range. - prevRaw, rangeRaw, err := write.GetLatest() + // Get the latest checkpoint. + prevRaw, err := write.GetLatest() if err != nil { // If there was nothing stored already then treat this new // checkpoint as trust-on-first-use (TOFU). @@ -176,7 +175,7 @@ func (w *Witness) Update(ctx context.Context, logID string, nextRaw []byte, cPro return nil, status.Errorf(codes.Internal, "couldn't sign input checkpoint: %v", err) } - if err := setInitChkptData(write, logInfo, next, signed, cProof); err != nil { + if err := write.Set(signed, nil); err != nil { return nil, status.Errorf(codes.Internal, "couldn't set TOFU checkpoint: %v", err) } counterUpdateSuccess.Inc(logID) @@ -189,13 +188,6 @@ func (w *Witness) Update(ctx context.Context, logID string, nextRaw []byte, cPro if err != nil { return nil, status.Errorf(codes.Internal, "couldn't parse stored checkpoint: %v", err) } - // Parse the compact range if we're using one. - var prevRange Proof - if logInfo.UseCompact { - if err := prevRange.Unmarshal(rangeRaw); err != nil { - return nil, status.Errorf(codes.InvalidArgument, "couldn't unmarshal proof: %v", err) - } - } if next.Size < prev.Size { // Complain if prev is bigger than next. return prevRaw, status.Errorf(codes.AlreadyExists, "cannot prove consistency backwards (%d < %d)", next.Size, prev.Size) @@ -218,7 +210,7 @@ func (w *Witness) Update(ctx context.Context, logID string, nextRaw []byte, cPro if err != nil { return nil, status.Errorf(codes.Internal, "couldn't sign input checkpoint: %v", err) } - if err := setInitChkptData(write, logInfo, next, signed, cProof); err != nil { + if err := write.Set(signed, nil); err != nil { return nil, status.Errorf(codes.Internal, "couldn't set first non-zero checkpoint: %v", err) } counterUpdateSuccess.Inc(logID) @@ -259,63 +251,3 @@ func (w *Witness) signChkpt(n *note.Note) ([]byte, error) { } return cosigned, nil } - -// verifyRange verifies the new checkpoint against the stored and given compact -// range and outputs the updated compact range if verification succeeds. -func verifyRange(next *log.Checkpoint, prev *log.Checkpoint, h merkle.LogHasher, rngRaw [][]byte, deltaRaw [][]byte) ([][]byte, error) { - rf := compact.RangeFactory{Hash: h.HashChildren} - rng, err := rf.NewRange(0, prev.Size, rngRaw) - if err != nil { - return nil, fmt.Errorf("can't form current compact range: %v", err) - } - // As a sanity check, make sure the stored checkpoint and range are consistent. - if err := verifyRangeHash(prev.Hash, rng); err != nil { - return nil, fmt.Errorf("old root hash doesn't verify: %v", err) - } - delta, err := rf.NewRange(prev.Size, next.Size, deltaRaw) - if err != nil { - return nil, fmt.Errorf("can't form delta compact range: %v", err) - } - // Merge the delta range into the existing one and compare root hashes. - if err := rng.AppendRange(delta, nil); err != nil { - return nil, fmt.Errorf("failed to append range: %v", err) - } - if err := verifyRangeHash(next.Hash, rng); err != nil { - return nil, fmt.Errorf("new root hash doesn't verify: %v", err) - } - return rng.Hashes(), nil -} - -// verifyRangeHash computes the root hash of the compact range and compares it -// against the one given as input, returning an error if they aren't equal. -func verifyRangeHash(rootHash []byte, rng *compact.Range) error { - h, err := rng.GetRootHash(nil) - if err != nil { - return fmt.Errorf("can't get root hash for range: %v", err) - } - if !bytes.Equal(rootHash, h) { - return fmt.Errorf("hashes aren't equal (got %x, given %x)", h, rootHash) - } - return nil -} - -// setInitChkptData stores the data for an initial checkpoint and, if using one, -// its associated compact range. -func setInitChkptData(write persistence.LogStateWriteOps, logInfo LogInfo, c *log.Checkpoint, cRaw []byte, rngRaw [][]byte) error { - // If we're using compact ranges then store the initial range, assuming - // it matches the initial checkpoint. - if logInfo.UseCompact { - rf := compact.RangeFactory{Hash: logInfo.Hasher.HashChildren} - rng, err := rf.NewRange(0, c.Size, rngRaw) - if err != nil { - return fmt.Errorf("can't form compact range: %v", err) - } - if err := verifyRangeHash(c.Hash, rng); err != nil { - return fmt.Errorf("input root hash doesn't verify: %v", err) - } - r := []byte(Proof(rngRaw).Marshal()) - return write.Set(cRaw, r) - } - // If we're not using compact ranges no need to store one. - return write.Set(cRaw, nil) -} diff --git a/internal/witness/witness_test.go b/internal/witness/witness_test.go index 826b64d..26ef59d 100644 --- a/internal/witness/witness_test.go +++ b/internal/witness/witness_test.go @@ -287,8 +287,6 @@ func TestUpdate(t *testing.T) { initC []byte newC []byte pf [][]byte - useCR bool - initCR [][]byte isGood bool wantCode codes.Code }{ @@ -297,7 +295,6 @@ func TestUpdate(t *testing.T) { initC: mustCreateCheckpoint(t, mSK, 5, dh("e35b268c1522014ef412d2a54fa94838862d453631617b0307e5c77dcbeefc11", 32)), newC: mNext, pf: consProof, - useCR: false, isGood: true, }, { @@ -305,7 +302,6 @@ func TestUpdate(t *testing.T) { initC: mustCreateCheckpoint(t, mSK, 0, rfc6962.DefaultHasher.EmptyRoot()), newC: mustCreateCheckpoint(t, mSK, 5, dh("e35b268c1522014ef412d2a54fa94838862d453631617b0307e5c77dcbeefc11", 32)), pf: consProof, - useCR: false, isGood: true, }, { @@ -313,7 +309,6 @@ func TestUpdate(t *testing.T) { initC: mustCreateCheckpoint(t, mSK, 0, rfc6962.DefaultHasher.EmptyRoot()), newC: mustCreateCheckpoint(t, mSK, 5, dh("e35b268c1522014ef412d2a54fa94838862d453631617b0307e5c77dcbeefc11", 32)), pf: [][]byte{}, - useCR: false, isGood: true, }, { @@ -321,7 +316,6 @@ func TestUpdate(t *testing.T) { initC: mustCreateCheckpoint(t, mSK, 5, dh("e35b268c1522014ef412d2a54fa94838862d453631617b0307e5c77dcbeefc11", 32)), newC: mustCreateCheckpoint(t, mSK, 5, dh("e35b268c1522014ef412d2a54fa94838862d453631617b0307e5c77dcbeefc11", 32)), pf: [][]byte{}, - useCR: false, isGood: true, }, { @@ -329,7 +323,6 @@ func TestUpdate(t *testing.T) { initC: mustCreateCheckpoint(t, mSK, 4, dh("e35b268c1522014ef412d2a54fa94838862d453631617b0307e5c77dcbeefc11", 32)), newC: mustCreateCheckpoint(t, mSK, 5, dh("e35b268c1522014ef412d2a54fa94838862d453631617b0307e5c77dcbeefc11", 32)), pf: [][]byte{}, - useCR: false, isGood: false, wantCode: codes.AlreadyExists, }, @@ -338,14 +331,12 @@ func TestUpdate(t *testing.T) { initC: mInit, newC: []byte("Frog Checkpoint v0\n8\nV8K9aklZ4EPB+RMOk1/8VsJUdFZR77GDtZUQq84vSbo=\n\n— monkeys 202ffgCVdfZmrroccRdQoEfn2TfmXHez4R++GvVrFvFiaI85O12aTV5GpNOvWsuQW77eNxQ2b7ggYeglzF/QSy/EBws=\n"), pf: consProof, - useCR: false, isGood: false, }, { desc: "vanilla consistency smaller checkpoint", initC: mNext, newC: mInit, pf: consProof, - useCR: false, isGood: false, }, { desc: "vanilla consistency garbage proof", @@ -358,27 +349,6 @@ func TestUpdate(t *testing.T) { dh("dddd", 2), }, isGood: false, - }, { - desc: "compact range happy path", - initC: crInit, - newC: crNext, - pf: crProof, - useCR: true, - initCR: crInitRange, - isGood: true, - }, { - desc: "compact range garbage proof", - initC: crInit, - newC: crNext, - pf: [][]byte{ - dh("aaaa", 2), - dh("bbbb", 2), - dh("cccc", 2), - dh("dddd", 2), - }, - useCR: true, - initCR: crInitRange, - isGood: false, }, } { t.Run(test.desc, func(t *testing.T) { @@ -389,9 +359,9 @@ func TestUpdate(t *testing.T) { ID: logID, origin: logOrigin, PK: mPK, - useCompact: test.useCR}}) + useCompact: false}}) // Set an initial checkpoint for the log. - if _, err := w.Update(ctx, logID, test.initC, test.initCR); err != nil { + if _, err := w.Update(ctx, logID, test.initC, nil); err != nil { t.Errorf("failed to set checkpoint: %v", err) } // Now update from this checkpoint to a newer one.