-
Notifications
You must be signed in to change notification settings - Fork 331
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
fix(tendermint-rpc): add earliest block to sync info #1449
fix(tendermint-rpc): add earliest block to sync info #1449
Conversation
See
for the place to put the tests. This is tested against a real Tendermint instance in CI |
0bbd28d
to
b840e1b
Compare
@webmaster128 I've added a few unit tests there. But it looks like the tests are failing because those new fields are missing. What nodes does the client connect to when it is being tested in CI? These fields have been added to Tendermint more than three years ago. So either there is some really old node running in CI or mock data without these fields is used there. But I don't see it defined anywhere in the codebase. |
b840e1b
to
c678aed
Compare
I've printed it to the console and it looks like the problem is only with Original data:
Processed object:
But there are other tests where it is defined properly:
Or:
When looking at the latest implementation of status endpoint, I can see there might be Golang zero values returned from it unless |
84ab4aa
to
c813712
Compare
@webmaster128 Can we merge this or is there something else I should do? |
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. Could you add a CHANGLOG.md entry to the Unreleadsed section please?
c813712
to
6f36b8c
Compare
6f36b8c
to
665fc3b
Compare
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.
Thank you!
This should fix issue #1448. Those fields are returned from
/status
endpoint but I haven't tested this code. I'm open to any suggestions how I can test it easily.