Skip to content

Commit

Permalink
fix: Added embedded fee offset calculation on feecheck + realistic un…
Browse files Browse the repository at this point in the history
…it test assurance (#846)

* fix: Added embedded fee offset calculation on feecheck + realistic unit test assurance

* Debug

* Fixed cast

* Bump gas adjustment

* Switch literal

* Bump significantly

* Bump

* Reducal to average

* Bump fixed

* Bump to greater / more stable value

* Recalc diff

* Debug

* Removed redundant

* Debug

* Added explicit initialisation

* Debug

* Removed redundant + fixed fee bump

* Bump
  • Loading branch information
Eengineer1 authored Jan 27, 2025
1 parent 1a4d415 commit 68249e0
Show file tree
Hide file tree
Showing 10 changed files with 221 additions and 12 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ jobs:
- name: Run integration tests on upgraded network
working-directory: ./tests/integration
run: |
ginkgo -r --tags integration --race --randomize-suites --keep-going --trace --junit-report ../../report-upgraded-integration.xml
ginkgo -r --tags integration --race --randomize-suites --keep-going --trace --skip-file cli_defi_test.go --junit-report ../../report-upgraded-integration.xml
- name: Upload post-upgrade integration tests result
uses: actions/upload-artifact@v4
Expand All @@ -286,7 +286,7 @@ jobs:
- name: Run pricing integration tests after successful param change proposal
working-directory: ./tests/integration
run: |
ginkgo -r --tags integration --race --randomize-suites --keep-going --trace --skip-file cli_diddoc_test.go --skip-file cli_diddoc_negative_test.go --skip-file cli_resource_test.go --skip-file cli_resource_negative_test.go --junit-report ../../report-pricing-change.xml
ginkgo -r --tags integration --race --randomize-suites --keep-going --trace --skip-file cli_diddoc_test.go --skip-file cli_diddoc_negative_test.go --skip-file cli_resource_test.go --skip-file cli_resource_negative_test.go --skip-file cli_defi_test.go --junit-report ../../report-pricing-change.xml
- name: Upload pricing change tests result
uses: actions/upload-artifact@v4
Expand Down
4 changes: 4 additions & 0 deletions ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
didtypes "github.com/cheqd/cheqd-node/x/did/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)

Expand Down Expand Up @@ -68,6 +69,9 @@ func CheckTxFee(ctx sdk.Context, gasPrice sdk.DecCoin, feeCoin sdk.Coin, feeGas
gcDec := sdkmath.LegacyNewDec(gasConsumed)
glDec := sdkmath.LegacyNewDec(feeGas)

// multiply gasPrice by offset
gasPrice.Amount = gasPrice.Amount.Mul(sdkmath.LegacyNewDec(didtypes.FeeOffset))

consumedFeeAmount := gasPrice.Amount.Mul(gcDec)
limitFee := gasPrice.Amount.Mul(glDec)

Expand Down
94 changes: 94 additions & 0 deletions ante/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,100 @@ var _ = Describe("Fee tests on DeliverTx", func() {
Expect((feeCollectorBalance.Amount).GT(math.NewInt(0)))
})

It("Non TaxableTx Lifecycle - Ensure minimum gas prices", func() {
// keys and addresses
priv1, _, addr1 := testdata.KeyTestPubAddr()

// msg and signatures
msg := testdata.NewTestMsg(addr1)
feeAmount := sdk.NewCoins(sdk.NewInt64Coin(didtypes.BaseMinimalDenom, 1*didtypes.BaseFactor)) // 1 CHEQ
gasLimit := testdata.NewTestGasLimit()
Expect(s.txBuilder.SetMsgs(msg)).To(BeNil())
s.txBuilder.SetFeeAmount(feeAmount)
s.txBuilder.SetGasLimit(gasLimit)
s.txBuilder.SetFeePayer(addr1)

privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0}
tx, err := s.CreateTestTx(privs, accNums, accSeqs, s.ctx.ChainID())
Expect(err).To(BeNil())

// set account with sufficient funds
acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr1)
s.app.AccountKeeper.SetAccount(s.ctx, acc)
amount := sdk.NewInt(300_000_000_000) // 300 CHEQ
err = testutil.FundAccount(s.app.BankKeeper, s.ctx, addr1, sdk.NewCoins(sdk.NewCoin(didtypes.BaseMinimalDenom, amount)))
Expect(err).To(BeNil())

dfd := cheqdante.NewOverAllDecorator(decorators...)
antehandler := sdk.ChainAnteDecorators(dfd)

// get feemarket params
feemarketParams, err := s.app.FeeMarketKeeper.GetParams(s.ctx)
Expect(err).To(BeNil())

// enforced enablement
feemarketParams.Enabled = true

// enforce burn
feemarketParams.DistributeFees = false

// enforce maximum block utilisation
feemarketParams.MaxBlockUtilization = feemarkettypes.DefaultMaxBlockUtilization

// set minimum gas prices to realistic value
feemarketParams.MinBaseGasPrice = sdk.MustNewDecFromStr("0.5")

err = s.app.FeeMarketKeeper.SetParams(s.ctx, feemarketParams)
Expect(err).To(BeNil())

// get feemarket state
feemarketState, err := s.app.FeeMarketKeeper.GetState(s.ctx)
Expect(err).To(BeNil())

// set base gas price to realistic value
feemarketState.BaseGasPrice = sdk.MustNewDecFromStr("0.5")

// set feemarket state
err = s.app.FeeMarketKeeper.SetState(s.ctx, feemarketState)
Expect(err).To(BeNil())

taxDecorator := cheqdpost.NewTaxDecorator(s.app.AccountKeeper, s.app.BankKeeper, s.app.FeeGrantKeeper, s.app.DidKeeper, s.app.ResourceKeeper, s.app.FeeMarketKeeper)
posthandler := sdk.ChainPostDecorators(taxDecorator)

// get supply before tx
supplyBefore, _, err := s.app.BankKeeper.GetPaginatedTotalSupply(s.ctx, &query.PageRequest{})
Expect(err).To(BeNil())

// antehandler should not error since we make sure that the fee is sufficient in DeliverTx (simulate=false only, Posthandler will check it otherwise)
newCtx, err := antehandler(s.ctx, tx, false)
Expect(err).To(BeNil(), "Tx errored when non-taxable on deliverTx")
_, _, proposer := testdata.KeyTestPubAddr()
s.ctx = newCtx
a := s.ctx.BlockHeader()
a.ProposerAddress = proposer
newCtx = s.ctx.WithBlockHeader(a)
s.ctx = newCtx
_, err = posthandler(s.ctx, tx, false, true)
Expect(err).To(BeNil(), "Tx errored when non-taxable on deliverTx from posthandler")

// check balance of fee payer
balance := s.app.BankKeeper.GetBalance(s.ctx, addr1, didtypes.BaseMinimalDenom)
Expect(amount.Sub(sdk.NewInt(feeAmount.AmountOf(didtypes.BaseMinimalDenom).Int64()))).To(Equal(balance.Amount), "Fee amount subtracted was not equal to fee amount required for non-taxable tx")

// get supply after tx
supplyAfter, _, err := s.app.BankKeeper.GetPaginatedTotalSupply(s.ctx, &query.PageRequest{})
Expect(err).To(BeNil())

// check that supply was not deflated
Expect(supplyBefore).To(Equal(supplyAfter), "Supply was deflated")

// check that reward has been sent to the fee collector
feeCollector := s.app.AccountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName)
feeCollectorBalance := s.app.BankKeeper.GetBalance(s.ctx, feeCollector, didtypes.BaseMinimalDenom)

Expect((feeCollectorBalance.Amount).GT(math.NewInt(0)))
})

It("TaxableTx Lifecycle on Simulation", func() {
// keys and addresses
priv1, _, addr1 := testdata.KeyTestPubAddr()
Expand Down
7 changes: 7 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1176,27 +1176,34 @@ func (app *App) setupUpgradeStoreLoaders() {
}

func ConfigureFeeMarketModule(ctx sdk.Context, keeper *feemarketkeeper.Keeper) error {
// get feemarket params
params, err := keeper.GetParams(ctx)
if err != nil {
return err
}

// configure feemarket params
params.Enabled = true
params.FeeDenom = resourcetypes.BaseMinimalDenom
params.DistributeFees = false // burn fees
params.MinBaseGasPrice = sdk.MustNewDecFromStr("0.5")
params.MaxBlockUtilization = feemarkettypes.DefaultMaxBlockUtilization

// set feemarket params
if err := keeper.SetParams(ctx, params); err != nil {
return err
}

// get feemarket state
state, err := keeper.GetState(ctx)
if err != nil {
return err
}

// configure feemarket state
state.BaseGasPrice = sdk.MustNewDecFromStr("0.5")

// set feemarket state
return keeper.SetState(ctx, state)
}

Expand Down
2 changes: 1 addition & 1 deletion app/upgrades/v3/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ package v3

const (
UpgradeName = "v3"
MinorUpgradeName = "v3.1-new"
MinorUpgradeName = "v3.1.5"
)
4 changes: 2 additions & 2 deletions tests/integration/cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const (
KeyringBackend = "test"
OutputFormat = "json"
Gas = "auto"
GasAdjustment = "2.5"
GasPrices = "60ncheq"
GasAdjustment = "3.5"
GasPrices = "10000ncheq"
)

const (
Expand Down
30 changes: 30 additions & 0 deletions tests/integration/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,36 @@ func QueryProposal(container, id string) (govtypesv1.Proposal, error) {
return resp, nil
}

func QueryFeemarketGasPrice(denom string) (feemarkettypes.GasPriceResponse, error) {
res, err := Query("feemarket", "gas-price", denom)
if err != nil {
return feemarkettypes.GasPriceResponse{}, err
}

var resp feemarkettypes.GasPriceResponse
err = helpers.Codec.UnmarshalJSON([]byte(res), &resp)
if err != nil {
return feemarkettypes.GasPriceResponse{}, err
}

return resp, nil
}

func QueryFeemarketGasPrices() (feemarkettypes.GasPricesResponse, error) {
res, err := Query("feemarket", "gas-prices")
if err != nil {
return feemarkettypes.GasPricesResponse{}, err
}

var resp feemarkettypes.GasPricesResponse
err = helpers.Codec.UnmarshalJSON([]byte(res), &resp)
if err != nil {
return feemarkettypes.GasPricesResponse{}, err
}

return resp, nil
}

func QueryFeemarketParams() (feemarkettypes.Params, error) {
res, err := Query("feemarket", "params")
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions tests/integration/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cli

import (
"encoding/json"
"fmt"

"github.com/cheqd/cheqd-node/tests/integration/helpers"
"github.com/cheqd/cheqd-node/tests/integration/network"
Expand Down Expand Up @@ -43,6 +44,8 @@ func Tx(module, tx, from string, feeParams []string, txArgs ...string) (sdk.TxRe
// Other args
args = append(args, txArgs...)

fmt.Println("args", args)

output, err := Exec(args...)
if err != nil {
return sdk.TxResponse{}, err
Expand Down Expand Up @@ -176,3 +179,7 @@ func SubmitProposalTx(from, pathToDir string, feeParams []string) (sdk.TxRespons
func VoteProposalTx(from, option, id string, feeParams []string) (sdk.TxResponse, error) {
return Tx("gov", "vote", from, feeParams, option, id)
}

func SendTokensTx(from, to, amount string, feeParams []string) (sdk.TxResponse, error) {
return Tx("bank", "send", from, feeParams, from, to, amount)
}
80 changes: 73 additions & 7 deletions tests/integration/cli_defi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
cli "github.com/cheqd/cheqd-node/tests/integration/cli"
helpers "github.com/cheqd/cheqd-node/tests/integration/helpers"

sdkmath "cosmossdk.io/math"
"github.com/cheqd/cheqd-node/tests/integration/testdata"
didtypes "github.com/cheqd/cheqd-node/x/did/types"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -24,8 +25,8 @@ var _ = Describe("Upgrade - Burn coins from relevant message signer", func() {
// assert no error
Expect(err).To(BeNil())

// generate fixed fees, in which case 500,000,000 ncheq or 0.5 cheq
fees := helpers.GenerateFees("500000000ncheq")
// generate fixed fees, in which case 3,500,000,000 ncheq or 3.5 cheq
fees := helpers.GenerateFees("3500000000ncheq")

// burn the coins
res, err := cli.BurnMsg(testdata.BASE_ACCOUNT_1, burnCoins.String(), fees)
Expand All @@ -46,13 +47,13 @@ var _ = Describe("Upgrade - Burn coins from relevant message signer", func() {
diff := balanceBefore.Sub(balanceAfter)

// assert the difference is equal to the coins burnt
total := burnCoins.Add(sdk.NewCoin(didtypes.BaseMinimalDenom, sdk.NewInt(500_000_000)))
total := burnCoins.Add(sdk.NewCoin(didtypes.BaseMinimalDenom, sdk.NewInt(3_500_000_000)))

// assert the difference is equal to the coins burnt
Expect(diff).To(Equal(total))
})

It("shouldn't burn as their are insufficient funds in the sender", func() {
It("shouldn't burn if sender has insufficient funds", func() {
// define the coins to burn, in which case 1,000,000 ncheq or 0.01 cheq
coins := sdk.NewCoin(didtypes.BaseMinimalDenom, sdk.NewInt(1_000_000))

Expand All @@ -62,13 +63,13 @@ var _ = Describe("Upgrade - Burn coins from relevant message signer", func() {
// assert no error
Expect(err).To(BeNil())

// generate fixed fees, in which case 500,000,000 ncheq or 0.5 cheq
fees := helpers.GenerateFees("500000000ncheq")
// generate fixed fees, in which case 3,500,000,000 ncheq or 3.5 cheq
fees := helpers.GenerateFees("3500000000ncheq")

// burn the coins
res, err := cli.BurnMsg(testdata.BASE_ACCOUNT_3, coins.String(), fees)

// assert no error
// assert error
Expect(err).NotTo(BeNil())

// assert the response code is 0
Expand All @@ -84,3 +85,68 @@ var _ = Describe("Upgrade - Burn coins from relevant message signer", func() {
Expect(balanceBefore).To(Equal(balanceAfter))
})
})

var _ = Describe("Upgrade - Feemarket fees (non-taxable transactions)", func() {
It("should successfully submit a non-taxable transaction with sufficient fees (--gas-prices)", func() {
// query feemarket gas price for the base minimal denom
gasPrice, err := cli.QueryFeemarketGasPrice(didtypes.BaseMinimalDenom)

// print the gas price
By("Gas Price: " + gasPrice.Price.String())

// assert no error
Expect(err).To(BeNil())

// define the coins to send, in which case 1,000,000,000 ncheq or 1 cheq
coins := sdk.NewCoin(didtypes.BaseMinimalDenom, sdk.NewInt(1_000_000_000))

// compute gas price, using offset
gasPrice.Price.Amount = gasPrice.Price.Amount.Mul(sdkmath.LegacyNewDec(didtypes.FeeOffset))

// define feeParams
feeParams := []string{
"--gas", cli.Gas,
"--gas-adjustment", cli.GasAdjustment,
"--gas-prices", gasPrice.Price.String(),
}

// send the coins, balance assertions are intentionally omitted or out of scope
res, err := cli.SendTokensTx(testdata.BASE_ACCOUNT_1, testdata.BASE_ACCOUNT_2_ADDR, coins.String(), feeParams)

// assert no error
Expect(err).To(BeNil())

// assert the response code is 0
Expect(res.Code).To(BeEquivalentTo(0))
})

It("should successfully submit a non-taxable transaction with sufficient fees (--fees)", func() {
// query feemarket gas price for the base minimal denom
gasPrice, err := cli.QueryFeemarketGasPrice(didtypes.BaseMinimalDenom)

// print the gas price
By("Gas Price: " + gasPrice.Price.String())

// assert no error
Expect(err).To(BeNil())

// define the coins to send, in which case 1,000,000,000 ncheq or 1 cheq
coins := sdk.NewCoin(didtypes.BaseMinimalDenom, sdk.NewInt(1_000_000_000))

// define static fees, in which case gas price is multiplied by roughly 3 or greater, times the minimal base denom
// consider multiplying in the range of [1.5, 3] times the gas price
gasPrice.Price.Amount = gasPrice.Price.Amount.Mul(sdkmath.LegacyNewDec(3)).Mul(sdkmath.LegacyNewDec(didtypes.BaseFactor))

// define feeParams
feeParams := helpers.GenerateFees(gasPrice.Price.String())

// send the coins, balance assertions are intentionally omitted or out of scope
res, err := cli.SendTokensTx(testdata.BASE_ACCOUNT_1, testdata.BASE_ACCOUNT_2_ADDR, coins.String(), feeParams)

// assert no error
Expect(err).To(BeNil())

// assert the response code is 0
Expect(res.Code).To(BeEquivalentTo(0))
})
})
1 change: 1 addition & 0 deletions x/did/types/denom.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ package types
const (
BaseMinimalDenom = "ncheq"
BaseFactor = 1e9
FeeOffset = 1e4
)

0 comments on commit 68249e0

Please sign in to comment.