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

Implement PrettyPrinter for various staking types and use it in stake CLI #3087

Merged
merged 3 commits into from
Jul 8, 2020

Conversation

tjanez
Copy link
Member

@tjanez tjanez commented Jul 6, 2020

Example of previous oasis-node stake account info CLI command output:

{"general":{"balance":"601492490765","nonce":8},"escrow":{"active":{"balance":"11242384816640","total_shares":"10000000000000"},"debonding":{"balance":"0","total_shares":"0"},"commission_schedule":{"rates":[{"start":16,"rate":"50000"}],"bounds":[{"start":16,"rate_min":"0","rate_max":"100000"}]},"stake_accumulator":{"claims":{"registry.RegisterEntity":[{"global":"entity"}]}}}}

Example of the new oasis-node stake account info CLI command output:

General Account:
  Balance: 601492490765
  Nonce:   8
Escrow Account:
  Active:
    Balance:      11242384816640
    Total Shares: 10000000000000
  Debonding:
    Balance:      0
    Total Shares: 0
  Commission Schedule:
    Rates:
      - Start: epoch 16
        Rate:  50 %
    Rate Bounds:
      - Start:        epoch 16
        Minimum Rate: 0 %
        Maximum Rate: 100 %
  Stake Accumulator:
    Claims:
      - Name: registry.RegisterEntity
        Staking Thresholds:
          - Global: entity

TODO:

  • Update e2e/stake-cli scenario.

@tjanez tjanez added c:staking Category: staking c:cli Category: command line interface labels Jul 6, 2020
@tjanez tjanez self-assigned this Jul 6, 2020
@kostko kostko marked this pull request as draft July 6, 2020 17:51
@pro-wh
Copy link
Contributor

pro-wh commented Jul 6, 2020

what do you think about this: annotate "Balance" field as "Balance (base units)"

@kostko
Copy link
Member

kostko commented Jul 6, 2020

I think the next step will be to actually support proper units based on the genesis document (#3061). This will probably require some additional context information for the pretty printer. But maybe (base units) could be a good default in case there is no context information available.

@tjanez
Copy link
Member Author

tjanez commented Jul 7, 2020

I think the next step will be to actually support proper units based on the genesis document (#3061).

Yes. Currently, the code base doesn't distinguish between tokens and base units. Everything is a token at the moment.

After #3061, I will augment the pretty printers to be able to print the balance in token units or base units.

This will probably require some additional context information for the pretty printer. But maybe (base units) could be a good default in case there is no context information available.

Agreed.

@tjanez tjanez force-pushed the tjanez/staking-pretty-print branch from 443d8e5 to 82d3f19 Compare July 8, 2020 10:39
@tjanez tjanez marked this pull request as ready for review July 8, 2020 10:44
@tjanez tjanez force-pushed the tjanez/staking-pretty-print branch from 82d3f19 to 1c6307c Compare July 8, 2020 11:01
go/staking/api/api.go Outdated Show resolved Hide resolved
@tjanez tjanez force-pushed the tjanez/staking-pretty-print branch from 1c6307c to ffe4281 Compare July 8, 2020 21:16
@codecov
Copy link

codecov bot commented Jul 8, 2020

Codecov Report

Merging #3087 into master will decrease coverage by 0.29%.
The diff coverage is 73.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3087      +/-   ##
==========================================
- Coverage   68.48%   68.19%   -0.30%     
==========================================
  Files         372      372              
  Lines       36734    36814      +80     
==========================================
- Hits        25156    25104      -52     
- Misses       8346     8465     +119     
- Partials     3232     3245      +13     
Impacted Files Coverage Δ
go/staking/api/api.go 72.72% <68.00%> (-1.40%) ⬇️
go/staking/api/commission.go 95.58% <80.64%> (-3.09%) ⬇️
go/oasis-node/cmd/stake/account.go 57.21% <100.00%> (-0.23%) ⬇️
go/oasis-node/cmd/ias/auth_registry.go 0.00% <0.00%> (-69.24%) ⬇️
go/worker/compute/executor/committee/batch.go 69.23% <0.00%> (-15.39%) ⬇️
go/consensus/tendermint/api/api.go 75.75% <0.00%> (-12.13%) ⬇️
go/worker/compute/executor/committee/state.go 74.07% <0.00%> (-11.12%) ⬇️
go/worker/storage/service_external.go 40.86% <0.00%> (-8.61%) ⬇️
go/consensus/tendermint/abci/state/state.go 61.53% <0.00%> (-7.70%) ⬇️
...ompute/txnscheduler/algorithm/batching/batching.go 78.66% <0.00%> (-6.67%) ⬇️
... and 36 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 0fd92a9...ffe4281. Read the comment docs.

@tjanez tjanez merged commit 7da6fb9 into master Jul 8, 2020
@tjanez tjanez deleted the tjanez/staking-pretty-print branch July 8, 2020 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:cli Category: command line interface c:staking Category: staking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants