Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(core): add a new codec to core #22326

Merged
merged 9 commits into from
Oct 28, 2024
Merged

feat(core): add a new codec to core #22326

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
beec467
add a new codec to avoid sdk imports in modules
tac0turtle Oct 21, 2024
c64ea01
add protocodec
tac0turtle Oct 21, 2024
71a20ab
revert change
tac0turtle Oct 21, 2024
c63aece
add go mod changes
tac0turtle Oct 22, 2024
376d1bf
address comments
tac0turtle Oct 25, 2024
eb5c4bf
lint
tac0turtle Oct 25, 2024
f1330c8
Merge branch 'main' into marko/propose_codecs
tac0turtle Oct 25, 2024
d89f56c
Merge branch 'main' into marko/propose_codecs
tac0turtle Oct 28, 2024
fab3136
Merge branch 'main' into marko/propose_codecs
tac0turtle Oct 28, 2024
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
38 changes: 4 additions & 34 deletions codec/collections.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,16 @@
"reflect"

"github.com/cosmos/gogoproto/proto"
gogotypes "github.com/cosmos/gogoproto/types"
"google.golang.org/protobuf/encoding/protojson"
protov2 "google.golang.org/protobuf/proto"

"cosmossdk.io/collections"
collcodec "cosmossdk.io/collections/codec"
"cosmossdk.io/collections/protocodec"

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / split-test-files 

module cosmossdk.io/collections/protocodec provides package cosmossdk.io/collections/protocodec and is replaced but not required; to add it:

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / dependency-review 

no required module provides package cosmossdk.io/collections/protocodec; to add it:

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / dependency-review 

no required module provides package cosmossdk.io/collections/protocodec; to add it:

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / dependency-review 

no required module provides package cosmossdk.io/collections/protocodec; to add it:

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / dependency-review 

no required module provides package cosmossdk.io/collections/protocodec; to add it:

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / dependency-review 

no required module provides package cosmossdk.io/collections/protocodec; to add it:

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / dependency-review 

module cosmossdk.io/collections/protocodec provides package cosmossdk.io/collections/protocodec and is replaced but not required; to add it:

Check failure on line 12 in codec/collections.go

View workflow job for this annotation

GitHub Actions / dependency-review 

could not import cosmossdk.io/collections/protocodec (invalid package name: "")
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the import path for 'protocodec' package

The import statement for "cosmossdk.io/collections/protocodec" may cause build failures due to missing module requirements. According to the static analysis hints, the module is not properly included in your go.mod file.

To resolve this issue, ensure that the cosmossdk.io/collections module is required in your go.mod file. You can add it by running:

go get cosmossdk.io/collections@latest

This command will fetch the latest version of the module and update your go.mod file accordingly.

🧰 Tools
🪛 GitHub Check: split-test-files

[failure] 12-12:
module cosmossdk.io/collections/protocodec provides package cosmossdk.io/collections/protocodec and is replaced but not required; to add it:

🪛 GitHub Check: dependency-review

[failure] 12-12:
no required module provides package cosmossdk.io/collections/protocodec; to add it:

[failure] 12-12:
no required module provides package cosmossdk.io/collections/protocodec; to add it:

[failure] 12-12:
no required module provides package cosmossdk.io/collections/protocodec; to add it:

[failure] 12-12:
no required module provides package cosmossdk.io/collections/protocodec; to add it:

[failure] 12-12:
no required module provides package cosmossdk.io/collections/protocodec; to add it:

[failure] 12-12:
module cosmossdk.io/collections/protocodec provides package cosmossdk.io/collections/protocodec and is replaced but not required; to add it:

[failure] 12-12:
could not import cosmossdk.io/collections/protocodec (invalid package name: "")

)

// BoolValue implements a ValueCodec that saves the bool value
// as if it was a prototypes.BoolValue. Required for backwards
// compatibility of state.
var BoolValue collcodec.ValueCodec[bool] = boolValue{}

type boolValue struct{}

func (boolValue) Encode(value bool) ([]byte, error) {
return (&gogotypes.BoolValue{Value: value}).Marshal()
}

func (boolValue) Decode(b []byte) (bool, error) {
v := new(gogotypes.BoolValue)
err := v.Unmarshal(b)
return v.Value, err
}

func (boolValue) EncodeJSON(value bool) ([]byte, error) {
return collections.BoolValue.EncodeJSON(value)
}

func (boolValue) DecodeJSON(b []byte) (bool, error) {
return collections.BoolValue.DecodeJSON(b)
}

func (boolValue) Stringify(value bool) string {
return collections.BoolValue.Stringify(value)
}

func (boolValue) ValueType() string {
return "protobuf/bool"
}
type (
BoolValue = protocodec.BoolValue
)

type protoMessage[T any] interface {
*T
Expand Down
137 changes: 137 additions & 0 deletions collections/protocodec/collections.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package codec

import (
"fmt"

"github.com/cosmos/gogoproto/proto"
gogotypes "github.com/cosmos/gogoproto/types"
"google.golang.org/protobuf/encoding/protojson"
protov2 "google.golang.org/protobuf/proto"

"cosmossdk.io/collections"
collcodec "cosmossdk.io/collections/codec"
corecodec "cosmossdk.io/core/codec"
)

// BoolValue implements a ValueCodec that saves the bool value
// as if it was a prototypes.BoolValue. Required for backwards
// compatibility of state.
var BoolValue collcodec.ValueCodec[bool] = boolValue{}

type boolValue struct{}

func (boolValue) Encode(value bool) ([]byte, error) {
return (&gogotypes.BoolValue{Value: value}).Marshal()
}

func (boolValue) Decode(b []byte) (bool, error) {
v := new(gogotypes.BoolValue)
err := v.Unmarshal(b)
return v.Value, err
}

func (boolValue) EncodeJSON(value bool) ([]byte, error) {
return collections.BoolValue.EncodeJSON(value)
}

func (boolValue) DecodeJSON(b []byte) (bool, error) {
return collections.BoolValue.DecodeJSON(b)
}

func (boolValue) Stringify(value bool) string {
return collections.BoolValue.Stringify(value)
}

func (boolValue) ValueType() string {
return "protobuf/bool"
}

type protoMessage[T any] interface {
*T
proto.Message
}

// CollValue inits a collections.ValueCodec for a generic gogo protobuf message.
func CollValue[T any, PT protoMessage[T]](cdc interface {
Marshal(proto.Message) ([]byte, error)
Unmarshal([]byte, proto.Message) error
},
) collcodec.ValueCodec[T] {
Comment on lines +55 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Define a Specific Interface Instead of Using an Anonymous Interface for cdc

Using an anonymous interface for the cdc parameter reduces clarity and type safety. Consider defining a specific interface or using an existing one that includes the required methods (Marshal and Unmarshal). This enhances readability and ensures better compile-time checks.

return &collValue[T, PT]{cdc.(corecodec.Codec), proto.MessageName(PT(new(T)))}
}
Comment on lines +55 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle Type Assertion Safely to Prevent Potential Panic

In the CollValue function, the type assertion cdc.(corecodec.Codec) may cause a panic if cdc does not implement corecodec.Codec. It's safer to perform a checked type assertion to prevent runtime errors.

Apply this diff to safely handle the type assertion:

 func CollValue[T any, PT protoMessage[T]](cdc interface {
 	Marshal(proto.Message) ([]byte, error)
 	Unmarshal([]byte, proto.Message) error
 },
 ) collcodec.ValueCodec[T] {
-	return &collValue[T, PT]{cdc.(corecodec.Codec), proto.MessageName(PT(new(T)))}
+	coreCdc, ok := cdc.(corecodec.Codec)
+	if !ok {
+		// Handle the error appropriately, perhaps return an error
+		return nil // or return an error
+	}
+	return &collValue[T, PT]{coreCdc, proto.MessageName(PT(new(T)))}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func CollValue[T any, PT protoMessage[T]](cdc interface {
Marshal(proto.Message) ([]byte, error)
Unmarshal([]byte, proto.Message) error
},
) collcodec.ValueCodec[T] {
return &collValue[T, PT]{cdc.(corecodec.Codec), proto.MessageName(PT(new(T)))}
}
func CollValue[T any, PT protoMessage[T]](cdc interface {
Marshal(proto.Message) ([]byte, error)
Unmarshal([]byte, proto.Message) error
},
) collcodec.ValueCodec[T] {
coreCdc, ok := cdc.(corecodec.Codec)
if !ok {
// Handle the error appropriately, perhaps return an error
return nil // or return an error
}
return &collValue[T, PT]{coreCdc, proto.MessageName(PT(new(T)))}
}


type collValue[T any, PT protoMessage[T]] struct {
cdc corecodec.Codec
messageName string
}

func (c collValue[T, PT]) Encode(value T) ([]byte, error) {
return c.cdc.Marshal(PT(&value))
}

func (c collValue[T, PT]) Decode(b []byte) (value T, err error) {
err = c.cdc.Unmarshal(b, PT(&value))
return value, err
}

func (c collValue[T, PT]) EncodeJSON(value T) ([]byte, error) {
return c.cdc.MarshalJSON(PT(&value))
}

func (c collValue[T, PT]) DecodeJSON(b []byte) (value T, err error) {
err = c.cdc.UnmarshalJSON(b, PT(&value))
return
}

func (c collValue[T, PT]) Stringify(value T) string {
return PT(&value).String()
}

func (c collValue[T, PT]) ValueType() string {
return "github.com/cosmos/gogoproto/" + c.messageName
}

type protoMessageV2[T any] interface {
*T
protov2.Message
}

// CollValueV2 is used for protobuf values of the newest google.golang.org/protobuf API.
func CollValueV2[T any, PT protoMessageV2[T]]() collcodec.ValueCodec[PT] {
return &collValue2[T, PT]{
messageName: string(PT(new(T)).ProtoReflect().Descriptor().FullName()),
}
}

type collValue2[T any, PT protoMessageV2[T]] struct {
messageName string
}

func (c collValue2[T, PT]) Encode(value PT) ([]byte, error) {
protov2MarshalOpts := protov2.MarshalOptions{Deterministic: true}
return protov2MarshalOpts.Marshal(value)
}

func (c collValue2[T, PT]) Decode(b []byte) (PT, error) {
var value T
err := protov2.Unmarshal(b, PT(&value))
return &value, err
}

func (c collValue2[T, PT]) EncodeJSON(value PT) ([]byte, error) {
return protojson.Marshal(value)
}

func (c collValue2[T, PT]) DecodeJSON(b []byte) (PT, error) {
var value T
err := protojson.Unmarshal(b, PT(&value))
return &value, err
}

func (c collValue2[T, PT]) Stringify(value PT) string {
return fmt.Sprintf("%v", value)
}

func (c collValue2[T, PT]) ValueType() string {
return "google.golang.org/protobuf/" + c.messageName
}
Comment on lines +63 to +137
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor to Eliminate Code Duplication Between collValue and collValue2

The structs collValue and collValue2, along with their methods, share similar logic. Consider refactoring to reduce code duplication by abstracting the common functionality into a single generic struct or interface. This adheres to the DRY (Don't Repeat Yourself) principle and improves maintainability.

57 changes: 57 additions & 0 deletions collections/protocodec/collections_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package codec_test

import (
"testing"

gogotypes "github.com/cosmos/gogoproto/types"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/wrapperspb"

"cosmossdk.io/collections/colltest"

codec "cosmossdk.io/collections/protocodec"
)

func TestCollectionsCorrectness(t *testing.T) {

t.Run("CollValueV2", func(t *testing.T) {
// NOTE: we cannot use colltest.TestValueCodec because protov2 has different
// compare semantics than protov1. We need to use protocmp.Transform() alongside
// cmp to ensure equality.
encoder := codec.CollValueV2[wrapperspb.UInt64Value]()
value := &wrapperspb.UInt64Value{Value: 500}
encodedValue, err := encoder.Encode(value)
require.NoError(t, err)
decodedValue, err := encoder.Decode(encodedValue)
require.NoError(t, err)
require.True(t, cmp.Equal(value, decodedValue, protocmp.Transform()), "encoding and decoding produces different values")

encodedJSONValue, err := encoder.EncodeJSON(value)
require.NoError(t, err)
decodedJSONValue, err := encoder.DecodeJSON(encodedJSONValue)
require.NoError(t, err)
require.True(t, cmp.Equal(value, decodedJSONValue, protocmp.Transform()), "encoding and decoding produces different values")
require.NotEmpty(t, encoder.ValueType())

_ = encoder.Stringify(value)
})

t.Run("BoolValue", func(t *testing.T) {
colltest.TestValueCodec(t, codec.BoolValue, true)
colltest.TestValueCodec(t, codec.BoolValue, false)

// asserts produced bytes are equal
valueAssert := func(b bool) {
wantBytes, err := (&gogotypes.BoolValue{Value: b}).Marshal()
require.NoError(t, err)
gotBytes, err := codec.BoolValue.Encode(b)
require.NoError(t, err)
require.Equal(t, wantBytes, gotBytes)
}

valueAssert(true)
valueAssert(false)
})
}
54 changes: 54 additions & 0 deletions collections/protocodec/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
module cosmossdk.io/collections/protocodec
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

go 1.23.2

require (
cosmossdk.io/collections v0.4.0
cosmossdk.io/core v0.11.1
github.com/cosmos/gogoproto v1.7.0
github.com/google/go-cmp v0.6.0
github.com/stretchr/testify v1.9.0
google.golang.org/protobuf v1.35.1
)

require (
cosmossdk.io/schema v0.3.0 // indirect
github.com/DataDog/zstd v1.5.5 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/cockroachdb/errors v1.11.3 // indirect
github.com/cockroachdb/fifo v0.0.0-20240606204812-0bbfbd93a7ce // indirect
github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b // indirect
github.com/cockroachdb/pebble v1.1.1 // indirect
github.com/cockroachdb/redact v1.1.5 // indirect
github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 // indirect
github.com/cosmos/cosmos-db v1.0.2 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/getsentry/sentry-go v0.27.0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/btree v1.1.2 // indirect
github.com/klauspost/compress v1.17.9 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/linxGnu/grocksdb v1.8.14 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/prometheus/client_golang v1.20.1 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.55.0 // indirect
github.com/prometheus/procfs v0.15.1 // indirect
github.com/rogpeppe/go-internal v1.12.0 // indirect
github.com/spf13/cast v1.6.0 // indirect
github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect
golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0 // indirect
golang.org/x/net v0.28.0 // indirect
golang.org/x/sys v0.24.0 // indirect
golang.org/x/text v0.17.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace cosmossdk.io/core => ../../core
Copy link
Member

Choose a reason for hiding this comment

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

TODO in follow up delete this.

Loading
Loading