cosmos / cosmjs

The Swiss Army knife to power JavaScript based client solutions ranging from Web apps/explorers over browser extensions to server-side clients like faucets/scrapers.
https://cosmos.github.io/cosmjs/
Apache License 2.0
649 stars 335 forks source link

Unable to perform `MsgExec` authz transaction in CosmJS #1155

Closed arnabghose997 closed 2 years ago

arnabghose997 commented 2 years ago

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 commented 2 years ago

CosmJS does not yet have a way to set the fee payer for the feegrant module. This is tracked as part of https://github.com/cosmos/cosmjs/issues/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 commented 2 years ago

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

webmaster128 commented 2 years ago

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

arnabghose997 commented 2 years ago

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

webmaster128 commented 2 years ago

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

arnabghose997 commented 2 years ago

Sure, will do

arnabghose997 commented 2 years ago

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-reporter
➤ YN0002: │ @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-reporter
➤ YN0002: │ @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-reporter
➤ YN0002: │ @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-reporter
➤ YN0002: │ @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-reporter
➤ YN0002: │ @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-reporter
➤ YN0002: │ @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-reporter
➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: │ ESM support for PnP uses the experimental loader API and is therefore experimental
➤ YN0007: │ protobufjs@npm:6.11.2 must be built because it never has been before or the last one failed
➤ YN0007: │ protobufjs@npm:6.10.2 must be built because it never has been before or the last one failed
➤ YN0007: │ node-hid@npm:2.1.1 must be built because it never has been before or the last one failed
➤ YN0007: │ usb@npm:1.7.1 must be built because it never has been before or the last one failed
➤ YN0000: └ Completed in 1s 197ms
➤ YN0000: Done with warnings in 1s 664ms

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

webmaster128 commented 2 years ago

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

arnabghose997 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 ?