diff --git a/pkg/database/keyvalue/kvtest/test.go b/pkg/database/keyvalue/kvtest/test.go index b778904ff..359be078c 100644 --- a/pkg/database/keyvalue/kvtest/test.go +++ b/pkg/database/keyvalue/kvtest/test.go @@ -54,7 +54,7 @@ func TestDatabase(t *testing.T, open Opener) { db := openDb(t, open) // Read when nothing exists - doBatch(t, db, func(batch keyvalue.ChangeSet) { + doBatch(t, db, nil, func(batch keyvalue.ChangeSet) { _, err := batch.Get(record.NewKey("answer", 0)) require.Error(t, err) require.ErrorAs(t, err, new(*database.NotFoundError)) @@ -63,34 +63,26 @@ func TestDatabase(t *testing.T, open Opener) { // Write values := map[record.KeyHash]string{} for i := range N { - if i > 0 { - doBatch(t, db, func(batch keyvalue.ChangeSet) { - _, err := batch.Get(record.NewKey("answer", i-1, 0)) - require.NoError(t, err) - }) - } - doBatch(t, db, func(batch keyvalue.ChangeSet) { + prefix := record.NewKey("answer", i) + doBatch(t, db, prefix, func(batch keyvalue.ChangeSet) { for j := range M { - key := record.NewKey("answer", i, j) value := fmt.Sprintf("%x/%x this much data ", i, j) - values[key.Hash()] = value - err := batch.Put(key, []byte(value)) + values[record.NewKey("answer", i, j).Hash()] = value + err := batch.Put(record.NewKey(j), []byte(value)) require.NoError(t, err, "Put") } }) - if i > 0 { - doBatch(t, db, func(batch keyvalue.ChangeSet) { - _, err := batch.Get(record.NewKey("answer", i-1, 0)) - require.NoError(t, err) - }) - } + doBatch(t, db, prefix, func(batch keyvalue.ChangeSet) { + _, err := batch.Get(record.NewKey(0)) + require.NoError(t, err) + }) } // Verify with a new batch - doBatch(t, db, func(batch keyvalue.ChangeSet) { + doBatch(t, db, record.NewKey("answer"), func(batch keyvalue.ChangeSet) { for i := range N { for j := range M { - val, err := batch.Get(record.NewKey("answer", i, j)) + val, err := batch.Get(record.NewKey(i, j)) require.NoError(t, err, "Get") require.Equal(t, fmt.Sprintf("%x/%x this much data ", i, j), string(val)) } @@ -101,7 +93,7 @@ func TestDatabase(t *testing.T, open Opener) { db.Close() db = openDb(t, open) - doBatch(t, db, func(batch keyvalue.ChangeSet) { + doBatch(t, db, nil, func(batch keyvalue.ChangeSet) { for i := range N { for j := range M { val, err := batch.Get(record.NewKey("answer", i, j)) @@ -112,7 +104,7 @@ func TestDatabase(t *testing.T, open Opener) { }) // Verify ForEach - doBatch(t, db, func(batch keyvalue.ChangeSet) { + doBatch(t, db, nil, func(batch keyvalue.ChangeSet) { require.NoError(t, batch.ForEach(func(key *record.Key, value []byte) error { expect, ok := values[key.Hash()] require.Truef(t, ok, "%v should exist", key) @@ -242,9 +234,9 @@ func TestDelete(t *testing.T, open Opener) { require.ErrorIs(t, err, errors.NotFound) } -func doBatch(t testing.TB, db keyvalue.Beginner, fn func(batch keyvalue.ChangeSet)) { +func doBatch(t testing.TB, db keyvalue.Beginner, prefix *record.Key, fn func(batch keyvalue.ChangeSet)) { t.Helper() - batch := db.Begin(nil, true) + batch := db.Begin(prefix, true) defer batch.Discard() fn(batch) require.NoError(t, batch.Commit()) diff --git a/pkg/database/keyvalue/overlay/overlay.go b/pkg/database/keyvalue/overlay/overlay.go new file mode 100644 index 000000000..4ad248562 --- /dev/null +++ b/pkg/database/keyvalue/overlay/overlay.go @@ -0,0 +1,101 @@ +// Copyright 2024 The Accumulate Authors +// +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +package overlay + +import ( + "gitlab.com/accumulatenetwork/accumulate/pkg/database" + "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue" + "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/memory" + "gitlab.com/accumulatenetwork/accumulate/pkg/errors" + "gitlab.com/accumulatenetwork/accumulate/pkg/types/record" +) + +func Open(a, b keyvalue.Beginner) keyvalue.Beginner { + return &Database{a, b} +} + +type Database struct { + a, b keyvalue.Beginner +} + +func (d *Database) Begin(prefix *database.Key, writable bool) keyvalue.ChangeSet { + a := d.a.Begin(prefix, writable) + b := d.b.Begin(prefix, false) + + return memory.NewChangeSet(memory.ChangeSetOptions{ + Get: func(key *record.Key) ([]byte, error) { return get(a, b, key) }, + ForEach: func(fn func(*record.Key, []byte) error) error { return forEach(a, b, fn) }, + Discard: func() { a.Discard(); b.Discard() }, + Commit: func(m map[[32]byte]memory.Entry) error { return commit(a, b, m) }, + }) +} + +func get(a, b keyvalue.ChangeSet, key *record.Key) ([]byte, error) { + // Get from a + v, err := a.Get(key) + switch { + case err == nil: + return v, nil + case !errors.Is(err, errors.NotFound): + return nil, err + } + + // Get from b + v, err = b.Get(key) + switch { + case err == nil: + return v, nil + case !errors.Is(err, errors.NotFound): + return nil, err + } + + return nil, (*database.NotFoundError)(key) +} + +func forEach(a, b keyvalue.ChangeSet, fn func(*record.Key, []byte) error) error { + seen := map[[32]byte]bool{} + + err := a.ForEach(func(key *record.Key, value []byte) error { + seen[key.Hash()] = true + return fn(key, value) + }) + if err != nil { + return err + } + return b.ForEach(func(key *record.Key, value []byte) error { + if seen[key.Hash()] { + return nil + } + return fn(key, value) + }) +} + +func commit(a, b keyvalue.ChangeSet, m map[[32]byte]memory.Entry) error { + for _, entry := range m { + if !entry.Delete { + err := a.Put(entry.Key, entry.Value) + if err != nil { + return err + } + continue + } + + _, err := b.Get(entry.Key) + switch { + case err == nil: + return errors.NotAllowed.WithFormat("cannot delete %v: it is present in the underlying database", entry.Key) + case !errors.Is(err, errors.NotFound): + return err + } + + err = a.Delete(entry.Key) + if err != nil { + return err + } + } + return a.Commit() +} diff --git a/pkg/database/keyvalue/overlay/overlay_test.go b/pkg/database/keyvalue/overlay/overlay_test.go new file mode 100644 index 000000000..3fe471d5b --- /dev/null +++ b/pkg/database/keyvalue/overlay/overlay_test.go @@ -0,0 +1,43 @@ +// Copyright 2024 The Accumulate Authors +// +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +package overlay + +import ( + "testing" + + "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue" + "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/kvtest" + "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/memory" +) + +func open(*testing.T) kvtest.Opener { + db := Open(memory.New(nil), memory.New(nil)) + return func() (keyvalue.Beginner, error) { + return db, nil + } +} + +func TestDatabase(t *testing.T) { + kvtest.TestDatabase(t, open(t)) +} + +func TestIsolation(t *testing.T) { + t.Skip("Not supported by the underlying databases") + kvtest.TestIsolation(t, open(t)) +} + +func TestSubBatch(t *testing.T) { + kvtest.TestSubBatch(t, open(t)) +} + +func TestPrefix(t *testing.T) { + kvtest.TestPrefix(t, open(t)) +} + +func TestDelete(t *testing.T) { + kvtest.TestDelete(t, open(t)) +} diff --git a/pkg/database/keyvalue/overlay/simulator_test.go b/pkg/database/keyvalue/overlay/simulator_test.go new file mode 100644 index 000000000..a444a8088 --- /dev/null +++ b/pkg/database/keyvalue/overlay/simulator_test.go @@ -0,0 +1,77 @@ +// Copyright 2024 The Accumulate Authors +// +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +package overlay_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "gitlab.com/accumulatenetwork/accumulate/pkg/build" + "gitlab.com/accumulatenetwork/accumulate/pkg/errors" + . "gitlab.com/accumulatenetwork/accumulate/protocol" + . "gitlab.com/accumulatenetwork/accumulate/test/harness" + "gitlab.com/accumulatenetwork/accumulate/test/simulator" +) + +func TestOverlay(t *testing.T) { + // Setup + alice := build. + Identity("alice").Create("book"). + Tokens("tokens").Create("ACME").Add(1e9).Identity(). + Book("book").Page(1).Create().AddCredits(1e9).Book().Identity() + aliceKey := alice.Book("book").Page(1). + GenerateKey(SignatureTypeED25519) + + badger := simulator.BadgerDbOpener(t.TempDir(), func(err error) { require.NoError(t, err) }) + simOpts := []simulator.Option{ + simulator.SimpleNetwork(t.Name(), 1, 1), + simulator.Genesis(GenesisTime).With(alice).WithVersion(ExecutorVersionV2Vandenberg), + } + + // Execute a transaction with the original database (timestamp = 1) + sim := NewSim(t, append(simOpts, + simulator.WithDatabase(badger), + )...) + + st := sim.BuildAndSubmitTxnSuccessfully( + build.Transaction().For(alice, "book", "1"). + BurnCredits(1). + SignWith(alice, "book", "1").Version(1).Timestamp(1).PrivateKey(aliceKey)) + sim.StepUntil( + Txn(st.TxID).Completes()) + + // Execute a transaction in an overlay (timestamp = 2) + sim = NewSim(t, append(simOpts, + simulator.OverlayDatabase(simulator.MemoryDbOpener, badger), + )...) + + st = sim.BuildAndSubmitTxnSuccessfully( + build.Transaction().For(alice, "book", "1"). + BurnCredits(1). + SignWith(alice, "book", "1").Version(1).Timestamp(2).PrivateKey(aliceKey)) + sim.StepUntil( + Txn(st.TxID).Completes()) + + // Verify that the same timestamp fails + st = sim.BuildAndSubmit( + build.Transaction().For(alice, "book", "1"). + BurnCredits(1). + SignWith(alice, "book", "1").Version(1).Timestamp(2).PrivateKey(aliceKey))[1] + require.ErrorIs(t, st.AsError(), errors.BadTimestamp) + + // Executing the transaction with the same timestamp in another overlay succeeds + sim = NewSim(t, append(simOpts, + simulator.OverlayDatabase(simulator.MemoryDbOpener, badger), + )...) + + st = sim.BuildAndSubmitTxnSuccessfully( + build.Transaction().For(alice, "book", "1"). + BurnCredits(1). + SignWith(alice, "book", "1").Version(1).Timestamp(2).PrivateKey(aliceKey)) + sim.StepUntil( + Txn(st.TxID).Completes()) +} diff --git a/pkg/database/keyvalue/remote/remote.go b/pkg/database/keyvalue/remote/remote.go index 832733fd0..12a30dc49 100644 --- a/pkg/database/keyvalue/remote/remote.go +++ b/pkg/database/keyvalue/remote/remote.go @@ -19,14 +19,26 @@ import ( // Serve opens a batch and serves it over the connection. Serve returns once the // connection is closed or the remote side calls Commit. See [Connect]. -func Serve(db keyvalue.Beginner, conn io.ReadWriteCloser, prefix *record.Key, writable bool) error { - defer conn.Close() +func Serve(db keyvalue.Beginner, conn io.ReadWriteCloser, prefix *record.Key, writable bool) <-chan error { batch := db.Begin(prefix, writable) - defer batch.Discard() + ch := make(chan error) + go func() { + defer conn.Close() + defer batch.Discard() + defer close(ch) + ch <- serve(batch, conn) + }() + return ch +} +func serve(batch keyvalue.ChangeSet, conn io.ReadWriteCloser) error { rd := bufio.NewReader(conn) run := true - for run { + for { + if !run { + return nil + } + c, err := read(rd, unmarshalCall) switch { case err == nil: @@ -41,6 +53,11 @@ func Serve(db keyvalue.Beginner, conn io.ReadWriteCloser, prefix *record.Key, wr switch c := c.(type) { case *commitCall: run = false + if c.Discard { + batch.Discard() + return new(okResponse) + } + err := batch.Commit() if err != nil { return errResp(err) @@ -58,7 +75,6 @@ func Serve(db keyvalue.Beginner, conn io.ReadWriteCloser, prefix *record.Key, wr return err } } - return nil } // DB is a remote key-value database client that creates a connection to the @@ -105,9 +121,12 @@ func (c *DB) Begin(prefix *database.Key, writable bool) keyvalue.ChangeSet { } discard := func() { - if err == nil { - _ = conn.Close() + if err != nil { + return } + + _ = c.discard(rd, conn) + _ = conn.Close() } return memory.NewChangeSet(memory.ChangeSetOptions{ @@ -127,6 +146,11 @@ func (c *DB) get(rd *bufio.Reader, wr io.Writer, key *record.Key) ([]byte, error return r.Value, nil } +func (c *DB) discard(rd *bufio.Reader, wr io.WriteCloser) error { + _, err := roundTrip[*okResponse](rd, wr, &commitCall{Discard: true}) + return err +} + func (c *DB) commit(rd *bufio.Reader, wr io.WriteCloser, entries map[[32]byte]memory.Entry) error { var err error for _, e := range entries { diff --git a/pkg/database/keyvalue/remote/remote_test.go b/pkg/database/keyvalue/remote/remote_test.go index 43178a74f..32733c0c0 100644 --- a/pkg/database/keyvalue/remote/remote_test.go +++ b/pkg/database/keyvalue/remote/remote_test.go @@ -14,25 +14,28 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue" - "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/badger" "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/kvtest" "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/memory" "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/remote" ) func open(t testing.TB) kvtest.Opener { - // Use a database that supports isolation - store, err := badger.New(t.TempDir(), badger.WithPlainKeys) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, store.Close()) - }) + // store, err := badger.New(t.TempDir(), badger.WithPlainKeys) + // require.NoError(t, err) + // t.Cleanup(func() { + // require.NoError(t, store.Close()) + // }) + + // BUG: Badger leads to strange concurrency errors that are still present + // even after ensuring serialization of calls. + store := memory.New(nil) return func() (keyvalue.Beginner, error) { return remote.Connect(func() (io.ReadWriteCloser, error) { rd1, wr1 := io.Pipe() rd2, wr2 := io.Pipe() - go func() { require.NoError(t, remote.Serve(store, &conn{rd1, wr2}, nil, true)) }() + done := remote.Serve(store, &conn{rd1, wr2}, nil, true) + go func() { require.NoError(t, <-done) }() return &conn{rd2, wr1}, nil }), nil } @@ -65,15 +68,14 @@ func TestCloseWhenDone(t *testing.T) { rd1, wr1 := io.Pipe() rd2, wr2 := io.Pipe() - errch := make(chan error) - go func() { errch <- remote.Serve(store, &conn{rd1, wr2}, nil, true) }() + done := remote.Serve(store, &conn{rd1, wr2}, nil, true) // Closing the connection should stop the server require.NoError(t, wr1.Close()) require.NoError(t, rd2.Close()) select { - case err := <-errch: + case err := <-done: require.NoError(t, err) case <-time.After(time.Second): t.Fatal("Server did not close") @@ -85,14 +87,13 @@ func TestCloseWhenDone(t *testing.T) { rd1, wr1 := io.Pipe() rd2, wr2 := io.Pipe() - errch := make(chan error) - go func() { errch <- remote.Serve(store, &conn{rd1, wr2}, nil, true) }() + done := remote.Serve(store, &conn{rd1, wr2}, nil, true) // Committing should stop the server (even if the connection doesn't close) require.NoError(t, remote.SendCommit(bufio.NewReader(rd2), wr1)) select { - case err := <-errch: + case err := <-done: require.NoError(t, err) case <-time.After(time.Second): t.Fatal("Server did not close") diff --git a/pkg/database/keyvalue/remote/types.yml b/pkg/database/keyvalue/remote/types.yml index e3f9123e8..397150d2d 100644 --- a/pkg/database/keyvalue/remote/types.yml +++ b/pkg/database/keyvalue/remote/types.yml @@ -32,7 +32,9 @@ forEachCall: commitCall: non-json: true union: { type: call, private: true } - fields: ~ + fields: + - name: Discard + type: bool batchCall: non-json: true diff --git a/pkg/database/keyvalue/remote/types_gen.go b/pkg/database/keyvalue/remote/types_gen.go index 9bafed03c..e56c62987 100644 --- a/pkg/database/keyvalue/remote/types_gen.go +++ b/pkg/database/keyvalue/remote/types_gen.go @@ -36,6 +36,7 @@ type batchResponse struct { type commitCall struct { fieldsSet []bool + Discard bool `json:"discard,omitempty" form:"discard" query:"discard" validate:"required"` extraData []byte } @@ -176,6 +177,7 @@ func (v *batchResponse) CopyAsInterface() interface{} { return v.Copy() } func (v *commitCall) Copy() *commitCall { u := new(commitCall) + u.Discard = v.Discard if len(v.extraData) > 0 { u.extraData = make([]byte, len(v.extraData)) copy(u.extraData, v.extraData) @@ -373,6 +375,9 @@ func (v *batchResponse) Equal(u *batchResponse) bool { } func (v *commitCall) Equal(u *commitCall) bool { + if !(v.Discard == u.Discard) { + return false + } return true } @@ -583,6 +588,7 @@ func (v *batchResponse) IsValid() error { var fieldNames_commitCall = []string{ 1: "Type", + 2: "Discard", } func (v *commitCall) MarshalBinary() ([]byte, error) { @@ -594,6 +600,9 @@ func (v *commitCall) MarshalBinary() ([]byte, error) { writer := encoding.NewWriter(buffer) writer.WriteEnum(1, v.Type()) + if !(!v.Discard) { + writer.WriteBool(2, v.Discard) + } _, _, err := writer.Reset(fieldNames_commitCall) if err != nil { @@ -609,6 +618,11 @@ func (v *commitCall) IsValid() error { if len(v.fieldsSet) > 0 && !v.fieldsSet[0] { errs = append(errs, "field Type is missing") } + if len(v.fieldsSet) > 1 && !v.fieldsSet[1] { + errs = append(errs, "field Discard is missing") + } else if !v.Discard { + errs = append(errs, "field Discard is not set") + } switch len(errs) { case 0: @@ -1243,6 +1257,9 @@ func (v *commitCall) UnmarshalBinaryFrom(rd io.Reader) error { } func (v *commitCall) UnmarshalFieldsFrom(reader *encoding.Reader) error { + if x, ok := reader.ReadBool(2); ok { + v.Discard = x + } seen, err := reader.Reset(fieldNames_commitCall) if err != nil { @@ -1639,6 +1656,7 @@ func init() { encoding.RegisterTypeDefinition(&[]*encoding.TypeField{ encoding.NewTypeField("type", "string"), + encoding.NewTypeField("discard", "bool"), }, "commitCall", "commitCall") encoding.RegisterTypeDefinition(&[]*encoding.TypeField{ diff --git a/pkg/database/values/value.go b/pkg/database/values/value.go index 7d5f96441..cc565a0ce 100644 --- a/pkg/database/values/value.go +++ b/pkg/database/values/value.go @@ -194,6 +194,7 @@ func (v *value[T]) Commit() error { return errors.UnknownError.Wrap(err) } + // TODO: v.status = valueClean? return nil } diff --git a/pkg/errors/inspect.go b/pkg/errors/inspect.go index cec5c3b57..ee5b98bb5 100644 --- a/pkg/errors/inspect.go +++ b/pkg/errors/inspect.go @@ -20,6 +20,15 @@ func Is(err, target error) bool { return errors.Is(err, target) } // Unwrap calls stdlib errors.Unwrap. func Unwrap(err error) error { return errors.Unwrap(err) } +func First(errs ...error) error { + for _, err := range errs { + if err != nil { + return err + } + } + return nil +} + // Code returns the status code if the error is an [Error], or 0. func Code(err error) Status { var err2 *ErrorBase[Status] diff --git a/pkg/types/record/key.go b/pkg/types/record/key.go index fe58c3202..1280b6d1a 100644 --- a/pkg/types/record/key.go +++ b/pkg/types/record/key.go @@ -15,12 +15,19 @@ import ( "fmt" "io" "strings" + "time" "gitlab.com/accumulatenetwork/accumulate/pkg/errors" "gitlab.com/accumulatenetwork/accumulate/pkg/types/encoding" + "gitlab.com/accumulatenetwork/accumulate/pkg/url" binary2 "gitlab.com/accumulatenetwork/core/schema/pkg/binary" + json2 "gitlab.com/accumulatenetwork/core/schema/pkg/json" ) +var binEncPool = binary2.NewEncoderPool() +var jsonEncPool = json2.NewEncoderPool() +var bufferPool = binary2.NewBufferPool() + // A Key is the key for a record. type Key struct { values []any @@ -28,7 +35,7 @@ type Key struct { } func NewKey(v ...any) *Key { - return &Key{values: v} + return (*Key)(nil).Append(v...) } func KeyFromHash(kh KeyHash) *Key { @@ -185,32 +192,14 @@ func (k *Key) Compare(l *Key) int { // MarshalBinary marshals the key to bytes. func (k *Key) MarshalBinary() ([]byte, error) { - buf := new(bytes.Buffer) - - // Write the length - _, _ = buf.Write(encoding.MarshalUint(uint64(k.Len()))) - if k.Len() == 0 { - return buf.Bytes(), nil - } + buf := bufferPool.Get() + defer bufferPool.Put(buf) - // Write each field using the encoding writer, but prefix values with their - // type code instead of with a field number. This is an abuse but 🤷 it - // works. - w := encoding.NewWriter(buf) - for _, v := range k.values { - p, err := asKeyPart(v) - if err != nil { - return nil, errors.UnknownError.Wrap(err) - } - p.WriteBinary(w) - } + enc := binEncPool.Get(buf, binary2.WithBufferPool(bufferPool)) + defer binEncPool.Put(enc) - // Finish up - _, _, err := w.Reset(nil) - if err != nil { - return nil, err - } - return buf.Bytes(), nil + err := k.MarshalBinaryV2(enc) + return buf.Bytes(), err } // UnmarshalBinary unmarshals a key from bytes. @@ -240,15 +229,24 @@ func (k *Key) MarshalBinaryV2(enc *binary2.Encoder) error { // type code instead of with a field number. This is an abuse but 🤷 it // works. for _, v := range k.values { - p, err := asKeyPart(v) - if err != nil { - return errors.UnknownError.WithFormat("encode Key: %w", err) + v, typ, ok := normalize(v) + if !ok { + return invalidKeyPart(v) } - err = enc.Field(uint(p.Type())) + err = enc.Field(uint(typ)) if err != nil { return errors.UnknownError.WithFormat("encode Key: %w", err) } - err = p.WriteBinary2(enc) + switch typ { + case typeCodeUrl: + err = enc.EncodeString(v.(*url.URL).String()) + case typeCodeTxid: + err = enc.EncodeString(v.(*url.TxID).String()) + case typeCodeTime: + err = enc.EncodeInt(v.(time.Time).UTC().Unix()) + default: + err = enc.Encode(v) + } if err != nil { return errors.UnknownError.WithFormat("encode Key: %w", err) } @@ -373,22 +371,39 @@ func (k *Key) UnmarshalBinaryFrom(rd io.Reader) error { // // [{"string": "Account"}, {"url": "foo.acme"}, {"string": "MainChain"}, {"string": "Element"}, {"int": 1}] func (k *Key) MarshalJSON() ([]byte, error) { - if k.Len() == 0 { - return []byte("[]"), nil + buf := bufferPool.Get() + defer bufferPool.Put(buf) + + enc := jsonEncPool.Get(buf) + defer jsonEncPool.Put(enc) + + err := k.MarshalJSONV2(enc) + return buf.Bytes(), err +} + +func (k *Key) MarshalJSONV2(enc *json2.Encoder) error { + err := enc.StartArray() + if err != nil { + return err } - parts := make([]map[string]any, len(k.values)) - for i, v := range k.values { - // Convert the value to a key part - p, err := asKeyPart(v) + for _, v := range k.values { + v, typ, ok := normalize(v) + if !ok { + return invalidKeyPart(v) + } + err = errors.First( + enc.StartObject(), + enc.Field(typ.String()), + enc.Encode(v), + enc.EndObject(), + ) if err != nil { - return nil, errors.UnknownError.Wrap(err) + return err } - - // Record as { [type]: value } - parts[i] = map[string]any{p.Type().String(): p} } - return json.Marshal(parts) + + return enc.EndArray() } // UnmarshalJSON unmarshals a key from JSON. @@ -427,7 +442,7 @@ func (k *Key) UnmarshalJSON(b []byte) error { } // Unmarshal the value - err = json.Unmarshal(b, &kp) + err = json.Unmarshal(b, kp) if err != nil { return errors.UnknownError.WithFormat("decode Key: %w", err) } diff --git a/pkg/types/record/key_part.go b/pkg/types/record/key_part.go index c438d25cc..dbd3b2682 100644 --- a/pkg/types/record/key_part.go +++ b/pkg/types/record/key_part.go @@ -19,8 +19,8 @@ import ( binary2 "gitlab.com/accumulatenetwork/core/schema/pkg/binary" ) -// keyPart is a part of a [Key]. keyPart is used for marshalling. -type keyPart interface { +// keyPartRd is a part of a [Key]. keyPartRd is used for marshalling. +type keyPartRd interface { // Type returns the key part's type. Type() typeCode @@ -36,6 +36,12 @@ type keyPart interface { // any attention to the field numbers so this still works. WriteBinary(*enc.Writer) + WriteBinary2(*binary2.Encoder) error +} + +type keyPartWr interface { + keyPartRd + // ReadBinary reads the key part from the binary reader. // // The intended use of the binary reader is reading field-number tagged @@ -47,7 +53,6 @@ type keyPart interface { // instructs it to skip the field number logic and read the value directly. ReadBinary(*enc.Reader) - WriteBinary2(*binary2.Encoder) error ReadBinary2(*binary2.Decoder) error } @@ -61,52 +66,44 @@ func (k uintKeyPart) WriteBinary(w *enc.Writer) { w.WriteUint(uint(k.Type()), func (k stringKeyPart) WriteBinary(w *enc.Writer) { w.WriteString(uint(k.Type()), string(k)) } func (k hashKeyPart) WriteBinary(w *enc.Writer) { w.WriteHash(uint(k.Type()), (*[32]byte)(&k)) } func (k bytesKeyPart) WriteBinary(w *enc.Writer) { w.WriteBytes(uint(k.Type()), k) } -func (k urlKeyPart) WriteBinary(w *enc.Writer) { w.WriteUrl(uint(k.Type()), k.URL) } -func (k txidKeyPart) WriteBinary(w *enc.Writer) { w.WriteTxid(uint(k.Type()), k.TxID) } -func (k timeKeyPart) WriteBinary(w *enc.Writer) { w.WriteTime(uint(k.Type()), k.Time) } - -func (k *intKeyPart) ReadBinary(r *enc.Reader) { ReadBinary((*int64)(k), r.ReadInt) } -func (k *uintKeyPart) ReadBinary(r *enc.Reader) { ReadBinary((*uint64)(k), r.ReadUint) } -func (k *stringKeyPart) ReadBinary(r *enc.Reader) { ReadBinary((*string)(k), r.ReadString) } -func (k *hashKeyPart) ReadBinary(r *enc.Reader) { ReadBinary((*[32]byte)(k), r.ReadHash2) } -func (k *bytesKeyPart) ReadBinary(r *enc.Reader) { ReadBinary((*[]byte)(k), r.ReadBytes) } -func (k *urlKeyPart) ReadBinary(r *enc.Reader) { ReadBinary(&k.URL, r.ReadUrl) } -func (k *txidKeyPart) ReadBinary(r *enc.Reader) { ReadBinary(&k.TxID, r.ReadTxid) } -func (k *timeKeyPart) ReadBinary(r *enc.Reader) { ReadBinary(&k.Time, r.ReadTime) } +func (k *urlKeyPart) WriteBinary(w *enc.Writer) { w.WriteUrl(uint(k.Type()), (*url.URL)(k)) } +func (k *txidKeyPart) WriteBinary(w *enc.Writer) { w.WriteTxid(uint(k.Type()), (*url.TxID)(k)) } +func (k timeKeyPart) WriteBinary(w *enc.Writer) { w.WriteTime(uint(k.Type()), time.Time(k)) } + +func (k *intKeyPart) ReadBinary(r *enc.Reader) { rdBinVal((*int64)(k), r.ReadInt) } +func (k *uintKeyPart) ReadBinary(r *enc.Reader) { rdBinVal((*uint64)(k), r.ReadUint) } +func (k *stringKeyPart) ReadBinary(r *enc.Reader) { rdBinVal((*string)(k), r.ReadString) } +func (k *hashKeyPart) ReadBinary(r *enc.Reader) { rdBinVal((*[32]byte)(k), r.ReadHash2) } +func (k *bytesKeyPart) ReadBinary(r *enc.Reader) { rdBinVal((*[]byte)(k), r.ReadBytes) } +func (k *urlKeyPart) ReadBinary(r *enc.Reader) { rdBinPtr((*url.URL)(k), r.ReadUrl) } +func (k *txidKeyPart) ReadBinary(r *enc.Reader) { rdBinPtr((*url.TxID)(k), r.ReadTxid) } +func (k *timeKeyPart) ReadBinary(r *enc.Reader) { rdBinVal((*time.Time)(k), r.ReadTime) } func (k intKeyPart) WriteBinary2(w *enc2) error { return w.EncodeInt(int64(k)) } func (k uintKeyPart) WriteBinary2(w *enc2) error { return w.EncodeUint(uint64(k)) } func (k stringKeyPart) WriteBinary2(w *enc2) error { return w.EncodeString(string(k)) } func (k hashKeyPart) WriteBinary2(w *enc2) error { return w.EncodeHash(k) } func (k bytesKeyPart) WriteBinary2(w *enc2) error { return w.EncodeBytes(k) } -func (k urlKeyPart) WriteBinary2(w *enc2) error { return w.EncodeString(k.URL.String()) } -func (k txidKeyPart) WriteBinary2(w *enc2) error { return w.EncodeString(k.TxID.String()) } -func (k timeKeyPart) WriteBinary2(w *enc2) error { return w.EncodeInt(k.Time.UTC().Unix()) } +func (k *urlKeyPart) WriteBinary2(w *enc2) error { return w.EncodeString((*url.URL)(k).String()) } +func (k *txidKeyPart) WriteBinary2(w *enc2) error { return w.EncodeString((*url.TxID)(k).String()) } +func (k timeKeyPart) WriteBinary2(w *enc2) error { return w.EncodeInt(time.Time(k).UTC().Unix()) } func (k *intKeyPart) ReadBinary2(r *dec2) error { return rdBin2((*int64)(k), r.DecodeInt) } func (k *uintKeyPart) ReadBinary2(r *dec2) error { return rdBin2((*uint64)(k), r.DecodeUint) } func (k *stringKeyPart) ReadBinary2(r *dec2) error { return rdBin2((*string)(k), r.DecodeString) } func (k *hashKeyPart) ReadBinary2(r *dec2) error { return rdBin2((*[32]byte)(k), r.DecodeHash) } func (k *bytesKeyPart) ReadBinary2(r *dec2) error { return rdBin2((*[]byte)(k), r.DecodeBytes) } - -func (k *urlKeyPart) ReadBinary2(r *dec2) error { - k.URL = new(url.URL) - return r.DecodeValueV2(k.URL) -} - -func (k *txidKeyPart) ReadBinary2(r *dec2) error { - k.TxID = new(url.TxID) - return r.DecodeValueV2(k.TxID) -} +func (k *urlKeyPart) ReadBinary2(r *dec2) error { return r.DecodeValueV2((*url.URL)(k)) } +func (k *txidKeyPart) ReadBinary2(r *dec2) error { return r.DecodeValueV2((*url.TxID)(k)) } func (k *timeKeyPart) ReadBinary2(r *dec2) error { v, err := r.DecodeInt() - k.Time = time.Unix(v, 0).UTC() + *k = timeKeyPart(time.Unix(v, 0).UTC()) return err } // newKeyPart returns a new key part for the type code. -func newKeyPart(typ typeCode) (keyPart, error) { +func newKeyPart(typ typeCode) (keyPartWr, error) { switch typ { case typeCodeInt: return new(intKeyPart), nil @@ -129,33 +126,30 @@ func newKeyPart(typ typeCode) (keyPart, error) { } } -// asKeyPart converts the value to a key part. -func asKeyPart(v any) (keyPart, error) { - switch v := v.(type) { +func normalize(v any) (any, typeCode, bool) { + switch u := v.(type) { case int64: - return (*intKeyPart)(&v), nil + return v, typeCodeInt, true case int: - u := int64(v) - return (*intKeyPart)(&u), nil + return int64(u), typeCodeInt, true case uint64: - return (*uintKeyPart)(&v), nil + return v, typeCodeUint, true case uint: - u := uint64(v) - return (*uintKeyPart)(&u), nil + return uint64(u), typeCodeUint, true case string: - return (*stringKeyPart)(&v), nil + return v, typeCodeString, true case [32]byte: - return (*hashKeyPart)(&v), nil + return v, typeCodeHash, true case []byte: - return (*bytesKeyPart)(&v), nil + return v, typeCodeBytes, true case *url.URL: - return &urlKeyPart{v}, nil + return v, typeCodeUrl, true case *url.TxID: - return &txidKeyPart{v}, nil + return v, typeCodeTxid, true case time.Time: - return &timeKeyPart{v}, nil + return v, typeCodeTime, true default: - return nil, errors.NotAllowed.WithFormat("%T is not a supported key part type", v) + return v, typeCodeUnknown, false } } @@ -164,17 +158,17 @@ type uintKeyPart uint64 type stringKeyPart string type hashKeyPart [32]byte type bytesKeyPart []byte -type urlKeyPart struct{ *url.URL } -type txidKeyPart struct{ *url.TxID } -type timeKeyPart struct{ time.Time } +type urlKeyPart url.URL +type txidKeyPart url.TxID +type timeKeyPart time.Time func (k intKeyPart) Type() typeCode { return typeCodeInt } func (k uintKeyPart) Type() typeCode { return typeCodeUint } func (k stringKeyPart) Type() typeCode { return typeCodeString } func (k hashKeyPart) Type() typeCode { return typeCodeHash } func (k bytesKeyPart) Type() typeCode { return typeCodeBytes } -func (k urlKeyPart) Type() typeCode { return typeCodeUrl } -func (k txidKeyPart) Type() typeCode { return typeCodeTxid } +func (k *urlKeyPart) Type() typeCode { return typeCodeUrl } +func (k *txidKeyPart) Type() typeCode { return typeCodeTxid } func (k timeKeyPart) Type() typeCode { return typeCodeTime } func (k intKeyPart) Value() any { return int64(k) } @@ -182,16 +176,22 @@ func (k uintKeyPart) Value() any { return uint64(k) } func (k stringKeyPart) Value() any { return string(k) } func (k hashKeyPart) Value() any { return [32]byte(k) } func (k bytesKeyPart) Value() any { return []byte(k) } -func (k urlKeyPart) Value() any { return k.URL } -func (k txidKeyPart) Value() any { return k.TxID } -func (k timeKeyPart) Value() any { return k.Time } +func (k *urlKeyPart) Value() any { return (*url.URL)(k) } +func (k *txidKeyPart) Value() any { return (*url.TxID)(k) } +func (k timeKeyPart) Value() any { return time.Time(k) } -func ReadBinary[V any](v *V, read func(uint) (V, bool)) { +func rdBinVal[V any](v *V, read func(uint) (V, bool)) { // Read with field = 0 to tell the reader to skip the field number u, _ := read(0) *v = u } +func rdBinPtr[V any](v *V, read func(uint) (*V, bool)) { + // Read with field = 0 to tell the reader to skip the field number + u, _ := read(0) + *v = *u +} + func rdBin2[V any](v *V, read func() (V, error)) error { u, err := read() *v = u @@ -237,109 +237,103 @@ func (k *bytesKeyPart) UnmarshalJSON(b []byte) error { } func (k *urlKeyPart) UnmarshalJSON(b []byte) error { - k.URL = new(url.URL) - return k.URL.UnmarshalJSON(b) + return (*url.URL)(k).UnmarshalJSON(b) } func (k *txidKeyPart) UnmarshalJSON(b []byte) error { - k.TxID = new(url.TxID) - return k.TxID.UnmarshalJSON(b) + return (*url.TxID)(k).UnmarshalJSON(b) } func (k *timeKeyPart) UnmarshalJSON(b []byte) error { - return json.Unmarshal(b, &k.Time) + return json.Unmarshal(b, (*time.Time)(k)) } // keyPartsEqual returns true if U and V are the same. func keyPartsEqual(v, u any) bool { - switch v := v.(type) { - case int64: - _, ok := u.(int64) - if !ok { - return false - } - case uint64: - _, ok := u.(uint64) - if !ok { - return false - } - case string: - _, ok := u.(string) - if !ok { - return false - } - case [32]byte: - _, ok := u.([32]byte) - if !ok { - return false - } - case []byte: - u, ok := u.([]byte) - if !ok { - return false - } + v, a, ok := normalize(v) + if !ok { + panic(errors.NotAllowed.WithFormat("%T is not a supported key part type", v)) + } + u, b, ok := normalize(u) + if !ok { + panic(errors.NotAllowed.WithFormat("%T is not a supported key part type", u)) + } + if a != b { + return false + } + + switch a { + case typeCodeInt: + v, u := v.(int64), u.(int64) + return v == u + case typeCodeUint: + v, u := v.(uint64), u.(uint64) + return v == u + case typeCodeString: + v, u := v.(string), u.(string) + return v == u + case typeCodeHash: + v, u := v.([32]byte), u.([32]byte) + return v == u + case typeCodeBytes: + v, u := v.([]byte), u.([]byte) return bytes.Equal(v, u) - case *url.URL: - u, ok := u.(*url.URL) - if !ok { - return false - } + case typeCodeUrl: + v, u := v.(*url.URL), u.(*url.URL) return v.Equal(u) - case *url.TxID: - u, ok := u.(*url.TxID) - if !ok { - return false - } + case typeCodeTxid: + v, u := v.(*url.TxID), u.(*url.TxID) return v.Equal(u) - case time.Time: - u, ok := u.(time.Time) - if !ok { - return false - } + case typeCodeTime: + v, u := v.(time.Time), u.(time.Time) return v.Equal(u) } return v == u } +func invalidKeyPart(v any) error { + return errors.NotAllowed.WithFormat("%T is not a supported key part type", v) +} + func keyPartsCompare(v, u any) int { - a, err := asKeyPart(v) - if err != nil { - panic(err) + v, a, ok := normalize(v) + if !ok { + panic(invalidKeyPart(v)) } - b, err := asKeyPart(u) - if err != nil { - panic(err) + u, b, ok := normalize(u) + if !ok { + panic(invalidKeyPart(u)) } - - if a.Type() != b.Type() { - return int(a.Type()) - int(b.Type()) + if a != b { + return int(a - b) } - switch v := a.(type) { - case *intKeyPart: - u := b.(*intKeyPart) - return int(*v) - int(*u) - case *uintKeyPart: - u := b.(*uintKeyPart) - return int(*v) - int(*u) - case *stringKeyPart: - u := b.(*stringKeyPart) - return strings.Compare(string(*v), string(*u)) - case *hashKeyPart: - u := b.(*hashKeyPart) + + switch a { + case typeCodeInt: + v, u := v.(int64), u.(int64) + return int(v - u) + case typeCodeUint: + v, u := v.(uint64), u.(uint64) + return int(v - u) + case typeCodeString: + v, u := v.(string), u.(string) + return strings.Compare(v, u) + case typeCodeHash: + v, u := v.([32]byte), u.([32]byte) return bytes.Compare(v[:], u[:]) - case *bytesKeyPart: - u := b.(*bytesKeyPart) - return bytes.Compare(*v, *u) - case *urlKeyPart: - u := b.(*urlKeyPart) - return v.Compare(u.URL) - case *txidKeyPart: - u := b.(*txidKeyPart) - return v.Compare(u.TxID) - case *timeKeyPart: - u := b.(*timeKeyPart) - return v.Compare(u.Time) + case typeCodeBytes: + v, u := v.([]byte), u.([]byte) + return bytes.Compare(v, u) + case typeCodeUrl: + v, u := v.(*url.URL), u.(*url.URL) + return v.Compare(u) + case typeCodeTxid: + v, u := v.(*url.TxID), u.(*url.TxID) + return v.Compare(u) + case typeCodeTime: + v, u := v.(time.Time), u.(time.Time) + return v.Compare(u) default: panic("unknown type") } diff --git a/pkg/types/record/key_test.go b/pkg/types/record/key_test.go index 6fbe81a70..712d8cacc 100644 --- a/pkg/types/record/key_test.go +++ b/pkg/types/record/key_test.go @@ -7,9 +7,11 @@ package record_test import ( + "encoding/hex" "testing" "github.com/stretchr/testify/require" + "gitlab.com/accumulatenetwork/accumulate/internal/database/record" . "gitlab.com/accumulatenetwork/accumulate/pkg/types/record" "gitlab.com/accumulatenetwork/accumulate/protocol" ) @@ -22,6 +24,11 @@ func TestKeyBinary(t *testing.T) { var l Key require.NoError(t, l.UnmarshalBinary(b)) require.True(t, k.Equal(&l)) + + k = NewKey(1) + b, err = k.MarshalBinary() + require.NoError(t, err) + require.Equal(t, "010102", hex.EncodeToString(b)) } func TestKeyJSON(t *testing.T) { @@ -32,4 +39,73 @@ func TestKeyJSON(t *testing.T) { var l Key require.NoError(t, l.UnmarshalJSON(b)) require.True(t, k.Equal(&l)) + + k = NewKey(1) + b, err = k.MarshalJSON() + require.NoError(t, err) + require.Equal(t, `[{"int":1}]`, string(b)) +} + +func BenchmarkKey_Append(b *testing.B) { + key := NewKey(1) + var x *record.Key + for range b.N { + x = key.Append("foo") + } + require.NotNil(b, x) +} + +func BenchmarkKey_Compare(b *testing.B) { + i, j := NewKey(1), NewKey(2) + var c int + for range b.N { + c = i.Compare(j) + } + require.Equal(b, -1, c) +} + +func BenchmarkKey_Hash(b *testing.B) { + key := NewKey("foo", 1) + var x [32]byte + for range b.N { + key := *key // Prevent Hash from saving the result + x = key.Hash() + } + require.NotZero(b, x) +} + +func BenchmarkKey_Copy(b *testing.B) { + key := NewKey("foo", 1) + var x *record.Key + for range b.N { + x = key.Copy() + } + require.NotNil(b, x) +} + +func BenchmarkKey_Equal(b *testing.B) { + i, j := NewKey(1), NewKey(2) + var c bool + for range b.N { + c = i.Equal(j) + } + require.False(b, c) +} + +func BenchmarkKey_MarshalJSON(b *testing.B) { + key := NewKey("foo", 1) + var err error + for range b.N { + _, err = key.MarshalJSON() + } + require.NoError(b, err) +} + +func BenchmarkKey_MarshalBinary(b *testing.B) { + key := NewKey("foo", 1) + var err error + for range b.N { + _, err = key.MarshalBinary() + } + require.NoError(b, err) } diff --git a/test/simulator/options.go b/test/simulator/options.go index bb39f3c38..bf3e41427 100644 --- a/test/simulator/options.go +++ b/test/simulator/options.go @@ -25,6 +25,8 @@ import ( ioutil2 "gitlab.com/accumulatenetwork/accumulate/internal/util/io" "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue" "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/badger" + "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/memory" + "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/overlay" "gitlab.com/accumulatenetwork/accumulate/pkg/errors" "gitlab.com/accumulatenetwork/accumulate/pkg/types/network" "gitlab.com/accumulatenetwork/accumulate/pkg/url" @@ -199,24 +201,48 @@ func NewLocalNetwork(name string, bvnCount, nodeCount int, baseIP net.IP, basePo // Deprecated: This is a no-op func MemoryDatabase(*simFactory) error { return nil } -func BadgerDatabaseFromDirectory(dir string, onErr func(error)) Option { - return WithDatabase(func(partition *protocol.PartitionInfo, node int, _ log.Logger) keyvalue.Beginner { +func MemoryDbOpener(partition *protocol.PartitionInfo, node int, logger log.Logger) keyvalue.Beginner { + return memory.New(nil) +} + +func BadgerDbOpener(dir string, onErr func(error)) OpenDatabaseFunc { + dbs := map[string]keyvalue.Beginner{} + return func(partition *protocol.PartitionInfo, node int, logger log.Logger) keyvalue.Beginner { + file := fmt.Sprintf("%s-%d.db", partition.ID, node) + if db, ok := dbs[file]; ok { + return db + } + err := os.MkdirAll(dir, 0700) if err != nil { onErr(err) panic(err) } - db, err := badger.New(filepath.Join(dir, fmt.Sprintf("%s-%d.db", partition.ID, node))) + db, err := badger.New(filepath.Join(dir, file)) if err != nil { onErr(err) panic(err) } + dbs[file] = db return db + } +} + +func OverlayDatabase(a, b OpenDatabaseFunc) Option { + return WithDatabase(func(partition *protocol.PartitionInfo, node int, logger log.Logger) keyvalue.Beginner { + return overlay.Open( + a(partition, node, logger), + b(partition, node, logger), + ) }) } +func BadgerDatabaseFromDirectory(dir string, onErr func(error)) Option { + return WithDatabase(BadgerDbOpener(dir, onErr)) +} + func SnapshotFromDirectory(dir string) Option { return WithSnapshot(func(partition string, network *accumulated.NetworkInit, logger log.Logger) (ioutil2.SectionReader, error) { return os.Open(filepath.Join(dir, fmt.Sprintf("%s.snapshot", partition))) diff --git a/test/simulator/simulator.go b/test/simulator/simulator.go index c25c4fd98..6c7d20d81 100644 --- a/test/simulator/simulator.go +++ b/test/simulator/simulator.go @@ -15,8 +15,6 @@ import ( "gitlab.com/accumulatenetwork/accumulate/internal/database" accumulated "gitlab.com/accumulatenetwork/accumulate/internal/node/daemon" ioutil2 "gitlab.com/accumulatenetwork/accumulate/internal/util/io" - "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue" - "gitlab.com/accumulatenetwork/accumulate/pkg/database/keyvalue/memory" "gitlab.com/accumulatenetwork/accumulate/pkg/errors" "gitlab.com/accumulatenetwork/accumulate/pkg/types/messaging" "gitlab.com/accumulatenetwork/accumulate/pkg/url" @@ -39,10 +37,8 @@ type Simulator struct { func New(opts ...Option) (*Simulator, error) { // Process options o := &simFactory{ - network: NewSimpleNetwork("Sim", 3, 3), - storeOpt: func(_ *protocol.PartitionInfo, _ int, logger log.Logger) keyvalue.Beginner { - return memory.New(nil) - }, + network: NewSimpleNetwork("Sim", 3, 3), + storeOpt: MemoryDbOpener, snapshot: func(string, *accumulated.NetworkInit, log.Logger) (ioutil2.SectionReader, error) { return new(ioutil2.Buffer), nil }, diff --git a/tools/cmd/debug/db_common.go b/tools/cmd/debug/db_common.go index cea2d4f26..1587e30ea 100644 --- a/tools/cmd/debug/db_common.go +++ b/tools/cmd/debug/db_common.go @@ -47,7 +47,8 @@ func serveKeyValueStore(ctx context.Context, store keyvalue.Beginner, addr net.A } } - err = remote.Serve(store, c, nil, false) + done := remote.Serve(store, c, nil, false) + err = <-done if err != nil { return err }