-
Notifications
You must be signed in to change notification settings - Fork 607
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
Conversation
// 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:] |
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.
Note:
The following are the reasons why this is done by string matching manually:
- https://github.com/pelletier/go-toml does not handle comments when writing. So all config comments would be removed if I were to load the file, update the value, and overwrite.
- SDK helpers either unexpectedly overwrite other config values or run into the same comment issue as above. I found it easier to simply string-match
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.
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 also ran into this when I attempted this a while ago, thanks for making an issue
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.
Nice work
// Check if the file is writable | ||
if fileInfo.Mode()&os.FileMode(0200) != 0 { |
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.
Closes: #XXX ## What is the purpose of the change To address the failing arbitrage spam taking up block space, we will increase [arbitrage-min-gas-fee](https://github.com/osmosis-labs/osmosis/blob/3aaf5d117f96df0092b0375d0c8de0a12850d092/cmd/osmosisd/cmd/root.go#L540) to be near the order of magnitude of 0.1 OSMO. CL Swap is around 600K gas wanted and 340K gas used. Let's assume a middle ground of 450K and aim to have the arb pay approximately 0.05 OSMO. ``` 0.045 = 450_000 * 0.1 / 10**6 ``` As a result, 0.1 is an appropriate choice. ## Testing and Verifying Tested the following vectors: - new config - does no overwrite other values - old value is "0.005" -> overwritten - old value is ".005" -> overwritten - old value is something else -> does not change ## Documentation and Release Note - [ ] Does this pull request introduce a new feature or user-facing behavior changes? - [ ] Changelog entry added to `Unreleased` section of `CHANGELOG.md`? Where is the change documented? - [ ] Specification (`x/{module}/README.md`) - [ ] Osmosis documentation site - [ ] Code comments? - [ ] N/A (cherry picked from commit af1ed8c)
…kport #7368) (#7383) * refactor: overwrite ArbitrageMinGasPrice config from .005 to 0.1 (#7368) Closes: #XXX ## What is the purpose of the change To address the failing arbitrage spam taking up block space, we will increase [arbitrage-min-gas-fee](https://github.com/osmosis-labs/osmosis/blob/3aaf5d117f96df0092b0375d0c8de0a12850d092/cmd/osmosisd/cmd/root.go#L540) to be near the order of magnitude of 0.1 OSMO. CL Swap is around 600K gas wanted and 340K gas used. Let's assume a middle ground of 450K and aim to have the arb pay approximately 0.05 OSMO. ``` 0.045 = 450_000 * 0.1 / 10**6 ``` As a result, 0.1 is an appropriate choice. ## Testing and Verifying Tested the following vectors: - new config - does no overwrite other values - old value is "0.005" -> overwritten - old value is ".005" -> overwritten - old value is something else -> does not change ## Documentation and Release Note - [ ] Does this pull request introduce a new feature or user-facing behavior changes? - [ ] Changelog entry added to `Unreleased` section of `CHANGELOG.md`? Where is the change documented? - [ ] Specification (`x/{module}/README.md`) - [ ] Osmosis documentation site - [ ] Code comments? - [ ] N/A (cherry picked from commit af1ed8c) * changelog --------- Co-authored-by: Roman <[email protected]>
why not make it just 1 osmo and leave this spam ahead? |
Generally, I agree with increasing it even higher. I was worried about making too much of a rapid change for all nodes at once. This is already a 20x increase |
Closes: #XXX
What is the purpose of the change
To address the failing arbitrage spam taking up block space, we will increase arbitrage-min-gas-fee to be near the order of magnitude of 0.1 OSMO.
CL Swap is around 600K gas wanted and 340K gas used. Let's assume a middle ground of 450K and aim to have the arb pay approximately 0.05 OSMO.
As a result, 0.1 is an appropriate choice.
Testing and Verifying
Tested the following vectors:
Documentation and Release Note
Unreleased
section ofCHANGELOG.md
?Where is the change documented?
x/{module}/README.md
)