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

AutoCLI deserializes Proto types into Amino types #21261

Closed
1 task done
mvid opened this issue Aug 12, 2024 · 2 comments · Fixed by #21712
Closed
1 task done

AutoCLI deserializes Proto types into Amino types #21261

mvid opened this issue Aug 12, 2024 · 2 comments · Fixed by #21712

Comments

@mvid
Copy link
Contributor

mvid commented Aug 12, 2024

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

AutoCLI seems to have issues with Any types in responses. Specifically, it returns the inner types as Amino definitions instead of Proto serialized definitions. I have validated this on a number of AutoCLI chains.

This may be related to #18310

Cosmos SDK Version

0.50.8

How to reproduce?

The expectations here are that type is @type and the value for that key is the proto message name, not the amino registered name

One example is here:

➜  neutron git:(main) neutrond tx feegrant grant neutron-test neutron17r4px32kw74hkp2l9ffx4282g4ex8vktvf8xq7 --period 3600 --period-limit 5000untrn --spend-limit 10000untrn --chain-id pion-1 --allowed-messages "/cosmos.bank.v1beta1.Msg/Send" --node https://rpc-palvus.pion-1.ntrn.tech:443 --gas auto --gas-adjustment 1.6 --gas-prices 0.1untrn

➜  neutron git:(main) neutrond q feegrant grant neutron1q8em7tjrlmck8rt8tj8ku7mva6gy5n6vqh5290 neutron17r4px32kw74hkp2l9ffx4282g4ex8vktvf8xq7  --node https://rpc-palvus.pion-1.ntrn.tech:443 --output json
{
  "allowance": {
    "granter": "neutron1q8em7tjrlmck8rt8tj8ku7mva6gy5n6vqh5290",
    "grantee": "neutron17r4px32kw74hkp2l9ffx4282g4ex8vktvf8xq7",
    "allowance": {
      "type": "cosmos-sdk/AllowedMsgAllowance",
      "value": {
        "allowance": {
          "type": "cosmos-sdk/PeriodicAllowance",
          "value": {
            "basic": {
              "spend_limit": [
                {
                  "denom": "untrn",
                  "amount": "10000"
                }
              ]
            },
            "period": "1h0m0s",
            "period_spend_limit": [
              {
                "denom": "untrn",
                "amount": "5000"
              }
            ],
            "period_can_spend": [
              {
                "denom": "untrn",
                "amount": "5000"
              }
            ],
            "period_reset": "2024-08-12T19:20:18.467835Z"
          }
        },
        "allowed_messages": [
          "/cosmos.bank.v1beta1.Msg/Send"
        ]
      }
    }
  }
}
@mvid mvid added the T:Bug label Aug 12, 2024
@julienrbrt
Copy link
Member

Right, so we use the amino json encoder, so this is the normal behavior of that json marshaller.
However, I agree that is something we can improve.

I won't keep this as a bug, but indeed as an enhancement. Let's track it in that issue 👍🏾, we'll look into it.

@julienrbrt julienrbrt changed the title [Bug]: AutoCLI incorrectly deserializes Proto types into Amino types AutoCLI deserializes Proto types into Amino types Aug 12, 2024
@mvid
Copy link
Contributor Author

mvid commented Aug 12, 2024

My only argument against it being an enhancement vs a regression is that the v47 feegrant and authz query CLI commands returned Proto json, and these new replacement AutoCLI cmds do not. If you are using the outputs of the CLI with any expectation, for instance with interchain-test, they will break

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🥳 Done
Development

Successfully merging a pull request may close this issue.

2 participants