-
Notifications
You must be signed in to change notification settings - Fork 289
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: backport features to allow node to handle gas price estimation (#…
…2134) Backports #2122 --------- Co-authored-by: Rootul P <[email protected]>
- Loading branch information
Showing
8 changed files
with
296 additions
and
37 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package errors | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "regexp" | ||
| "strconv" | ||
|
|
||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
| ) | ||
|
|
||
| var ( | ||
| // This is relatively brittle. It would be better if going below the min gas price | ||
| // had a specific error type. | ||
| regexpMinGasPrice = regexp.MustCompile(`insufficient fees; got: \d+utia required: \d+utia`) | ||
| regexpInt = regexp.MustCompile(`[0-9]+`) | ||
| ) | ||
|
|
||
| // ParseInsufficientMinGasPrice checks if the error is due to the gas price being too low. | ||
| // Given the previous gas price and gas limit, it returns the new minimum gas price that | ||
| // the node should accept. | ||
| // If the error is not due to the gas price being too low, it returns 0, nil | ||
| func ParseInsufficientMinGasPrice(err error, gasPrice float64, gasLimit uint64) (float64, error) { | ||
| // first work out if the error is ErrInsufficientFunds | ||
| if err == nil || !sdkerrors.ErrInsufficientFee.Is(err) { | ||
| return 0, nil | ||
| } | ||
|
|
||
| // As there are multiple cases of ErrInsufficientFunds, we need to check the error message | ||
| // matches the regexp | ||
| substr := regexpMinGasPrice.FindAllString(err.Error(), -1) | ||
| if len(substr) != 1 { | ||
| return 0, nil | ||
| } | ||
|
|
||
| // extract the first and second numbers from the error message (got and required) | ||
| numbers := regexpInt.FindAllString(substr[0], -1) | ||
| if len(numbers) != 2 { | ||
| return 0, fmt.Errorf("expected two numbers in error message got %d", len(numbers)) | ||
| } | ||
|
|
||
| // attempt to parse them into float64 values | ||
| got, err := strconv.ParseFloat(numbers[0], 64) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| required, err := strconv.ParseFloat(numbers[1], 64) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
|
|
||
| // catch rare condition that required is zero. This should theoretically | ||
| // never happen as a min gas price of zero should always be accepted. | ||
| if required == 0 { | ||
| return 0, errors.New("unexpected case: required gas price is zero (why was an error returned)") | ||
| } | ||
|
|
||
| // calculate the actual min gas price of the node based on the difference | ||
| // between the got and required values. If gas price was zero, we need to use | ||
| // the gasLimit to infer this. | ||
| if gasPrice == 0 || got == 0 { | ||
| if gasLimit == 0 { | ||
| return 0, fmt.Errorf("gas limit and gas price cannot be zero") | ||
| } | ||
| return required / float64(gasLimit), nil | ||
| } | ||
| return required / got * gasPrice, nil | ||
| } | ||
|
|
||
| // IsInsufficientMinGasPrice checks if the error is due to the gas price being too low. | ||
| func IsInsufficientMinGasPrice(err error) bool { | ||
| // first work out if the error is ErrInsufficientFunds | ||
| if err == nil || !sdkerrors.ErrInsufficientFee.Is(err) { | ||
| return false | ||
| } | ||
|
|
||
| // As there are multiple cases of ErrInsufficientFunds, we need to check the error message | ||
| // matches the regexp | ||
| return regexpMinGasPrice.MatchString(err.Error()) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| package errors_test | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "cosmossdk.io/errors" | ||
| "github.com/celestiaorg/celestia-app/app" | ||
| apperr "github.com/celestiaorg/celestia-app/app/errors" | ||
| "github.com/celestiaorg/celestia-app/pkg/appconsts" | ||
| "github.com/celestiaorg/celestia-app/pkg/namespace" | ||
| testutil "github.com/celestiaorg/celestia-app/test/util" | ||
| blob "github.com/celestiaorg/celestia-app/x/blob/types" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
| "github.com/cosmos/cosmos-sdk/x/auth/ante" | ||
| "github.com/stretchr/testify/require" | ||
| tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | ||
| ) | ||
|
|
||
| // This will detect any changes to the DeductFeeDecorator which may cause a | ||
| // different error message that does not match the regexp. | ||
| func TestInsufficientMinGasPriceIntegration(t *testing.T) { | ||
| var ( | ||
| gasLimit uint64 = 1_000_000 | ||
| feeAmount int64 = 10 | ||
| gasPrice = float64(feeAmount) / float64(gasLimit) | ||
| ) | ||
| account := "test" | ||
| testApp, kr := testutil.SetupTestAppWithGenesisValSet(app.DefaultConsensusParams(), account) | ||
| minGasPrice, err := sdk.ParseDecCoins(fmt.Sprintf("%v%s", appconsts.DefaultMinGasPrice, app.BondDenom)) | ||
| require.NoError(t, err) | ||
| ctx := testApp.NewContext(true, tmproto.Header{}).WithMinGasPrices(minGasPrice) | ||
| signer := blob.NewKeyringSigner(kr, account, testutil.ChainID) | ||
| builder := signer.NewTxBuilder() | ||
| builder.SetGasLimit(gasLimit) | ||
| builder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(app.BondDenom, sdk.NewInt(feeAmount)))) | ||
|
|
||
| address, err := signer.GetSignerInfo().GetAddress() | ||
| require.NoError(t, err) | ||
|
|
||
| _, err = sdk.AccAddressFromBech32(address.String()) | ||
| require.NoError(t, err, address) | ||
|
|
||
| b, err := blob.NewBlob(namespace.RandomNamespace(), []byte("hello world"), 0) | ||
| require.NoError(t, err) | ||
|
|
||
| pfb, err := blob.NewMsgPayForBlobs(address.String(), b) | ||
| require.NoError(t, err, address) | ||
|
|
||
| tx, err := signer.BuildSignedTx(builder, pfb) | ||
| require.NoError(t, err) | ||
|
|
||
| decorator := ante.NewDeductFeeDecorator(testApp.AccountKeeper, testApp.BankKeeper, testApp.FeeGrantKeeper, nil) | ||
| anteHandler := sdk.ChainAnteDecorators(decorator) | ||
|
|
||
| _, err = anteHandler(ctx, tx, false) | ||
| require.True(t, apperr.IsInsufficientMinGasPrice(err)) | ||
| actualGasPrice, err := apperr.ParseInsufficientMinGasPrice(err, gasPrice, gasLimit) | ||
| require.NoError(t, err) | ||
| require.Equal(t, appconsts.DefaultMinGasPrice, actualGasPrice, err) | ||
| } | ||
|
|
||
| func TestInsufficientMinGasPriceTable(t *testing.T) { | ||
| testCases := []struct { | ||
| name string | ||
| err error | ||
| inputGasPrice float64 | ||
| inputGasLimit uint64 | ||
| isInsufficientMinGasPriceErr bool | ||
| expectParsingError bool | ||
| expectedGasPrice float64 | ||
| }{ | ||
| { | ||
| name: "nil error", | ||
| err: nil, | ||
| isInsufficientMinGasPriceErr: false, | ||
| }, | ||
| { | ||
| name: "not insufficient fee error", | ||
| err: errors.Wrap(sdkerrors.ErrInsufficientFee, "not enough gas to pay for blobs (minimum: 1000000, got: 100000)"), | ||
| isInsufficientMinGasPriceErr: false, | ||
| }, | ||
| { | ||
| name: "not insufficient fee error 2", | ||
| err: errors.Wrap(sdkerrors.ErrInsufficientFunds, "not enough gas to pay for blobs (got: 1000000, required: 100000)"), | ||
| isInsufficientMinGasPriceErr: false, | ||
| }, | ||
| { | ||
| name: "insufficient fee error", | ||
| err: errors.Wrap(sdkerrors.ErrInsufficientFee, "insufficient fees; got: 10utia required: 100utia"), | ||
| inputGasPrice: 0.01, | ||
| expectedGasPrice: 0.1, | ||
| isInsufficientMinGasPriceErr: true, | ||
| }, | ||
| { | ||
| name: "insufficient fee error with zero gas price", | ||
| err: errors.Wrap(sdkerrors.ErrInsufficientFee, "insufficient fees; got: 0utia required: 100utia"), | ||
| inputGasPrice: 0, | ||
| inputGasLimit: 100, | ||
| expectedGasPrice: 1, | ||
| isInsufficientMinGasPriceErr: true, | ||
| }, | ||
| { | ||
| name: "insufficient fee error with zero gas price and zero gas limit", | ||
| err: errors.Wrap(sdkerrors.ErrInsufficientFee, "insufficient fees; got: 0utia required: 100utia"), | ||
| inputGasPrice: 0, | ||
| inputGasLimit: 0, | ||
| isInsufficientMinGasPriceErr: true, | ||
| expectParsingError: true, | ||
| }, | ||
| { | ||
| name: "incorrectly formatted error", | ||
| err: errors.Wrap(sdkerrors.ErrInsufficientFee, "insufficient fees; got: 0uatom required: 100uatom"), | ||
| isInsufficientMinGasPriceErr: false, | ||
| }, | ||
| { | ||
| name: "error with zero required gas price", | ||
| err: errors.Wrap(sdkerrors.ErrInsufficientFee, "insufficient fees; got: 10utia required: 0utia"), | ||
| isInsufficientMinGasPriceErr: true, | ||
| expectParsingError: true, | ||
| }, | ||
| { | ||
| name: "error with extra wrapping", | ||
| err: errors.Wrap(errors.Wrap(sdkerrors.ErrInsufficientFee, "insufficient fees; got: 10utia required: 100utia"), "extra wrapping"), | ||
| inputGasPrice: 0.01, | ||
| expectedGasPrice: 0.1, | ||
| isInsufficientMinGasPriceErr: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| require.Equal(t, tc.isInsufficientMinGasPriceErr, apperr.IsInsufficientMinGasPrice(tc.err)) | ||
| actualGasPrice, err := apperr.ParseInsufficientMinGasPrice(tc.err, tc.inputGasPrice, tc.inputGasLimit) | ||
| if tc.expectParsingError { | ||
| require.Error(t, err) | ||
| require.Zero(t, actualGasPrice) | ||
| } else { | ||
| require.NoError(t, err) | ||
| require.Equal(t, tc.expectedGasPrice, actualGasPrice) | ||
| } | ||
| }) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.