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 expired to getstakeinfo command #360

Merged
merged 4 commits into from
Oct 31, 2016
Merged

Conversation

alexlyp
Copy link
Member

@alexlyp alexlyp commented Oct 11, 2016

No description provided.

Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

Just some questions.

@@ -2825,6 +2825,7 @@ type StakeInfoData struct {
Voted uint32
Missed uint32
Revoked uint32
Expired uint32
Copy link
Member

@chappjc chappjc Oct 11, 2016

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

added

}

// Check the expired ticket pool for the presense of tickets.
expiredBitSetBStr, err := chainClient.ExistsExpiredTickets(revokeTicketPtrs)
Copy link
Member

Choose a reason for hiding this comment

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

Then does chainClient.MissedTickets() no longer include expired? Or does that count need to be decremented?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess that's a good point, I was using a inclusive/exclusive view of them. All expired are missed, but not all missed are expired.

Copy link
Member

Choose a reason for hiding this comment

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

To me, missed implies someone wasn't listening and missed their opportunity.

Copy link
Member Author

Choose a reason for hiding this comment

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

ya i agree with that. though not sure the current bucketing in dcrd. i believe missed is inclusive of expired, so you'd have to do the changes in wallet. i'm going to keep it for now but i'll open an issue that we can discuss/resolve this in the near future

Copy link
Member Author

Choose a reason for hiding this comment

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

@chappjc
Copy link
Member

chappjc commented Oct 24, 2016

tACK
Calling getstakeinfo with dcrctl seemed to trigger a CPU-intensive operation with dcrd, dcrwallet's RPC session timed out, dcrctl said client shutdown, and dcrwallet reconnected shortly after. Further calls to getstakeinfo worked as expected. However, this only seemed to happen on one machine. It worked without hiccup on another.

@chappjc
Copy link
Member

chappjc commented Oct 29, 2016

Is this going in? I'd like it.

Copy link
Member

@jrick jrick left a comment

Choose a reason for hiding this comment

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

This adds yet another dcrd RPC to getstakeinfo that will make this RPC even slower than it already is, but functionally it looks ok.

@jrick jrick merged commit fcbf13b into decred:master Oct 31, 2016
@alexlyp alexlyp deleted the ayp_expired branch August 25, 2017 17:36
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