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

Skip binary search if observation exists #88

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Conversation

trmid
Copy link
Member

@trmid trmid commented Feb 13, 2024

Motivation:

In normal prize pool operations, we will have an observation for every draw except draws that had no contributions. This means that we shouldn't need to binary search for the observation in the ring buffer unless the draw had no contributions.

The following changes check to see if the draw observation has any non-zero values (available or disbursed) and if so, we can use the observation instead of binary searching for it. This should save gas in normal operating conditions.

Base automatically changed from gen-1141-eliminate-smoothing-to-reduce-code-size to main February 13, 2024 17:11
@trmid trmid force-pushed the suggestions-for-1141 branch from a689140 to f593ce9 Compare February 13, 2024 17:17
@GenerationSoftware GenerationSoftware deleted a comment from github-actions bot Feb 13, 2024
Copy link

LCOV of commit f593ce9 during Tests with 100% Coverage #412

Summary coverage rate:
  lines......: 100.0% (397 of 397 lines)
  functions..: 100.0% (63 of 63 functions)
  branches...: no data found

Files changed coverage rate:
                                             |Lines       |Functions  |Branches    
  Filename                                   |Rate     Num|Rate    Num|Rate     Num
  =================================================================================
  src/libraries/DrawAccumulatorLib.sol       | 100%     58| 100%     4|    -      0

Copy link
Contributor

@asselstine asselstine left a comment

Choose a reason for hiding this comment

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

Nice improvement!

src/libraries/DrawAccumulatorLib.sol Outdated Show resolved Hide resolved
src/libraries/DrawAccumulatorLib.sol Outdated Show resolved Hide resolved
Copy link

LCOV of commit ea5a5b2 during Tests with 100% Coverage #416

Summary coverage rate:
  lines......: 100.0% (399 of 399 lines)
  functions..: 100.0% (63 of 63 functions)
  branches...: no data found

Files changed coverage rate:
                                             |Lines       |Functions  |Branches    
  Filename                                   |Rate     Num|Rate    Num|Rate     Num
  =================================================================================
  src/libraries/DrawAccumulatorLib.sol       | 100%     60| 100%     4|    -      0

@trmid trmid merged commit 900b980 into main Feb 13, 2024
2 checks passed
@trmid trmid deleted the suggestions-for-1141 branch February 13, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants