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

Updated slot subscriber provider #798

Merged
merged 9 commits into from
Jan 4, 2024

Conversation

lukecaan
Copy link
Member

@lukecaan lukecaan commented Dec 28, 2023

Slightly changed slot subscriber logic:

  • Removed the lookback slots params from the strategies, because this param is going through the subscriber anyway, and it makes more sense for the subscriber to just do the relevant filtering before passing into the the strategies. It reduces complexity+duplicated code in the strategies.

Copy link

codecov bot commented Dec 28, 2023

Codecov Report

Merging #798 (63948a6) into master (fc70257) will increase coverage by 0.00%.
Report is 13 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #798   +/-   ##
=======================================
  Coverage   74.65%   74.65%           
=======================================
  Files         128      128           
  Lines       29772    29772           
=======================================
+ Hits        22225    22226    +1     
+ Misses       7547     7546    -1     
Components Coverage Δ
drift 74.85% <ø> (+<0.01%) ⬆️

@lukecaan lukecaan changed the title Updated slot subscriber provider api Updated slot subscriber provider Dec 28, 2023
calculate(samples: { slot: number; prioritizationFee: number }[]): number {
if (samples.length === 0) {
return 0;
}
const stopSlot = samples[0].slot - this.lookbackSlots;
let runningSumFees = 0;
let countFees = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is countFees still needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah look down a few lines

@lukecaan lukecaan merged commit 1d80b21 into master Jan 4, 2024
7 of 9 checks passed
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