-
Notifications
You must be signed in to change notification settings - Fork 143
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
chore: replace PlatformStateAccessor.getAddressBook() with calls to R… #17047
chore: replace PlatformStateAccessor.getAddressBook() with calls to R… #17047
Conversation
…osterRetriever Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
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 from hedera-services
file changes. Thanks @anthony-swirldslabs
hedera-node/hedera-app/src/main/java/com/hedera/node/app/info/StateNetworkInfo.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
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, tyvm @anthony-swirldslabs !
platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/roster/RosterUtils.java
Show resolved
Hide resolved
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
4c4c4e3
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
…stream Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
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, tyvm @anthony-swirldslabs !
…osterRetriever
Description:
This fix replaces meaningful usages of the
PlatformStateAccessor.getAddressBook()
with calls to theRosterRetriever
in order to fetch the currentRoster
/AddressBook
in the app, platform, and test code.The
RosterRetriever
implementation would first check the RosterService state, and only if it's not populated with data yet, then it will fall back to reading the AddressBook from the PlatformState. In other words, the new code path will automatically activate the moment we start writing to the RosterService states. Please note that all the tests code has been updated to go through this new code path already thanks to the newly introducedRosterServiceStateMock
utility class, as well as the most crucial call to theRosterUtils.setActiveRoster()
in theRandomSignedStateGenerator.build()
.The remaining usages of this method support the
PlatformState.addressBook
field directly and shouldn't be removed until we actually deprecate this field physically, or at least stop populating it altogether, which is going to be a part of a follow-up ticket/PR.Related issue(s):
Fixes #16904
Notes for reviewer:
All tests should pass.
Checklist