Skip to content
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

Add new getMiningInfo for multiple masternode support #229

Merged

Conversation

benzumbrunn
Copy link
Contributor

@benzumbrunn benzumbrunn commented May 16, 2021

What kind of PR is this?:

/kind feature

What this PR does / why we need it:

getMintingInfo is deprecated since adding multiple masternode support from within one server:
https://github.com/DeFiCh/ain/blob/df796cb7d6dbbe0ed7d051e79db0f36630f77afb/src/rpc/mining.cpp#L220

This PR adds getMiningInfo which is the implementation to support multiple mining masternodes:
https://github.com/DeFiCh/ain/blob/df796cb7d6dbbe0ed7d051e79db0f36630f77afb/src/rpc/mining.cpp#L276

Additional comments?:

  • Added docs with a deprecated comment for the old one, but please advise if this is the correct way
  • getMintingInfo is used as an example in some places, let me know if this PR should replace the examples as well
  • Let me know if the test data should include a second node now, since there's an array to test now

@netlify
Copy link

netlify bot commented May 16, 2021

Deploy preview for jellyfish-defi ready!

Built with commit 84a6e0d

https://deploy-preview-229--jellyfish-defi.netlify.app

@codecov
Copy link

codecov bot commented May 16, 2021

Codecov Report

Merging #229 (84a6e0d) into main (904edd3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #229   +/-   ##
=======================================
  Coverage   96.36%   96.37%           
=======================================
  Files          65       65           
  Lines        1652     1654    +2     
  Branches      222      222           
=======================================
+ Hits         1592     1594    +2     
  Misses         60       60           
Impacted Files Coverage Δ
packages/jellyfish-api-core/src/category/mining.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 904edd3...84a6e0d. Read the comment docs.

Copy link
Contributor

@fuxingloh fuxingloh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I made some minor changes to how the docs are formatted. GitHub Actions is having a Major outage today, I will set this PR to auto-merge when all the checks passes.

https://www.githubstatus.com/

@fuxingloh fuxingloh merged commit e609025 into BirthdayResearch:main May 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants