cosmology-tech / telescope

A TypeScript Transpiler for Cosmos Protobufs ⚛️
https://cosmology.zone/products/telescope
Apache License 2.0
146 stars 43 forks source link

`fromAmino` discard message type #660

Open fmorency opened 1 day ago

fmorency commented 1 day ago

Problem

The call to fromAmino removes the nested message type indicator in a group proposal, causing an issue when multiple message types contain the same fields.

Context

Telescope configuration

The POA module contains MsgRemovePending and MsgRemoveValidator differing only by message name, i.e., the message fields are the same in both messages.

The remove pending validator test creates a Group Proposal containing a Remove Pending Validator message, submits the proposal, vote on it and execute the proposal.

Encoding and signing the message using AMINO works fine

{
  "chain_id": "manifest-ledger-beta",
  "account_number": "2",
  "sequence": "8",
  "fee": {
    "amount": [
      {
        "denom": "umfx",
        "amount": "100000"
      }
    ],
    "gas": "550000"
  },
  "msgs": [
    {
      "type": "cosmos-sdk/group/MsgSubmitProposal",
      "value": {
        "group_policy_address": "manifest1afk9zr2hn2jsac63h4hm60vl9z3e5u69gndzf7c99cqge3vzwjzsfmy9qj",
        "proposers": [
          "manifest1pss7nxeh3f9md2vuxku8q99femnwdjtcwhpj7f"
        ],
        "messages": [
          {
            "type": "poa/MsgRemovePending",
            "value": {
              "sender": "manifest1afk9zr2hn2jsac63h4hm60vl9z3e5u69gndzf7c99cqge3vzwjzsfmy9qj",
              "validator_address": "manifestvaloper1pss7nxeh3f9md2vuxku8q99femnwdjtcjhuxjm"
            }
          }
        ],
        "title": "remove pending",
        "summary": "some remove pending"
      }
    }
  ],
  "memo": ""
}

The nested message is of the right type. However, the signedTxBodyEncodedObject created in the signAmino method and broadcasted to the server is

{
  "typeUrl": "/cosmos.tx.v1beta1.TxBody",
  "value": {
    "messages": [
      {
        "typeUrl": "/cosmos.group.v1.MsgSubmitProposal",
        "value": {
          "groupPolicyAddress": "manifest1afk9zr2hn2jsac63h4hm60vl9z3e5u69gndzf7c99cqge3vzwjzsfmy9qj",
          "proposers": [
            "manifest1pss7nxeh3f9md2vuxku8q99femnwdjtcwhpj7f"
          ],
          "metadata": "",
          "messages": [
            {
              "sender": "manifest1afk9zr2hn2jsac63h4hm60vl9z3e5u69gndzf7c99cqge3vzwjzsfmy9qj",
              "validatorAddress": "manifestvaloper1pss7nxeh3f9md2vuxku8q99femnwdjtcjhuxjm"
            }
          ],
          "exec": 0,
          "title": "remove pending",
          "summary": "some remove pending"
        }
      }
    ],
    "memo": ""
  }
}

Notice that the nested message field has no type indicator. This field is created by the call to fromAmino

const signedTxBody = {
    messages: signed.msgs.map((msg) => this.aminoTypes.fromAmino(msg)),
    memo: signed.memo,
    timeoutHeight: timeoutHeight,
};

The message decoded by the server is of type poa/RemoveValidator instead of poa/RemovePending causing the signature check to fail. I suspect the lack of type indication in the nested message causes the issue. One is unable to match the type only from the instance.

Everything works fine when using the DIRECT signer. Everything also works fine using the AMINO signer from the manifestd CLI.

Running the test

yarn starship start
yarn starship:test starship/__tests__/poa.group.test.ts
fmorency commented 2 hours ago

I also tried using the MsgSubmitProposalEncoded as

...
  const submitMsg: MsgSubmitProposalEncoded = {
    groupPolicyAddress: POA_GROUP_ADDRESS,
    title,
    summary,
    proposers,
    exec: Exec.EXEC_UNSPECIFIED,
    messages: messages.map((msg) => Any.toProtoMsg(msg)),
    metadata: "",
  }

  const msg = GroupMessageComposer.fromPartial.submitProposal(submitMsg);
  const result = await client.signAndBroadcast(signer, [msg], fee);
  ...

where messages[0] is, e.g.,

ManifestMessageComposer.encoded.payout({
  authority: POA_GROUP_ADDRESS,
  payoutPairs: [
    {
      address: test2Address,
      coin: { denom, amount: "1000" },
    },
  ],
});

without luck. I'm getting the following error

TypeError: decoder.toAminoMsg is not a function

      196 |     }
      197 |
    > 198 |     return decoder.toAminoMsg!(data);
          |                    ^
      199 |   }
      200 | }
      201 |

      at Function.toAminoMsg (src/codegen/registry.ts:198:20)
      at src/codegen/cosmos/group/v1/tx.ts:2870:74
          at Array.map (<anonymous>)
      at Object.toAmino (src/codegen/cosmos/group/v1/tx.ts:2870:39)
      at AminoTypes.toAmino (node_modules/@cosmjs/stargate/src/aminotypes.ts:40:24)
      at node_modules/@cosmjs/stargate/src/signingstargateclient.ts:404:56
          at Array.map (<anonymous>)
      at SigningStargateClient.signAmino (node_modules/@cosmjs/stargate/src/signingstargateclient.ts:404:27)
      at SigningStargateClient.signAndBroadcast (node_modules/@cosmjs/stargate/src/signingstargateclient.ts:319:19)
      at submitGroupProposal (starship/src/test_helper.ts:105:18)
      at submitVoteExecGroupProposal (starship/src/test_helper.ts:162:22)
      at Object.<anonymous> (starship/__tests__/manifest.group.test.ts:123:5)