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

Abstract away Any in the generated types #118

Closed
Tracked by #278 ...
assafmo opened this issue Jun 29, 2022 · 12 comments
Closed
Tracked by #278 ...

Abstract away Any in the generated types #118

assafmo opened this issue Jun 29, 2022 · 12 comments

Comments

@assafmo
Copy link

assafmo commented Jun 29, 2022

Complex Msgs in cosmos-sdk usually have an Any field in them. It might be a good Idea to abstract this away from the users by recursively following implements_interface & accepts_interface.

For example:

Then the Typescript output should probably look like this:

/**
 * MsgGrantAllowance adds permission for Grantee to spend up to Allowance
 * of fees from the account of Granter.
 */
interface MsgGrantAllowance {
  granter: string;
  grantee: string;
  allowance: BasicAllowance | PeriodicAllowance | AllowedMsgAllowance;
}

/**
 * BasicAllowance implements Allowance with a one-time grant of tokens
 * that optionally expires. The grantee can use up to SpendLimit to cover fees.
 */
interface BasicAllowance {
  /**
   * spend_limit specifies the maximum amount of tokens that can be spent
   * by this allowance and will be updated as tokens are spent. If it is
   * empty, there is no spend limit and any amount of coins can be spent.
   */
  spendLimit: Coin[];
  /** expiration specifies an optional time when this allowance expires */
  expiration?: Timestamp;
}

/**
 * PeriodicAllowance extends Allowance to allow for both a maximum cap,
 * as well as a limit per time period.
 */
interface PeriodicAllowance {
  /** basic specifies a struct of `BasicAllowance` */
  basic?: BasicAllowance;
  /**
   * period specifies the time duration in which period_spend_limit coins can
   * be spent before that allowance is reset
   */
  period?: Duration;
  /**
   * period_spend_limit specifies the maximum number of coins that can be spent
   * in the period
   */
  periodSpendLimit: Coin[];
  /** period_can_spend is the number of coins left to be spent before the period_reset time */
  periodCanSpend: Coin[];
  /**
   * period_reset is the time at which this period resets and a new one begins,
   * it is calculated from the start time of the first transaction after the
   * last period ended
   */
  periodReset?: Timestamp;
}

/** AllowedMsgAllowance creates allowance only for specified message types. */
export interface AllowedMsgAllowance {
  /** allowance can be any of basic and filtered fee allowance. */
  allowance?: BasicAllowance | PeriodicAllowance | AllowedMsgAllowance;
  /** allowed_messages are the messages for which the grantee has the access. */
  allowedMessages: string[];
}

(BTW I'm still not sure how to handle optional fields, as some of them are definitely not optional)

@assafmo
Copy link
Author

assafmo commented Jun 29, 2022

For reference, here's how it looks when generated with ts-proto:

/**
 * MsgGrantAllowance adds permission for Grantee to spend up to Allowance
 * of fees from the account of Granter.
 */
interface MsgGrantAllowance {
  granter: string;
  grantee: string;
-  allowance: BasicAllowance | PeriodicAllowance | AllowedMsgAllowance;
+  allowance?: Any;
}

/**
 * BasicAllowance implements Allowance with a one-time grant of tokens
 * that optionally expires. The grantee can use up to SpendLimit to cover fees.
 */
interface BasicAllowance {
  /**
   * spend_limit specifies the maximum amount of tokens that can be spent
   * by this allowance and will be updated as tokens are spent. If it is
   * empty, there is no spend limit and any amount of coins can be spent.
   */
  spendLimit: Coin[];
  /** expiration specifies an optional time when this allowance expires */
  expiration?: Timestamp;
}

/**
 * PeriodicAllowance extends Allowance to allow for both a maximum cap,
 * as well as a limit per time period.
 */
interface PeriodicAllowance {
  /** basic specifies a struct of `BasicAllowance` */
  basic?: BasicAllowance;
  /**
   * period specifies the time duration in which period_spend_limit coins can
   * be spent before that allowance is reset
   */
  period?: Duration;
  /**
   * period_spend_limit specifies the maximum number of coins that can be spent
   * in the period
   */
  periodSpendLimit: Coin[];
  /** period_can_spend is the number of coins left to be spent before the period_reset time */
  periodCanSpend: Coin[];
  /**
   * period_reset is the time at which this period resets and a new one begins,
   * it is calculated from the start time of the first transaction after the
   * last period ended
   */
  periodReset?: Timestamp;
}

/** AllowedMsgAllowance creates allowance only for specified message types. */
export interface AllowedMsgAllowance {
  /** allowance can be any of basic and filtered fee allowance. */
-  allowance: BasicAllowance | PeriodicAllowance | AllowedMsgAllowance;
+  allowance?: Any;
  /** allowed_messages are the messages for which the grantee has the access. */
  allowedMessages: string[];
}

@assafmo
Copy link
Author

assafmo commented Jun 29, 2022

From looking in the cosmos-sdk code there aren't a lot of these, but by abstracting this away from devs Telescope will make it easy for them to use complex Msgs and will empower blockchain devs to create complex Msgs in the future knowing that it'll not be hard for devs to use them.

image

pyramation added a commit that referenced this issue Jul 16, 2022
@pyramation
Copy link
Collaborator

cosmos/cosmos-proto#5

@pyramation
Copy link
Collaborator

pyramation commented Jul 18, 2022

cosmos_proto.accepts_interface

https://github.com/regen-network/cosmos-proto#accepts_interface

An informational extension to be added to google.protobuf.Any fields to indicate which interface that field accepts. The interface name should be independent of any programming language and either be unqualified to indicate the current package of fully-qualified to indicate another protobuf package.

message SomeContainer {
    google.protobuf.Any some_interface = 1 [(cosmos_proto.accepts_interface) = "SomeInterface"];
}

message AnotherContainer {
    google.protobuf.Any another_interface = 1 [(cosmos_proto.accepts_interface) = "another.package.AnotherInterface"];
}

implements_interface

https://github.com/regen-network/cosmos-proto#implements_interface

An informational extension to be added to messages to indicate which interface that message implements. The interface name should be independent of any programming language and either be unqualified to indicate the current package of fully-qualified to indicate another protobuf package.

message A {
    option (cosmos_proto.implements_interface) = "SomeInterface";
    int64 x = 1;
}

message B {
    option (cosmos_proto.implements_interface) = "another.package.AnotherInterface";
    double x = 1;
}

questions

message MsgExec {
  option (cosmos.msg.v1.signer) = "grantee";

  string grantee = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
  // Authorization Msg requests to execute. Each msg must implement Authorization interface
  // The x/authz will try to find a grant matching (msg.signers[0], grantee, MsgTypeURL(msg))
  // triple and validate it.
  repeated google.protobuf.Any msgs = 2 [(cosmos_proto.accepts_interface) = "sdk.Msg, authz.Authorization"];
}

other notes

@assafmo
Copy link
Author

assafmo commented Jul 19, 2022

So things just became more complicated. Reposting from Discord:

Hi all, I have a question about the protobuf definitions of MsgSubmitProposal. 👋

In MsgSubmitProposal content is (cosmos_proto.accepts_interface) = "Content", but only TextProposal is (cosmos_proto.implements_interface) = "Content".

In practice there are at least 7 proposal types that I manged to find by hand, so my question is how can client library effectively find them?

  1. SoftwareUpgradeProposal
  2. CancelSoftwareUpgradeProposal
  3. TextProposal
  4. ParameterChangeProposal
  5. CommunityPoolSpendProposal

And two more in ibc-go:
6. ClientUpdateProposal
7. UpgradeProposal

Seems to me that they're all manually implementing the Content interface in Go after being derived from protobuf, and that's what ultimately decides if it's a valid proposal type or not.

Also seems like CommunityPoolSpendProposalWithDeposit is defined but doesn't implement the Content interface.

Would love to get some feedback and more info on this. 🙏

@pyramation
Copy link
Collaborator

pyramation commented Jul 19, 2022

@ValarDragon another question I have — would it be a breaking change to add the (cosmos_proto.implements_interface) = "Content" to every proposal message that actually does implement it?

It seems that there was a convention, but perhaps it didn't get followed. If this would be a non-breaking change, adding this option would allow us to automate all the proposal serialization logic.

@pyramation
Copy link
Collaborator

pyramation commented Jul 19, 2022

we also need to figure out where in exported .proto files the Content and Authorization message are.

@faddat @nghuyenthevinh2000 do you know if the cosmos-sdk exports these two objects to .proto files?

From what I can tell, these objects aren't included in the export.

@ValarDragon
Copy link

(cosmos_proto.implements_interface) = "Content"

This should be possible and not a breaking change to do, as far as I understand! I'd test it on a node before releasing, but that should be easy to do

@pyramation
Copy link
Collaborator

some examples I've found in the wild:

export const createSubmitProposalMsg = (
    typeUrl,
    title,
    description,
    proposer,
    deposit,
    denom,
) => {
    const msgSubmitProposal = {
        content: {
            typeUrl: typeUrl,
            value: gov_1.TextProposal.encode(gov_1.TextProposal.fromPartial({
                title: title,
                description: description,
            })).finish(),
        },
        initialDeposit: coins(deposit, denom),
        proposer: proposer
    }

    const msg = {
        typeUrl: "/cosmos.gov.v1beta1.MsgSubmitProposal",
        value: msgSubmitProposal,
    }

    return msg
}

@JakeHartnell
Copy link

JakeHartnell commented Sep 7, 2022

Really glad to see this... this is the kind of annoying stuff we have to do to encode nested proto messages in JS right now: https://github.com/DA0-DA0/dao-dao-ui/blob/940462a5210c507a731289b3dc2cdcc36299c33d/packages/utils/messages.ts#L205

@pyramation
Copy link
Collaborator

due to infinite recursion of the authz messages types — pretty certain we need to re-write how amino types are encoded: this will be more maintainable as well: #49

@pyramation
Copy link
Collaborator

pyramation commented Dec 10, 2022

@assafmo @webmaster128 making some progress, these are auto-generated decoders:

import { DepositDeploymentAuthorization as DepositDeploymentAuthorization1 } from "../../../akash/deployment/v1beta1/authz";
import { DepositDeploymentAuthorization as DepositDeploymentAuthorization2 } from "../../../akash/deployment/v1beta2/authz";
import { SendAuthorization } from "../../bank/v1beta1/authz";
import { StakeAuthorization } from "../../staking/v1beta1/authz";

...

export const Authorization_InterfaceDecoder = (input: _m0.Reader | Uint8Array): DepositDeploymentAuthorization1 | DepositDeploymentAuthorization2 | GenericAuthorization | SendAuthorization | StakeAuthorization | Any => {
  const reader = input instanceof _m0.Reader ? input : new _m0.Reader(input);
  const data = Any.decode(reader, reader.uint32());

  switch (data.typeUrl) {
    case "/akash.deployment.v1beta1.DepositDeploymentAuthorization":
      return DepositDeploymentAuthorization1.decode(data.value);

    case "/akash.deployment.v1beta2.DepositDeploymentAuthorization":
      return DepositDeploymentAuthorization2.decode(data.value);

    case "/cosmos.authz.v1beta1.GenericAuthorization":
      return GenericAuthorization.decode(data.value);

    case "/cosmos.bank.v1beta1.SendAuthorization":
      return SendAuthorization.decode(data.value);

    case "/cosmos.staking.v1beta1.StakeAuthorization":
      return StakeAuthorization.decode(data.value);

    default:
      return data;
  }
};

the idea is to replace the type decoders anywhere they are found in decode function:

  decode(input: _m0.Reader | Uint8Array, length?: number): Grant {
    const reader = input instanceof _m0.Reader ? input : new _m0.Reader(input);
    let end = length === undefined ? reader.len : reader.pos + length;
    const message = createBaseGrant();

    while (reader.pos < end) {
      const tag = reader.uint32();

      switch (tag >>> 3) {
        case 1:
          message.authorization = Authorization_InterfaceDecoder(reader);
          break;

        case 2:
          message.expiration = fromTimestamp(Timestamp.decode(reader, reader.uint32()));
          break;

        default:
          reader.skipType(tag & 7);
          break;
      }
    }

    return message;
  },

related osmosis-labs/osmojs#5

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

4 participants