Skip to content
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

Reactive gas price oracle #314

Merged

Conversation

uprendis
Copy link
Collaborator

@uprendis uprendis commented May 19, 2022

rework gasprice oracle to rely on two parts:

  • Constructive part: increases gasprice according to accumulated gas power of validators (less -> higher price). This part is needed to protect network from draining validators. This part is needed because minGasPrice/baseFee balancing cannot be done very quickly per-block due to epoch-based consensus structure
  • Reactive: takes gas price from top G's gas position in an average of txpool statistic for last 12 seconds, where G depends on chosen certainty (higher certainty -> smaller G). Certainty is a percentile of eth_feeHistory

Final gasprice estimation is a maximum of these 2 parts

Previously gasPrice estimation was based only on constructive part. This PR adjusts weights for the constructive part and improves calculation of accumulated gas power of validators

Note: like before, meeting the constructive part is not optional but mandatory, it serves as a soft minGasPrice/baseFee limit. This PR adds ftm_effectiveBaseFee API call for retrieving current soft limit for gas price

Copy link
Contributor

@hadv hadv left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -555,6 +555,17 @@ func (pool *TxPool) Pending(enforceTips bool) (map[common.Address]types.Transact
return pending, nil
}

func (pool *TxPool) PendingSlice() types.Transactions {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it'd better to rename this method to PendingTxs

Suggested change
func (pool *TxPool) PendingSlice() types.Transactions {
func (pool *TxPool) PendingTxs() types.Transactions {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh it's just because there's already a method called Pending which returns a map, I wanted to emphasise the difference between them

Copy link
Contributor

@rus-alex rus-alex left a comment

Choose a reason for hiding this comment

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

go test ./gossip failed:
*dummyTxPool does not implement TxPool (missing PendingSlice method)

@cyberbono3
Copy link
Collaborator

I suggest to increase test coverage of gossip/gasprice package
ok github.com/Fantom-foundation/go-opera/gossip/gasprice 0.148s coverage: 0.0% of statements [no tests to run]

@uprendis uprendis force-pushed the feature/reactive-gas-oracle branch from 37ec55a to 54a672f Compare May 24, 2022 02:40
@uprendis
Copy link
Collaborator Author

@cyberbono3 added tests in e4ddb6d

@uprendis uprendis merged commit f591585 into Fantom-foundation:develop May 24, 2022
@BMItr
Copy link

BMItr commented May 24, 2022

👌 great work

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.

None yet

5 participants