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

R4R: Query staking Pool and Params #2099

Merged
merged 13 commits into from
Aug 21, 2018

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Aug 20, 2018

Closes #2011, #2012

API docs for LCD endpoints are being updated in #2066

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
  • Updated all relevant documentation (docs/)
  • Updated all relevant code comments
  • Wrote tests
  • Added entries in PENDING.md that include links to the relevant issue or PR that most accurately describes the change.
  • Updated cmd/gaia and examples/

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Aug 20, 2018

Codecov Report

Merging #2099 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2099   +/-   ##
========================================
  Coverage    63.82%   63.82%           
========================================
  Files          113      113           
  Lines         6684     6684           
========================================
  Hits          4266     4266           
  Misses        2135     2135           
  Partials       283      283

@fedekunze fedekunze requested a review from zramsay as a code owner August 20, 2018 15:44
@fedekunze fedekunze changed the title WIP: Query staking Pool and Params R4R: Query staking Pool and Params Aug 20, 2018
Copy link
Contributor

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

Minor comments on LCD tests. Haven't looked at the rest of the code.

@@ -353,6 +353,35 @@ func TestTxs(t *testing.T) {
require.Equal(t, resultTx.Height, indexedTxs[0].Height)
}

func TestPoolQuery(t *testing.T) {
_, password := "test", "1234567890"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make the first parameter a variable, and use it in the subsequent line.

}

func TestParamsQuery(t *testing.T) {
_, password := "test", "1234567890"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will merge both into one test

defer cleanup()

res, body := Request(t, port, "GET", "/stake/pool", nil)
require.Equal(t, http.StatusOK, res.StatusCode, body)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do some check on the body? Currently this test just ensures that the endpoint doesn't error, not that it actually does anything.

Copy link
Collaborator Author

@fedekunze fedekunze Aug 20, 2018

Choose a reason for hiding this comment

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

@ValarDragon I'm trying to compare the initial pool with the returned one

expected: types.Pool{
LooseTokens:1000000000000,
BondedTokens:0,
InflationLastTime:time.Time{wall:0x0, ext:62135596800, loc:(*time.Location)(0x22aad00)}, 
Inflation:700000000,
DateLastCommissionReset:0,
PrevBondedShares:0}
		
actual  : types.Pool{
LooseTokens:1500019965200, 
BondedTokens:1000000000000,
InflationLastTime:time.Time{wall:0x1d6c1165, ext:63670381882, loc:(*time.Location)(nil)},
Inflation:700059763, 
DateLastCommissionReset:0, 
PrevBondedShares:0}

Copy link
Collaborator Author

@fedekunze fedekunze Aug 20, 2018

Choose a reason for hiding this comment

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

@ValarDragon could you help me figuring out where LooseTokens, BondedTokens and Inflation are updated once you initialize LCD?

@cwgoes
Copy link
Contributor

cwgoes commented Aug 21, 2018

Please rebase on develop.

@fedekunze fedekunze dismissed ValarDragon’s stale review August 21, 2018 14:10

addressed. Tests for Pool are working now

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

utACK

@cwgoes cwgoes merged commit cfb5acc into develop Aug 21, 2018
@cwgoes cwgoes deleted the fedekunze/new-query-staking-endpoints branch August 21, 2018 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants