Skip to content

Commit

Permalink
refactor(x/group)!: remove MustAccAddressFromBech32 (cosmos#20082)
Browse files Browse the repository at this point in the history
  • Loading branch information
JulianToledano authored Apr 18, 2024
1 parent c6b8d7d commit 90a0114
Show file tree
Hide file tree
Showing 22 changed files with 147 additions and 99 deletions.
9 changes: 6 additions & 3 deletions testutil/testdata/table.go
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
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.
* [#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
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
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
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
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
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
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
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
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"

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

// 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 @@ type PrimaryKeyed interface {
//
// 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)
}
key, err := buildKeyFromParts(fields)
if err != nil {
panic(err)
Expand All @@ -65,7 +69,7 @@ func PrimaryKey(obj PrimaryKeyed) []byte {
// 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 @@ func (a PrimaryKeyTable) Create(store storetypes.KVStore, obj PrimaryKeyed) erro
// 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 @@ func (a PrimaryKeyTable) Update(store storetypes.KVStore, newValue PrimaryKeyed)
// 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 @@ func (a PrimaryKeyTable) Set(store storetypes.KVStore, newValue PrimaryKeyed) er
// 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 @@ func (a PrimaryKeyTable) Contains(store storetypes.KVStore, obj PrimaryKeyed) bo
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

0 comments on commit 90a0114

Please sign in to comment.