-
Notifications
You must be signed in to change notification settings - Fork 268
turn getRetiringFederationCreationTime to return an optional #3426
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
turn getRetiringFederationCreationTime to return an optional #3426
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
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.
Pull request overview
This PR refactors the getRetiringFederationCreationTime method to return an Optional<Instant> instead of a nullable Instant, improving null safety and making the API more explicit about the possibility of no retiring federation existing.
Key Changes:
- Changed return type from
InstanttoOptional<Instant>forgetRetiringFederationCreationTimeacross the interface and implementation layers - Updated test cases to verify
Optional.isEmpty()instead of null checks - Refactored the
Bridgeclass to use functional programming patterns withOptional.map()andOptional.orElse()
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rskj-core/src/main/java/co/rsk/peg/federation/FederationSupport.java | Updated interface method signature to return Optional<Instant> |
| rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java | Changed implementation to return Optional directly instead of .orElse(null) |
| rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java | Updated method signature to match interface change |
| rskj-core/src/main/java/co/rsk/peg/Bridge.java | Refactored to use Optional and extracted epoch time logic into a private method |
| rskj-core/src/test/java/co/rsk/peg/federation/FederationSupportImplTest.java | Updated test assertions to check isEmpty() instead of null values |
| rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java | Updated mock returns and assertions to use Optional |
| rskj-core/src/test/java/co/rsk/peg/BridgeTest.java | Updated mock to return Optional value |
| rskj-core/src/test/java/co/rsk/peg/BridgeSupportIT.java | Updated integration test to unwrap Optional before assertions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rskj-core/src/main/java/co/rsk/peg/federation/FederationSupport.java
Outdated
Show resolved
Hide resolved
343dbbb to
8e4e700
Compare
jeremy-then
left a comment
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.
8e4e700 to
ae2226b
Compare
| .orElse((long) FederationChangeResponseCode.FEDERATION_NON_EXISTENT.getCode()); | ||
| } | ||
|
|
||
| private Long getEpochTimeBasedOnActivation(Instant retiringFederationCreationTime) { |
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.
| private Long getEpochTimeBasedOnActivation(Instant retiringFederationCreationTime) { | |
| private Long getFederationCreationTiimeEpochBasedOnActivation(Instant federationCreationTime) { |
Make it clear that it applies to federation creation time, not to any epoch. Can/should this be re-used when getting active federation creation time?
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.
marcos is right, this same logic is being applied for getFederationCreationTime
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.
done!
ae2226b to
94f1ade
Compare
…hBasedOnActivation and reuse the method in getFederationCreationTime
|



Description
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist: