-
Notifications
You must be signed in to change notification settings - Fork 328
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
[api] readState support to non-rolldpos mode #4247
Conversation
inputEpochNum := rp.GetEpochNum(inputHeight) | ||
if inputEpochNum < tipEpochNum { | ||
rp := rolldpos.FindProtocol(core.registry) | ||
if rp != nil { |
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.
what if rp == nil
, still cannot read?
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.
it will go to L949 / L957 to read
api/coreservice.go
Outdated
rp := rolldpos.FindProtocol(core.registry) | ||
if rp != nil { | ||
tipEpochNum := rp.GetEpochNum(tipHeight) | ||
if height != "" { |
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.
no need, L932 already checked
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.
indeed
tipEpochNum := rp.GetEpochNum(tipHeight) | ||
inputEpochNum := rp.GetEpochNum(inputHeight) | ||
if inputEpochNum < tipEpochNum { | ||
inputHeight = rp.GetEpochHeight(inputEpochNum) |
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.
can't understand why the height of state is changed in rp, but the changes are acceptable for me compared to the prev
Quality Gate failedFailed conditions |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4247 +/- ##
==========================================
- Coverage 77.02% 76.97% -0.05%
==========================================
Files 342 343 +1
Lines 29406 29438 +32
==========================================
+ Hits 22651 22661 +10
- Misses 5650 5670 +20
- Partials 1105 1107 +2 ☔ View full report in Codecov by Sentry. |
Description
STANDALONE
andNOOP
is easy to use thanROLLDPOS
in writing e2etest, andreadState
is used to checking things in testing. But it's only supported in ROLLDPOS.Fixes #(issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: