From 55247cb0a1d2bb05d3aca761cb80e6834a48be07 Mon Sep 17 00:00:00 2001
From: Callum Waters <cmwaters19@gmail.com>
Date: Fri, 8 Nov 2024 13:17:11 +0100
Subject: [PATCH] feat: check for correct signer in PFB construction (#4027)

We already check this in `CheckTx` and `ProcessProposal`. This adds the
same check to the client side construction so it errors before being
submitted to the network. (the check being that the signer in the blob
and the actual signer of the PFB are the same)
---
 app/test/check_tx_test.go         | 11 +++++++++--
 app/test/process_proposal_test.go |  3 ++-
 pkg/user/signer.go                |  4 ----
 x/blob/types/payforblob.go        | 26 ++++++++++++++++++++++++--
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/app/test/check_tx_test.go b/app/test/check_tx_test.go
index 45ac426d88..aba7720547 100644
--- a/app/test/check_tx_test.go
+++ b/app/test/check_tx_test.go
@@ -189,11 +189,18 @@ func TestCheckTx(t *testing.T) {
 			checkType: abci.CheckTxType_New,
 			getTx: func() []byte {
 				signer := createSigner(t, kr, accs[10], encCfg.TxConfig, 11)
-				blob, err := share.NewV1Blob(share.RandomBlobNamespace(), []byte("data"), testnode.RandomAddress().(sdk.AccAddress))
+				blob, err := share.NewV1Blob(share.RandomBlobNamespace(), []byte("data"), signer.Account(accs[10]).Address())
 				require.NoError(t, err)
 				blobTx, _, err := signer.CreatePayForBlobs(accs[10], []*share.Blob{blob}, opts...)
 				require.NoError(t, err)
-				return blobTx
+				blob, err = share.NewV1Blob(share.RandomBlobNamespace(), []byte("data"), testnode.RandomAddress().(sdk.AccAddress))
+				require.NoError(t, err)
+				bTx, _, err := tx.UnmarshalBlobTx(blobTx)
+				require.NoError(t, err)
+				bTx.Blobs[0] = blob
+				blobTxBytes, err := tx.MarshalBlobTx(bTx.Tx, bTx.Blobs[0])
+				require.NoError(t, err)
+				return blobTxBytes
 			},
 			expectedABCICode: blobtypes.ErrInvalidBlobSigner.ABCICode(),
 		},
diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go
index 69f18b8bd4..33b1d406c5 100644
--- a/app/test/process_proposal_test.go
+++ b/app/test/process_proposal_test.go
@@ -321,8 +321,9 @@ func TestProcessProposal(t *testing.T) {
 				falseAddr := testnode.RandomAddress().(sdk.AccAddress)
 				blob, err := share.NewV1Blob(ns1, data, falseAddr)
 				require.NoError(t, err)
-				msg, err := blobtypes.NewMsgPayForBlobs(addr.String(), appconsts.LatestVersion, blob)
+				msg, err := blobtypes.NewMsgPayForBlobs(falseAddr.String(), appconsts.LatestVersion, blob)
 				require.NoError(t, err)
+				msg.Signer = addr.String()
 
 				rawTx, err := signer.CreateTx([]sdk.Msg{msg}, user.SetGasLimit(100000), user.SetFee(100000))
 				require.NoError(t, err)
diff --git a/pkg/user/signer.go b/pkg/user/signer.go
index 4b39c9bd31..0340e3ad57 100644
--- a/pkg/user/signer.go
+++ b/pkg/user/signer.go
@@ -92,10 +92,6 @@ func (s *Signer) CreatePayForBlobs(accountName string, blobs []*share.Blob, opts
 		return nil, 0, fmt.Errorf("account %s not found", accountName)
 	}
 
-	if err := blobtypes.ValidateBlobs(blobs...); err != nil {
-		return nil, 0, err
-	}
-
 	msg, err := blobtypes.NewMsgPayForBlobs(acc.address.String(), s.appVersion, blobs...)
 	if err != nil {
 		return nil, 0, err
diff --git a/x/blob/types/payforblob.go b/x/blob/types/payforblob.go
index b49eb05394..b282925323 100644
--- a/x/blob/types/payforblob.go
+++ b/x/blob/types/payforblob.go
@@ -1,6 +1,7 @@
 package types
 
 import (
+	"bytes"
 	fmt "fmt"
 
 	"cosmossdk.io/errors"
@@ -42,12 +43,23 @@ const (
 // See: https://github.com/cosmos/cosmos-sdk/blob/v0.46.15/docs/building-modules/messages-and-queries.md#legacy-amino-legacymsgs
 var _ legacytx.LegacyMsg = &MsgPayForBlobs{}
 
-func NewMsgPayForBlobs(signer string, version uint64, blobs ...*share.Blob) (*MsgPayForBlobs, error) {
+func NewMsgPayForBlobs(signer string, appVersion uint64, blobs ...*share.Blob) (*MsgPayForBlobs, error) {
 	err := ValidateBlobs(blobs...)
 	if err != nil {
 		return nil, err
 	}
-	commitments, err := inclusion.CreateCommitments(blobs, merkle.HashFromByteSlices, appconsts.SubtreeRootThreshold(version))
+
+	signerBytes, err := sdk.AccAddressFromBech32(signer)
+	if err != nil {
+		return nil, err
+	}
+
+	err = ValidateBlobShareVersion(signerBytes, blobs...)
+	if err != nil {
+		return nil, err
+	}
+
+	commitments, err := inclusion.CreateCommitments(blobs, merkle.HashFromByteSlices, appconsts.SubtreeRootThreshold(appVersion))
 	if err != nil {
 		return nil, fmt.Errorf("creating commitments: %w", err)
 	}
@@ -217,6 +229,16 @@ func ValidateBlobs(blobs ...*share.Blob) error {
 	return nil
 }
 
+// ValidateBlobShareVersion validates any share version specific rules
+func ValidateBlobShareVersion(signer sdk.AccAddress, blobs ...*share.Blob) error {
+	for _, blob := range blobs {
+		if blob.ShareVersion() == share.ShareVersionOne && !bytes.Equal(blob.Signer(), []byte(signer)) {
+			return ErrInvalidBlobSigner.Wrapf("blob signer %X does not match msgPFB signer %X", blob.Signer(), signer)
+		}
+	}
+	return nil
+}
+
 // ExtractBlobComponents separates and returns the components of a slice of
 // blobs.
 func ExtractBlobComponents(pblobs []*share.Blob) (namespaces []share.Namespace, sizes []uint32, shareVersions []uint32) {