Refactor to use a single transaction builder

.github/workflows/release-sims.yml

@@ -30,7 +30,7 @@ jobs:
       - name: install runsim
         run: |
           export GO111MODULE="on" && go get github.com/cosmos/tools/cmd/[email protected]
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-runsim-binary
@@ -40,7 +40,7 @@ jobs:
     needs: [build, install-runsim]
     steps:
       - uses: actions/checkout@v2
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-runsim-binary

.github/workflows/sims.yml

@@ -39,7 +39,7 @@ jobs:
         run: go version
       - name: Install runsim
         run: export GO111MODULE="on" && go get github.com/cosmos/tools/cmd/[email protected]
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-runsim-binary
@@ -60,7 +60,7 @@ jobs:
             **/**.go
             go.mod
             go.sum
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-runsim-binary
@@ -88,7 +88,7 @@ jobs:
             go.sum
           SET_ENV_NAME_INSERTIONS: 1
           SET_ENV_NAME_LINES: 1
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-runsim-binary
@@ -116,7 +116,7 @@ jobs:
             go.sum
           SET_ENV_NAME_INSERTIONS: 1
           SET_ENV_NAME_LINES: 1
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-runsim-binary
@@ -144,7 +144,7 @@ jobs:
             go.sum
           SET_ENV_NAME_INSERTIONS: 1
           SET_ENV_NAME_LINES: 1
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-runsim-binary

.github/workflows/tag.yml

@@ -19,7 +19,7 @@ jobs:
       - name: Unshallow
         run: git fetch --prune --unshallow
       - name: Create release
-        uses: goreleaser/goreleaser-action@v2.7.0
+        uses: goreleaser/goreleaser-action@v2.8.0
         with:
           args: release --rm-dist --release-notes ./RELEASE_CHANGELOG.md
         env:

.github/workflows/test.yml

@@ -26,7 +26,7 @@ jobs:
       - name: install tparse
         run: |
           export GO111MODULE="on" && go get github.com/mfridman/[email protected]
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-tparse-binary
@@ -218,7 +218,7 @@ jobs:
         with:
           name: "${{ github.sha }}-03-race-output"
         if: env.GIT_DIFF
-      - uses: actions/[email protected].6
+      - uses: actions/[email protected].7
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-go-tparse-binary

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)
 	}

go.mod

@@ -3,7 +3,7 @@ module github.com/celestiaorg/celestia-app
 go 1.17
 
 require (
-	github.com/celestiaorg/nmt v0.7.0
+	github.com/celestiaorg/nmt v0.8.0
 	github.com/cosmos/cosmos-sdk v0.44.0
 	github.com/cosmos/ibc-go v1.2.0
 	github.com/gogo/protobuf v1.3.3

go.sum

@@ -143,8 +143,9 @@ github.com/celestiaorg/go-leopard v0.1.0 h1:28z2EkvKJIez5J9CEaiiUEC+OxalRLtTGJJ1
 github.com/celestiaorg/go-leopard v0.1.0/go.mod h1:NtO/rjlB8dw2aq7jr06vZFKGvryQcTDXaNHelmPNOAM=
 github.com/celestiaorg/merkletree v0.0.0-20210714075610-a84dc3ddbbe4 h1:CJdIpo8n5MFP2MwK0gSRcOVlDlFdQJO1p+FqdxYzmvc=
 github.com/celestiaorg/merkletree v0.0.0-20210714075610-a84dc3ddbbe4/go.mod h1:fzuHnhzj1pUygGz+1ZkB3uQbEUL4htqCGJ4Qs2LwMZA=
-github.com/celestiaorg/nmt v0.7.0 h1:XRYh7F7TH6ewD3Gybk1L1zjyvgkWPt1iGqWtbMtB2Lk=
 github.com/celestiaorg/nmt v0.7.0/go.mod h1:3bqzTj8xKj0DgQUpOgZzoxvtNkC3MS/hTbQ6dn8SIa0=
+github.com/celestiaorg/nmt v0.8.0 h1:wtX7GRouLbmBe+ffnc8+cOg2UbWteM+Y1imZuZ/EeqU=
+github.com/celestiaorg/nmt v0.8.0/go.mod h1:3bqzTj8xKj0DgQUpOgZzoxvtNkC3MS/hTbQ6dn8SIa0=
 github.com/celestiaorg/rsmt2d v0.3.0 h1:c82dh9J3DBcIQu9OND+5HViUKb5NmpKYVvf8z3zjlfA=
 github.com/celestiaorg/rsmt2d v0.3.0/go.mod h1:2Frw4GEYUnVu6Mvlo+CUzuC2/8wn+zLwVVtp+muN6vg=
 github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4=

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
 			}

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
+	}
+}

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)

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
+}

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