Fix fee-less transactions, and refactor it to have fee-less addresses 🚀#1389
Fix fee-less transactions, and refactor it to have fee-less addresses 🚀#1389ducksquaddd wants to merge 10 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1389 +/- ##
==========================================
+ Coverage 57.43% 57.47% +0.04%
==========================================
Files 1002 1002
Lines 42408 42417 +9
==========================================
+ Hits 24355 24380 +25
+ Misses 16364 16350 -14
+ Partials 1689 1687 -2 🚀 New features to boost your workflow:
|
app/ante/validator_tx_fee.go
Outdated
| "elys16qgewtplahkqwqa0aqv4pxnxa58ulu48k6crhj", // Price Feeder 1 mainnet | ||
| "elys1zwexzk6ns5ermvag5fc0gtyvrnxyaz9kzaflqf", // Price Feeder 2 mainnet | ||
| "elys1nelawytdfdk4af3z0sy2p8vkrllk8zw9g32jmf", // Execution bot 1 mainnet | ||
| "elys1gszp63euzm0ecs3qwu0j6mexjr97hjs7x5gvzk", // Execution bot 2 mainnet |
There was a problem hiding this comment.
we should not hardcode these addresses, should be stored or removed via gov
There was a problem hiding this comment.
Building that logic out is definitely outside my scope of Cosmos SDK. But I also never saw a project do it like that, in everyone else's implementation things were hard coded.
But maybe can implement some logic so that if its testnet only testnet addresses are in the array, etc.
There was a problem hiding this comment.
ref this: #1389 (comment)
In pre-blocker, you can call params and use a setter function.
app/ante/validator_tx_fee.go
Outdated
| } | ||
| minGasPrices = sdk.NewDecCoins(minGasPrice) | ||
| "elys1gy503ty29ydute5rksnkwgmtatelret9d455lt", // Execution bot devnet | ||
| "elys1dae9z45ccetfwr208ghya6npntg75qvxgmg4p9", // Price Feeder devnet |
| priority = getTxPriority(feeCoins, int64(gas)) | ||
| } | ||
|
|
||
| return feeCoins, priority, nil |
There was a problem hiding this comment.
in this we are making all txs gases for hardcoded addresses, is that correct ?
I guess we should only allow price txs as gasless otherwise someone can spam the chain with other txs if compromised.
There was a problem hiding this comment.
The priority changes the priority of the TX, allowing our gasless TXs to execute before everyone else otherwise our gasless TXs always execute last.
This logic also does not remove gas from the tx it simply makes it so the minimum fee is 0, so if you upload a TX with a fee 0 it can pass. But you can still pass in a fee.
Also because this uses the feepayer address to identify who can send 0 uelys fees, we can actually make it one address then do a feegrant, from say governance. So we could control who has access.
| // GaslessAddrs contains only the governance module address. | ||
| // Governance can grant feegrants to operational addresses (price feeders, liquidation bots, etc.) | ||
| // When those addresses use feegrants, the fee payer becomes governance, enabling gasless transactions. | ||
| var GaslessAddrs = []string{ |
There was a problem hiding this comment.
hard coding sender of tx makes tit difficult to change on the fly. Why not set it when chain starts by fetching params of respective module?
Only con is it might have to be done every block or we can do periodically or some other design
There was a problem hiding this comment.
I mean this makes it so governance can change it on the fly. If a wallet gets compromised then the governance module simply revokes the fee grant.
| feePayer := sdk.AccAddress(feePayerBytes).String() | ||
|
|
||
| for _, ga := range GaslessAddrs { | ||
| if feePayer == ga { |
There was a problem hiding this comment.
this allow address to do all txs gasless
There was a problem hiding this comment.
Yeah I mean this was just a design choice for flexibility. Allows governance to pick and choose what they want to allow as fee-less too. Then if it's abused the abuser can get removed it's not really a huge risk IMHO.
But I mean I don't mind either way we can limit it to specific TXs.
3 participants
The old system which existed to allow fee-less price feeder TXs simply didnt work, due to it comparing lowercase msg type URLs to mixed case msg type URLs.
This new system replaces that logic, and allows certain addresses to ability to define 0uelys fees and have it get into the block. It also forces highest execution priority to TXs of such nature, so Price-Feeder TXs should always execute before anything else for example.