-
Notifications
You must be signed in to change notification settings - Fork 119
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
feature(rpc): add real data to getinfo
method
#3660
Conversation
@oxarbitrage this seems like it might be a decision for the whole team. Did you want to put it on the engineering sync agenda for tomorrow? |
Codecov Report
@@ Coverage Diff @@
## main #3660 +/- ##
==========================================
- Coverage 80.06% 79.98% -0.09%
==========================================
Files 284 288 +4
Lines 32740 32755 +15
==========================================
- Hits 26214 26198 -16
- Misses 6526 6557 +31 |
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.
I haven't tried this myself, but I think it might simplify things.
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.
Looks good, let's check what tests we need with the team before we merge.
Lets add some basic test here before we merge. This will also define the test structure for all the methods. |
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.
Looks great, thanks!
Motivation
Implement the
getinfo
RPC method with the fields used by lightwalletd. Close #3142 if merged.Solution
I had a hard time trying to pass the app version into the method. This approach is with
lazy_static
. I am open to change this if i find something that works and it is better.This PR does not have a specific test, i am unsure if the tests we will use will be acceptance or something else for each method. If they are acceptance then the test for this call should be added after #3641 as it is very similar to what we have now.
The code in this pull was tested manually:
The docs for the method are in a discussion at #3638. Open to change the format.
Review
Probably @teor2345 will be the best option if they have time so i can have some feedback for my morning.
Reviewer Checklist