Zebra: Use Display for Block in errors, logs, and spans

[teor2345]

We have a Display for Block impl that shows the block height and hash.

We should change our error enums, error messages, tracing logs, and spans so they use Display, rather than separate block heights or hashes.

TODO

Replace height and hash with block, and debug with display, in:

• [ ] error enums
• [ ] error messages
• [ ] tracing logs
• [ ] tracing spans

Once these changes are done, we should also:

• [ ] eliminate any duplicate fields, logs, or spans

For example, the ChainVerifier has a height span, and the BlockVerifier has a hash span. After this change, the BlockVerifier's span will be redundant.

Context

https://github.com/ZcashFoundation/zebra/pull/1153#discussion_r504320596

[hdevalence]

I'm not sure this is a good idea -- the tracing output is supposed to be a trace of execution, so I don't think we should be omitting data from it. If there are spans where the full contents of the block are not relevant, a better solution would be to change the span fields, rather than to change the formatting.

[teor2345]

I don't have a strong opinion about this, it was a change suggested by @yaahc.

I also don't know if I wrote up her suggestion accurately.

So I'll let you both decide if you want to close or modify this ticket.

[yaahc]

I just worry that we're inconsistently identifying blocks based on random subsets of their fields across the codebase, and that we need some short, consistent format for identifying blocks in our logs. I'd go so far as to say the full context of the block is never relevant in logs, because that would be pages and pages of information. Changing the span fields doesn't really solve the issue of inconsistency across the code base.

The problem as I see it is that we are inconsistently using height / hash as a proxy for the block itself, because we cannot format the block and we don't want to include all the information from the block. Maybe we should introduce a new BlockID type that just wraps the block and has the short form format as its display impl, and in spans where we just want to indicate that a specific block was encountered we'd add block.id() to the span, rather than grabbing the height and hash and having to remember how they were formatted in the logs everywhere else to maintain any sort of consistency.

[teor2345]

The problem as I see it is that we are inconsistently using height / hash as a proxy for the block itself, because we cannot format the block and we don't want to include all the information from the block.

hash is a proxy for the block, because block hashes are unique.

height is useful information, but ambiguous by itself, because there can be multiple blocks at the same height. And height is always ambiguous if the logs don't show the current network.

Maybe we should introduce a new BlockID type that just wraps the block and has the short form format as its display impl,

Which block fields are in the short form format?

[yaahc]

I think height+hash are the fields I'd want most of the time in logs, so that's what I'd put in it.

[hdevalence]

After working more with the spans, I think that my original concern is misplaced and that this is a good idea.