From 8d52f4f076c0f3e8a360b7b36b8f0a8f35b40c38 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Thu, 7 Nov 2024 17:12:27 +0100 Subject: [PATCH 1/2] feat: check for correct signer in PFB construction --- pkg/user/signer.go | 4 ---- x/blob/types/payforblob.go | 26 ++++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) 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..02764c17a1 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) { From 940492da83e1b9b266ea318a8d44ad29f5f486dc Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Thu, 7 Nov 2024 17:36:05 +0100 Subject: [PATCH 2/2] fix tests --- app/test/check_tx_test.go | 11 +++++++++-- app/test/process_proposal_test.go | 3 ++- x/blob/types/payforblob.go | 2 +- 3 files changed, 12 insertions(+), 4 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/x/blob/types/payforblob.go b/x/blob/types/payforblob.go index 02764c17a1..b282925323 100644 --- a/x/blob/types/payforblob.go +++ b/x/blob/types/payforblob.go @@ -232,7 +232,7 @@ func ValidateBlobs(blobs ...*share.Blob) error { // 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)) { + 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) } }