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.

Sign up for GitHub

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

Jump to bottom

Pretty print transaction's gas amount in token units #3167

Merged
merged 2 commits into from
Aug 6, 2020
Merged

Pretty print transaction's gas amount in token units #3167

merged 2 commits into from
Aug 6, 2020

Conversation

tjanez
Copy link
Member

@tjanez tjanez commented Aug 4, 2020 •
edited

An example of the new output:

You are about to sign the following transaction:
  Nonce:  7
  Fee:
    Amount: AMBER 0.000002
    Gas limit: 1000
    (gas price: AMBER 0.000000002 per gas unit)
  Method: staking.Transfer
  Body:
    To:     oasis1qr3jc2yfhszpyy0daha2l9xjlkrxnzas0uaje4t3
    Amount: AMBER 170.0
Other info:
  Genesis document's hash: da80e19432de40911469314ee5aa02ee9ea5e5518b01b0b1eb3a13e526a8e3ca

TODO:

  • Finish go/consensus/api/transaction.Fee's PrettyPrint().

Closes #3151.

@tjanez tjanez added c:consensus/tendermint Category: Tendermint-based consensus c:staking Category: staking c:cli Category: command line interface labels Aug 4, 2020
@tjanez tjanez force-pushed the tjanez/gas-amount-in-tokens branch 2 times, most recently from 0dc0c40 to 4dad065 Compare August 5, 2020 12:36
@tjanez tjanez marked this pull request as ready for review August 5, 2020 12:38
@codecov
Copy link

codecov bot commented Aug 5, 2020 •
edited

Codecov Report

Merging #3167 into master will decrease coverage by 0.06%.
The diff coverage is 93.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3167      +/-   ##
==========================================
- Coverage   68.57%   68.50%   -0.07%     
==========================================
  Files         385      386       +1     
  Lines       37951    37962      +11     
==========================================
- Hits        26023    26005      -18     
- Misses       8630     8652      +22     
- Partials     3298     3305       +7
Impacted Files Coverage Δ
go/staking/api/prettyprint.go 100.00% <ø> (ø)
go/staking/api/sanity_check.go 32.60% <25.00%> (ø)
go/consensus/api/transaction/gas.go 70.00% <80.00%> (+10.00%) ⬆️
go/consensus/api/transaction/transaction.go 56.56% <100.00%> (+0.44%) ⬆️
go/oasis-node/cmd/consensus/consensus.go 49.46% <100.00%> (ø)
go/oasis-node/cmd/stake/account.go 60.19% <100.00%> (ø)
go/oasis-node/cmd/stake/stake.go 54.66% <100.00%> (ø)
go/staking/api/api.go 72.44% <100.00%> (ø)
go/staking/api/token/prettyprint.go 100.00% <100.00%> (ø)
go/consensus/tendermint/abci/state/state.go 61.53% <0.00%> (-7.70%) ⬇️
... and 38 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 9053680...2141b4f. Read the comment docs.

kostko
kostko approved these changes Aug 6, 2020
View reviewed changes
go/consensus/api/transaction/gas.go Outdated Show resolved Hide resolved
@tjanez
go/staking/api: Extract token-related things to go/staking/api/token
966434e 
So it can be imported by other packages without causing import cycles.

go/staking/api: Move token-related context keys to
go/common/prettyprint.
@tjanez
go/consensus/api/transaction: Implement PrettyPrinter interface for Fee
2141b4f 
Use it in Transaction's PrettyPrinter to print a transaction's fee.
@tjanez tjanez force-pushed the tjanez/gas-amount-in-tokens branch from 4dad065 to 2141b4f Compare August 6, 2020 11:26
@tjanez tjanez merged commit 1bb5b80 into master Aug 6, 2020
@tjanez tjanez deleted the tjanez/gas-amount-in-tokens branch August 6, 2020 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kostko kostko kostko approved these changes

@peterjgilbert peterjgilbert Awaiting requested review from peterjgilbert peterjgilbert is a code owner

@pro-wh pro-wh Awaiting requested review from pro-wh pro-wh is a code owner

@Yawning Yawning Awaiting requested review from Yawning

Assignees
No one assigned
Labels
c:cli Category: command line interface c:consensus/tendermint Category: Tendermint-based consensus c:staking Category: staking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pretty print transaction's gas amount in token units
2 participants
@tjanez @kostko