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

Adapt arbitrage weight calculations to proof sizes #1120

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

maltekliemann
Copy link
Contributor

What does it do?

Previously, execute_arbitrage_all did the following calculation to estimate how many pools can be arbitrages. Let $w(a)$ be the weight required for arbitraging $a$ pools. We assume that $w$ is a linear function, $w(a) = ma + b$. We therefore have $w(1) - w(0) = m + b - b = m$. Suppose that $W$ is the available weight. Then the maximum number of pools we can arbitrage in the time available is $\frac{W - b}{m}$ (just solve $W = w(a)$ for $a$), ignoring the fact that we need to round down to make $a$ an integer.

But weights now have two components, ref time and proof size. The changes in this PR are the following:

  • Introduce a minimum available proof size of 100 KB for on_idle to even do the calculation described above. This is definitely an overestimation, but we should be on the safe side.
  • Perform the calculation described above both for the ref time and the proof size. This gives us a maximum amount of pools that can be arbitraged using the available ref time and a maximum amount of pools that can be arbitraged using the available proof size. Taking the minimum of both gives us the maximum number of pools we can arbitrage with the available resources.

What important points should reviewers know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues?

References

@maltekliemann maltekliemann added the s:review-needed The pull request requires reviews label Sep 19, 2023
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 88.34% and project coverage change: -0.10% ⚠️

Comparison is base (851c803) 93.53% compared to head (3412ffa) 93.43%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1120      +/-   ##
==========================================
- Coverage   93.53%   93.43%   -0.10%     
==========================================
  Files          95       95              
  Lines       29137    29552     +415     
==========================================
+ Hits        27252    27612     +360     
- Misses       1885     1940      +55     
Flag Coverage Δ
tests 93.43% <88.34%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
zrml/liquidity-mining/src/weights.rs 0.00% <0.00%> (ø)
zrml/styx/src/weights.rs 0.00% <0.00%> (ø)
zrml/swaps/src/lib.rs 86.77% <76.92%> (-0.07%) ⬇️
zrml/court/src/weights.rs 79.03% <79.78%> (+0.12%) ⬆️
zrml/swaps/src/weights.rs 83.55% <83.54%> (-1.18%) ⬇️
zrml/prediction-markets/src/weights.rs 96.94% <96.91%> (+<0.01%) ⬆️
zrml/authorized/src/weights.rs 100.00% <100.00%> (ø)
zrml/global-disputes/src/weights.rs 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maltekliemann maltekliemann changed the base branch from main to release-v0.4.0 September 20, 2023 11:40
Copy link
Member

@sea212 sea212 left a comment

Choose a reason for hiding this comment

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

LGTM

@sea212 sea212 added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels Sep 20, 2023
@sea212 sea212 merged commit 628c559 into release-v0.4.0 Sep 23, 2023
@sea212 sea212 deleted the mkl-fix-arbitrage branch September 23, 2023 15:16
sea212 added a commit that referenced this pull request Oct 11, 2023
* Update versions to v0.4.0 (#1098)

* Update weights (#1101)

* Reduce length of `MarketsCollectingSubsidy` (#1118)

* Add bad block of the proof size fiasko to Battery Station chain spec (#1119)

Add bad block to Battery Station chain spec

* Update weights v0.4.0 (#1121)

* Update moonbeam dependencies (bench fix)

* Update weights

* Adapt arbitrage weight calculations to proof sizes (#1120)

Include proof size into arbitrage weight estimates

---------

Co-authored-by: Malte Kliemann <mail@maltekliemann.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:accepted This pull request is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants