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

feat(dk): log(n) draw #89

Merged
merged 13 commits into from
Jun 20, 2022
Merged

feat(dk): log(n) draw #89

merged 13 commits into from
Jun 20, 2022

Conversation

shotaronowhere
Copy link
Contributor

Original implementation sload's the entire sortition tree node array and mstores this array.

This implementation only accesses the necessary sortition tree indexes; hence log(n) complexity per draw.

There are a few more low hanging fruit sortition tree related optimizations, will address these in the future.

@netlify
Copy link

netlify bot commented Jun 10, 2022

Deploy Preview for kleros-v2 ready!

Name Link
🔨 Latest commit e6e5216
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2/deploys/62b0fac96ceee500085458d0
😎 Deploy Preview https://deploy-preview-89--kleros-v2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@jaybuidl
Copy link
Member

jaybuidl commented Jun 13, 2022

It would be useful to have a benchmark to ensure that it does save gas because it saves 1 SLOAD+MSTORE operation at the expense of several CALL to KlerosCore at each iteration.

@shotaronowhere
Copy link
Contributor Author

Updating with some gas benchmarks reported from hardhat with the updated test file. The efficient implementation is more gas efficient with ~ 16 > jurors staked in the sortition tree under the test scenario.

n = number of staked jurors

n=16
| Original · draw · - · - · 477138 gas
| PR · draw · - · - · 481326 gas
n=32
| Original · draw · - · - · 549101 ·gas
| PR · draw · - · - · 487179 ·gas
n=64
| Original · draw · - · - · 719880 ·gas
| PR · draw · - · - · 492766 ·gas
n=128
| Original · draw · - · - · 1006146 gas
| PR · draw · - · - · 500206 ·gas
n=256
| Original · draw · - · - · 1572408 gas
| PR · draw · - · - · 502057 ·gas

@codeclimate
Copy link

codeclimate bot commented Jun 20, 2022

Code Climate has analyzed commit e6e5216 and detected 0 issues on this pull request.

View more on Code Climate.

@jaybuidl jaybuidl merged commit 5256d8b into master Jun 20, 2022
@jaybuidl jaybuidl deleted the feat/dk-draw-efficiency branch June 20, 2022 22:56
@sonarcloud
Copy link

sonarcloud bot commented Jun 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jaybuidl jaybuidl restored the feat/dk-draw-efficiency branch November 24, 2022 02:58
@jaybuidl jaybuidl deleted the feat/dk-draw-efficiency branch November 24, 2022 03:10
Params10 pushed a commit that referenced this pull request Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants