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

Cannot use AccountI inside Protobuf messages #8876

Closed
4 tasks
RiccardoM opened this issue Mar 15, 2021 · 5 comments
Closed
4 tasks

Cannot use AccountI inside Protobuf messages #8876

RiccardoM opened this issue Mar 15, 2021 · 5 comments

Comments

@RiccardoM
Copy link
Contributor

RiccardoM commented Mar 15, 2021

Summary of Bug

Currently, it is not possible to use the AccountI interface inside Protobuf messages. This makes it impossible to implement that interface and create a new custom account type that wraps all already existing account types. It is only possible to extend BaseAccount, DelayedVestingAccount and other types individually.

Version

v0.42.1

Steps to Reproduce

Currently, inside our application we have the following custom account type:

// Profile represents a generic first on Desmos, containing the information of a
// single user
message Profile {
  option (gogoproto.goproto_getters) = false;
  option (gogoproto.goproto_stringer) = false;

  cosmos.auth.v1beta1.BaseAccount base_account = 1 [(gogoproto.embed) = true];
  string dtag = 2 [ (gogoproto.moretags) = "yaml:\"dtag\"" ];
  string moniker = 3 [ (gogoproto.moretags) = "yaml:\"moniker\"" ];
  string bio = 4 [ (gogoproto.moretags) = "yaml:\"bio\"" ];
  Pictures pictures = 5 [
    (gogoproto.nullable) = false,
    (gogoproto.moretags) = "yaml:\"pictures\""
  ];
  google.protobuf.Timestamp creation_date = 6 [
    (gogoproto.stdtime) = true,
    (gogoproto.moretags) = "yaml:\"creation_date\"",
    (gogoproto.jsontag) = "creation_date",
    (gogoproto.nullable) = false
  ];
}

Objects of such type are then created as follows:

func NewProfile(
	dtag string, moniker, bio string, pictures Pictures, creationDate time.Time,
	baseAccount *authtypes.BaseAccount,
) Profile {
	return Profile{
		Dtag:         dtag,
		Moniker:      moniker,
		Bio:          bio,
		Pictures:     pictures,
		CreationDate: creationDate,
		BaseAccount:  baseAccount,
	}
}

Inside our message handler, we then use the following code to create a new Profile instance:

profile = types.NewProfile(
	msg.Dtag,
	"",
	"",
	types.NewPictures("", ""),
	ctx.BlockTime(),
	k.ak.GetAccount(ctx, addr).(*authtypes.BaseAccount),
)

The problem with this is that the line

k.ak.GetAccount(ctx, addr).(*authtypes.BaseAccount)

Can throw the following error:

panic: interface conversion: types.AccountI is *types.DelayedVestingAccount, not *types.BaseAccount

After I saw that error, I tried searching a way to use types.AccountI inside the Protobuf message directly. Unfortunately, I found now way of doing this. IMO, I think there should be a way for zone developers to use that interface inside their Protobuf message, otherwise it might become extremely difficult for them to create custom account implementations that should work based on any account type (and not only BaseAccount).


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@RiccardoM RiccardoM changed the title Cannot extend all account types at once Cannot use AccountI inside Protobuf messages Mar 15, 2021
@amaury1093
Copy link
Contributor

amaury1093 commented Mar 15, 2021

The way we decided to encode interfaces in Protobuf is using Any.

message Profile {
  google.Any account = 1 [
     (cosmos_proto.accepts_interface) = "AccountI" // At some point we want to make this work, but today it's just informational.
  ];
  string dtag = 2;
  // -- snip --
}

Then you would do:

var myAccount AccountI // Populate it.
// Make sure myAccount is a proto.Message, e.g. a BaseAccount etc.
protoAccount, ok := myAccount.(proto.Message)
myAccountAny := codec.NewAnyWithValue(protoAccount)
myProfile := types.Profile{
  Account: myAccountAny,
  Dtag: "",
  // -- snip --
}

Related #8053 (any usage part)

@RiccardoM
Copy link
Contributor Author

@AmauryM Thanks for the example. I've thought of this as well, but this makes it extremely difficult to properly implement the AccountI interface correctly inside the new Profile type. Since the interface is encoded as Any, I then need to use the ModuleCdc to implement all its method (eg. GetPubKey, GetAddress, etc) which I don't think it's the best way to do it. And, without that implementation, I can't pass a *Profile to the authkeeper.Keeper#SetAccount method.

@amaury1093
Copy link
Contributor

amaury1093 commented Mar 16, 2021

Since the interface is encoded as Any, I then need to use the ModuleCdc to implement all its method (eg. GetPubKey, GetAddress, etc) which I don't think it's the best way to do it.

I believe you can do:

func (p Profile) GetPubKey() {
  return p.Account.GetCachedValue().(AccountI).GetPubKey()
}

GetCachedValue retrieves the actual Go type from inside an Any.

@aaronc
Copy link
Member

aaronc commented Mar 16, 2021

Closing this as it is not a bug although maybe the documentation could be clearer. Note also that in order to take the approach @AmauryM described you will need to implement UnpackInterfacesMessage correctly. @AmauryM is there some documentation that we could point @RiccardoM to and if not let's open an issue to make sure this is all clearly documented.

@aaronc aaronc closed this as completed Mar 16, 2021
@amaury1093
Copy link
Contributor

is there some documentation that we could point @RiccardoM

I'm writing those in #8895, as tracked in #8053.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants