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

added RL sliding windows and osmo RLs #4461

Merged
merged 7 commits into from
Mar 1, 2023
Merged

Conversation

nicolaslara
Copy link
Contributor

What is the purpose of the change

We had discussed adding sliding windows to rate limits to avoid allowing double the max at the time where the RL falls over. This wasn't included in the original code that was going to accompany the proposal, but adding it here now.

Brief Changelog

  • Added sliding windows
  • Added RLs for uosmo

Testing and Verifying

all tests pass

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (yes / no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
  • How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

@github-actions github-actions bot added the C:app-wiring Changes to the app folder label Feb 28, 2023
@nicolaslara nicolaslara added V:state/breaking State machine breaking PR A:backport/v15.x backport patches to v15.x branch labels Feb 28, 2023
Comment on lines 137 to 140
{"name":"ATOM-DAY-1","duration":86400,"send_recv":[30,30]},
{"name":"ATOM-DAY-2","duration":129600,"send_recv":[30,30]},
{"name":"ATOM-WEEK-1","duration":604800,"send_recv":[60,60]},
{"name":"ATOM-WEEK-2","duration":907200,"send_recv":[60,60]}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't name be UOSMO-DAY1 etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! copy/paste mistake

{"name":"ATOM-DAY","duration":86400,"send_recv":[30,30]},
{"name":"ATOM-WEEK","duration":604800,"send_recv":[60,60]}
{"name":"ATOM-DAY-1","duration":86400,"send_recv":[30,30]},
{"name":"ATOM-DAY-2","duration":129600,"send_recv":[30,30]},
Copy link
Member

Choose a reason for hiding this comment

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

Asking to learn: can you please elaborate on the sliding window method and how this change enabled it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. If we have a limit of 30% every 24h, for example, that means that at the seams (when one period ends and the other begins) you could actually transfer 60%, 30% in the block before, and then 30% in the block after.

To avoid this, we stagger the windows by half the time, so if you tried to do the above, the second quota would prevent it.

@p0mvn
Copy link
Member

p0mvn commented Feb 28, 2023

Also, what's the decision about adding the rate limits for ion?

@p0mvn
Copy link
Member

p0mvn commented Feb 28, 2023

Please add a changelog entry for this

@nicolaslara
Copy link
Contributor Author

Also, what's the decision about adding the rate limits for ion?

I don't think we have one. I think osmo can be considered as implied by https://commonwealth.im/osmosis/discussion/8543-set-ibc-rate-limits as it was listed in all the charts, but prob not the same fore ion?

@p0mvn p0mvn merged commit 03f2a98 into main Mar 1, 2023
@p0mvn p0mvn deleted the nicolas/RL-windows-and-osmo branch March 1, 2023 14:55
mergify bot pushed a commit that referenced this pull request Mar 1, 2023
* added RL sliding windows and osmo RLs

* added missing comma

* osmo name

* changelog

* updated test

* removed uosmo until we can get the proper channel value from ibc

(cherry picked from commit 03f2a98)
p0mvn pushed a commit that referenced this pull request Mar 1, 2023
* added RL sliding windows and osmo RLs

* added missing comma

* osmo name

* changelog

* updated test

* removed uosmo until we can get the proper channel value from ibc

(cherry picked from commit 03f2a98)

Co-authored-by: Nicolas Lara <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v15.x backport patches to v15.x branch C:app-wiring Changes to the app folder V:state/breaking State machine breaking PR
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants