Skip to content

Commit

Permalink
feat: check for correct signer in PFB construction (#4027)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
cmwaters authored Nov 8, 2024
1 parent 00088ee commit 55247cb
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
11 changes: 9 additions & 2 deletions app/test/check_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
Expand Down
3 changes: 2 additions & 1 deletion app/test/process_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions pkg/user/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 24 additions & 2 deletions x/blob/types/payforblob.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types

import (
"bytes"
fmt "fmt"

"cosmossdk.io/errors"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 55247cb

Please sign in to comment.