From a5fed12e0db3d15812880c791cd21019e5a37c08 Mon Sep 17 00:00:00 2001 From: StrathCole Date: Wed, 11 Sep 2024 15:16:20 +0200 Subject: [PATCH 1/5] - improve gas simulation for taxable transactions --- custom/auth/ante/fee.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/custom/auth/ante/fee.go b/custom/auth/ante/fee.go index 8e8cf402..8b18a150 100644 --- a/custom/auth/ante/fee.go +++ b/custom/auth/ante/fee.go @@ -101,9 +101,28 @@ func (fd FeeDecorator) checkDeductFee(ctx sdk.Context, feeTx sdk.FeeTx, taxes sd feesOrTax := fee - // deduct the fees - if fee.IsZero() && simulate { - feesOrTax = taxes + if simulate { + // we need to check all taxes if they are GTE 10 because otherwise we will not be able to + // simulate the split processes (i.e. BurnTaxSplit and OracleSplit) + // if they are less than 10, we will set them to 10 + for i := range taxes { + if taxes[i].Amount.LT(sdk.NewInt(10)) { + taxes[i].Amount = sdk.NewInt(10) + } + } + + if fee.IsZero() { + feesOrTax = taxes + } + + // even if fee is not zero it might be it is lower than the increased tax + // so we need to check if the tax is higher than the fee to not run into deduction errors + for i := range feesOrTax { + feeDenom := feesOrTax[i].Denom + if feesOrTax[i].Amount.LT(taxes.AmountOf(feeDenom)) { + feesOrTax[i].Amount = taxes.AmountOf(feeDenom) + } + } } if !feesOrTax.IsZero() { From 4a3c59f39c168c641e508a5a24e13dabc1c5fc11 Mon Sep 17 00:00:00 2001 From: StrathCole Date: Wed, 11 Sep 2024 15:49:45 +0200 Subject: [PATCH 2/5] - taxable tx simulation improvements --- custom/auth/ante/fee.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/custom/auth/ante/fee.go b/custom/auth/ante/fee.go index 8b18a150..23f00e4c 100644 --- a/custom/auth/ante/fee.go +++ b/custom/auth/ante/fee.go @@ -6,6 +6,7 @@ import ( errorsmod "cosmossdk.io/errors" + "github.com/classic-terra/core/v3/types/assets" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/ante" @@ -111,6 +112,16 @@ func (fd FeeDecorator) checkDeductFee(ctx sdk.Context, feeTx sdk.FeeTx, taxes sd } } + // it might also be that there is no taxes at all, then we need to add one + if len(taxes) == 0 { + denom := assets.MicroLunaDenom + if !fee.IsZero() { + denom = fee[0].Denom + } + + taxes = sdk.NewCoins(sdk.NewCoin(denom, sdk.NewInt(10))) + } + if fee.IsZero() { feesOrTax = taxes } From c21c4d4ba2de1d47d3307d76501a2a5bee0c487c Mon Sep 17 00:00:00 2001 From: StrathCole Date: Wed, 11 Sep 2024 17:02:18 +0200 Subject: [PATCH 3/5] - improve tax simulation handling method --- custom/auth/ante/fee.go | 24 ++---------------------- custom/auth/ante/fee_tax.go | 24 +++++++++++++++--------- custom/auth/ante/fee_test.go | 2 +- custom/auth/tx/service.go | 2 +- custom/wasm/keeper/handler_plugin.go | 4 +++- 5 files changed, 22 insertions(+), 34 deletions(-) diff --git a/custom/auth/ante/fee.go b/custom/auth/ante/fee.go index 23f00e4c..e3f10d5f 100644 --- a/custom/auth/ante/fee.go +++ b/custom/auth/ante/fee.go @@ -6,7 +6,6 @@ import ( errorsmod "cosmossdk.io/errors" - "github.com/classic-terra/core/v3/types/assets" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/ante" @@ -52,7 +51,7 @@ func (fd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, nex msgs := feeTx.GetMsgs() // Compute taxes - taxes := FilterMsgAndComputeTax(ctx, fd.treasuryKeeper, msgs...) + taxes := FilterMsgAndComputeTax(ctx, fd.treasuryKeeper, simulate, msgs...) if !simulate { priority, err = fd.checkTxFee(ctx, tx, taxes) @@ -103,30 +102,11 @@ func (fd FeeDecorator) checkDeductFee(ctx sdk.Context, feeTx sdk.FeeTx, taxes sd feesOrTax := fee if simulate { - // we need to check all taxes if they are GTE 10 because otherwise we will not be able to - // simulate the split processes (i.e. BurnTaxSplit and OracleSplit) - // if they are less than 10, we will set them to 10 - for i := range taxes { - if taxes[i].Amount.LT(sdk.NewInt(10)) { - taxes[i].Amount = sdk.NewInt(10) - } - } - - // it might also be that there is no taxes at all, then we need to add one - if len(taxes) == 0 { - denom := assets.MicroLunaDenom - if !fee.IsZero() { - denom = fee[0].Denom - } - - taxes = sdk.NewCoins(sdk.NewCoin(denom, sdk.NewInt(10))) - } - if fee.IsZero() { feesOrTax = taxes } - // even if fee is not zero it might be it is lower than the increased tax + // even if fee is not zero it might be it is lower than the increased tax from computeTax // so we need to check if the tax is higher than the fee to not run into deduction errors for i := range feesOrTax { feeDenom := feesOrTax[i].Denom diff --git a/custom/auth/ante/fee_tax.go b/custom/auth/ante/fee_tax.go index 96a0e48d..be76869b 100644 --- a/custom/auth/ante/fee_tax.go +++ b/custom/auth/ante/fee_tax.go @@ -20,14 +20,14 @@ func isIBCDenom(denom string) bool { } // FilterMsgAndComputeTax computes the stability tax on messages. -func FilterMsgAndComputeTax(ctx sdk.Context, tk TreasuryKeeper, msgs ...sdk.Msg) sdk.Coins { +func FilterMsgAndComputeTax(ctx sdk.Context, tk TreasuryKeeper, simulate bool, msgs ...sdk.Msg) sdk.Coins { taxes := sdk.Coins{} for _, msg := range msgs { switch msg := msg.(type) { case *banktypes.MsgSend: if !tk.HasBurnTaxExemptionAddress(ctx, msg.FromAddress, msg.ToAddress) { - taxes = taxes.Add(computeTax(ctx, tk, msg.Amount)...) + taxes = taxes.Add(computeTax(ctx, tk, msg.Amount, simulate)...) } case *banktypes.MsgMultiSend: @@ -47,28 +47,28 @@ func FilterMsgAndComputeTax(ctx sdk.Context, tk TreasuryKeeper, msgs ...sdk.Msg) if tainted != len(msg.Inputs)+len(msg.Outputs) { for _, input := range msg.Inputs { - taxes = taxes.Add(computeTax(ctx, tk, input.Coins)...) + taxes = taxes.Add(computeTax(ctx, tk, input.Coins, simulate)...) } } case *marketexported.MsgSwapSend: - taxes = taxes.Add(computeTax(ctx, tk, sdk.NewCoins(msg.OfferCoin))...) + taxes = taxes.Add(computeTax(ctx, tk, sdk.NewCoins(msg.OfferCoin), simulate)...) case *wasmtypes.MsgInstantiateContract: - taxes = taxes.Add(computeTax(ctx, tk, msg.Funds)...) + taxes = taxes.Add(computeTax(ctx, tk, msg.Funds, simulate)...) case *wasmtypes.MsgInstantiateContract2: - taxes = taxes.Add(computeTax(ctx, tk, msg.Funds)...) + taxes = taxes.Add(computeTax(ctx, tk, msg.Funds, simulate)...) case *wasmtypes.MsgExecuteContract: if !tk.HasBurnTaxExemptionContract(ctx, msg.Contract) { - taxes = taxes.Add(computeTax(ctx, tk, msg.Funds)...) + taxes = taxes.Add(computeTax(ctx, tk, msg.Funds, simulate)...) } case *authz.MsgExec: messages, err := msg.GetMessages() if err == nil { - taxes = taxes.Add(FilterMsgAndComputeTax(ctx, tk, messages...)...) + taxes = taxes.Add(FilterMsgAndComputeTax(ctx, tk, simulate, messages...)...) } } } @@ -77,7 +77,7 @@ func FilterMsgAndComputeTax(ctx sdk.Context, tk TreasuryKeeper, msgs ...sdk.Msg) } // computes the stability tax according to tax-rate and tax-cap -func computeTax(ctx sdk.Context, tk TreasuryKeeper, principal sdk.Coins) sdk.Coins { +func computeTax(ctx sdk.Context, tk TreasuryKeeper, principal sdk.Coins, simulate bool) sdk.Coins { taxRate := tk.GetTaxRate(ctx) if taxRate.Equal(sdk.ZeroDec()) { return sdk.Coins{} @@ -95,6 +95,12 @@ func computeTax(ctx sdk.Context, tk TreasuryKeeper, principal sdk.Coins) sdk.Coi } taxDue := sdk.NewDecFromInt(coin.Amount).Mul(taxRate).TruncateInt() + // we need to check all taxes if they are GTE 100 because otherwise we will not be able to + // simulate the split processes (i.e. BurnTaxSplit and OracleSplit) + // if they are less than 100, we will set them to 100 + if simulate && taxDue.LT(sdk.NewInt(100)) { + taxDue = sdk.NewInt(100) + } // If tax due is greater than the tax cap, cap! taxCap := tk.GetTaxCap(ctx, coin.Denom) diff --git a/custom/auth/ante/fee_test.go b/custom/auth/ante/fee_test.go index ecd8a634..68f8482f 100644 --- a/custom/auth/ante/fee_test.go +++ b/custom/auth/ante/fee_test.go @@ -832,7 +832,7 @@ func (s *AnteTestSuite) runBurnSplitTaxTest(burnSplitRate sdk.Dec, oracleSplitRa feeCollectorAfter := bk.GetAllBalances(s.ctx, ak.GetModuleAddress(authtypes.FeeCollectorName)) oracleAfter := bk.GetAllBalances(s.ctx, ak.GetModuleAddress(oracletypes.ModuleName)) - taxes := ante.FilterMsgAndComputeTax(s.ctx, tk, msg) + taxes := ante.FilterMsgAndComputeTax(s.ctx, tk, false, msg) communityPoolAfter, _ := dk.GetFeePoolCommunityCoins(s.ctx).TruncateDecimal() if communityPoolAfter.IsZero() { communityPoolAfter = sdk.NewCoins(sdk.NewCoin(core.MicroSDRDenom, sdk.ZeroInt())) diff --git a/custom/auth/tx/service.go b/custom/auth/tx/service.go index 63710d04..a9c803c2 100644 --- a/custom/auth/tx/service.go +++ b/custom/auth/tx/service.go @@ -52,7 +52,7 @@ func (ts txServer) ComputeTax(c context.Context, req *ComputeTaxRequest) (*Compu return nil, status.Errorf(codes.InvalidArgument, "empty txBytes is not allowed") } - taxAmount := customante.FilterMsgAndComputeTax(ctx, ts.treasuryKeeper, msgs...) + taxAmount := customante.FilterMsgAndComputeTax(ctx, ts.treasuryKeeper, false, msgs...) return &ComputeTaxResponse{ TaxAmount: taxAmount, }, nil diff --git a/custom/wasm/keeper/handler_plugin.go b/custom/wasm/keeper/handler_plugin.go index f3a24801..479f9684 100644 --- a/custom/wasm/keeper/handler_plugin.go +++ b/custom/wasm/keeper/handler_plugin.go @@ -78,7 +78,9 @@ func (h SDKMessageHandler) DispatchMsg(ctx sdk.Context, contractAddr sdk.AccAddr for _, sdkMsg := range sdkMsgs { // Charge tax on result msg - taxes := ante.FilterMsgAndComputeTax(ctx, h.treasuryKeeper, sdkMsg) + // we set simulate to false here as it is not available and we don't need to + // increase the tax amount for simulation inside of wasm + taxes := ante.FilterMsgAndComputeTax(ctx, h.treasuryKeeper, false, sdkMsg) if !taxes.IsZero() { eventManager := sdk.NewEventManager() contractAcc := h.accountKeeper.GetAccount(ctx, contractAddr) From ee1ab391be002cf6018b182d263db4010fbc6fb4 Mon Sep 17 00:00:00 2001 From: StrathCole Date: Wed, 11 Sep 2024 17:32:30 +0200 Subject: [PATCH 4/5] - improve logic --- custom/auth/ante/fee.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/custom/auth/ante/fee.go b/custom/auth/ante/fee.go index e3f10d5f..f30d9150 100644 --- a/custom/auth/ante/fee.go +++ b/custom/auth/ante/fee.go @@ -108,10 +108,14 @@ func (fd FeeDecorator) checkDeductFee(ctx sdk.Context, feeTx sdk.FeeTx, taxes sd // even if fee is not zero it might be it is lower than the increased tax from computeTax // so we need to check if the tax is higher than the fee to not run into deduction errors - for i := range feesOrTax { - feeDenom := feesOrTax[i].Denom - if feesOrTax[i].Amount.LT(taxes.AmountOf(feeDenom)) { - feesOrTax[i].Amount = taxes.AmountOf(feeDenom) + for _, tax := range taxes { + feeAmount := feesOrTax.AmountOf(tax.Denom) + // if the fee amount is zero, add the tax amount to feesOrTax + if feeAmount.IsZero() { + feesOrTax = feesOrTax.Add(tax) + } else if feeAmount.LT(tax.Amount) { + // Update feesOrTax if the tax amount is higher + feesOrTax = feesOrTax.Add(tax.Sub(sdk.NewCoin(tax.Denom, feeAmount))) } } } From f71014e6f2467a5bd86c1efda2483badf6e65a43 Mon Sep 17 00:00:00 2001 From: StrathCole Date: Wed, 11 Sep 2024 17:45:09 +0200 Subject: [PATCH 5/5] - fix sub handling --- custom/auth/ante/fee.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/custom/auth/ante/fee.go b/custom/auth/ante/fee.go index f30d9150..a292be06 100644 --- a/custom/auth/ante/fee.go +++ b/custom/auth/ante/fee.go @@ -115,7 +115,8 @@ func (fd FeeDecorator) checkDeductFee(ctx sdk.Context, feeTx sdk.FeeTx, taxes sd feesOrTax = feesOrTax.Add(tax) } else if feeAmount.LT(tax.Amount) { // Update feesOrTax if the tax amount is higher - feesOrTax = feesOrTax.Add(tax.Sub(sdk.NewCoin(tax.Denom, feeAmount))) + missingAmount := tax.Amount.Sub(feeAmount) + feesOrTax = feesOrTax.Add(sdk.NewCoin(tax.Denom, missingAmount)) } } }