Skip to content

Commit

Permalink
Merge branch 'main' into fix/go-critics-support
Browse files Browse the repository at this point in the history
  • Loading branch information
rootulp authored Jan 21, 2024
2 parents 5a466bc + e05d105 commit 6ff49d8
Show file tree
Hide file tree
Showing 15 changed files with 324 additions and 239 deletions.
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@
# owner is fully deferring ownership to the directory owner
docs @liamsi @adlerjohn @evan-forbes @rootulp @cmwaters
specs @liamsi @adlerjohn @evan-forbes @rootulp @cmwaters
x/blobstream @SweeXordious @evan-forbes
x/blobstream @rach-id @evan-forbes
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -197,5 +197,5 @@ prebuilt-binary:

## govulncheck: Check for vulnerabilities in dependencies.
govulncheck:
@go run golang.org/x/vuln/cmd/govulncheck@latest ./...
@go run golang.org/x/vuln/cmd/govulncheck@v1.0.1 ./...
.PHONY: govulncheck
2 changes: 1 addition & 1 deletion app/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func NewAnteHandler(
ante.NewConsumeGasForTxSizeDecorator(accountKeeper),
// Ensure the feepayer (fee granter or first signer) has enough funds to pay for the tx.
// Side effect: deducts fees from the fee payer. Sets the tx priority in context.
ante.NewDeductFeeDecorator(accountKeeper, bankKeeper, feegrantKeeper, checkTxFeeWithValidatorMinGasPrices),
ante.NewDeductFeeDecorator(accountKeeper, bankKeeper, feegrantKeeper, CheckTxFeeWithGlobalMinGasPrices),
// Set public keys in the context for fee-payer and all signers.
// Contract: must be called before all signature verification decorators.
ante.NewSetPubKeyDecorator(accountKeeper),
Expand Down
44 changes: 21 additions & 23 deletions app/ante/fee_checker.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package ante

import (
"fmt"

errors "cosmossdk.io/errors"
"github.com/celestiaorg/celestia-app/pkg/appconsts"
v1 "github.com/celestiaorg/celestia-app/pkg/appconsts/v1"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerror "github.com/cosmos/cosmos-sdk/types/errors"
)
Expand All @@ -11,41 +15,35 @@ const (
priorityScalingFactor = 1_000_000
)

// checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per
// unit of gas is fixed and set by each validator, and the tx priority is computed from the gas price.
func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
// CheckTxFeeWithGlobalMinGasPrices implements the default fee logic, where the minimum price per
// unit of gas is fixed and set globally, and the tx priority is computed from the gas price.
func CheckTxFeeWithGlobalMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
return nil, 0, errors.Wrap(sdkerror.ErrTxDecode, "Tx must be a FeeTx")
}

feeCoins := feeTx.GetFee()
fee := feeTx.GetFee().AmountOf(appconsts.BondDenom)
gas := feeTx.GetGas()

// Ensure that the provided fees meet a minimum threshold for the validator,
// if this is a CheckTx. This is only for local mempool purposes, and thus
// is only ran on check tx.
if ctx.IsCheckTx() {
minGasPrices := ctx.MinGasPrices()
if !minGasPrices.IsZero() {
requiredFees := make(sdk.Coins, len(minGasPrices))
// global minimum fee only applies to app versions greater than one
if ctx.BlockHeader().Version.App > v1.Version {
// convert the global minimum gas price to a big.Int
globalMinGasPrice, err := sdk.NewDecFromStr(fmt.Sprintf("%f", appconsts.GlobalMinGasPrice))
if err != nil {
return nil, 0, errors.Wrap(err, "invalid GlobalMinGasPrice")
}

// Determine the required fees by multiplying each required minimum gas
// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
glDec := sdk.NewDec(int64(gas))
for i, gp := range minGasPrices {
fee := gp.Amount.Mul(glDec)
requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
}
gasInt := sdk.NewIntFromUint64(gas)
minFee := globalMinGasPrice.MulInt(gasInt).RoundInt()

if !feeCoins.IsAnyGTE(requiredFees) {
return nil, 0, errors.Wrapf(sdkerror.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees)
}
if !fee.GTE(minFee) {
return nil, 0, errors.Wrapf(sdkerror.ErrInsufficientFee, "insufficient fees; got: %s required: %s", fee, minFee)
}
}

priority := getTxPriority(feeCoins, int64(gas))
return feeCoins, priority, nil
priority := getTxPriority(feeTx.GetFee(), int64(gas))
return feeTx.GetFee(), priority, nil
}

// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price
Expand Down
File renamed without changes.
111 changes: 111 additions & 0 deletions app/ante/min_fee_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package ante_test

import (
"math"
"testing"

"github.com/celestiaorg/celestia-app/app"
"github.com/celestiaorg/celestia-app/app/ante"
"github.com/celestiaorg/celestia-app/app/encoding"
"github.com/celestiaorg/celestia-app/pkg/appconsts"
"github.com/celestiaorg/celestia-app/test/util/testnode"
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
version "github.com/tendermint/tendermint/proto/tendermint/version"
)

func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)

builder := encCfg.TxConfig.NewTxBuilder()
err := builder.SetMsgs(banktypes.NewMsgSend(
testnode.RandomAddress().(sdk.AccAddress),
testnode.RandomAddress().(sdk.AccAddress),
sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, 10))),
)
require.NoError(t, err)

feeAmount := int64(1000)

ctx := sdk.Context{}

testCases := []struct {
name string
fee sdk.Coins
gasLimit uint64
appVersion uint64
expErr bool
}{
{
name: "bad tx; fee below required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
gasLimit: uint64(float64(feeAmount) / appconsts.GlobalMinGasPrice),
appVersion: uint64(2),
expErr: true,
},
{
name: "good tx; fee equal to required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: uint64(float64(feeAmount) / appconsts.GlobalMinGasPrice),
appVersion: uint64(2),
expErr: false,
},
{
name: "good tx; fee above required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
gasLimit: uint64(float64(feeAmount) / appconsts.GlobalMinGasPrice),
appVersion: uint64(2),
expErr: false,
},
{
name: "good tx; with no fee (v1)",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: uint64(float64(feeAmount) / appconsts.GlobalMinGasPrice),
appVersion: uint64(1),
expErr: false,
},
{
name: "good tx; gas limit and fee are maximum values",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, math.MaxInt64)),
gasLimit: math.MaxUint64,
appVersion: uint64(2),
expErr: false,
},
{
name: "bad tx; gas limit and fee are 0",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, 0)),
gasLimit: 0,
appVersion: uint64(2),
expErr: false,
},
{
name: "good tx; minFee = 0.8, rounds up to 1",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: 400,
appVersion: uint64(2),
expErr: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
builder.SetGasLimit(tc.gasLimit)
builder.SetFeeAmount(tc.fee)
tx := builder.GetTx()

ctx = ctx.WithBlockHeader(tmproto.Header{
Version: version.Consensus{
App: tc.appVersion,
},
})
_, _, err := ante.CheckTxFeeWithGlobalMinGasPrices(ctx, tx)
if tc.expErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}
13 changes: 9 additions & 4 deletions app/process_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"github.com/celestiaorg/celestia-app/app/ante"
v1 "github.com/celestiaorg/celestia-app/pkg/appconsts/v1"
"github.com/celestiaorg/celestia-app/pkg/blob"
"github.com/celestiaorg/celestia-app/pkg/da"
"github.com/celestiaorg/celestia-app/pkg/shares"
Expand Down Expand Up @@ -58,10 +59,14 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp

sdkTx, err := app.txConfig.TxDecoder()(tx)
if err != nil {
// we don't reject the block here because it is not a block validity
// rule that all transactions included in the block data are
// decodable
continue
if req.Header.Version.App == v1.Version {
// For appVersion 1, there was no block validity rule that all
// transactions must be decodable.
continue
}
// An error here means that a tx was included in the block that is not decodable.
logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("tx %d is not decodable", idx))
return reject()
}

// handle non-blob transactions first
Expand Down
2 changes: 1 addition & 1 deletion app/test/max_total_blob_size_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (s *MaxTotalBlobSizeSuite) TestErrTotalBlobSizeTooLarge() {

for _, tc := range testCases {
s.Run(tc.name, func() {
blobTx, err := signer.CreatePayForBlob([]*blob.Blob{tc.blob}, user.SetGasLimit(1e9))
blobTx, err := signer.CreatePayForBlob([]*blob.Blob{tc.blob}, user.SetGasLimit(1e9), user.SetFee(2000000))
require.NoError(t, err)
subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), 30*time.Second)
defer cancel()
Expand Down
2 changes: 1 addition & 1 deletion app/test/prepare_proposal_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestTimeInPrepareProposalContext(t *testing.T) {
addr := testfactory.GetAddress(cctx.Keyring, account)
signer, err := user.SetupSigner(cctx.GoContext(), cctx.Keyring, cctx.GRPCClient, addr, ecfg)
require.NoError(t, err)
res, err := signer.SubmitTx(cctx.GoContext(), msgs, user.SetGasLimit(1000000), user.SetFee(1))
res, err := signer.SubmitTx(cctx.GoContext(), msgs, user.SetGasLimit(1000000), user.SetFee(2000))
if tt.expectedCode != abci.CodeTypeOK {
require.Error(t, err)
} else {
Expand Down
Loading

0 comments on commit 6ff49d8

Please sign in to comment.