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

refactor(x/group)!: remove MustAccAddressFromBech32 #20082

Merged
merged 4 commits into from
Apr 18, 2024
Merged

refactor(x/group)!: remove MustAccAddressFromBech32 #20082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0994098
update: remove MustAccAddressFromBech32
JulianToledano Apr 18, 2024
7f495c7
lint
JulianToledano Apr 18, 2024
9f45bfd
CHANGELOG
JulianToledano Apr 18, 2024
64085a5
Merge branch 'main' into julian/group-remove-MustAccAddressFromBech32
JulianToledano Apr 18, 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
9 changes: 6 additions & 3 deletions testutil/testdata/table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package testdata

import "cosmossdk.io/errors"
import (
"cosmossdk.io/core/address"
"cosmossdk.io/errors"
)

var ErrTest = errors.Register("table_testdata", 2, "test")

func (g TableModel) PrimaryKeyFields() []interface{} {
return []interface{}{g.Id}
func (g TableModel) PrimaryKeyFields(_ address.Codec) ([]interface{}, error) {
return []interface{}{g.Id}, nil
}

func (g TableModel) ValidateBasic() error {
Expand Down
3 changes: 3 additions & 0 deletions x/group/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* [#20082](https://github.com/cosmos/cosmos-sdk/pull/20082) Removes the use of `MustAccAddressFromBech32`:
* `PrimaryKeyFields` function from interface `PrimaryKeyed` now takes an address codec as argument.
* `PrimaryKey`, `NewAutoUInt64Table` and `NewPrimaryKeyTable` now take an address codec as argument.
Comment on lines +35 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure consistent indentation for list items in the changelog to improve readability.

-    * `PrimaryKeyFields` function from interface `PrimaryKeyed` now takes an address codec as argument.
-    * `PrimaryKey`, `NewAutoUInt64Table` and `NewPrimaryKeyTable` now take an address codec as argument.
+  * `PrimaryKeyFields` function from interface `PrimaryKeyed` now takes an address codec as argument.
+  * `PrimaryKey`, `NewAutoUInt64Table` and `NewPrimaryKeyTable` now take an address codec as argument.
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.

Suggested change
* [#20082](https://github.com/cosmos/cosmos-sdk/pull/20082) Removes the use of `MustAccAddressFromBech32`:
* `PrimaryKeyFields` function from interface `PrimaryKeyed` now takes an address codec as argument.
* `PrimaryKey`, `NewAutoUInt64Table` and `NewPrimaryKeyTable` now take an address codec as argument.
* [#20082](https://github.com/cosmos/cosmos-sdk/pull/20082) Removes the use of `MustAccAddressFromBech32`:
* `PrimaryKeyFields` function from interface `PrimaryKeyed` now takes an address codec as argument.
* `PrimaryKey`, `NewAutoUInt64Table` and `NewPrimaryKeyTable` now take an address codec as argument.

* [#19916](https://github.com/cosmos/cosmos-sdk/pull/19916) Removes the use of Address String methods:
* `NewMsgCreateGroupPolicy` now takes a string as argument instead of an `AccAddress`.
* `NewMsgUpdateGroupPolicyDecisionPolicy` now takes strings as argument instead of `AccAddress`.
Expand Down
5 changes: 3 additions & 2 deletions x/group/internal/orm/auto_uint64.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package orm
import (
"github.com/cosmos/gogoproto/proto"

"cosmossdk.io/core/address"
storetypes "cosmossdk.io/core/store"
"cosmossdk.io/errors"

Expand All @@ -21,8 +22,8 @@ type AutoUInt64Table struct {
}

// NewAutoUInt64Table creates a new AutoUInt64Table.
func NewAutoUInt64Table(prefixData [2]byte, prefixSeq byte, model proto.Message, cdc codec.Codec) (*AutoUInt64Table, error) {
table, err := newTable(prefixData, model, cdc)
func NewAutoUInt64Table(prefixData [2]byte, prefixSeq byte, model proto.Message, cdc codec.Codec, addressCodec address.Codec) (*AutoUInt64Table, error) {
table, err := newTable(prefixData, model, cdc, addressCodec)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion x/group/internal/orm/auto_uint64_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"cosmossdk.io/x/group/errors"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
Expand All @@ -23,7 +24,7 @@ func TestAutoUInt64PrefixScan(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)

tb, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc)
tb, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc, address.NewBech32Codec("cosmos"))
require.NoError(t, err)

key := storetypes.NewKVStoreKey("test")
Expand Down
6 changes: 4 additions & 2 deletions x/group/internal/orm/example_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package orm

import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
)

Expand All @@ -27,8 +28,9 @@ var (
func NewTestKeeper(cdc codec.Codec) TestKeeper {
k := TestKeeper{}
var err error
ac := address.NewBech32Codec("cosmos")

k.autoUInt64Table, err = NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc)
k.autoUInt64Table, err = NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc, ac)
if err != nil {
panic(err.Error())
}
Expand All @@ -39,7 +41,7 @@ func NewTestKeeper(cdc codec.Codec) TestKeeper {
panic(err.Error())
}

k.primaryKeyTable, err = NewPrimaryKeyTable(PrimaryKeyTablePrefix, &testdata.TableModel{}, cdc)
k.primaryKeyTable, err = NewPrimaryKeyTable(PrimaryKeyTablePrefix, &testdata.TableModel{}, cdc, ac)
if err != nil {
panic(err.Error())
}
Expand Down
3 changes: 2 additions & 1 deletion x/group/internal/orm/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
Expand All @@ -18,7 +19,7 @@ func TestImportExportTableData(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)

table, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc)
table, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc, address.NewBech32Codec("cosmos"))
require.NoError(t, err)

key := storetypes.NewKVStoreKey("test")
Expand Down
17 changes: 9 additions & 8 deletions x/group/internal/orm/index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"cosmossdk.io/x/group/errors"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
Expand All @@ -33,7 +34,7 @@ func TestNewIndex(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)

myTable, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc)
myTable, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc, address.NewBech32Codec("cosmos"))
require.NoError(t, err)
indexer := func(val interface{}) ([]interface{}, error) {
return []interface{}{val.(*testdata.TableModel).Metadata}, nil
Expand Down Expand Up @@ -91,7 +92,7 @@ func TestIndexPrefixScan(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)

tb, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc)
tb, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc, address.NewBech32Codec("cosmos"))
require.NoError(t, err)
idx, err := NewIndex(tb, AutoUInt64TableModelByMetadataPrefix, func(val interface{}) ([]interface{}, error) {
i := []interface{}{val.(*testdata.TableModel).Metadata}
Expand Down Expand Up @@ -297,8 +298,8 @@ func TestIndexPrefixScan(t *testing.T) {
func TestUniqueIndex(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)

myTable, err := NewPrimaryKeyTable(PrimaryKeyTablePrefix, &testdata.TableModel{}, cdc)
ac := address.NewBech32Codec("cosmos")
myTable, err := NewPrimaryKeyTable(PrimaryKeyTablePrefix, &testdata.TableModel{}, cdc, ac)
require.NoError(t, err)
uniqueIdx, err := NewUniqueIndex(myTable, 0x10, func(val interface{}) (interface{}, error) {
return []byte{val.(*testdata.TableModel).Metadata[0]}, nil
Expand Down Expand Up @@ -330,7 +331,7 @@ func TestUniqueIndex(t *testing.T) {
var loaded testdata.TableModel
rowID, err := it.LoadNext(&loaded)
require.NoError(t, err)
require.Equal(t, RowID(PrimaryKey(&m)), rowID)
require.Equal(t, RowID(PrimaryKey(&m, ac)), rowID)
require.Equal(t, m, loaded)

// GetPaginated
Expand All @@ -357,7 +358,7 @@ func TestUniqueIndex(t *testing.T) {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, RowID(PrimaryKey(&m)), rowID)
require.Equal(t, RowID(PrimaryKey(&m, ac)), rowID)
require.Equal(t, m, loaded)
}
})
Expand All @@ -368,7 +369,7 @@ func TestUniqueIndex(t *testing.T) {
require.NoError(t, err)
rowID, err = it.LoadNext(&loaded)
require.NoError(t, err)
require.Equal(t, RowID(PrimaryKey(&m)), rowID)
require.Equal(t, RowID(PrimaryKey(&m, ac)), rowID)
require.Equal(t, m, loaded)

// PrefixScan no match
Expand All @@ -382,7 +383,7 @@ func TestUniqueIndex(t *testing.T) {
require.NoError(t, err)
rowID, err = it.LoadNext(&loaded)
require.NoError(t, err)
require.Equal(t, RowID(PrimaryKey(&m)), rowID)
require.Equal(t, RowID(PrimaryKey(&m, ac)), rowID)
require.Equal(t, m, loaded)

// ReversePrefixScan no match
Expand Down
3 changes: 2 additions & 1 deletion x/group/internal/orm/iterator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"cosmossdk.io/x/group/errors"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
Expand Down Expand Up @@ -204,7 +205,7 @@ func TestPaginate(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)

tb, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc)
tb, err := NewAutoUInt64Table(AutoUInt64TablePrefix, AutoUInt64TableSeqPrefix, &testdata.TableModel{}, cdc, address.NewBech32Codec("cosmos"))
require.NoError(t, err)
idx, err := NewIndex(tb, AutoUInt64TableModelByMetadataPrefix, func(val interface{}) ([]interface{}, error) {
return []interface{}{val.(*testdata.TableModel).Metadata}, nil
Expand Down
12 changes: 7 additions & 5 deletions x/group/internal/orm/orm_scenario_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"cosmossdk.io/x/group/errors"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
Expand Down Expand Up @@ -122,7 +123,7 @@ func TestKeeperEndToEndWithPrimaryKeyTable(t *testing.T) {
err := k.primaryKeyTable.Create(store, &tm)
require.NoError(t, err)
// then we should find it by primary key
primaryKey := PrimaryKey(&tm)
primaryKey := PrimaryKey(&tm, address.NewBech32Codec("cosmos"))
exists := k.primaryKeyTable.Has(store, primaryKey)
require.True(t, exists)

Expand Down Expand Up @@ -192,6 +193,7 @@ func TestKeeperEndToEndWithPrimaryKeyTable(t *testing.T) {
func TestGasCostsPrimaryKeyTable(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)
ac := address.NewBech32Codec("cosmos")

key := storetypes.NewKVStoreKey("test")
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
Expand All @@ -216,7 +218,7 @@ func TestGasCostsPrimaryKeyTable(t *testing.T) {
// get by primary key
testCtx.Ctx = testCtx.Ctx.WithGasMeter(storetypes.NewInfiniteGasMeter())
var loaded testdata.TableModel
err = k.primaryKeyTable.GetOne(store, PrimaryKey(&tm), &loaded)
err = k.primaryKeyTable.GetOne(store, PrimaryKey(&tm, ac), &loaded)
require.NoError(t, err)
t.Logf("gas consumed on get by primary key: %d", testCtx.Ctx.GasMeter().GasConsumed())

Expand Down Expand Up @@ -260,7 +262,7 @@ func TestGasCostsPrimaryKeyTable(t *testing.T) {
Number: 123,
Metadata: []byte("metadata"),
}
err = k.primaryKeyTable.GetOne(store, PrimaryKey(&tm), &loaded)
err = k.primaryKeyTable.GetOne(store, PrimaryKey(&tm, ac), &loaded)
require.NoError(t, err)
t.Logf("%d: gas consumed on get by primary key: %d", i, testCtx.Ctx.GasMeter().GasConsumed())
}
Expand Down Expand Up @@ -391,7 +393,7 @@ func TestExportImportStatePrimaryKeyTable(t *testing.T) {
keys, err := ReadAll(it, &loaded)
require.NoError(t, err)
for i := range keys {
assert.Equal(t, PrimaryKey(&testRecords[i]), keys[i].Bytes())
assert.Equal(t, PrimaryKey(&testRecords[i], address.NewBech32Codec("cosmos")), keys[i].Bytes())
}
assert.Equal(t, testRecords, loaded)

Expand All @@ -411,7 +413,7 @@ func assertIndex(t *testing.T, store corestore.KVStore, index Index, v testdata.
var loaded []testdata.TableModel
keys, err := ReadAll(it, &loaded)
require.NoError(t, err)
assert.Equal(t, []RowID{PrimaryKey(&v)}, keys)
assert.Equal(t, []RowID{PrimaryKey(&v, address.NewBech32Codec("cosmos"))}, keys)
assert.Equal(t, []testdata.TableModel{v}, loaded)
}

Expand Down
24 changes: 14 additions & 10 deletions x/group/internal/orm/primary_key.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import (
"github.com/cosmos/gogoproto/proto"

"cosmossdk.io/core/address"
storetypes "cosmossdk.io/core/store"

"github.com/cosmos/cosmos-sdk/codec"
Expand All @@ -20,8 +21,8 @@
}

// NewPrimaryKeyTable creates a new PrimaryKeyTable.
func NewPrimaryKeyTable(prefixData [2]byte, model PrimaryKeyed, cdc codec.Codec) (*PrimaryKeyTable, error) {
table, err := newTable(prefixData, model, cdc)
func NewPrimaryKeyTable(prefixData [2]byte, model PrimaryKeyed, cdc codec.Codec, addressCodec address.Codec) (*PrimaryKeyTable, error) {
table, err := newTable(prefixData, model, cdc, addressCodec)
if err != nil {
return nil, err
}
Expand All @@ -42,16 +43,19 @@
//
// IMPORTANT: []byte parts are encoded with a single byte length prefix,
// so cannot be longer than 255 bytes.
PrimaryKeyFields() []interface{}
PrimaryKeyFields(address.Codec) ([]interface{}, error)
proto.Message
}

// PrimaryKey returns the immutable and serialized primary key of this object.
// The primary key has to be unique within it's domain so that not two with same
// value can exist in the same table. This means PrimaryKeyFields() has to
// return a unique value for each object.
func PrimaryKey(obj PrimaryKeyed) []byte {
fields := obj.PrimaryKeyFields()
func PrimaryKey(obj PrimaryKeyed, addressCodec address.Codec) []byte {
fields, err := obj.PrimaryKeyFields(addressCodec)
if err != nil {
panic(err)
Dismissed Show dismissed Hide dismissed
}
key, err := buildKeyFromParts(fields)
if err != nil {
panic(err)
Expand All @@ -65,7 +69,7 @@
// Create iterates through the registered callbacks that may add secondary
// index keys.
func (a PrimaryKeyTable) Create(store storetypes.KVStore, obj PrimaryKeyed) error {
rowID := PrimaryKey(obj)
rowID := PrimaryKey(obj, a.ac)
return a.table.Create(store, rowID, obj)
}

Expand All @@ -77,7 +81,7 @@
// Update iterates through the registered callbacks that may add or remove
// secondary index keys.
func (a PrimaryKeyTable) Update(store storetypes.KVStore, newValue PrimaryKeyed) error {
return a.table.Update(store, PrimaryKey(newValue), newValue)
return a.table.Update(store, PrimaryKey(newValue, a.ac), newValue)
}

// Set persists the given object under the rowID key. It does not check if the
Expand All @@ -86,7 +90,7 @@
// Set iterates through the registered callbacks that may add secondary index
// keys.
func (a PrimaryKeyTable) Set(store storetypes.KVStore, newValue PrimaryKeyed) error {
return a.table.Set(store, PrimaryKey(newValue), newValue)
return a.table.Set(store, PrimaryKey(newValue, a.ac), newValue)
}

// Delete removes the object. It expects the primary key to exists already and
Expand All @@ -96,7 +100,7 @@
// Delete iterates through the registered callbacks that remove secondary index
// keys.
func (a PrimaryKeyTable) Delete(store storetypes.KVStore, obj PrimaryKeyed) error {
return a.table.Delete(store, PrimaryKey(obj))
return a.table.Delete(store, PrimaryKey(obj, a.ac))
}

// Has checks if a key exists. Always returns false on nil or empty key.
Expand All @@ -109,7 +113,7 @@
if err := assertCorrectType(a.table.model, obj); err != nil {
return false
}
return a.table.Has(store, PrimaryKey(obj))
return a.table.Has(store, PrimaryKey(obj, a.ac))
}

// GetOne loads the object persisted for the given primary Key into the dest parameter.
Expand Down
Loading
Loading