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.

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

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

Merged
merged 9 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
9 changes: 9 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,15 @@ updates:
labels:
- "A:automerge"
- dependencies
- package-ecosystem: gomod
directory: "/collections/protocodec"
schedule:
interval: weekly
day: friday
time: "02:20"
labels:
- "A:automerge"
- dependencies
- package-ecosystem: gomod
directory: "x/accounts"
schedule:
Expand Down
2 changes: 2 additions & 0 deletions .github/pr_labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
- store/**/*
"C:collections":
- collections/**/*
"C:collections/protocodec":
- collections/protocodec/*
"C:core/testing":
- core/testing/**/*
"C:log":
Expand Down
31 changes: 31 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,37 @@ jobs:
with:
projectBaseDir: collections/

test-collections-protocodec:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: "1.23"
check-latest: true
cache: true
cache-dependency-path: collections/protocodec/go.sum
- uses: technote-space/[email protected]
id: git_diff
with:
PATTERNS: |
collections/protocodec/**/*.go
collections/protocodec/go.mod
collections/protocodec/go.sum
- name: tests
if: env.GIT_DIFF
run: |
cd collections/protocodec
go test -mod=readonly -timeout 30m -coverprofile=coverage.out -covermode=atomic -tags='norace ledger test_ledger_mock' ./...
- name: sonarcloud
if: ${{ env.GIT_DIFF && !github.event.pull_request.draft && env.SONAR_TOKEN != null }}
uses: SonarSource/sonarcloud-github-action@master
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
with:
projectBaseDir: collections/protocodec

test-orm:
runs-on: ubuntu-latest
steps:
Expand Down
137 changes: 137 additions & 0 deletions collections/protocodec/collections.go
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
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)
})
}
55 changes: 55 additions & 0 deletions collections/protocodec/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
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
)

// TODO: remove this once core is tagged.
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