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

Unable to perform MsgExec authz transaction in CosmJS #1155

Closed
arnabghose997 opened this issue May 24, 2022 · 16 comments · Fixed by #1262
Closed

Unable to perform MsgExec authz transaction in CosmJS #1155

arnabghose997 opened this issue May 24, 2022 · 16 comments · Fixed by #1262

Comments

@arnabghose997
Copy link
Contributor

We are building a frontend where the facility of paying gas fee for someone else is being implemented. To do this, we are using the x/authz and x/feegrant module to do the job.

While we could run the /cosmos.authz.v1beta1.MsgGrant and /cosmos.feegrant.v1beta1.MsgGrantAllowance in CosmJS, we were not able to do cosmos.authz.v1beta1.MsgExec transaction, as it gave the error 0uhid is smaller than 100uhid: insufficient funds: insufficient funds.

This is the overall setup of the code for executing MsgExec Tx:

async function execTx() {
  const grantee_mnemonic = " ----- grantee's mnemonic ------"
  const wallet =  await DirectSecp256k1HdWallet.fromMnemonic(grantee_mnemonic, options = { prefix: "hid" })
  
  const client = SigningStargateClient.connectWithSigner(
    "http://localhost:26657",
     wallet,
    { 
        registry: // Custom Module TypeURLs are added ,
        gasPrice: GasPrice.fromString("0.0001uhid"),
    },
  )
  
  let grantee = "hid1k77resf8gktl5wh8fhwlqt7pccandeyj9z5702"  // account with no money
  let customTypeUrl = "/hypersignprotocol.hidnode.ssi.MsgCreateDID"
  
  // The Custom Module Message that the grantee needs to execute
  const txCreateDIDMessage = {
  typeUrl: customTypeUrl,
  value: MsgCreateDID.encode(
      MsgCreateDID.fromPartial({
          didDocString: "---",
          signatures: "---",
          creator: "---",
      })).finish(),
  };
  
  // MsgExec Tx Object
  const txAuthMessage = {
    typeUrl: "/cosmos.authz.v1beta1.MsgExec",
    value: {
        grantee: grantee,
        msgs: [
            txCreateDIDMessage
        ]
    },
  };
  
  const txResult = await client.signAndBroadcast(grantee, [txAuthMessage], "auto");
  return txResult
}

CLI execution (hid-noded tx authz exec tx.json --from <grantee-address> --fee-account <granter's-address> --fees 90uhid) worked fine, but we are struggling with CosmJs execution.

@webmaster128
Copy link
Member

CosmJS does not yet have a way to set the fee payer for the feegrant module. This is tracked as part of #1105. So it seems like the message signer (grantee) is supposed to pay the fee and this account has a 0 balance ("0uhid is smaller than 100uhid").

Would you be interested in working on that feature for CosmJS? I'm happy to help in that case.

@arnabghose997
Copy link
Contributor Author

arnabghose997 commented May 24, 2022

@webmaster128 Sure, I would try my best to work on it.

@webmaster128
Copy link
Member

Amazing! Have a look at HACKING.md for how to start developing CosmJS.

@arnabghose997
Copy link
Contributor Author

Thank you! I will post regarding any dev updates on the build-frontend channel on Cosmos Discord Server.

@webmaster128
Copy link
Member

webmaster128 commented May 24, 2022

Better post here or open a draft PR to discuss inline. Otherwise it get's lost in the noise of the chat.

@arnabghose997
Copy link
Contributor Author

Sure, will do

@arnabghose997
Copy link
Contributor Author

I am getting the following warnings while running yarn install:

 YN0000:  Resolution step
 YN0002:  @cosmjs/amino@workspace:packages/amino doesn't provide jasmine-core (p3b488), requested by karma-jasmine-html-reporterYN0002:  @cosmjs/cosmwasm-stargate@workspace:packages/cosmwasm-stargate doesn't provide jasmine-core (p4bb4d), requested by karma-jasmine-html-reporter
➤ YN0002: │ @cosmjs/crypto@workspace:packages/crypto doesn't provide jasmine-core (p39c2c), requested by karma-jasmine-html-reporterYN0002:  @cosmjs/encoding@workspace:packages/encoding doesn't provide jasmine-core (p56d68), requested by karma-jasmine-html-reporter
➤ YN0002: │ @cosmjs/faucet-client@workspace:packages/faucet-client doesn't provide jasmine-core (p1ee44), requested by karma-jasmine-html-reporterYN0002:  @cosmjs/json-rpc@workspace:packages/json-rpc doesn't provide jasmine-core (pe3fca), requested by karma-jasmine-html-reporter
➤ YN0002: │ @cosmjs/math@workspace:packages/math doesn't provide jasmine-core (p00bbe), requested by karma-jasmine-html-reporterYN0002:  @cosmjs/proto-signing@workspace:packages/proto-signing doesn't provide jasmine-core (p8d482), requested by karma-jasmine-html-reporter
➤ YN0002: │ @cosmjs/socket@workspace:packages/socket doesn't provide jasmine-core (paca33), requested by karma-jasmine-html-reporterYN0002:  @cosmjs/stargate@workspace:packages/stargate doesn't provide jasmine-core (p3bbdc), requested by karma-jasmine-html-reporter
➤ YN0002: │ @cosmjs/stream@workspace:packages/stream doesn't provide jasmine-core (pc3b6f), requested by karma-jasmine-html-reporterYN0002:  @cosmjs/tendermint-rpc@workspace:packages/tendermint-rpc doesn't provide jasmine-core (pf034a), requested by karma-jasmine-html-reporter
➤ YN0002: │ @cosmjs/utils@workspace:packages/utils doesn't provide jasmine-core (pd0a2f), requested by karma-jasmine-html-reporterYN0000:  Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed codeYN0000:  CompletedYN0000:  Fetch stepYN0000:  CompletedYN0000:  Link stepYN0000:  ESM support for PnP uses the experimental loader API and is therefore experimentalYN0007:  protobufjs@npm:6.11.2 must be built because it never has been before or the last one failedYN0007:  protobufjs@npm:6.10.2 must be built because it never has been before or the last one failedYN0007:  node-hid@npm:2.1.1 must be built because it never has been before or the last one failedYN0007:  usb@npm:1.7.1 must be built because it never has been before or the last one failedYN0000:  Completed in 1s 197msYN0000: Done with warnings in 1s 664ms

These seem to be related with jasmine-core, but I am unsure about any workarounds for this.

@webmaster128
Copy link
Member

Looks like warnings only. What does yarn install && echo "all good" say?

@arnabghose997
Copy link
Contributor Author

In my VSCode IDE, I saw import errors in typescript files, and I guessed that it might have something to do with warning logs of yarn install. The yarn test suggests its all working fine.

@webmaster128
Copy link
Member

Yeah, this can happen when VSCode's TypeScript server does not find the type definitions of other packages in the same repo because they are not built yet. Running yarn build in the repo root and restarting the IDE should help.

@arnabghose997
Copy link
Contributor Author

Based on my exploration, it is clear that for MsgExec to work, the granter field of AuthInfo in Tx needs to be populated with the address who will pay for the fees.

The fee argument of signAndBroadcast() accepts value of type StdFee, which comprises of two attributes namely amount and gas. When signAndBroadcast() executes, the fee value is passed down, to finally reach to makeAuthInfoBytes(), which composes AuthInfo. Hence, it is intuitive to think about adding an optional field, say feePayer, in StdFee interface, which can be assigned in the granter field of AuthInfo.

However, signAndBroadcast() also accepts "auto" and Number type as well, which leaves no scope to pass the fee payer's address.

My suggestion is to code a method similar to sendTokens(), specifically meant to perform the MsgExec transaction. It will include feePayer and fee as the input parameters, making it easier to pass feePayer to AuthInfo.

@webmaster128 What are you thoughts on this approach?

@webmaster128
Copy link
Member

What about this: only allow setting the fee payer when the fee is set explicitly (via StdFee)? Once this is available we can think about making "auto" fee and fee payer work together.

@webmaster128
Copy link
Member

In Cosmos SDK the payer field was added to the StdFee type as well:

type StdFee struct {
	Amount  sdk.Coins `json:"amount" yaml:"amount"`
	Gas     uint64    `json:"gas" yaml:"gas"`
	Payer   string    `json:"payer,omitempty" yaml:"payer"`
	Granter string    `json:"granter,omitempty" yaml:"granter"`
}

We need the field in this structure for Amino JSON signing compatibility. So StdFee is the best place for it.

@webmaster128
Copy link
Member

Here are some "payer"/"granter" docs from Cosmos SDK:

// Fee includes the amount of coins paid in fees and the maximum
// gas to be used by the transaction. The ratio yields an effective "gasprice",
// which must be above some miminum to be accepted into the mempool.
message Fee {
  // amount is the amount of coins to be paid as a fee
  repeated cosmos.base.v1beta1.Coin amount = 1
      [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];

  // gas_limit is the maximum gas that can be used in transaction processing
  // before an out of gas error occurs
  uint64 gas_limit = 2;

  // if unset, the first signer is responsible for paying the fees. If set, the specified account must pay the fees.
  // the payer must be a tx signer (and thus have signed this field in AuthInfo).
  // setting this field does *not* change the ordering of required signers for the transaction.
  string payer = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];

  // if set, the fee payer (either the first signer or the value of the payer field) requests that a fee grant be used
  // to pay fees instead of the fee payer's own balance. If an appropriate fee grant does not exist or the chain does
  // not support fee grants, this will fail
  string granter = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
}

@arnabghose997
Copy link
Contributor Author

arnabghose997 commented May 30, 2022

What about this: only allow setting the fee payer when the fee is set explicitly (via StdFee)? Once this is available we can think about making "auto" fee and fee payer work together.

Sounds great! Let's go ahead with this approach

@arnabghose997
Copy link
Contributor Author

@webmaster128 I have raised the PR where I have added the granter field in StdFee and made necessary changes in other files that dependent on it.

All there's left to do is to write test for verifying MsgExec transaction. But I am having a query with the full fledged test. In HACKING.md, there are mentions of running few scripts in order to perform a full test, however one of scripts ./scripts/launchpad/start.sh isn't present in the repo. Are there any other scripts that we need to run ?

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

Successfully merging a pull request may close this issue.

2 participants