Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Sync gas price #144

Open
wants to merge 40 commits into
base: snowbridge
Choose a base branch
from
Open

Sync gas price #144

wants to merge 40 commits into from

Conversation

claravanstaden
Copy link
Collaborator

@claravanstaden claravanstaden commented May 2, 2024

snowbridge companion: Snowfork/snowbridge#1188

bridges/snowbridge/pallets/gas-price/src/lib.rs Outdated Show resolved Hide resolved
bridges/snowbridge/pallets/gas-price/src/lib.rs Outdated Show resolved Hide resolved
bridges/snowbridge/pallets/gas-price/src/lib.rs Outdated Show resolved Hide resolved
@yrong yrong marked this pull request as ready for review June 13, 2024 09:34
@vgeddes
Copy link

vgeddes commented Jun 15, 2024

Since beacon headers are now imported on-demand, we need to adjust the EMA algorithm. Updates which are much more recent, ie thousands of blocks compared to the last recorded value) should dominate the EMA.

The scaling factor should be dynamically adjusted so that it ranges between [0.2, 1]:

Window = EPOCHS_PER_SYNC_COMMITTEE_PERIOD * SLOTS_PER_EPOCH
ScalingFactor = Min(
   0.2,
   (Min(Window, Update.Slot) - LastRecordedSlot) / Window
)
EMA = EMA + ScalingFactor * (Update.GasPrice - EMA)

Let me know what you think. This is just an idea.

@yrong
Copy link

yrong commented Jun 16, 2024

Updates which are much more recent should dominate the EMA, ...scaling factor should be dynamically adjusted so that it ranges between [0.2, 1]

Yeah, that make sense.

Btw, is there something wrong with the formula for ScalingFactor? Assume the LastRecordedSlot is 1 and Update.Slot is 8192, the formula should be something as following?

image

yrong and others added 6 commits June 17, 2024 22:08
* Improve gas price implementation

* Fix build error

* Fix breaking tests

* Fix unit test

* Fix integration test

---------

Co-authored-by: ron <[email protected]>
@vgeddes vgeddes requested a review from musnit July 15, 2024 10:02
@alistair-singh
Copy link

The scaling factor should be dynamically adjusted so that it ranges between [0.2, 1]:

Seems like this code was reverted? Did we find that it did not matter? The new implementation is much simpler.

@yrong yrong closed this Jul 30, 2024
@yrong yrong reopened this Aug 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants