feat: buy back and burn osmo using non osmo taker fee burn portion#9541
feat: buy back and burn osmo using non osmo taker fee burn portion#9541
Conversation
WalkthroughAdds a dedicated non-native taker-fee burn path: introduces a burn module account and constant, updates v31 upgrade handling to create the account and propagate errors, changes txfees keeper to compute distributions from original amounts and perform swap-and-burn for non-native fees (with guards), generalizes a telemetry helper, and expands tests and changelog. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as App / EndBlocker
participant TxFees as x/txfees Keeper
participant Bank as BankKeeper
participant BurnMod as non_native_fee_collector_burn (ModuleAcct)
participant Swap as PoolManager / Swap
participant Null as Null Address (Burn)
User->>App: Trades produce taker fees (multi-denom)
App->>TxFees: DistributeTakerFees(ctx, fees)
rect rgba(230,240,255,0.35)
note over TxFees: Split by denom (OSMO vs non-native)\nCompute shares from originalAmount (staking, community, burn)
end
alt Base denom (OSMO)
TxFees->>Bank: Send staking & community shares
TxFees->>Null: Send burn share (immediate burn)
TxFees->>TxFees: Update OSMO trackers
else Non-native denoms
TxFees->>Bank: Send staking & community shares (community uses originalAmount)
TxFees->>BurnMod: Send burn share (accumulate nonOsmoForBurn)
TxFees->>TxFees: Update non-OSMO trackers
end
opt Epoch-end / Deferred burn processing
BurnMod->>Swap: Swap accumulated non-native coins -> base denom (skip zero amounts)
Swap-->>BurnMod: Swapped base-denom coins
BurnMod->>Null: Burn swapped base-denom coins
BurnMod->>TxFees: Report burned amount -> update burn trackers
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
x/txfees/keeper/hooks.go (1)
170-185: Fix logger to use trackerErr (not err) when logging taker-fee tracker failuresReplace the incorrect
"error", errwith"error", trackerErrin x/txfees/keeper/hooks.go at lines: 116, 147, 180, 222, 244, 268.
🧹 Nitpick comments (7)
x/txfees/keeper/hooks_test.go (2)
831-843: Do not mutate pool state when computing expectations; this double‑swaps before AfterEpochEnd.Calling SwapOutAmtGivenIn here changes reserves; AfterEpochEnd then swaps again, so your expected vs actual will diverge and the test becomes brittle.
Apply this diff to avoid pre‑mutating pools (compute expected via CalcOut only or via PoolManager’s estimator):
- // update state - _, err = daiCfmmPool.SwapOutAmtGivenIn(s.Ctx, sdk.Coins{sdk.NewCoin(daiDenom, daiForBurn)}, baseDenom, osmomath.ZeroDec()) - s.Require().NoError(err) - _, err = usdcCfmmPool.SwapOutAmtGivenIn(s.Ctx, sdk.Coins{sdk.NewCoin(usdcDenom, usdcForBurn)}, baseDenom, osmomath.ZeroDec()) - s.Require().NoError(err)Optionally prefer:
- PoolManagerKeeper.MultihopEstimateOutGivenExactAmountInNoTakerFee(...) for parity with production routing.
915-923: Same issue: pre‑mutating the failed‑token pool skews second‑epoch expectations.Remove the swap; rely on CalcOut or the PoolManager estimator to derive expected amounts.
- // update state - _, err = failedTokenCfmmPool.SwapOutAmtGivenIn(s.Ctx, sdk.Coins{sdk.NewCoin(failedSwapDenom, actualBurnTokenAmount)}, baseDenom, osmomath.ZeroDec()) - s.Require().NoError(err)x/txfees/keeper/hooks.go (5)
173-184: Use cacheCtx inside ApplyFuncIfNoError closures; fix trackerErr logging.Leverage the cached context to ensure atomicity on error. Also log trackerErr, not err.
Apply this diff:
- applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { - err := k.distributionKeeper.FundCommunityPool(ctx, sdk.NewCoins(nonOsmoTakerFeeToCommunityPoolCoin), takerFeeModuleAccount) + applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { + err := k.distributionKeeper.FundCommunityPool(cacheCtx, sdk.NewCoins(nonOsmoTakerFeeToCommunityPoolCoin), takerFeeModuleAccount) if err == nil { takerFeeCoin.Amount = takerFeeCoin.Amount.Sub(nonOsmoTakerFeeToCommunityPoolCoin.Amount) } trackerErr := k.poolManager.UpdateTakerFeeTrackerForCommunityPoolByDenom(ctx, nonOsmoTakerFeeToCommunityPoolCoin.Denom, nonOsmoTakerFeeToCommunityPoolCoin.Amount) if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for community pool by denom", "error", err) + ctx.Logger().Error("Error updating taker fee tracker for community pool by denom", "error", trackerErr) } return err }, txfeestypes.TakerFeeFailedCommunityPoolUpdateMetricName, nonOsmoTakerFeeToCommunityPoolCoin)
219-226: Use cacheCtx and log trackerErr, not err.Apply this diff:
- applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { - err := k.distributionKeeper.FundCommunityPool(ctx, sdk.NewCoins(totalCoinOut), takerFeeCommunityPoolModuleAccount) + applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { + err := k.distributionKeeper.FundCommunityPool(cacheCtx, sdk.NewCoins(totalCoinOut), takerFeeCommunityPoolModuleAccount) trackerErr := k.poolManager.UpdateTakerFeeTrackerForCommunityPoolByDenom(ctx, totalCoinOut.Denom, totalCoinOut.Amount) if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for community pool by denom", "error", err) + ctx.Logger().Error("Error updating taker fee tracker for community pool by denom", "error", trackerErr) } return err }, txfeestypes.TakerFeeFailedCommunityPoolUpdateMetricName, totalCoinOut)
228-234: Guard empty coins and use cacheCtx for the burn module transfer.Avoid no-op/invalid send on empty coins and ensure changes are wrapped in the cached context.
Apply this diff:
- applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { - err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, takerFeeModuleAccount, txfeestypes.TakerFeeBurnName, nonOsmoForBurn) - return err - }, txfeestypes.TakerFeeFailedBurnUpdateMetricName, nonOsmoForBurn) + if len(nonOsmoForBurn) > 0 { + applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { + err := k.bankKeeper.SendCoinsFromAccountToModule(cacheCtx, takerFeeModuleAccount, txfeestypes.TakerFeeBurnName, nonOsmoForBurn) + return err + }, txfeestypes.TakerFeeFailedBurnUpdateMetricName, nonOsmoForBurn) + }
239-248: Use cacheCtx and log trackerErr, not err, when burning swapped OSMO.Apply this diff:
- applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { - err := k.bankKeeper.SendCoins(ctx, takerFeeBurnModuleAccount, txfeestypes.DefaultNullAddress, sdk.NewCoins(totalCoinOutForBurn)) + applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { + err := k.bankKeeper.SendCoins(cacheCtx, takerFeeBurnModuleAccount, txfeestypes.DefaultNullAddress, sdk.NewCoins(totalCoinOutForBurn)) trackerErr := k.poolManager.UpdateTakerFeeTrackerForBurnByDenom(ctx, totalCoinOutForBurn.Denom, totalCoinOutForBurn.Amount) if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for burn by denom", "error", err) + ctx.Logger().Error("Error updating taker fee tracker for burn by denom", "error", trackerErr) } return err }, txfeestypes.TakerFeeFailedBurnUpdateMetricName, totalCoinOutForBurn)
263-272: Use cacheCtx and log trackerErr for the stakers transfer.Keep ApplyFuncIfNoError semantics consistent; log the tracker error.
Apply this diff:
- applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { - err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.TakerFeeStakersName, authtypes.FeeCollectorName, sdk.NewCoins(totalCoinOut)) + applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { + err := k.bankKeeper.SendCoinsFromModuleToModule(cacheCtx, txfeestypes.TakerFeeStakersName, authtypes.FeeCollectorName, sdk.NewCoins(totalCoinOut)) trackerErr := k.poolManager.UpdateTakerFeeTrackerForStakersByDenom(ctx, totalCoinOut.Denom, totalCoinOut.Amount) if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for stakers by denom", "error", err) + ctx.Logger().Error("Error updating taker fee tracker for stakers by denom", "error", trackerErr) } return err }, txfeestypes.TakerFeeFailedNativeRewardUpdateMetricName, totalCoinOut)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/modules.go(1 hunks)app/upgrades/v31/upgrades.go(1 hunks)app/upgrades/v31/upgrades_test.go(1 hunks)x/txfees/keeper/hooks.go(4 hunks)x/txfees/keeper/hooks_test.go(2 hunks)x/txfees/types/keys.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T15:40:00.224Z
Learnt from: czarcas7ic
PR: osmosis-labs/osmosis#8310
File: x/poolmanager/store.go:129-143
Timestamp: 2024-10-08T15:40:00.224Z
Learning: Caching is not implemented for the `GetTakerFeeShareDenomsToAccruedValue` function in `x/poolmanager/store.go` due to efficient direct key access, specific usage patterns where values are updated immediately after retrieval, and the minor overhead of unmarshaling integers. This decision may be revisited if performance issues arise.
Applied to files:
x/txfees/keeper/hooks.go
🧬 Code graph analysis (5)
x/txfees/keeper/hooks_test.go (5)
app/test_helpers.go (1)
Setup(199-201)x/txfees/types/expected_keepers.go (3)
TxFeesKeeper(87-91)AccountKeeper(65-68)BankKeeper(76-84)x/txfees/types/constants.go (1)
DefaultNullAddress(16-16)x/protorev/types/expected_keepers.go (4)
GAMMKeeper(32-34)PoolManagerKeeper(38-72)AccountKeeper(16-18)BankKeeper(22-28)x/txfees/types/keys.go (3)
TakerFeeCollectorName(34-34)TakerFeeBurnName(31-31)TakerFeeStakersName(26-26)
app/upgrades/v31/upgrades_test.go (2)
osmomath/sdk_math_alias.go (1)
MustNewDecFromStr(32-32)x/poolmanager/types/params.go (1)
OneDec(27-27)
app/upgrades/v31/upgrades.go (1)
osmomath/sdk_math_alias.go (1)
MustNewDecFromStr(32-32)
app/modules.go (1)
x/txfees/types/keys.go (1)
TakerFeeBurnName(31-31)
x/txfees/keeper/hooks.go (3)
x/txfees/types/keys.go (1)
TakerFeeBurnName(31-31)x/txfees/types/telemetry.go (1)
TakerFeeFailedBurnUpdateMetricName(27-27)x/txfees/types/constants.go (1)
DefaultNullAddress(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: go-split-test-files
- GitHub Check: e2e
- GitHub Check: Run golangci-lint
- GitHub Check: test
- GitHub Check: Summary
🔇 Additional comments (10)
app/modules.go (1)
137-137: Registering the burn module account looks correct.TakerFeeBurnName added with nil perms aligns with other per‑bucket module accounts.
app/upgrades/v31/upgrades.go (1)
49-52: Distribution values LGTM; they sum to 1.0.Staking=0.225, Burn=0.525, Community=0.25 is consistent with PR intent.
Please confirm the keeper enforces sum==1 invariant (or safe normalization) for NonOsmoTakerFeeDistribution at SetParams time.
x/txfees/types/keys.go (1)
28-31: LGTM: dedicated burn bucket name and docs.Name is consistent with existing stakers/community pool buckets and matches app/modules registration.
app/upgrades/v31/upgrades_test.go (1)
81-101: LGTM: asserts validate both OSMO and non‑OSMO distributions and totals.Covers exact values and sum‑to‑one checks post‑upgrade.
x/txfees/keeper/hooks_test.go (2)
695-749: LGTM: OSMO taker fee burn test is precise and self‑contained.Verifies null address, 70/30 split, and empty collector.
814-824: Confirm rounding semantics match keeper implementation.You use TruncateInt on per‑bucket splits; ensure keeper uses identical rounding to avoid off‑by‑one deltas.
x/txfees/keeper/hooks.go (4)
289-293: Good zero-amount guard in swap loop.Prevents “token amount must be positive” errors on edge cases.
158-160: Non‑OSMO burn accumulator addition looks good.
193-200: Burn share computed from originalAmount is correct.Avoids compounding from “remaining” amounts.
401-406: Generic applyFuncIfNoErrorAndLog helper looks good.Type-bounded fmt.Stringer keeps telemetry flexible.
| // Loop through all remaining tokens in the taker fee module account. | ||
| for _, takerFeeCoin := range takerFeeModuleAccountCoins { | ||
| // Store original amount to calculate percentages from the total, not from remaining amounts | ||
| originalAmount := takerFeeCoin.Amount | ||
|
|
||
| // Community Pool: | ||
| if nonOsmoTakerFeeDistribution.CommunityPool.GT(zeroDec) && takerFeeCoin.Amount.GT(osmomath.ZeroInt()) { | ||
| if nonOsmoTakerFeeDistribution.CommunityPool.GT(zeroDec) && originalAmount.GT(osmomath.ZeroInt()) { | ||
| denomIsWhitelisted := isDenomWhitelisted(takerFeeCoin.Denom, communityPoolDenomWhitelist) | ||
| // If the non osmo denom is a whitelisted quote asset, we directly send to the community pool | ||
| if denomIsWhitelisted { |
There was a problem hiding this comment.
Skip base denom in the “non‑native” loop to avoid double-processing OSMO.
OSMO leftover (e.g., rounding dust) will be re-processed under the non‑native path. Add a guard to continue when denom == defaultFeesDenom.
Apply this diff:
for _, takerFeeCoin := range takerFeeModuleAccountCoins {
- // Store original amount to calculate percentages from the total, not from remaining amounts
- originalAmount := takerFeeCoin.Amount
+ // Skip base denom; it is handled in the native path above.
+ if takerFeeCoin.Denom == defaultFeesDenom {
+ continue
+ }
+ // Store original amount to calculate percentages from the total, not from remaining amounts
+ originalAmount := takerFeeCoin.Amount📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Loop through all remaining tokens in the taker fee module account. | |
| for _, takerFeeCoin := range takerFeeModuleAccountCoins { | |
| // Store original amount to calculate percentages from the total, not from remaining amounts | |
| originalAmount := takerFeeCoin.Amount | |
| // Community Pool: | |
| if nonOsmoTakerFeeDistribution.CommunityPool.GT(zeroDec) && takerFeeCoin.Amount.GT(osmomath.ZeroInt()) { | |
| if nonOsmoTakerFeeDistribution.CommunityPool.GT(zeroDec) && originalAmount.GT(osmomath.ZeroInt()) { | |
| denomIsWhitelisted := isDenomWhitelisted(takerFeeCoin.Denom, communityPoolDenomWhitelist) | |
| // If the non osmo denom is a whitelisted quote asset, we directly send to the community pool | |
| if denomIsWhitelisted { | |
| // Loop through all remaining tokens in the taker fee module account. | |
| for _, takerFeeCoin := range takerFeeModuleAccountCoins { | |
| // Skip base denom; it is handled in the native path above. | |
| if takerFeeCoin.Denom == defaultFeesDenom { | |
| continue | |
| } | |
| // Store original amount to calculate percentages from the total, not from remaining amounts | |
| originalAmount := takerFeeCoin.Amount | |
| // Community Pool: | |
| if nonOsmoTakerFeeDistribution.CommunityPool.GT(zeroDec) && originalAmount.GT(osmomath.ZeroInt()) { | |
| denomIsWhitelisted := isDenomWhitelisted(takerFeeCoin.Denom, communityPoolDenomWhitelist) | |
| // If the non osmo denom is a whitelisted quote asset, we directly send to the community pool | |
| if denomIsWhitelisted { |
🤖 Prompt for AI Agents
In x/txfees/keeper/hooks.go around lines 161 to 170, the loop processing
non-native takerFeeModuleAccountCoins must skip the base/default fee denom to
avoid re-processing OSMO; add a guard at the top of the loop that checks if
takerFeeCoin.Denom == defaultFeesDenom and continue if true, so the remainder
handling for the native denom is not executed here.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
x/txfees/keeper/hooks.go (1)
262-272: Don’t strand OSMO in the stakers module; forward the full post‑swap OSMO balance.Currently only the swapped amount (
totalCoinOut) is forwarded; any pre‑existing OSMO moved into the stakers module (e.g., leftover native OSMO) remains there.Apply this diff:
- if totalCoinOut.Amount.GT(osmomath.ZeroInt()) { - // Now that the assets have been swapped, transfer any base denom existing in the taker fee module account to the auth fee collector module account (indirectly distributing to stakers) - applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { - err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.TakerFeeStakersName, authtypes.FeeCollectorName, sdk.NewCoins(totalCoinOut)) - trackerErr := k.poolManager.UpdateTakerFeeTrackerForStakersByDenom(ctx, totalCoinOut.Denom, totalCoinOut.Amount) - if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for stakers by denom", "error", err) - } - return err - }, txfeestypes.TakerFeeFailedNativeRewardUpdateMetricName, totalCoinOut) - } + // After swap, forward the entire OSMO balance (including any pre-existing OSMO) to the fee collector. + osmoBalAfterSwap := k.bankKeeper.GetBalance(ctx, takerFeeStakersModuleAccount, defaultFeesDenom) + if osmoBalAfterSwap.Amount.GT(osmomath.ZeroInt()) { + applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { + err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.TakerFeeStakersName, authtypes.FeeCollectorName, sdk.NewCoins(osmoBalAfterSwap)) + trackerErr := k.poolManager.UpdateTakerFeeTrackerForStakersByDenom(ctx, osmoBalAfterSwap.Denom, osmoBalAfterSwap.Amount) + if trackerErr != nil { + ctx.Logger().Error("Error updating taker fee tracker for stakers by denom", "error", trackerErr) + } + return err + }, txfeestypes.TakerFeeFailedNativeRewardUpdateMetricName, osmoBalAfterSwap) + }
🧹 Nitpick comments (3)
x/txfees/keeper/hooks.go (3)
228-234: Use module→module transfer helper for clarity and consistency.Prefer
SendCoinsFromModuleToModulewhen both ends are modules.-applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { - err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, takerFeeModuleAccount, txfeestypes.TakerFeeBurnName, nonOsmoForBurn) - return err -}, txfeestypes.TakerFeeFailedBurnUpdateMetricName, nonOsmoForBurn) +applyFuncIfNoErrorAndLog(ctx, func(cacheCtx sdk.Context) error { + err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.TakerFeeCollectorName, txfeestypes.TakerFeeBurnName, nonOsmoForBurn) + return err +}, txfeestypes.TakerFeeFailedBurnUpdateMetricName, nonOsmoForBurn)Consider making the same change for the community‑pool and stakers sends above for consistency.
235-248: LGTM: swap then burn flow.Optional: If chain policy allows, consider
bankKeeper.BurnCoins(ctx, txfeestypes.TakerFeeBurnName, sdk.NewCoins(totalCoinOutForBurn))instead of sending to the null address to keep supply accounting explicit. Keep as‑is if null‑address burn is the established pattern.
112-119: Fix logger variable: log trackerErr, not err.Multiple places log
errinstead oftrackerErr, obscuring the real failure cause.Apply these diffs:
- if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for community pool by denom", "error", err) - } + if trackerErr != nil { + ctx.Logger().Error("Error updating taker fee tracker for community pool by denom", "error", trackerErr) + }- if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for stakers by denom", "error", err) - } + if trackerErr != nil { + ctx.Logger().Error("Error updating taker fee tracker for stakers by denom", "error", trackerErr) + }- if trackerErr != nil { - ctx.Logger().Error("Error updating taker fee tracker for burn by denom", "error", err) - } + if trackerErr != nil { + ctx.Logger().Error("Error updating taker fee tracker for burn by denom", "error", trackerErr) + }Also applies to: 144-151, 218-226, 241-248, 265-272
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)x/txfees/keeper/hooks.go(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T15:40:00.224Z
Learnt from: czarcas7ic
PR: osmosis-labs/osmosis#8310
File: x/poolmanager/store.go:129-143
Timestamp: 2024-10-08T15:40:00.224Z
Learning: Caching is not implemented for the `GetTakerFeeShareDenomsToAccruedValue` function in `x/poolmanager/store.go` due to efficient direct key access, specific usage patterns where values are updated immediately after retrieval, and the minor overhead of unmarshaling integers. This decision may be revisited if performance issues arise.
Applied to files:
x/txfees/keeper/hooks.go
🧬 Code graph analysis (1)
x/txfees/keeper/hooks.go (4)
osmomath/sdk_math_alias.go (1)
ZeroInt(43-43)x/txfees/types/keys.go (1)
TakerFeeBurnName(31-31)x/txfees/types/telemetry.go (1)
TakerFeeFailedBurnUpdateMetricName(27-27)x/txfees/types/constants.go (1)
DefaultNullAddress(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: go (02)
- GitHub Check: go (03)
- GitHub Check: go (01)
- GitHub Check: e2e
- GitHub Check: Summary
🔇 Additional comments (6)
x/txfees/keeper/hooks.go (6)
158-160: LGTM: dedicated accumulator for burn.
nonOsmoForBurnseparation is correct and makes the flow explicit.
167-173: LGTM: compute CP portion from originalAmount.Deriving from the original amount (not the running remainder) avoids compounding/truncation drift.
186-190: LGTM: aggregate non‑whitelisted CP portion from originalAmount.Keeps leftovers consistent for the stakers bucket.
193-200: LGTM: burn portion derived from originalAmount.Correct and aligns with the new design.
289-293: LGTM: zero‑amount guard.Prevents “token amount must be positive” errors during swap.
162-165: Skip base denom in the non‑native loop to avoid double‑processing OSMO.Otherwise leftover OSMO (e.g., rounding dust) is re-processed under the non‑native path.
Apply this diff:
for _, takerFeeCoin := range takerFeeModuleAccountCoins { + // Skip base denom; it is handled in the native path above. + if takerFeeCoin.Denom == defaultFeesDenom { + continue + } // Store original amount to calculate percentages from the total, not from remaining amounts originalAmount := takerFeeCoin.Amount
|
|
||
| // TakerFeeBurnName is the name of the module account that collects non-native taker fees, swaps them to OSMO, and burns them. | ||
| // Note, all taker fees initially get sent to the TakerFeeCollectorName, and then prior to the taker fees slated for burn being swapped and burned, they are sent to this account. | ||
| // This is done so that, in the event of a failed swap, the funds slated for burn are not grouped back with the rest of the taker fees in the next epoch. | ||
| TakerFeeBurnName = "non_native_fee_collector_burn" |
2 participants
Closes: CHAIN-1129
What is the purpose of the change
Use non-osmo taker fee burn portion to buy osmo and burn them.
Testing and Verifying
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)