Skip to content

Conversation

@tmckenzie51
Copy link
Contributor

@tmckenzie51 tmckenzie51 commented Mar 25, 2020

What was wrong?

construct_time_based_gas_price_strategy is unable to handle cases when network suddenly requires higher gas prices due to many variables such as network congestion. This has resulted in more transactions to revert.

Related to Issue #1463

How was it fixed?

A recency based weighted average of the block time is calculated. This weighted_avg_block_time is then used to calculate the probabilities required to determine the best gas price.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@tmckenzie51
Copy link
Contributor Author

Hi there! @Jmjin @kclowes

I just created this weighted average function that uses the weighted average by recency of the block in the sample, in order to calculate the weighted average block time for the time_based_gas_price_strategy function. I used n/N (where N is the sample size and n is the number of the block in the sample 1 through to N, with the Nth block being equivalent to the 'latest' block) as a simple weighting function that could capture the idea of weighting more recent blocks more heavily. Do you think that such a weighting function would achieve the goal of more accurately deciding a gas price during a time of sudden network congestion?

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like the right approach to me @tmckenzie51! Thank you! I think this deserves to be it's own strategy, meaning that I think we should leave the simple time based strategy, and have a separate weighted strategy. I'd like to see some more testing around this strategy to make sure it's working as expected, and there is also some white space that needs to be cleaned up. I'll plan on addressing this feedback since I'm sure you are busy with other things now :)

@tmckenzie51
Copy link
Contributor Author

This looks like the right approach to me @tmckenzie51! Thank you! I think this deserves to be it's own strategy, meaning that I think we should leave the simple time based strategy, and have a separate weighted strategy. I'd like to see some more testing around this strategy to make sure it's working as expected, and there is also some white space that needs to be cleaned up. I'll plan on addressing this feedback since I'm sure you are busy with other things now :)

@kclowes That makes sense to have it as a separate strategy. Thanks for pointing that out, and thank you for the feedback and for handling this for me. :)

@kclowes kclowes force-pushed the tiffany/gas-pricing branch from a6520d9 to 68a1fae Compare April 10, 2020 22:12
@kclowes kclowes force-pushed the tiffany/gas-pricing branch 2 times, most recently from 3ffcf6e to 06f4de7 Compare April 13, 2020 21:12
@kclowes
Copy link
Collaborator

kclowes commented Apr 13, 2020

@marcgarreau I think this is good to go if you want to take a look and give final approval!

Copy link
Member

@wolovim wolovim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice contribution, thanks @tmckenzie51!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants