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

fix(group): migrate group policy account to base accounts with credentials #13742

Merged
merged 8 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 4 additions & 3 deletions api/cosmos/auth/v1beta1/auth.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions proto/cosmos/auth/v1beta1/auth.proto
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ message ModuleAccount {
repeated string permissions = 3;
}

// ModuleCredential represents a unclaimable pubkey for base accounts
// ModuleCredential represents a unclaimable pubkey for base accounts controlled by modules.
//
// Since: cosmos-sdk 0.47
message ModuleCredential {
// module_name is the name of the module used for address derivation (passed into address.Module).
string module_name = 1;
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
// derivation_keys is for deriving a module account (passed into address.Module)
// adding more keys creates sub-accounts (passed into address.Derive)
// derivation_keys is for deriving a module account address (passed into address.Module)
// adding more keys creates sub-account addresses (passed into address.Derive)
repeated bytes derivation_keys = 2;
}

Expand Down
7 changes: 4 additions & 3 deletions x/auth/types/auth.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions x/auth/types/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/cosmos/cosmos-sdk/types/address"
)

// NewAccountWithModuleCredential creates an account with an unclaimable module credential.
func NewAccountWithModuleCredential(pubkey cryptotypes.PubKey) (*BaseAccount, error) {
// NewBaseAccountWithPubKey creates an account with an a pubkey.
func NewBaseAccountWithPubKey(pubkey cryptotypes.PubKey) (*BaseAccount, error) {
if pubkey == nil {
return nil, fmt.Errorf("pubkey cannot be nil")
}
Expand Down Expand Up @@ -39,17 +39,17 @@ func NewModuleCredential(moduleName string, derivationKeys [][]byte) *ModuleCred
}

func (m *ModuleCredential) Address() cryptotypes.Address {
var module []byte
var addr []byte
for i, dk := range m.DerivationKeys {
if i == 0 {
module = address.Module(m.ModuleName, dk)
addr = address.Module(m.ModuleName, dk)
continue
}

module = address.Derive(module, dk)
addr = address.Derive(addr, dk)
}

return module
return addr
}

func (m *ModuleCredential) Bytes() []byte {
Expand All @@ -62,7 +62,7 @@ func (m *ModuleCredential) VerifySignature(_ []byte, _ []byte) bool {
}

func (m *ModuleCredential) Equals(other cryptotypes.PubKey) bool {
return false
return m.String() == other.String()
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
}

func (m *ModuleCredential) Type() string {
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this PR, but is this method on PubKey even used anymore? Maybe we should get rid of it

Copy link
Member Author

Choose a reason for hiding this comment

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

After a brief look, it is used for comparison (easy to remove) and in keyring

return crypto.ArmorPubKeyBytes(bz, key.Type()), nil
.

Expand Down
11 changes: 7 additions & 4 deletions x/auth/types/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,22 @@ func TestNewModuleCrendentials(t *testing.T) {
addr, err := sdk.AccAddressFromHexUnsafe(credential.Address().String())
require.NoError(t, err)
require.Equal(t, expected.String(), addr.String())

require.Equal(t, credential, authtypes.NewModuleCredential("group", [][]byte{{0x20}, {0x0}}))
require.NotEqual(t, credential, authtypes.NewModuleCredential("group", [][]byte{{0x20}, {0x1}}))
}

func TestNewAccountWithModuleCredential(t *testing.T) {
func TestNewBaseAccountWithPubKey(t *testing.T) {
expected := sdk.MustAccAddressFromBech32("cosmos1fpn0w0yf4x300llf5r66jnfhgj4ul6cfahrvqsskwkhsw6sv84wsmz359y")

credential := authtypes.NewModuleCredential("group", [][]byte{{0x20}, {0x0}})
account, err := authtypes.NewAccountWithModuleCredential(credential)
account, err := authtypes.NewBaseAccountWithPubKey(credential)
require.NoError(t, err)
require.Equal(t, expected, account.GetAddress())
require.Equal(t, credential, account.GetPubKey())
}

func TestNewAccountWithModuleCredential_WrongCredentials(t *testing.T) {
_, err := authtypes.NewAccountWithModuleCredential(cryptotypes.PubKey(nil))
func TestNewBaseAccountWithPubKey_WrongCredentials(t *testing.T) {
_, err := authtypes.NewBaseAccountWithPubKey(cryptotypes.PubKey(nil))
require.Error(t, err)
}
4 changes: 2 additions & 2 deletions x/group/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ func (s TestSuite) setNextAccount() {

accountCredentials := authtypes.NewModuleCredential(group.ModuleName, [][]byte{{keeper.GroupPolicyTablePrefix}, derivationKey})

groupPolicyAcc, err := authtypes.NewAccountWithModuleCredential(accountCredentials)
groupPolicyAcc, err := authtypes.NewBaseAccountWithPubKey(accountCredentials)
s.Require().NoError(err)

groupPolicyAccBumpAccountNumber, err := authtypes.NewAccountWithModuleCredential(accountCredentials)
groupPolicyAccBumpAccountNumber, err := authtypes.NewBaseAccountWithPubKey(accountCredentials)
s.Require().NoError(err)
groupPolicyAccBumpAccountNumber.SetAccountNumber(nextAccVal)

Expand Down
2 changes: 1 addition & 1 deletion x/group/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func (k Keeper) CreateGroupPolicy(goCtx context.Context, req *group.MsgCreateGro
}

// group policy accounts are unclaimable base accounts
account, err := authtypes.NewAccountWithModuleCredential(accountCredentials)
account, err := authtypes.NewBaseAccountWithPubKey(accountCredentials)
if err != nil {
return nil, sdkerrors.Wrap(err, "could not create group policy account")
}
Expand Down
2 changes: 1 addition & 1 deletion x/group/migrations/v2/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func Migrate(
}

accountCredentials := authtypes.NewModuleCredential(group.ModuleName, [][]byte{{GroupPolicyTablePrefix}, derivationKey})
baseAccount, err := authtypes.NewAccountWithModuleCredential(accountCredentials)
baseAccount, err := authtypes.NewBaseAccountWithPubKey(accountCredentials)
if err != nil {
return fmt.Errorf("failed to create new group policy account: %w", err)
}
Expand Down