Skip to content
This repository has been archived by the owner on Jun 11, 2022. It is now read-only.

More pretty printer features #109

Merged
merged 15 commits into from
May 30, 2018
Merged

More pretty printer features #109

merged 15 commits into from
May 30, 2018

Conversation

ghost
Copy link

@ghost ghost commented May 28, 2018

implementing aux requests noted in a comment in #90; also see commit messages

  • add color to previous_hash (field of BlockHeader)
  • add color to block_signature (field of Consensus)
  • extend the AST to allow Val::List<Vec<Value>>, use it for lists like transactions and the long list in genesisblocks
  • implement Pretty for genesis blocks (use same approach as blockchain::normal::Block implementations)

main block

image

genesis block

image

@ghost ghost requested a review from NicolasDP May 28, 2018 18:32
@vincenthz
Copy link
Member

vincenthz commented May 29, 2018

This look fine apart from the colour handling. the idea is to have high level values in the AST like Number and Hash, that will be rendered at the print site with color, hex escape, etc.. depending on their ast type.

For example the previous header hash should be a new Val::Hash not a Val::Raw with the color cyan. we want to capture the a high level semantics of the object in the intermediate representation Val

@ghost
Copy link
Author

ghost commented May 29, 2018

👍 handle colors with additional Val constructors

@vincenthz i have two questions:

  1. what are some other components which require their own constructor? (ie. you mentioned Number, is that just any i32/i64/u32/u64 value?)
  2. block-signature is a rich value with many subcomponents; how would you like it to be colored?

@NicolasDP
Copy link
Contributor

I think I can help with that:

  1. I can think of these: Addresses; hashes, the Epoch/Slotid; Signatures, public keys
  2. There might be some special objects relevant from the point 1. (signature, hashes, pk), that might be enough for now. We can enhance it later.

@ghost
Copy link
Author

ghost commented May 29, 2018

  • Val::Hash
  • Val::Epoch and Val::SlotId
  • Val::BlockSig

There's only very slight changes in output.

mb
gb

@ghost
Copy link
Author

ghost commented May 29, 2018

I only see something like a public key under tx::TxProof.witness_hash. Are there others?

I don't see any addresses in blockchain::normal or blockchain::genesis.

@ghost
Copy link
Author

ghost commented May 29, 2018

Checking out the cardano code reveals what I was misunderstanding:

  1. Pos/Core/Common/Types.hs#L81-L85
  2. Pos/Crypto/Hashing.hs#L86-L90
  3. Pos/Explorer/Web/ClientTypes.hs#L91-L105
    -- See this page for more explanation - https://cardanodocs.com/cardano/addresses/
    -- We have the general type @AbstractHash@ for all hashes we use. It's being parametrized
    -- by two types - AbstractHash algo a - the hashing algorithm and the phantom type for
    -- extra safety (can be a @Tx@, an @Address@ and so on, ...).
    --
    -- The following types explain the situation better:
    
    type AddressHash   = AbstractHash Blake2b_224
    type Hash          = AbstractHash Blake2b_256
    
    type TxId          = Hash Tx               = AbstractHash Blake2b_256 Tx
    type StakeholderId = AddressHash PublicKey = AbstractHash Blake2b_224 PublicKey

@NicolasDP From this I suppose it would make sense for us to four types, but i'm not yet sure which are used for what

  1. Val::Hash ..?
  2. Val::AddressHash ..?
  3. Val::TxId ..?
  4. Val::StakeholderId -- used for genesis::Block.body stakeholder list

@ghost ghost mentioned this pull request May 30, 2018
@NicolasDP
Copy link
Contributor

You don't need AddressHash, it is hidden in the ExtendedAddr and we prefer to display the extended address as base58 all serialised in the appropriate format.

While TxId would be useful, it is not in the data structure to be displayed. But it is still a Hash, so no need to duplicate it in its special sum_type constructor.

@NicolasDP NicolasDP merged commit 5ad421a into master May 30, 2018
@ghost ghost deleted the more-pretty-printer-features branch May 30, 2018 21:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants