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

txsource/queries: workload doing various queries #2769

Merged
merged 3 commits into from
Mar 24, 2020

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented Mar 16, 2020

Queries workload, part of: #2506

  • Also removes GetNodeList() client query as it's almost identical to the GetNodes one (minus the sorting) and it's not used anywhere at the moment - can revert that commit if desired. (extracted out)
  • adds LastBlockFees query method to staking client, so that client is able to obtain all funds and confirm total supply matches correctly

Feel free to comment in case any relevant queries/checks should be added.

TODO:

@ptrus ptrus self-assigned this Mar 16, 2020
@ptrus ptrus force-pushed the ptrus/feature/queries-workload branch from 7b3f865 to cf3e6cc Compare March 17, 2020 09:26
@codecov
Copy link

codecov bot commented Mar 17, 2020

Codecov Report

Merging #2769 into master will increase coverage by 0.21%.
The diff coverage is 57.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2769      +/-   ##
==========================================
+ Coverage   62.74%   62.95%   +0.21%     
==========================================
  Files         387      388       +1     
  Lines       36526    36828     +302     
==========================================
+ Hits        22917    23185     +268     
+ Misses      10708    10686      -22     
- Partials     2901     2957      +56     
Impacted Files Coverage Δ
go/oasis-node/cmd/debug/txsource/txsource.go 67.18% <0.00%> (-0.51%) ⬇️
...sis-node/cmd/debug/txsource/workload/delegation.go 62.60% <ø> (ø)
...asis-node/cmd/debug/txsource/workload/oversized.go 66.00% <ø> (ø)
...oasis-node/cmd/debug/txsource/workload/parallel.go 58.90% <ø> (ø)
...s-node/cmd/debug/txsource/workload/registration.go 75.93% <ø> (ø)
...oasis-node/cmd/debug/txsource/workload/transfer.go 63.80% <ø> (+63.80%) ⬆️
...oasis-node/cmd/debug/txsource/workload/workload.go 67.07% <ø> (ø)
go/registry/api/api.go 38.13% <0.00%> (ø)
go/staking/api/api.go 71.42% <ø> (ø)
.../oasis-node/cmd/debug/txsource/workload/queries.go 56.00% <56.00%> (ø)
... and 28 more

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 6f327d1...398f914. Read the comment docs.

@ptrus ptrus force-pushed the ptrus/feature/queries-workload branch 8 times, most recently from a8882e6 to 07f39d2 Compare March 19, 2020 12:19
@ptrus ptrus marked this pull request as ready for review March 19, 2020 12:39
@pro-wh
Copy link
Contributor

pro-wh commented Mar 19, 2020

Also removes GetNodeList() client query as it's almost identical to the GetNodes one (minus the sorting) and it's not used anywhere at the moment - can revert that commit if desired.

weird, why did we even have both?

Copy link
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

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

also, are we testing these?

go/consensus/tendermint/registry/registry.go Outdated Show resolved Hide resolved
go/oasis-node/cmd/debug/txsource/txsource.go Show resolved Hide resolved
go/oasis-node/cmd/debug/txsource/workload/queries.go Outdated Show resolved Hide resolved
go/oasis-node/cmd/debug/txsource/workload/queries.go Outdated Show resolved Hide resolved
go/oasis-node/cmd/debug/txsource/workload/queries.go Outdated Show resolved Hide resolved
@ptrus ptrus force-pushed the ptrus/feature/queries-workload branch 4 times, most recently from 4d67e87 to 525bfad Compare March 20, 2020 15:42
@@ -0,0 +1,3 @@
go/staking: add LastBlockFees query method

LastBlockFees returns the collected fees for previous block.
Copy link
Member

Choose a reason for hiding this comment

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

Slightly unrelated, but this is also something that should be part of the genesis state dump as otherwise it could cause the total token amount to become inconsistent on dump/restore.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh we don't do that at the moment :o will do a PR that adds that tomorrow 😁

@ptrus ptrus force-pushed the ptrus/feature/queries-workload branch from 525bfad to 398f914 Compare March 24, 2020 08:14
@ptrus
Copy link
Member Author

ptrus commented Mar 24, 2020

thanks for the reviews @pro-wh

@ptrus ptrus merged commit ea2c74d into master Mar 24, 2020
@ptrus ptrus deleted the ptrus/feature/queries-workload branch March 24, 2020 08:41
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