Skip to content

Commit

Permalink
feat(accounts): use gogoproto API instead of protov2. (#18653)
Browse files Browse the repository at this point in the history
Co-authored-by: unknown unknown <unknown@unknown>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 8, 2023
1 parent 833547f commit 112f6cb
Show file tree
Hide file tree
Showing 43 changed files with 4,450 additions and 1,114 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (simulation) [#17911](https://github.com/cosmos/cosmos-sdk/pull/17911) Fix all problems with executing command `make test-sim-custom-genesis-fast` for simulation test.
* (simulation) [#18196](https://github.com/cosmos/cosmos-sdk/pull/18196) Fix the problem of `validator set is empty after InitGenesis` in simulation test.
* (baseapp) [#18551](https://github.com/cosmos/cosmos-sdk/pull/18551) Fix SelectTxForProposal the calculation method of tx bytes size is inconsistent with CometBFT
* (baseapp) [#18653](https://github.com/cosmos/cosmos-sdk/pull/18654) Fixes an issue in which gogoproto.Merge does not work with gogoproto messages with custom types.
* (baseapp) [#18654](https://github.com/cosmos/cosmos-sdk/pull/18654) Fixes an issue in which gogoproto.Merge does not work with gogoproto messages with custom types.

### API Breaking Changes

Expand Down
278 changes: 156 additions & 122 deletions api/cosmos/accounts/v1/query.pulsar.go

Large diffs are not rendered by default.

412 changes: 238 additions & 174 deletions api/cosmos/accounts/v1/tx.pulsar.go

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions codec/unknownproto/unknown_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,7 @@ func RejectUnknownFields(bz []byte, msg proto.Message, allowUnknownNonCriticals
return hasUnknownNonCriticals, nil
}

desc, ok := msg.(descriptorIface)
if !ok {
return hasUnknownNonCriticals, fmt.Errorf("%T does not have a Descriptor() method", msg)
}

fieldDescProtoFromTagNum, _, err := getDescriptorInfo(desc, msg)
fieldDescProtoFromTagNum, _, err := getDescriptorInfo(msg)
if err != nil {
return hasUnknownNonCriticals, err
}
Expand Down Expand Up @@ -345,7 +340,11 @@ func unnestDesc(mdescs []*descriptorpb.DescriptorProto, indices []int) *descript

// Invoking descriptorpb.ForMessage(proto.Message.(Descriptor).Descriptor()) is incredibly slow
// for every single message, thus the need for a hand-rolled custom version that's performant and cacheable.
func extractFileDescMessageDesc(desc descriptorIface) (*descriptorpb.FileDescriptorProto, *descriptorpb.DescriptorProto, error) {
func extractFileDescMessageDesc(msg proto.Message) (*descriptorpb.FileDescriptorProto, *descriptorpb.DescriptorProto, error) {
desc, ok := msg.(descriptorIface)
if !ok {
return nil, nil, fmt.Errorf("%T does not have a Descriptor() method", msg)
}
gzippedPb, indices := desc.Descriptor()

protoFileToDescMu.RLock()
Expand Down Expand Up @@ -391,7 +390,8 @@ var (
)

// getDescriptorInfo retrieves the mapping of field numbers to their respective field descriptors.
func getDescriptorInfo(desc descriptorIface, msg proto.Message) (map[int32]*descriptorpb.FieldDescriptorProto, *descriptorpb.DescriptorProto, error) {
func getDescriptorInfo(msg proto.Message) (map[int32]*descriptorpb.FieldDescriptorProto, *descriptorpb.DescriptorProto, error) {
// we immediately check if the desc is present in the desc
key := reflect.ValueOf(msg).Type()

descprotoCacheMu.RLock()
Expand All @@ -403,7 +403,7 @@ func getDescriptorInfo(desc descriptorIface, msg proto.Message) (map[int32]*desc
}

// Now compute and cache the index.
_, md, err := extractFileDescMessageDesc(desc)
_, md, err := extractFileDescMessageDesc(msg)
if err != nil {
return nil, nil, err
}
Expand Down
8 changes: 3 additions & 5 deletions crypto/keys/secp256k1/keys.pb.go

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

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package cosmos.accounts.interfaces.account_abstraction.v1;
import "google/protobuf/any.proto";
import "cosmos/accounts/v1/account_abstraction.proto";

option go_package = "cosmossdk.io/x/accounts/interfaces/account_abstraction/v1";

// MsgAuthenticate is a message that an x/account account abstraction implementer
// must handle to authenticate a state transition.
message MsgAuthenticate {
Expand Down
2 changes: 2 additions & 0 deletions proto/cosmos/accounts/testing/counter/v1/counter.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ syntax = "proto3";

package cosmos.accounts.testing.counter.v1;

option go_package = "cosmossdk.io/x/accounts/testing/counter/v1";

// MsgInit defines a message which initializes the counter with a given amount.
message MsgInit {
// initial_value is the initial amount to set the counter to.
Expand Down
2 changes: 2 additions & 0 deletions proto/cosmos/accounts/testing/rotation/v1/partial.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ syntax = "proto3";

package cosmos.accounts.testing.rotation.v1;

option go_package = "cosmossdk.io/x/accounts/testing/rotation/v1";

// MsgInit is the init message used to create a new account
// abstraction implementation that we use for testing, this account
// also allows for rotating the public key.
Expand Down
6 changes: 4 additions & 2 deletions proto/cosmos/accounts/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ package cosmos.accounts.v1;

option go_package = "cosmossdk.io/x/accounts/v1";

import "google/protobuf/any.proto";

// Query defines the Query service for the x/accounts module.
service Query {
// AccountQuery runs an account query.
Expand All @@ -19,13 +21,13 @@ message AccountQueryRequest {
// target defines the account to be queried.
string target = 1;
// request defines the query message being sent to the account.
bytes request = 2;
google.protobuf.Any request = 2;
}

// AccountQueryResponse is the response type for the Query/AccountQuery RPC method.
message AccountQueryResponse {
// response defines the query response of the account.
bytes response = 1;
google.protobuf.Any response = 1;
}

// SchemaResponse is the response type for the Query/Schema RPC method.
Expand Down
15 changes: 7 additions & 8 deletions proto/cosmos/accounts/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package cosmos.accounts.v1;

option go_package = "cosmossdk.io/x/accounts/v1";

import "google/protobuf/any.proto";
import "cosmos/msg/v1/msg.proto";
import "cosmos/accounts/v1/account_abstraction.proto";

Expand All @@ -30,18 +31,16 @@ message MsgInit {
string sender = 1;
// account_type is the type of the account to be created.
string account_type = 2;
// message is the message to be sent to the account, it's up to the account
// implementation to decide what encoding format should be used to interpret
// this message.
bytes message = 3;
// message is the message to be sent to the account.
google.protobuf.Any message = 3;
}

// MsgInitResponse defines the Create response type for the Msg/Create RPC method.
message MsgInitResponse {
// account_address is the address of the newly created account.
string account_address = 1;
// response is the response returned by the account implementation.
bytes response = 2;
google.protobuf.Any response = 2;
}

// MsgExecute defines the Execute request type for the Msg/Execute RPC method.
Expand All @@ -51,14 +50,14 @@ message MsgExecute {
string sender = 1;
// target is the address of the account to be executed.
string target = 2;
// message is the message to be sent to the account, it's up to the account
bytes message = 3;
// message is the message to be sent to the account.
google.protobuf.Any message = 3;
}

// MsgExecuteResponse defines the Execute response type for the Msg/Execute RPC method.
message MsgExecuteResponse {
// response is the response returned by the account implementation.
bytes response = 1;
google.protobuf.Any response = 1;
}

// -------- Account Abstraction ---------
Expand Down
3 changes: 2 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,10 @@ func NewSimApp(
runtime.EventService{},
runtime.BranchService{},
app.AuthKeeper.AddressCodec(),
appCodec.InterfaceRegistry().SigningContext(),
appCodec,
app.MsgServiceRouter(),
app.GRPCQueryRouter(),
appCodec.InterfaceRegistry(),
accountstd.AddAccount("counter", counter.NewAccount),
accountstd.AddAccount("aa_minimal", account_abstraction.NewMinimalAbstractedAccount),
accountstd.AddAccount("aa_full", account_abstraction.NewFullAbstractedAccount),
Expand Down
64 changes: 27 additions & 37 deletions tests/e2e/accounts/account_abstraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,19 @@ import (
"context"
"testing"

rotationv1 "cosmossdk.io/api/cosmos/accounts/testing/rotation/v1"
accountsv1 "cosmossdk.io/api/cosmos/accounts/v1"
bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
v1beta1 "cosmossdk.io/api/cosmos/base/v1beta1"
nftv1beta1 "cosmossdk.io/api/cosmos/nft/v1beta1"
stakingv1beta1 "cosmossdk.io/api/cosmos/staking/v1beta1"
"cosmossdk.io/simapp"
"cosmossdk.io/x/accounts"
rotationv1 "cosmossdk.io/x/accounts/testing/rotation/v1"
accountsv1 "cosmossdk.io/x/accounts/v1"
"cosmossdk.io/x/bank/testutil"
banktypes "cosmossdk.io/x/bank/types"
"cosmossdk.io/x/nft"
"github.com/cosmos/cosmos-proto/anyutil"
stakingtypes "cosmossdk.io/x/staking/types"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
sdk "github.com/cosmos/cosmos-sdk/types"
gogoproto "github.com/cosmos/gogoproto/proto"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/anypb"
)

var (
Expand Down Expand Up @@ -70,13 +67,13 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "secp256k1",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &bankv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: bundlerAddrStr,
Amount: coins(t, "1stake"), // the sender is the AA, so it has the coins and wants to pay the bundler for the gas
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: aliceAddrStr,
Amount: coins(t, "2000stake"), // as the real action the sender wants to send coins to alice
Expand All @@ -103,13 +100,13 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "secp256k1",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &bankv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: bundlerAddrStr, // abstracted account tries to send money from bundler to itself.
ToAddress: aaAddrStr,
Amount: coins(t, "1stake"),
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: aliceAddrStr,
Amount: coins(t, "2000stake"), // as the real action the sender wants to send coins to alice
Expand All @@ -130,13 +127,13 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "secp256k1",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &bankv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: bundlerAddrStr,
Amount: coins(t, "1stake"),
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aliceAddrStr, // abstracted account attempts to send money from alice to itself
ToAddress: aaAddrStr,
Amount: coins(t, "2000stake"),
Expand All @@ -159,13 +156,13 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "invalid",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &bankv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: bundlerAddrStr,
Amount: coins(t, "1stake"),
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aliceAddrStr, // abstracted account attempts to send money from alice to itself
ToAddress: aaAddrStr,
Amount: coins(t, "2000stake"),
Expand All @@ -189,13 +186,13 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "secp256k1",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &bankv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: bundlerAddrStr,
Amount: coins(t, "1atom"), // abstracted account does not have enough money to pay the bundler, since it does not hold atom
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aliceAddrStr, // abstracted account attempts to send money from alice to itself
ToAddress: aaAddrStr,
Amount: coins(t, "2000stake"),
Expand All @@ -218,13 +215,13 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "secp256k1",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &bankv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: bundlerAddrStr,
Amount: coins(t, "1stake"),
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaAddrStr,
ToAddress: aliceAddrStr,
Amount: coins(t, "2000atom"), // abstracted account does not have enough money to pay alice, since it does not hold atom
Expand All @@ -247,13 +244,13 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "secp256k1",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &bankv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaFullAddrStr,
ToAddress: bundlerAddrStr,
Amount: coins(t, "1stake"), // we expect this to fail since the account is implement in such a way not to allow bank sends.
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaFullAddrStr,
ToAddress: aliceAddrStr,
Amount: coins(t, "2000stake"),
Expand All @@ -273,7 +270,7 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: nil,
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &stakingv1beta1.MsgDelegate{
ExecutionMessages: intoAny(t, &stakingtypes.MsgDelegate{
DelegatorAddress: aaFullAddrStr,
ValidatorAddress: "some-validator",
Amount: coins(t, "2000stake")[0],
Expand All @@ -300,14 +297,14 @@ func TestAccountAbstraction(t *testing.T) {
AuthenticationMethod: "secp256k1",
AuthenticationData: []byte("signature"),
AuthenticationGasLimit: 10000,
BundlerPaymentMessages: intoAny(t, &nftv1beta1.MsgSend{
BundlerPaymentMessages: intoAny(t, &nft.MsgSend{
ClassId: "omega-rare",
Id: "the-most-rare",
Sender: aaFullAddrStr,
Receiver: bundlerAddrStr,
}),
BundlerPaymentGasLimit: 50000,
ExecutionMessages: intoAny(t, &bankv1beta1.MsgSend{
ExecutionMessages: intoAny(t, &banktypes.MsgSend{
FromAddress: aaFullAddrStr,
ToAddress: aliceAddrStr,
Amount: coins(t, "2000stake"),
Expand All @@ -318,28 +315,21 @@ func TestAccountAbstraction(t *testing.T) {
})
}

func intoAny(t *testing.T, msgs ...proto.Message) (anys []*anypb.Any) {
func intoAny(t *testing.T, msgs ...gogoproto.Message) (anys []*codectypes.Any) {
t.Helper()
for _, msg := range msgs {
any, err := anyutil.New(msg)
any, err := codectypes.NewAnyWithValue(msg)
require.NoError(t, err)
anys = append(anys, any)
}
return
}

func coins(t *testing.T, s string) []*v1beta1.Coin {
func coins(t *testing.T, s string) sdk.Coins {
t.Helper()
coins, err := sdk.ParseCoinsNormalized(s)
require.NoError(t, err)
coinsv2 := make([]*v1beta1.Coin, len(coins))
for i, coin := range coins {
coinsv2[i] = &v1beta1.Coin{
Denom: coin.Denom,
Amount: coin.Amount.String(),
}
}
return coinsv2
return coins
}

func balanceIs(t *testing.T, ctx context.Context, app *simapp.SimApp, addr sdk.AccAddress, s string) {
Expand Down
13 changes: 0 additions & 13 deletions tests/e2e/accounts/setup_test.go

This file was deleted.

Loading

0 comments on commit 112f6cb

Please sign in to comment.