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 a masternode return of investment RPC #2933

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

techy2
Copy link

@techy2 techy2 commented Jul 30, 2024

update PR #2929 to current "master" branch
accurate CLI function for staking and masternode ROI
getroi
{
"3 hour avg ROI": "23.9%",
"30 min stk ROI": "29.5%",
"network stake": "8,564,072",
"--------------": "--------------",
"masternode ROI": "16.0%",
"tot collateral": "19,150,000",
"enabled nodes": "1915",
"blocks per day": "1401.1"
}

@Duddino Duddino changed the title update PR #2929 simpleroi Add a masternode return of investment RPC Oct 30, 2024
Copy link
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The code needs to be formatted and redundant comments cleaned up.
I'm not sure if this RPC is actually needed, It seems like something a third party tool like toolbox.pivx.org would provide, rather than the core wallet.

return csra.pb->GetBlockTime() - csra.pb0->GetBlockTime();
}

// returns count enabled or zero on error
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// returns count enabled or zero on error
// returns count enabled or zero on error

This says it will return zero on error, but later it returns negative numbers on error

Copy link
Author

Choose a reason for hiding this comment

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

see line 76

@Fuzzbawls
Copy link
Collaborator

I think that this kind of functionality is better suited to a website and not the core wallet. this introduces features that are not reasonably tested in a meaningful way (code coverage / regression tests).

@techy2
Copy link
Author

techy2 commented Nov 4, 2024

I don't have any PIVX so it make no difference to me. I provided this as a courtesy.
This is what it looks like fully implemented if you bring your code up to snuff.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants