From f36d909ae7e61885cf2ad7a7073afb3bfccc9e7c Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Thu, 16 May 2024 00:35:16 +0700 Subject: [PATCH 1/5] adjust Bytes() methods and add test for nested msig --- client/keys/output_test.go | 22 ++++++++++++++++++++++ crypto/keys/multisig/multisig.go | 2 +- crypto/keys/multisig/multisig_test.go | 15 +++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/client/keys/output_test.go b/client/keys/output_test.go index 9bc77841c51a..a2c75634aad1 100644 --- a/client/keys/output_test.go +++ b/client/keys/output_test.go @@ -45,6 +45,28 @@ func TestBech32KeysOutput(t *testing.T) { require.Equal(t, "{Name:multisig Type:multi Address:cosmos1nf8lf6n4wa43rzmdzwe6hkrnw5guekhqt595cw PubKey:{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"}]} Mnemonic:}", fmt.Sprintf("%+v", out)) } +// TestBech32KeysOutputNestedMsig tests that the output of a nested multisig key is correct +func TestBech32KeysOutputNestedMsig(t *testing.T) { + sk := secp256k1.PrivKey{Key: []byte{154, 49, 3, 117, 55, 232, 249, 20, 205, 216, 102, 7, 136, 72, 177, 2, 131, 202, 234, 81, 31, 208, 46, 244, 179, 192, 167, 163, 142, 117, 246, 13}} + tmpKey := sk.PubKey() + nestedMultiSig := kmultisig.NewLegacyAminoPubKey(1, []types.PubKey{tmpKey}) + multisigPk := kmultisig.NewLegacyAminoPubKey(1, []types.PubKey{nestedMultiSig}) + k, err := keyring.NewMultiRecord("multisig", multisigPk) + require.NotNil(t, k) + require.NoError(t, err) + pubKey, err := k.GetPubKey() + fmt.Println(pubKey.Bytes()) + require.NoError(t, err) + accAddr := sdk.AccAddress(pubKey.Address()) + expectedOutput, err := NewKeyOutput(k.Name, k.GetType(), accAddr, multisigPk, addresscodec.NewBech32Codec("cosmos")) + require.NoError(t, err) + + out, err := MkAccKeyOutput(k, addresscodec.NewBech32Codec("cosmos")) + require.NoError(t, err) + require.Equal(t, expectedOutput, out) + require.Equal(t, "{Name:multisig Type:multi Address:cosmos16snhe32gdcfhzsk62xdtuhnwextdm939e5snax PubKey:{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"}]}]} Mnemonic:}", fmt.Sprintf("%+v", out)) +} + func TestProtoMarshalJSON(t *testing.T) { require := require.New(t) pubkeys := generatePubKeys(3) diff --git a/crypto/keys/multisig/multisig.go b/crypto/keys/multisig/multisig.go index 7ea52f751828..d49769b1a264 100644 --- a/crypto/keys/multisig/multisig.go +++ b/crypto/keys/multisig/multisig.go @@ -41,7 +41,7 @@ func (m *LegacyAminoPubKey) Address() cryptotypes.Address { // Bytes returns the proto encoded version of the LegacyAminoPubKey func (m *LegacyAminoPubKey) Bytes() []byte { - return AminoCdc.MustMarshal(m) + return AminoCdc.MustMarshalJSON(m) } // VerifyMultisignature implements the multisigtypes.PubKey VerifyMultisignature method. diff --git a/crypto/keys/multisig/multisig_test.go b/crypto/keys/multisig/multisig_test.go index e8e75a61ab02..7b7c00e68eeb 100644 --- a/crypto/keys/multisig/multisig_test.go +++ b/crypto/keys/multisig/multisig_test.go @@ -389,6 +389,21 @@ func TestAminoBinary(t *testing.T) { require.Equal(t, msig.Threshold, newMsig.(*kmultisig.LegacyAminoPubKey).Threshold) } +// func TestAminoBinaryNested(t *testing.T) { +// pubkeys := generatePubKeys(4) +// nestedMsig := kmultisig.NewLegacyAminoPubKey(2, pubkeys[:2]) +// msig := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{pubkeys[2], pubkeys[3], nestedMsig}) + +// // Do a round-trip key->bytes->key. +// bz, err := legacy.Cdc.Marshal(msig) +// require.NoError(t, err) +// var newMsig cryptotypes.PubKey +// err = legacy.Cdc.Unmarshal(bz, &newMsig) +// require.NoError(t, err) +// require.Equal(t, msig.Threshold, newMsig.(*kmultisig.LegacyAminoPubKey).Threshold) +// require.Equal(t, len(msig.PubKeys), len(newMsig.(*kmultisig.LegacyAminoPubKey).PubKeys)) +// } + func TestAminoMarshalJSON(t *testing.T) { pubkeys := generatePubKeys(2) multisigKey := kmultisig.NewLegacyAminoPubKey(2, pubkeys) From b5f8985c42e3b85a639cc8e0f95a23c221116554 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Thu, 16 May 2024 00:43:54 +0700 Subject: [PATCH 2/5] remove println --- client/keys/output_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/keys/output_test.go b/client/keys/output_test.go index a2c75634aad1..f5fecdf491a3 100644 --- a/client/keys/output_test.go +++ b/client/keys/output_test.go @@ -55,7 +55,6 @@ func TestBech32KeysOutputNestedMsig(t *testing.T) { require.NotNil(t, k) require.NoError(t, err) pubKey, err := k.GetPubKey() - fmt.Println(pubKey.Bytes()) require.NoError(t, err) accAddr := sdk.AccAddress(pubKey.Address()) expectedOutput, err := NewKeyOutput(k.Name, k.GetType(), accAddr, multisigPk, addresscodec.NewBech32Codec("cosmos")) From 18a7b95da607844b823e23ac867dc06ae40dffe3 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Thu, 16 May 2024 08:08:24 +0700 Subject: [PATCH 3/5] update test --- client/keys/output_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/keys/output_test.go b/client/keys/output_test.go index f5fecdf491a3..88361e74e8cd 100644 --- a/client/keys/output_test.go +++ b/client/keys/output_test.go @@ -50,20 +50,23 @@ func TestBech32KeysOutputNestedMsig(t *testing.T) { sk := secp256k1.PrivKey{Key: []byte{154, 49, 3, 117, 55, 232, 249, 20, 205, 216, 102, 7, 136, 72, 177, 2, 131, 202, 234, 81, 31, 208, 46, 244, 179, 192, 167, 163, 142, 117, 246, 13}} tmpKey := sk.PubKey() nestedMultiSig := kmultisig.NewLegacyAminoPubKey(1, []types.PubKey{tmpKey}) - multisigPk := kmultisig.NewLegacyAminoPubKey(1, []types.PubKey{nestedMultiSig}) + multisigPk := kmultisig.NewLegacyAminoPubKey(2, []types.PubKey{tmpKey, nestedMultiSig}) k, err := keyring.NewMultiRecord("multisig", multisigPk) require.NotNil(t, k) require.NoError(t, err) + pubKey, err := k.GetPubKey() require.NoError(t, err) + accAddr := sdk.AccAddress(pubKey.Address()) expectedOutput, err := NewKeyOutput(k.Name, k.GetType(), accAddr, multisigPk, addresscodec.NewBech32Codec("cosmos")) require.NoError(t, err) out, err := MkAccKeyOutput(k, addresscodec.NewBech32Codec("cosmos")) require.NoError(t, err) + require.Equal(t, expectedOutput, out) - require.Equal(t, "{Name:multisig Type:multi Address:cosmos16snhe32gdcfhzsk62xdtuhnwextdm939e5snax PubKey:{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"}]}]} Mnemonic:}", fmt.Sprintf("%+v", out)) + require.Equal(t, "{Name:multisig Type:multi Address:cosmos13ytvp7lwj947gpa5gz0zmlr9drvkdcykk4lmcj PubKey:{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":2,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"},{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"}]}]} Mnemonic:}", fmt.Sprintf("%+v", out)) } func TestProtoMarshalJSON(t *testing.T) { From fc94fbff5b1933e854d42fc00549c00f5c8d184d Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Thu, 16 May 2024 17:04:48 +0700 Subject: [PATCH 4/5] use MustMarshal and override the MarshalAmino --- crypto/keys/multisig/amino.go | 10 ++++++++++ crypto/keys/multisig/multisig.go | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crypto/keys/multisig/amino.go b/crypto/keys/multisig/amino.go index 6ac90d0bdd11..17d3ba46a0d7 100644 --- a/crypto/keys/multisig/amino.go +++ b/crypto/keys/multisig/amino.go @@ -65,6 +65,16 @@ func tmToProto(tmPk tmMultisig) (*LegacyAminoPubKey, error) { }, nil } +// MarshalAmino overrides amino binary marshaling. +func (m LegacyAminoPubKey) MarshalAmino() ([]byte, error) { + return m.Marshal() +} + +// UnmarshalAmino overrides amino binary marshaling. +func (m *LegacyAminoPubKey) UnmarshalAmino(bz []byte) error { + return m.Unmarshal(bz) +} + // MarshalAminoJSON overrides amino JSON unmarshaling. func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { //nolint:golint // we need to override the default amino JSON marshaling return protoToTm(&m) diff --git a/crypto/keys/multisig/multisig.go b/crypto/keys/multisig/multisig.go index d49769b1a264..7ea52f751828 100644 --- a/crypto/keys/multisig/multisig.go +++ b/crypto/keys/multisig/multisig.go @@ -41,7 +41,7 @@ func (m *LegacyAminoPubKey) Address() cryptotypes.Address { // Bytes returns the proto encoded version of the LegacyAminoPubKey func (m *LegacyAminoPubKey) Bytes() []byte { - return AminoCdc.MustMarshalJSON(m) + return AminoCdc.MustMarshal(m) } // VerifyMultisignature implements the multisigtypes.PubKey VerifyMultisignature method. From dbeb76974441811d97e2413d9496455d43d12996 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Fri, 17 May 2024 08:33:43 +0700 Subject: [PATCH 5/5] add test for nested multisig marshal/unmarshal --- crypto/keys/multisig/multisig_test.go | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/crypto/keys/multisig/multisig_test.go b/crypto/keys/multisig/multisig_test.go index 7b7c00e68eeb..4f0b94745997 100644 --- a/crypto/keys/multisig/multisig_test.go +++ b/crypto/keys/multisig/multisig_test.go @@ -389,20 +389,20 @@ func TestAminoBinary(t *testing.T) { require.Equal(t, msig.Threshold, newMsig.(*kmultisig.LegacyAminoPubKey).Threshold) } -// func TestAminoBinaryNested(t *testing.T) { -// pubkeys := generatePubKeys(4) -// nestedMsig := kmultisig.NewLegacyAminoPubKey(2, pubkeys[:2]) -// msig := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{pubkeys[2], pubkeys[3], nestedMsig}) - -// // Do a round-trip key->bytes->key. -// bz, err := legacy.Cdc.Marshal(msig) -// require.NoError(t, err) -// var newMsig cryptotypes.PubKey -// err = legacy.Cdc.Unmarshal(bz, &newMsig) -// require.NoError(t, err) -// require.Equal(t, msig.Threshold, newMsig.(*kmultisig.LegacyAminoPubKey).Threshold) -// require.Equal(t, len(msig.PubKeys), len(newMsig.(*kmultisig.LegacyAminoPubKey).PubKeys)) -// } +func TestAminoBinaryNested(t *testing.T) { + pubkeys := generatePubKeys(4) + nestedMsig := kmultisig.NewLegacyAminoPubKey(2, pubkeys[:2]) + msig := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{pubkeys[2], pubkeys[3], nestedMsig}) + + // Do a round-trip key->bytes->key. + bz, err := legacy.Cdc.Marshal(msig) + require.NoError(t, err) + var newMsig cryptotypes.PubKey + err = legacy.Cdc.Unmarshal(bz, &newMsig) + require.NoError(t, err) + require.Equal(t, msig.Threshold, newMsig.(*kmultisig.LegacyAminoPubKey).Threshold) + require.Equal(t, len(msig.PubKeys), len(newMsig.(*kmultisig.LegacyAminoPubKey).PubKeys)) +} func TestAminoMarshalJSON(t *testing.T) { pubkeys := generatePubKeys(2)