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 cardano-cli transaction view #2348

Merged
merged 1 commit into from
Feb 17, 2021
Merged

Conversation

cblp
Copy link
Contributor

@cblp cblp commented Feb 10, 2021

No description provided.

Copy link
Contributor

@erikd erikd left a comment

Choose a reason for hiding this comment

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

Looking good. Is this done now? Its still marked as draft.

Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Looking good. Can you paste an example of the output on this PR?

cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs Outdated Show resolved Hide resolved
@cblp
Copy link
Contributor Author

cblp commented Feb 12, 2021

Minimal transaction:

$ cardano-cli transaction view --tx-body-file ../example/_tx43.tx.json
{
    "auxiliary_data": null,
    "auxiliary_data_hash": null,
    "certificates": [],
    "era": "Allegra",
    "fee": 43,
    "inputs": [
        "123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0#1"
    ],
    "mint": 0,
    "outputs": [],
    "update": null,
    "validity_interval": {
        "invalid_before": null,
        "invalid_hereafter": null
    },
    "withdrawals": []
}

@cblp cblp marked this pull request as ready for review February 12, 2021 14:23
@cblp
Copy link
Contributor Author

cblp commented Feb 12, 2021

@erikd @disassembler
The task is done to some sensible degree. Further prettifying may be done in subsequent changes.

@cblp
Copy link
Contributor Author

cblp commented Feb 12, 2021

There is some duplication of code, but I can't figure out a good solution due to complicated type inference in presence of open type families.

@Jimbo4350
Copy link
Contributor

@cblp I would squash the commits into one commit. I'd also update the commit message with something like:

Implement ability to JSON pretty print txs and txbodies with `cardano-cli transation view` 

When we get closer to a release, someone has to go through the commit messages to figure out what to include in the change log. It's better to be a bit more descriptive to make their life easier. 😄

@cblp
Copy link
Contributor Author

cblp commented Feb 12, 2021

With YAML, we can remove syntactical noise at no pay:

withdrawals: []
auxiliary_data: null
inputs:
- 372178064d7c33c0569c9ec34083723edd6e92f4efe16a6644652f2835b59fe9#0
timetolive: 1000000
fee: 150000
certificates: []
outputs:
- amount: 1000000
  address: 00f2998eb67942c4674d01e2cd435e1f17919e095eec43807bb0010313c0a060899d6806f810547e2cb66f92d5c817a16af2a20f269e258ee0
metadata_hash: null
era: Shelley
update: null

Should we?

@cblp
Copy link
Contributor Author

cblp commented Feb 12, 2021

Aren't commits squashed automatically during merge?

@cblp cblp force-pushed the cblp/cardano-cli-transaction-view branch 2 times, most recently from 8c73d3b to 59c2775 Compare February 12, 2021 14:50
@cblp
Copy link
Contributor Author

cblp commented Feb 12, 2021

Tests failed with

cardano-node-chairman.exe: ProgressFailure (BlockNo 50) "\\\\.\\pipe\\chairman\\test-95b4cd7bb96718a4\\socket\\node-bft1" (Tip (SlotNo 10) 0ccefc1084d7e73ee63de18ed9cf76cc153e0fad08eb10c6ea411bf149d4ee4d (BlockNo 6))

How to investigate this?

@cblp cblp changed the title Add cardano-cli transation view Add cardano-cli transaction view Feb 15, 2021
@cblp cblp force-pushed the cblp/cardano-cli-transaction-view branch from 59c2775 to d5c9660 Compare February 16, 2021 16:08
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM!

@cblp
Copy link
Contributor Author

cblp commented Feb 17, 2021

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 17, 2021

@iohk-bors iohk-bors bot merged commit 7280023 into master Feb 17, 2021
@iohk-bors iohk-bors bot deleted the cblp/cardano-cli-transaction-view branch February 17, 2021 10:31
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