Skip to content

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
Contributor

@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 <[email protected]>
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.

4 participants