-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: overwrite ArbitrageMinGasPrice config from .005 to 0.1 #7368
Changes from all commits
0eebe9f
dc776e0
10e44d4
cee96d9
33a0a44
aaeda8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,6 +94,11 @@ type DenomUnitMap struct { | |
} | ||
|
||
const ( | ||
mempoolConfigName = "osmosis-mempool" | ||
arbitrageMinGasFeeConfigName = "arbitrage-min-gas-fee" | ||
oldArbitrageMinGasFeeValue = ".005" | ||
newArbitrageMinGasFeeValue = "0.1" | ||
|
||
consensusConfigName = "consensus" | ||
timeoutCommitConfigName = "timeout_commit" | ||
) | ||
|
@@ -477,6 +482,79 @@ func overwriteConfigTomlValues(serverCtx *server.Context) error { | |
return nil | ||
} | ||
|
||
// overwrites app.toml values. Returns error if app.toml does not exist | ||
// | ||
// Currently, overwrites arbitrage-min-gas-fee value in app.toml if it is set to 0.005. Similarly, | ||
// overwrites the given viper config value. | ||
// Silently handles and skips any error/panic due to write permission issues. | ||
// No-op otherwise. | ||
func overwriteAppTomlValues(serverCtx *server.Context) error { | ||
// Get paths to config.toml and config parent directory | ||
rootDir := serverCtx.Viper.GetString(tmcli.HomeFlag) | ||
|
||
configParentDirPath := filepath.Join(rootDir, "config") | ||
configFilePath := filepath.Join(configParentDirPath, "app.toml") | ||
|
||
fileInfo, err := os.Stat(configFilePath) | ||
if err != nil { | ||
return fmt.Errorf("failed to read in %s: %w", configFilePath, err) | ||
} else { | ||
// app.toml exists | ||
|
||
// Get setting | ||
currentArbitrageMinGasFeeValue := serverCtx.Viper.Get(mempoolConfigName + "." + arbitrageMinGasFeeConfigName) | ||
|
||
// .x format at 0.x format are both valid. | ||
if currentArbitrageMinGasFeeValue == oldArbitrageMinGasFeeValue || currentArbitrageMinGasFeeValue == "0"+oldArbitrageMinGasFeeValue { | ||
// Set new value in viper | ||
serverCtx.Viper.Set(mempoolConfigName+"."+arbitrageMinGasFeeConfigName, newArbitrageMinGasFeeValue) | ||
|
||
defer func() { | ||
if err := recover(); err != nil { | ||
fmt.Printf("failed to write to %s: %s\n", configFilePath, err) | ||
} | ||
}() | ||
|
||
// Check if the file is writable | ||
if fileInfo.Mode()&os.FileMode(0200) != 0 { | ||
// Read the entire content of the file | ||
content, err := os.ReadFile(configFilePath) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Convert the content to a string | ||
fileContent := string(content) | ||
|
||
// Find the index of the search line | ||
index := strings.Index(fileContent, arbitrageMinGasFeeConfigName) | ||
if index == -1 { | ||
return fmt.Errorf("search line not found in the file") | ||
} | ||
|
||
// Find the opening and closing quotes | ||
openQuoteIndex := strings.Index(fileContent[index:], "\"") | ||
openQuoteIndex += index | ||
|
||
closingQuoteIndex := strings.Index(fileContent[openQuoteIndex+1:], "\"") | ||
closingQuoteIndex += openQuoteIndex + 1 | ||
|
||
// Replace the old value with the new value | ||
newFileContent := fileContent[:openQuoteIndex+1] + newArbitrageMinGasFeeValue + fileContent[closingQuoteIndex:] | ||
Comment on lines
+526
to
+543
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: The following are the reasons why this is done by string matching manually:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also ran into this when I attempted this a while ago, thanks for making an issue |
||
|
||
// Write the modified content back to the file | ||
err = os.WriteFile(configFilePath, []byte(newFileContent), 0644) | ||
if err != nil { | ||
return err | ||
} | ||
} else { | ||
fmt.Println("app.toml is not writable. Cannot apply update. Please consder manually changing arbitrage-min-gas-fee to " + newArbitrageMinGasFeeValue) | ||
} | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func getHomeEnvironment() string { | ||
envPath := filepath.Join(osmosis.DefaultNodeHome, ".env") | ||
|
||
|
@@ -519,7 +597,7 @@ func initAppConfig() (string, interface{}) { | |
// 128MB IAVL cache | ||
srvCfg.IAVLCacheSize = 781250 | ||
|
||
memCfg := OsmosisMempoolConfig{ArbitrageMinGasPrice: "0.01"} | ||
memCfg := OsmosisMempoolConfig{ArbitrageMinGasPrice: "0.1"} | ||
|
||
sqsConfig := sqs.DefaultConfig | ||
|
||
|
@@ -536,8 +614,8 @@ func initAppConfig() (string, interface{}) { | |
max-gas-wanted-per-tx = "25000000" | ||
|
||
# This is the minimum gas fee any arbitrage tx should have, denominated in uosmo per gas | ||
# Default value of ".005" then means that a tx with 1 million gas costs (.005 uosmo/gas) * 1_000_000 gas = .005 osmo | ||
arbitrage-min-gas-fee = ".005" | ||
# Default value of ".1" then means that a tx with 1 million gas costs (.1 uosmo/gas) * 1_000_000 gas = .1 osmo | ||
arbitrage-min-gas-fee = ".1" | ||
|
||
# This is the minimum gas fee any tx with high gas demand should have, denominated in uosmo per gas | ||
# Default value of ".0025" then means that a tx with 1 million gas costs (.0025 uosmo/gas) * 1_000_000 gas = .0025 osmo | ||
|
@@ -619,6 +697,12 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) { | |
return err | ||
} | ||
|
||
// overwrite app.toml values | ||
err = overwriteAppTomlValues(serverCtx) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return startRunE(cmd, args) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this implies if the file is not writeable, then the defer func wont trigger so there will be no feedback for the operator right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They would just not have their config updated which IMO is fine. The intention is to silently ignore the update if their file isn't writeable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I just think they would want to still get a log print saying it wasn't overwritten due to permission issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33a0a44