chore!: rename tx option for better clarity (#3649)

We had an instance of someone putting the fee as the second argument instead of the gas price. I realised having `SetFee` in the function signature is misleading so I'm proposing to change it
celestiaorg · Jul 3, 2024 · 8caa580 · 8caa580
1 parent 97121d0
commit 8caa580
Show file tree

Hide file tree

Showing 8 changed files with 12 additions and 24 deletions.
diff --git a/app/errors/insufficient_gas_price_test.go b/app/errors/insufficient_gas_price_test.go
@@ -26,7 +26,7 @@ import (
 func TestInsufficientMinGasPriceIntegration(t *testing.T) {
 	var (
 		gasLimit  uint64 = 1_000_000
-		feeAmount int64  = 10
+		feeAmount uint64 = 10
 		gasPrice         = float64(feeAmount) / float64(gasLimit)
 	)
 	account := "test"
@@ -40,14 +40,13 @@ func TestInsufficientMinGasPriceIntegration(t *testing.T) {
 	signer, err := user.NewSigner(kr, enc.TxConfig, testutil.ChainID, appconsts.LatestVersion, user.NewAccount(account, acc.GetAccountNumber(), acc.GetSequence()))
 	require.NoError(t, err)
 
-	fee := sdk.NewCoins(sdk.NewCoin(app.BondDenom, sdk.NewInt(feeAmount)))
 	b, err := blob.NewBlob(namespace.RandomNamespace(), []byte("hello world"), 0)
 	require.NoError(t, err)
 
 	msg, err := blob.NewMsgPayForBlobs(signer.Account(account).Address().String(), appconsts.LatestVersion, b)
 	require.NoError(t, err)
 
-	rawTx, err := signer.CreateTx([]sdk.Msg{msg}, user.SetGasLimit(gasLimit), user.SetFeeAmount(fee))
+	rawTx, err := signer.CreateTx([]sdk.Msg{msg}, user.SetGasLimit(gasLimit), user.SetFee(feeAmount))
 	require.NoError(t, err)
 
 	decorator := ante.NewDeductFeeDecorator(testApp.AccountKeeper, testApp.BankKeeper, testApp.FeeGrantKeeper, nil)

diff --git a/app/test/big_blob_test.go b/app/test/big_blob_test.go
@@ -80,7 +80,7 @@ func (s *BigBlobSuite) TestErrBlobsTooLarge() {
 		s.Run(tc.name, func() {
 			subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), 30*time.Second)
 			defer cancel()
-			res, err := txClient.SubmitPayForBlob(subCtx, []*blob.Blob{tc.blob}, user.SetGasLimitAndFee(1e9, appconsts.DefaultMinGasPrice))
+			res, err := txClient.SubmitPayForBlob(subCtx, []*blob.Blob{tc.blob}, user.SetGasLimitAndGasPrice(1e9, appconsts.DefaultMinGasPrice))
 			require.Error(t, err)
 			require.NotNil(t, res)
 			require.Equal(t, tc.want, res.Code, res.Logs)

diff --git a/app/test/fuzz_abci_test.go b/app/test/fuzz_abci_test.go
@@ -104,7 +104,7 @@ func TestPrepareProposalConsistency(t *testing.T) {
 						true,
 						testutil.ChainID,
 						accounts[:tt.count],
-						user.SetGasLimitAndFee(1_000_000_000, 0.1),
+						user.SetGasLimitAndGasPrice(1_000_000_000, 0.1),
 					)
 					// create 100 send transactions
 					sendTxs := testutil.SendTxsWithAccounts(
@@ -116,7 +116,7 @@ func TestPrepareProposalConsistency(t *testing.T) {
 						accounts[0],
 						accounts[len(accounts)-sendTxCount:],
 						testutil.ChainID,
-						user.SetGasLimitAndFee(1_000_000, 0.1),
+						user.SetGasLimitAndGasPrice(1_000_000, 0.1),
 					)
 					txs = append(txs, sendTxs...)
 

diff --git a/app/test/priority_test.go b/app/test/priority_test.go
@@ -90,7 +90,7 @@ func (s *PriorityTestSuite) TestPriorityByGasPrice() {
 				s.cctx.GoContext(),
 				accName,
 				blobs,
-				user.SetGasLimitAndFee(gasLimit, gasPrice),
+				user.SetGasLimitAndGasPrice(gasLimit, gasPrice),
 			)
 			require.NoError(t, err)
 			require.Equal(t, abci.CodeTypeOK, resp.Code, resp.RawLog)

diff --git a/pkg/user/e2e_test.go b/pkg/user/e2e_test.go
@@ -49,7 +49,7 @@ func TestConcurrentTxSubmission(t *testing.T) {
 		wg.Add(1)
 		go func(b *blob.Blob) {
 			defer wg.Done()
-			_, err := txClient.SubmitPayForBlob(subCtx, []*blob.Blob{b}, user.SetGasLimitAndFee(500_000, appconsts.DefaultMinGasPrice))
+			_, err := txClient.SubmitPayForBlob(subCtx, []*blob.Blob{b}, user.SetGasLimitAndGasPrice(500_000, appconsts.DefaultMinGasPrice))
 			if err != nil && !errors.Is(err, context.Canceled) {
 				// only catch the first error
 				select {

diff --git a/pkg/user/tx_client.go b/pkg/user/tx_client.go
@@ -375,7 +375,7 @@ func (client *TxClient) retryBroadcastingTx(ctx context.Context, txBytes []byte)
 		opts = append(opts, SetMemo(memo))
 	}
 	if fee := tx.GetFee(); fee != nil {
-		opts = append(opts, SetFeeAmount(fee))
+		opts = append(opts, SetFee(fee.AmountOf(appconsts.BondDenom).Uint64()))
 	}
 	if gas := tx.GetGas(); gas > 0 {
 		opts = append(opts, SetGasLimit(gas))

diff --git a/pkg/user/tx_options.go b/pkg/user/tx_options.go
@@ -18,13 +18,6 @@ func SetGasLimit(limit uint64) TxOption {
 	}
 }
 
-func SetFeeAmount(fees sdk.Coins) TxOption {
-	return func(builder sdkclient.TxBuilder) sdkclient.TxBuilder {
-		builder.SetFeeAmount(fees)
-		return builder
-	}
-}
-
 func SetFee(fees uint64) TxOption {
 	return func(builder sdkclient.TxBuilder) sdkclient.TxBuilder {
 		builder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(appconsts.BondDenom, sdk.NewInt(int64(fees)))))
@@ -67,10 +60,10 @@ func SetFeeGranter(feeGranter sdk.AccAddress) TxOption {
 	}
 }
 
-// SetGasLimitAndFee sets the gas limit and fee using the provided gas price and
+// SetGasLimitAndGasPrice sets the gas limit and fee using the provided gas price and
 // gas limit. Note that this could overwrite or be overwritten by other
 // conflicting TxOptions.
-func SetGasLimitAndFee(gasLimit uint64, gasPrice float64) TxOption {
+func SetGasLimitAndGasPrice(gasLimit uint64, gasPrice float64) TxOption {
 	return func(builder sdkclient.TxBuilder) sdkclient.TxBuilder {
 		builder.SetGasLimit(gasLimit)
 		builder.SetFeeAmount(

diff --git a/x/blob/types/estimate_gas_test.go b/x/blob/types/estimate_gas_test.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"testing"
 
-	"cosmossdk.io/math"
 	"github.com/celestiaorg/celestia-app/v2/app"
 	"github.com/celestiaorg/celestia-app/v2/app/encoding"
 	"github.com/celestiaorg/celestia-app/v2/pkg/appconsts"
@@ -16,7 +15,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	blobtypes "github.com/celestiaorg/celestia-app/v2/x/blob/types"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
 )
@@ -44,8 +42,7 @@ func TestPFBGasEstimation(t *testing.T) {
 			require.NoError(t, err)
 			blobs := blobfactory.ManyRandBlobs(rand, tc.blobSizes...)
 			gas := blobtypes.DefaultEstimateGas(toUint32(tc.blobSizes))
-			fee := sdk.NewCoins(sdk.NewCoin(app.BondDenom, math.NewInt(int64(gas))))
-			tx, _, err := signer.CreatePayForBlobs(accnts[0], blobs, user.SetGasLimit(gas), user.SetFeeAmount(fee))
+			tx, _, err := signer.CreatePayForBlobs(accnts[0], blobs, user.SetGasLimitAndGasPrice(gas, appconsts.DefaultMinGasPrice))
 			require.NoError(t, err)
 			blobTx, ok := blob.UnmarshalBlobTx(tx)
 			require.True(t, ok)
@@ -88,8 +85,7 @@ func FuzzPFBGasEstimation(f *testing.F) {
 		require.NoError(t, err)
 		blobs := blobfactory.ManyRandBlobs(rand, blobSizes...)
 		gas := blobtypes.DefaultEstimateGas(toUint32(blobSizes))
-		fee := sdk.NewCoins(sdk.NewCoin(app.BondDenom, math.NewInt(int64(gas))))
-		tx, _, err := signer.CreatePayForBlobs(accnts[0], blobs, user.SetGasLimit(gas), user.SetFeeAmount(fee))
+		tx, _, err := signer.CreatePayForBlobs(accnts[0], blobs, user.SetGasLimitAndGasPrice(gas, appconsts.DefaultMinGasPrice))
 		require.NoError(t, err)
 		blobTx, ok := blob.UnmarshalBlobTx(tx)
 		require.True(t, ok)