-
Notifications
You must be signed in to change notification settings - Fork 651
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
Feat(stateless-validation): Dynamically compute mandate price from target number of mandates per shard #11044
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11044 +/- ##
==========================================
+ Coverage 71.06% 71.10% +0.03%
==========================================
Files 767 768 +1
Lines 153338 153513 +175
Branches 153338 153513 +175
==========================================
+ Hits 108976 109158 +182
+ Misses 39922 39912 -10
- Partials 4440 4443 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
<p>This PR was automatically created by Snyk using the credentials of a real user.</p><br /><h3>Snyk has created this PR to upgrade react-router-dom from 6.19.0 to 6.20.0.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **2 versions** ahead of your current version. - The recommended version was released **21 days ago**, on 2023-11-22. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>react-router-dom</b></summary> <ul> <li> <b>6.20.0</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.20.0">2023-11-22</a></br><p>react-router-native@6.20.0</p> </li> <li> <b>6.20.0-pre.0</b> - 2023-11-21 </li> <li> <b>6.19.0</b> - 2023-11-16 </li> </ul> from <a href="https://snyk.io/redirect/github/remix-run/react-router/releases">react-router-dom GitHub release notes</a> </details> </details> <details> <summary><b>Commit messages</b></summary> </br> <details> <summary>Package name: <b>react-router-dom</b></summary> <ul> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/3cc38eac4753702a9a8a1fe239e2138d63ac6cc5">3cc38ea</a> chore: Update version for release (near#11050)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/77862f95f71397d92b8b583a16674c56efccc55f">77862f9</a> Exit prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1621319ffa353bf33f33064d7611859df16286ee">1621319</a> Update Release Notes TOC</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f1f8ed0acffb3d6c2860c362fc2b376dbf87df24">f1f8ed0</a> Update release notes</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1e026b6f1ac34a774b4f77e5e3696251e8f79940">1e026b6</a> chore: Update version for release (pre) (near#11047)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/4a08d64c368c07816e753632345a13b8da050111">4a08d64</a> Enter prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/c0b4e12d12c5abdf5f7723e71959c4eb5e9effd9">c0b4e12</a> Merge branch 'main' into release-next</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/58d421fc4c592661a68dea59edc507fc4668ba5d">58d421f</a> Fix other code paths for resolveTo from a splat route (near#11045)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/5f530a775cd266940f725894277b6ea7bc55b5d0">5f530a7</a> Do not revalidate unmounted fetchers when v7_persistFetcher is enabled (near#11044)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f320378b5145f59bb266a35a7655b563f712daef">f320378</a> Add additional test case for near#10983</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/a48c43c8118bbf75b23b3ee748648bb3ee4d688e">a48c43c</a> feat: export `PathParam` type (near#10719)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1d56e55d3f95730f99617dff23cf153f82394921">1d56e55</a> Remove tag links from headings in CHANGELOG.md</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/3b1a7364c730209b4baed9454c7f6c17c55e3ba8">3b1a736</a> Fix flaky test</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/406a1ddecd399ede2a517d7cae5f3ee63d02ed91">406a1dd</a> Merge branch 'release-next' into dev</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/6a5939b07c06c9dacd82705645dbbbe46de90e5e">6a5939b</a> Merge branch 'release-next'</li> </ul> <a href="https://snyk.io/redirect/github/remix-run/react-router/compare/dcf0c2a85aac3a78059a287ea478ff12adcb6a2d...3cc38eac4753702a9a8a1fe239e2138d63ac6cc5">Compare</a> </details> </details> <hr/> **Note:** *You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs.* For more information: <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiIyZmI3YjkxYi0zN2NiLTQzYzgtOWNkYS02ODAzNTFmYzMwNmQiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjJmYjdiOTFiLTM3Y2ItNDNjOC05Y2RhLTY4MDM1MWZjMzA2ZCJ9fQ==" width="0" height="0"/> 🧐 [View latest project report](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source=github&utm_medium=referral&page=upgrade-pr) 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?pkg=react-router-dom&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) <!--- (snyk:metadata:{"prId":"2fb7b91b-37cb-43c8-9cda-680351fc306d","prPublicId":"2fb7b91b-37cb-43c8-9cda-680351fc306d","dependencies":[{"name":"react-router-dom","from":"6.19.0","to":"6.20.0"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"98480bdc-d80b-4fd1-89d7-c4c56a706763","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":2,"publishedDate":"2023-11-22T16:56:32.720Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]}) ---> Co-authored-by: snyk-bot <snyk-bot@snyk.io> Co-authored-by: nikurt <86772482+nikurt@users.noreply.github.com>
…rget number of mandates per shard
e3ee74e
to
b2b08de
Compare
Actually, what prevents us from implementing a binary search instead? The function mandates(m) = sum(ceil(a[i]/m)) is non-increasing, so in 128 iterations we always find exact answer. This would make behaviour and testing much more consistent. |
Good suggestion @Longarithm ! Implemented the binary search in 5ca2a48 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
let target_mandates_per_shard = stakes.len(); | ||
let config = ValidatorMandatesConfig::new(target_mandates_per_shard, num_shards); | ||
let price = compute_mandate_price(config, &stakes); | ||
assert_eq!(count_whole_mandates(&stakes, price), target_mandates_per_shard + 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't understand how to explain + 3
probabilistically, may worth a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment in 5326630
It's not very explanatory, but does point out that it is sometimes impossible to get the exact target because we are working with integers.
This PR completes the remaining tasks in #10014 though in a slightly different way that initially conceived. This PR adds a function to compute the mandate price from a target number of mandates and the distribution of stakes. The function works by looking for the fixed point of a particular function, where this fixed point should exist because it arises from calculating the same value in two different ways. More details on this are found in the comments on the new code.
This is a little different than the initial design where we define a minimum number of mandates per shard. Instead we give a target number for the function to attempt to achieve. For many stake distributions it will exactly hit the target, but it is possible that the target cannot be exactly achieved due to the discrete nature of the problem. In this case the function will get as close as possible to the target; even if that means getting a value slightly lower. Importantly, the mandate counting done by the function does not include partial mandates, therefore if the number of whole mandates ends up a little below the target the partial mandates should make up for it to keep the security of the protocol high enough.
As part of this PR, the target number of mandates per shard in production is set to 68. This number was chosen based on some theory calculations. In a future PR this number should probably be extracted out as a genesis config parameter, but I think it's ok for now (stateless validation MVP).