Skip to content

Commit f313395

Browse files
rootulpclaude
andcommitted
test: add regression test for multisig MsgSetRoutingIsmDomain (#6541)
Add a regression test that reproduces the failure when submitting MsgSetRoutingIsmDomain from a multisig account. The malformed amino.name in the upstream hyperlane-cosmos proto definition causes SIGN_MODE_LEGACY_AMINO_JSON signing to fail with "unknown protobuf field". Includes commented-out code for the full end-to-end multisig flow to uncomment once the upstream fix lands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f45c140 commit f313395

File tree

1 file changed

+167
-0
lines changed

1 file changed

+167
-0
lines changed
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
package interop
2+
3+
import (
4+
"context"
5+
6+
"cosmossdk.io/math"
7+
ismtypes "github.com/bcp-innovations/hyperlane-cosmos/x/core/01_interchain_security/types"
8+
cosmostx "github.com/cosmos/cosmos-sdk/client/tx"
9+
"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
10+
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
11+
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
12+
multisigtypes "github.com/cosmos/cosmos-sdk/crypto/types/multisig"
13+
sdk "github.com/cosmos/cosmos-sdk/types"
14+
"github.com/cosmos/cosmos-sdk/types/tx/signing"
15+
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
16+
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
17+
)
18+
19+
// TestMultisigSetRoutingIsmDomain is a regression test for
20+
// https://github.com/celestiaorg/celestia-app/issues/6541.
21+
// MsgSetRoutingIsmDomain has a malformed amino.name in its proto definition
22+
// which causes multisig transactions (that use SIGN_MODE_LEGACY_AMINO_JSON)
23+
// to fail with signature verification or protobuf unmarshal errors.
24+
func (s *HyperlaneTestSuite) TestMultisigSetRoutingIsmDomain() {
25+
chain := s.celestia
26+
27+
// Generate two private keys and create a 2-of-2 multisig public key.
28+
priv1 := secp256k1.GenPrivKey()
29+
priv2 := secp256k1.GenPrivKey()
30+
pubKeys := []cryptotypes.PubKey{priv1.PubKey(), priv2.PubKey()}
31+
msigPubKey := multisig.NewLegacyAminoPubKey(2, pubKeys)
32+
msigAddr := sdk.AccAddress(msigPubKey.Address())
33+
34+
// Fund the multisig account. The test infra uses "stake" as the default
35+
// bond denom.
36+
fundMsg := &banktypes.MsgSend{
37+
FromAddress: chain.SenderAccount.GetAddress().String(),
38+
ToAddress: msigAddr.String(),
39+
Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(1_000_000))),
40+
}
41+
_, err := chain.SendMsgs(fundMsg)
42+
s.Require().NoError(err)
43+
44+
// Create a noop ISM to use as the route target.
45+
noopIsmID := s.SetupNoopISM(chain)
46+
47+
// Create a routing ISM (owned by the default sender).
48+
msgCreateRoutingIsm := &ismtypes.MsgCreateRoutingIsm{
49+
Creator: chain.SenderAccount.GetAddress().String(),
50+
}
51+
res, err := chain.SendMsgs(msgCreateRoutingIsm)
52+
s.Require().NoError(err)
53+
54+
var createResp ismtypes.MsgCreateRoutingIsmResponse
55+
err = unmarshalMsgResponses(chain.Codec, res.GetData(), &createResp)
56+
s.Require().NoError(err)
57+
routingIsmID := createResp.Id
58+
59+
// Transfer routing ISM ownership to the multisig address.
60+
msgTransferOwnership := &ismtypes.MsgUpdateRoutingIsmOwner{
61+
IsmId: routingIsmID,
62+
Owner: chain.SenderAccount.GetAddress().String(),
63+
NewOwner: msigAddr.String(),
64+
}
65+
_, err = chain.SendMsgs(msgTransferOwnership)
66+
s.Require().NoError(err)
67+
68+
// Build the MsgSetRoutingIsmDomain from the multisig.
69+
msgSetDomain := &ismtypes.MsgSetRoutingIsmDomain{
70+
IsmId: routingIsmID,
71+
Owner: msigAddr.String(),
72+
Route: ismtypes.Route{
73+
Ism: noopIsmID,
74+
Domain: 1,
75+
},
76+
}
77+
78+
// Sign the transaction with the multisig using SIGN_MODE_LEGACY_AMINO_JSON
79+
// (this is what real multisig accounts use).
80+
celestiaApp := s.GetCelestiaApp(chain)
81+
txCfg := celestiaApp.GetTxConfig()
82+
txBuilder := txCfg.NewTxBuilder()
83+
84+
err = txBuilder.SetMsgs(msgSetDomain)
85+
s.Require().NoError(err)
86+
txBuilder.SetGasLimit(400_000)
87+
txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(0))))
88+
89+
// Look up the multisig account to get its account number and sequence.
90+
msigAcc := celestiaApp.AccountKeeper.GetAccount(chain.GetContext(), msigAddr)
91+
s.Require().NotNil(msigAcc, "multisig account should exist after funding")
92+
accNum := msigAcc.GetAccountNumber()
93+
accSeq := msigAcc.GetSequence()
94+
95+
signMode := signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
96+
97+
// Round 1: Set an empty multisig signature to populate signer infos on the
98+
// tx builder, so that sign bytes are computed correctly.
99+
emptyMsigData := multisigtypes.NewMultisig(len(pubKeys))
100+
emptySig := signing.SignatureV2{
101+
PubKey: msigPubKey,
102+
Data: emptyMsigData,
103+
Sequence: accSeq,
104+
}
105+
err = txBuilder.SetSignatures(emptySig)
106+
s.Require().NoError(err)
107+
108+
// Round 2: Each key signs the transaction.
109+
signerData := authsigning.SignerData{
110+
Address: msigAddr.String(),
111+
ChainID: chain.ChainID,
112+
AccountNumber: accNum,
113+
Sequence: accSeq,
114+
PubKey: msigPubKey,
115+
}
116+
117+
_, err = cosmostx.SignWithPrivKey(
118+
context.Background(), signMode, signerData, txBuilder, priv1, txCfg, accSeq,
119+
)
120+
// TODO: remove this assertion once the upstream amino.name fix lands.
121+
// The malformed amino.name on MsgSetRoutingIsmDomain causes the amino
122+
// JSON sign mode handler to reject the Route field as unknown. See
123+
// https://github.com/celestiaorg/celestia-app/issues/6541
124+
s.Require().Error(err, "signing MsgSetRoutingIsmDomain with amino JSON should fail due to malformed amino.name")
125+
s.Require().ErrorContains(err, "unknown protobuf field")
126+
127+
// Once the bug is fixed, replace the above assertions with the following
128+
// to verify the full multisig flow works end-to-end:
129+
//
130+
// s.Require().NoError(err)
131+
//
132+
// sig2, err := cosmostx.SignWithPrivKey(
133+
// context.Background(), signMode, signerData, txBuilder, priv2, txCfg, accSeq,
134+
// )
135+
// s.Require().NoError(err)
136+
//
137+
// msigData := multisigtypes.NewMultisig(len(pubKeys))
138+
// err = multisigtypes.AddSignatureV2(msigData, sig1, pubKeys)
139+
// s.Require().NoError(err)
140+
// err = multisigtypes.AddSignatureV2(msigData, sig2, pubKeys)
141+
// s.Require().NoError(err)
142+
//
143+
// finalSig := signing.SignatureV2{
144+
// PubKey: msigPubKey,
145+
// Data: msigData,
146+
// Sequence: accSeq,
147+
// }
148+
// err = txBuilder.SetSignatures(finalSig)
149+
// s.Require().NoError(err)
150+
//
151+
// txBytes, err := txCfg.TxEncoder()(txBuilder.GetTx())
152+
// s.Require().NoError(err)
153+
//
154+
// resp, err := chain.App.GetBaseApp().FinalizeBlock(&abci.RequestFinalizeBlock{
155+
// Height: chain.App.GetBaseApp().LastBlockHeight() + 1,
156+
// Time: chain.CurrentHeader.GetTime(),
157+
// NextValidatorsHash: chain.NextVals.Hash(),
158+
// Txs: [][]byte{txBytes},
159+
// })
160+
// s.Require().NoError(err)
161+
// s.Require().Len(resp.TxResults, 1)
162+
//
163+
// txResult := resp.TxResults[0]
164+
// s.Require().Equal(uint32(0), txResult.Code,
165+
// "MsgSetRoutingIsmDomain from multisig should succeed but got code %d: %s",
166+
// txResult.Code, txResult.Log)
167+
}

0 commit comments

Comments
 (0)