F/gas price increment #106
Conversation
nhannamsiu
requested review from
albertchon,
vinhphuctadang and
achilleas-kal
|
Down the road we can add more fancy strategies like slowly reducing the gas over the period of time instead of just resetting it to baseline. |
|
We're very far from reaching tx limits per block and inclusion as of now is guaranteed. When the network grows and gas price increase becomes relevant towards inclusion I believe market makers will aim to increase gas prices (hardcoded) instead of arbitrarily setting gas prices for them. The network will most of the times operate under the same frequency in a 3-minute interval. Tx timeouts currently take place if the node does not respond in time based on I don't believe there's a need for these changes. |
|
@achilleas-kal I created this PR to follow up this issue https://injectivelabsinc.slack.com/archives/C03AHBPV7B7/p1676168357053979 |
|
The issue was not related to block inclusion and higher gas prices needed though, the client just didn't have enough INJ for gas which should be tracked individually per address on their end and not through the SDK. This specific client had actually set double gas prices from the minimum in the network which is a waste of gas since gas currently does not affect block inclusion. Even when gas does affect block inclusion though, market makers will aim to find the optimal value based on their tolerance and hardcode it to ensure their txs get finalized first. The activity is the same over a 1-minute window, I don't believe the SDK should or even can optimally determine differences in gas prices over such low periods of time. What would be helpful for clients when gas becomes relevant would be an API to determine average gas price in the network from all txs based on all activity etc. so they can choose their value based on preference. |
Overall, I think this PR provides a convenient behavior when it can auto increase gas prices; on high level, client only needs to retry, finally the tx will be succeeded, before they were panic because tx keeps failing. So it's better than before It's also a good point to have an API to check average tx gas, but actually out of this PR's scope |
| logger := c.logger.WithField("size", len(msgs)) | ||
| if strings.Contains(err.Error(), "tx timed out") { | ||
| if c.gasPriceIncrement.IsPositive() { | ||
| logger = logger.WithField("hint", "please check your wallet inj balance, gas price, tx timeout configs") |
There was a problem hiding this comment.
we should log this line even if the gasIncrement is not positive (0 e.g)
| logger := c.logger.WithField("size", len(toSubmit)) | ||
| if strings.Contains(err.Error(), "tx timed out") { | ||
| if c.gasPriceIncrement.IsPositive() { | ||
| logger = logger.WithField("hint", "please check your wallet inj balance, gas price, tx timeout configs") |
| } | ||
|
|
||
| // resetGasPrices resets gas price to default value to avoid wasting gas when it's network is not busy | ||
| func (c *chainClient) resetGasPrices(defaultGasPrices string, timer *time.Ticker) { |
There was a problem hiding this comment.
We should have a ResetGasPrice func to explicit reset gas. It's helpful when the bot realize that all txs has passed so it don't need adjusted gas price anymore, this go routine is not controllable
Add 2 new chain config options and hint to resolve tx timeout issue
common.OptionGasPriceIncrement(10)every timetx timed outerror happend, gas prices will be increased by 10% of current gas prices to prioritize txs into block inclusion when network is busycommon.OptionsGasPriceResetInterval(time.NewTicker(time.Minute*3))gas prices will be reset back to baseline every 3 minutes to avoid wasting gas when network is not busy.