diff --git a/app/abci_test.go b/app/abci_test.go index 05ea46a6c7..b0d206587c 100644 --- a/app/abci_test.go +++ b/app/abci_test.go @@ -241,7 +241,7 @@ func generateSignedWirePayForMessage(t *testing.T, k uint64, ns, message []byte, t.Error(err) } - err = msg.SignShareCommitments(signer, signer.NewTxBuilder()) + err = msg.SignShareCommitments(signer) if err != nil { t.Error(err) } diff --git a/x/payment/client/cli/wirepayformessage.go b/x/payment/client/cli/wirepayformessage.go index 179da13c87..f54666b5f1 100644 --- a/x/payment/client/cli/wirepayformessage.go +++ b/x/payment/client/cli/wirepayformessage.go @@ -57,6 +57,7 @@ func CmdWirePayForMessage() *cobra.Command { return err } + // use the keyring to programmatically sign multiple PayForMessage txs signer := types.NewKeyringSigner(clientCtx.Keyring, accName, clientCtx.ChainID) signer.SetAccountNumber(account.GetAccountNumber()) @@ -82,13 +83,12 @@ func CmdWirePayForMessage() *cobra.Command { return err } - // get the gas price and such and add it to the tx builder that is used to create the signed share commitments - builder := signer.NewTxBuilder() - builder.SetGasLimit(gasSetting.Gas) - builder.SetFeeAmount(parsedFees) - // sign the MsgPayForMessage's ShareCommitments - err = pfmMsg.SignShareCommitments(signer, builder) + err = pfmMsg.SignShareCommitments( + signer, + types.SetGasLimit(gasSetting.Gas), + types.SetFeeAmount(parsedFees), + ) if err != nil { return err } diff --git a/x/payment/types/builder_options.go b/x/payment/types/builder_options.go new file mode 100644 index 0000000000..76a5e1670d --- /dev/null +++ b/x/payment/types/builder_options.go @@ -0,0 +1,22 @@ +package types + +import ( + sdkclient "github.com/cosmos/cosmos-sdk/client" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +type TxBuilderOption func(builder sdkclient.TxBuilder) sdkclient.TxBuilder + +func SetGasLimit(limit uint64) TxBuilderOption { + return func(builder sdkclient.TxBuilder) sdkclient.TxBuilder { + builder.SetGasLimit(limit) + return builder + } +} + +func SetFeeAmount(fees sdk.Coins) TxBuilderOption { + return func(builder sdkclient.TxBuilder) sdkclient.TxBuilder { + builder.SetFeeAmount(fees) + return builder + } +} diff --git a/x/payment/types/builder_test.go b/x/payment/types/builder_test.go index 097fb69f91..a4554843a1 100644 --- a/x/payment/types/builder_test.go +++ b/x/payment/types/builder_test.go @@ -15,7 +15,7 @@ import ( "google.golang.org/grpc" ) -func TestBuildPayForMessage(t *testing.T) { +func TestBuildWirePayForMessage(t *testing.T) { testRing := generateKeyring(t) info, err := testRing.Key(testAccName) @@ -36,8 +36,8 @@ func TestBuildPayForMessage(t *testing.T) { rawTx, err := makeEncodingConfig().TxConfig.TxEncoder()(signedTx) require.NoError(t, err) - _, _, isChild := coretypes.UnwrapMalleatedTx(rawTx) - require.False(t, isChild) + _, _, isMalleated := coretypes.UnwrapMalleatedTx(rawTx) + require.False(t, isMalleated) sigs, err := signedTx.GetSignaturesV2() require.NoError(t, err) diff --git a/x/payment/types/payformessage_test.go b/x/payment/types/payformessage_test.go index 2ee63b408a..e071c7bd8e 100644 --- a/x/payment/types/payformessage_test.go +++ b/x/payment/types/payformessage_test.go @@ -2,8 +2,11 @@ package types import ( "bytes" + "fmt" "testing" + sdkclient "github.com/cosmos/cosmos-sdk/client" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx/signing" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/stretchr/testify/assert" @@ -136,11 +139,15 @@ func TestPadMessage(t *testing.T) { } } -func TestSignShareCommitments(t *testing.T) { +// TestSignMalleatedTxs checks to see that the signatures that are generated for +// the PayForMessages malleated from the original WirePayForMessage are actually +// valid. +func TestSignMalleatedTxs(t *testing.T) { type test struct { name string ns, msg []byte - ss uint64 + ss []uint64 + options []TxBuilderOption } kb := generateKeyring(t, "test") @@ -149,52 +156,67 @@ func TestSignShareCommitments(t *testing.T) { tests := []test{ { - name: "single share square size 2", - ns: []byte{1, 1, 1, 1, 1, 1, 1, 1}, - msg: bytes.Repeat([]byte{1}, ShareSize-8), - ss: 2, + name: "single share", + ns: []byte{1, 1, 1, 1, 1, 1, 1, 1}, + msg: bytes.Repeat([]byte{1}, ShareSize-8), + ss: []uint64{2, 4, 8, 16}, + options: []TxBuilderOption{SetGasLimit(2000000)}, }, { - name: "15 shares square size 4", + name: "15 shares", ns: []byte{1, 1, 1, 1, 1, 1, 1, 2}, - msg: bytes.Repeat([]byte{2}, ShareSize*15), - ss: 4, + msg: bytes.Repeat([]byte{2}, ShareSize*12), + ss: []uint64{4, 8, 16, 64}, + options: []TxBuilderOption{ + SetGasLimit(123456789), + SetFeeAmount(sdk.NewCoins(sdk.NewCoin("tio", sdk.NewInt(987654321))))}, }, } for _, tt := range tests { - wpfm, err := NewWirePayForMessage(tt.ns, tt.msg, tt.ss) + wpfm, err := NewWirePayForMessage(tt.ns, tt.msg, tt.ss...) require.NoError(t, err, tt.name) - err = wpfm.SignShareCommitments(signer, signer.NewTxBuilder()) + err = wpfm.SignShareCommitments(signer, tt.options...) // there should be no error assert.NoError(t, err) // the signature should exist assert.Equal(t, len(wpfm.MessageShareCommitment[0].Signature), 64) - // verify the signature - unsignedPFM, err := wpfm.unsignedPayForMessage(tt.ss) - require.NoError(t, err) - tx, err := signer.BuildSignedTx(signer.NewTxBuilder(), unsignedPFM) - require.NoError(t, err) - - // Generate the bytes to be signed. - bytesToSign, err := signer.encCfg.TxConfig.SignModeHandler().GetSignBytes( - signing.SignMode_SIGN_MODE_DIRECT, - authsigning.SignerData{ - ChainID: signer.chainID, - AccountNumber: signer.accountNumber, - Sequence: signer.sequence, - }, - tx, - ) - require.NoError(t, err) - - // verify the signature using the public key - assert.True(t, signer.GetSignerInfo().GetPubKey().VerifySignature( - bytesToSign, - wpfm.MessageShareCommitment[0].Signature, - )) - + // verify the signature for the PayForMessages derived from the + // WirePayForMessage + for i, size := range tt.ss { + unsignedPFM, err := wpfm.unsignedPayForMessage(size) + require.NoError(t, err) + + // create a new tx builder to create an unsigned PayForMessage + builder := applyOptions(signer.NewTxBuilder(), tt.options...) + tx, err := signer.BuildSignedTx(builder, unsignedPFM) + require.NoError(t, err) + + // Generate the bytes to be signed. + bytesToSign, err := signer.encCfg.TxConfig.SignModeHandler().GetSignBytes( + signing.SignMode_SIGN_MODE_DIRECT, + authsigning.SignerData{ + ChainID: signer.chainID, + AccountNumber: signer.accountNumber, + Sequence: signer.sequence, + }, + tx, + ) + require.NoError(t, err) + + // compare the commitments generated by the WirePayForMessage with + // that of independently generated PayForMessage + assert.Equal(t, unsignedPFM.MessageShareCommitment, wpfm.MessageShareCommitment[i].ShareCommitment) + + // verify the signature + assert.True(t, signer.GetSignerInfo().GetPubKey().VerifySignature( + bytesToSign, + wpfm.MessageShareCommitment[i].Signature, + ), + fmt.Sprintf("test: %s size: %d", tt.name, size), + ) + } } } @@ -325,7 +347,7 @@ func TestProcessMessage(t *testing.T) { for _, tt := range tests { wpfm, err := NewWirePayForMessage(tt.ns, tt.msg, tt.ss) require.NoError(t, err, tt.name) - err = wpfm.SignShareCommitments(signer, signer.NewTxBuilder()) + err = wpfm.SignShareCommitments(signer) assert.NoError(t, err) wpfm = tt.modify(wpfm) @@ -358,9 +380,16 @@ func validWirePayForMessage(t *testing.T) *MsgWirePayForMessage { signer := generateKeyringSigner(t) - err = msg.SignShareCommitments(signer, signer.NewTxBuilder()) + err = msg.SignShareCommitments(signer) if err != nil { panic(err) } return msg } + +func applyOptions(builder sdkclient.TxBuilder, options ...TxBuilderOption) sdkclient.TxBuilder { + for _, option := range options { + builder = option(builder) + } + return builder +} diff --git a/x/payment/types/wirepayformessage.go b/x/payment/types/wirepayformessage.go index df1d797990..5ad1a0761d 100644 --- a/x/payment/types/wirepayformessage.go +++ b/x/payment/types/wirepayformessage.go @@ -41,10 +41,16 @@ func NewWirePayForMessage(namespace, message []byte, sizes ...uint64) (*MsgWireP // SignShareCommitments creates and signs MsgPayForMessages for each square size configured in the MsgWirePayForMessage // to complete each shares commitment. -func (msg *MsgWirePayForMessage) SignShareCommitments(signer *KeyringSigner, builder sdkclient.TxBuilder) error { +func (msg *MsgWirePayForMessage) SignShareCommitments(signer *KeyringSigner, options ...TxBuilderOption) error { msg.Signer = signer.GetSignerInfo().GetAddress().String() // create an entire MsgPayForMessage and signing over it, including the signature in each commitment for i, commit := range msg.MessageShareCommitment { + builder := signer.NewTxBuilder() + + for _, option := range options { + builder = option(builder) + } + sig, err := msg.createPayForMessageSignature(signer, builder, commit.K) if err != nil { return err