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

Some CosmJs-Types Txs Don't Work (e.g. Unjail) #1417

Open
anthonyjoeseph opened this issue May 12, 2023 · 5 comments
Open

Some CosmJs-Types Txs Don't Work (e.g. Unjail) #1417

anthonyjoeseph opened this issue May 12, 2023 · 5 comments

Comments

@anthonyjoeseph
Copy link

anthonyjoeseph commented May 12, 2023

A simple Unjail doesn't work, even when I add a custom Registry and AminoConverter

{
  typeUrl: "/cosmos.slashing.v1beta1.MsgUnjail",
  value: {
    validatorAddr: "osmovaloper1wqx3uvz9xry82jy2suppg57mpeg50dwd6qldu8",
  },
}

However, some txs do work, like msgEdit

{
  typeUrl: "/cosmos.staking.v1beta1.MsgEditValidator",
  value: {
    description: {
      moniker: "New-Moniker",
      identity: "[do-not-modify]",
      website: "[do-not-modify]",
      securityContact: "[do-not-modify]",
      details: "[do-not-modify]",
    },
    validatorAddress: "osmovaloper1wqx3uvz9xry82jy2suppg57mpeg50dwd6qldu8",
    commissionRate: null,
    minSelfDelegation: null,
  },
}

Here is a gist of my minimal working example

Possibly related:

  1. why does direct signing fail for multisigs? Only amino seems to work with cosmjs. Replacing Secp256k1HdWallet with DirectSecp256k1HdWallet breaks it
  2. why does toBase64(signature) return a different value than this cli command? I would expect them to be the same:
  • osmosisd tx sign unjail_unsigned.json --multisig=osmo1wqx3uvz9xry82jy2suppg57mpeg50dwdqhhwtq --from fourth --chain-id osmo-test-5 --node=https://rpc.osmotest5.osmosis.zone:443 --output-document signed.json && cat signed.json | jq '.signatures[0].data.single.signature'
  1. is there a good way to get mnemonic(s) for the validator created by the simd genesis file - src?
  • this would allow inclusion & unit testing of more validator-related messages like Unjail etc
@anthonyjoeseph
Copy link
Author

anthonyjoeseph commented May 12, 2023

Found a related issues for #1 - #1273 , #1097 (comment)

@anthonyjoeseph
Copy link
Author

tangentially related - #837

@anthonyjoeseph anthonyjoeseph changed the title Some Multisign Txs Don't Work (e.g. Unjail) Some CosmJs Txs Don't Work (e.g. Unjail) May 12, 2023
@anthonyjoeseph anthonyjoeseph changed the title Some CosmJs Txs Don't Work (e.g. Unjail) Some CosmJs-Types Txs Don't Work (e.g. Unjail) May 12, 2023
@chalabi2
Copy link

Similar issues

hyperweb-io/cosmos-kit#187

#1414

@webmaster128
Copy link
Member

Thanks a lot for bringing this up.

Here is a gist of my minimal working example

This looks good as far as I can tell. However, it looks like the field is renamed to just address in the Amino JSON. I found this in cosmos-sdk code:

func TestMsgUnjailGetSignBytes(t *testing.T) {
	addr := sdk.AccAddress("abcd")
	msg := NewMsgUnjail(sdk.ValAddress(addr))
	bytes := msg.GetSignBytes()
	require.Equal(
		t,
		`{"type":"cosmos-sdk/MsgUnjail","value":{"address":"cosmosvaloper1v93xxeqhg9nn6"}}`,
		string(bytes),
	)
}

Would you submit your Amino converter as an implementation for https://github.com/cosmos/cosmjs/blob/main/packages/stargate/src/modules/slashing/aminomessages.ts#L21-L23 and fix AminoMsgUnjail?

why does direct signing fail for multisigs? Only amino seems to work with cosmjs. Replacing Secp256k1HdWallet with DirectSecp256k1HdWallet breaks it

Because multisigs work fundamentally different in sign mode direct. See #1400 for a proof of concept how this can be done. For now I'd consider it unsupported in CosmJS.

why does toBase64(signature) return a different value than this cli command? I would expect them to be the same:

secp256k1 signatures are not unique. I.e. different signing libraries can produce different signatures what are all valid.

is there a good way to get mnemonic(s) for the validator created by the simd genesis file - src?

Yeah, this was changed recently to a hardcoded mnemonic: https://github.com/cosmos/cosmjs/blob/v0.31.0/scripts/simapp44/setup.sh#L26-L27

this would allow inclusion & unit testing of more validator-related messages like Unjail etc

👍 but right now there is only one validator running the test chains. If this gets jailed, the chain probably halts.

@liray-unendlich
Copy link

This implementation works well on multisig unjail

export function createSlashingAminoConverters(): AminoConverters {
  return {
    "/cosmos.slashing.v1beta1.MsgUnjail": {
      aminoType: "cosmos-sdk/MsgUnjail",
      toAmino: ({validatorAddr}: MsgUnjail): AminoMsgUnjail["value"] => {
        return {
          address: validatorAddr,
        };
      },
      fromAmino: ({ address }: AminoMsgUnjail["value"]): MsgUnjail => {
        return {
          validatorAddr: address,
        };
      },
    }
  }
}

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