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

refactor: move EpochManager functions as default impl for EpochManagerAdapter #12851

Merged
merged 1 commit into from
Feb 2, 2025

Conversation

pugachAG
Copy link
Contributor

@pugachAG pugachAG commented Jan 31, 2025

This PR moves implementation for a bunch of functions from EpochManager to be the default implementation in EpochManagerAdapter trait.
Also is_slashed is removed from the API since we do not properly support slashing.

@pugachAG pugachAG requested a review from a team as a code owner January 31, 2025 13:27
@pugachAG pugachAG requested review from Longarithm and stedfn January 31, 2025 13:27
@pugachAG pugachAG changed the title refactor: move possible_epochs_of_height_around_tip as default impl refactor: move EpochManager functions as default impl for EpochManagerAdapter Jan 31, 2025
@pugachAG pugachAG force-pushed the inline-epochs-around-tip branch from eb3e3ad to ed9682e Compare January 31, 2025 13:46
@@ -239,7 +253,7 @@ pub trait EpochManagerAdapter: Send + Sync {
/// it for "production" code.
fn get_validator_info(
&self,
epoch_id: ValidatorInfoIdentifier,
epoch_identifier: ValidatorInfoIdentifier,
Copy link
Contributor

Choose a reason for hiding this comment

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

why rename

Copy link
Contributor Author

Choose a reason for hiding this comment

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

epoch_id usually corresponds to a variable of type EpochId in this file, so renaming to avoid confusing

@pugachAG pugachAG force-pushed the inline-epochs-around-tip branch 5 times, most recently from a0c0f69 to a8e0051 Compare February 1, 2025 21:03
Copy link

codecov bot commented Feb 1, 2025

Codecov Report

Attention: Patch coverage is 69.54733% with 74 lines in your changes missing coverage. Please review.

Project coverage is 70.38%. Comparing base (b1bb76c) to head (a8e0051).

Files with missing lines Patch % Lines
chain/epoch-manager/src/adapter.rs 68.18% 28 Missing and 14 partials ⚠️
chain/chain/src/runtime/tests.rs 40.00% 9 Missing ⚠️
chain/client/src/debug.rs 0.00% 7 Missing ⚠️
chain/chain/src/chain.rs 40.00% 0 Missing and 3 partials ⚠️
chain/chunks/src/shards_manager_actor.rs 76.92% 0 Missing and 3 partials ⚠️
chain/epoch-manager/src/lib.rs 76.92% 0 Missing and 3 partials ⚠️
chain/chain/src/approval_verification.rs 60.00% 2 Missing ⚠️
chain/client/src/client.rs 50.00% 0 Missing and 2 partials ⚠️
chain/chain/src/lightclient.rs 50.00% 0 Missing and 1 partial ⚠️
chain/chain/src/test_utils/kv_runtime.rs 83.33% 0 Missing and 1 partial ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12851      +/-   ##
==========================================
- Coverage   70.39%   70.38%   -0.01%     
==========================================
  Files         853      853              
  Lines      174093   174006      -87     
  Branches   174093   174006      -87     
==========================================
- Hits       122548   122473      -75     
+ Misses      46324    46317       -7     
+ Partials     5221     5216       -5     
Flag Coverage Δ
backward-compatibility 0.35% <0.00%> (+<0.01%) ⬆️
db-migration 0.35% <0.00%> (+<0.01%) ⬆️
genesis-check 1.41% <0.00%> (+<0.01%) ⬆️
linux 70.07% <67.90%> (-0.01%) ⬇️
linux-nightly 70.01% <69.54%> (-0.02%) ⬇️
pytests 1.72% <0.00%> (+<0.01%) ⬆️
sanity-checks 1.52% <0.00%> (+<0.01%) ⬆️
unittests 70.20% <69.54%> (-0.01%) ⬇️
upgradability 0.35% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pugachAG pugachAG force-pushed the inline-epochs-around-tip branch from a8e0051 to 279d62b Compare February 2, 2025 21:28
@pugachAG pugachAG enabled auto-merge February 2, 2025 21:29
@pugachAG pugachAG added this pull request to the merge queue Feb 2, 2025
Merged via the queue into master with commit 6e4b731 Feb 2, 2025
24 checks passed
@pugachAG pugachAG deleted the inline-epochs-around-tip branch February 2, 2025 22:06
@pugachAG
Copy link
Contributor Author

pugachAG commented Feb 3, 2025

@Longarithm just FYU I've pushed quite a few changes after your approval, so maybe you would like to take another look

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.

3 participants