-
Notifications
You must be signed in to change notification settings - Fork 28
Description
This issue documents a design problem discovered during PR #888 (improved logging) and is closely related to #640 (order susceptible fields).
While adding truncated hash display for cleaner logs in #888, we discovered that changing the Display implementation for BlockHash and TransactionHash raised concerns about breaking "contracts" elsewhere in the codebase. The worry was that somewhere, code might be calling .to_string() on these types and passing the result into a JSON-RPC response or similar machine interface.
This reveals a deeper architectural issue: we've been implicitly treating Display as if it were a stable, machine-readable serialization format. But that's not what Display is for. In idiomatic Rust, Display is meant for human-readable output — console printing, log messages, error formatting. It should be free to evolve for better human readability without breaking machine interfaces.
When Display gets conflated with "the way we serialize this for JSON", we lose the ability to improve logging ergonomics. We can't show truncated hashes in logs (which dramatically improves readability) because somewhere, someone might be relying on .to_string() producing a full 64-character hex string for a wire protocol.
How we got here
This is a natural consequence of the broader problem described in #640. When types like BlockHash have ambiguous interfaces — public inner fields, generic From<[u8; 32]> implementations that don't specify byte order, and Display implementations that happen to produce full hex — it's tempting to reach for whatever method is convenient. .to_string() works, so it gets used, and now it's load-bearing.
The ToHex trait already provides encode_hex() for producing full hexadecimal strings. That's the appropriate method for machine interfaces. But because Display also happened to produce hex (just a side effect of its implementation), the distinction was never enforced.
A better approach
Display should be understood as one of many "destinations" for a value, alongside JSON-RPC, gRPC, database storage, and others. Each destination might need a different representation. Display for logs and console can be human-friendly, truncated, include formatting like BlockHash(abc123..). Meanwhile ToHex::encode_hex() provides the full hexadecimal string in a stable format for machine consumption. ZainoVersionedSerde handles binary encoding for database storage. Proto/gRPC uses raw bytes in canonical order.
The #640 proposal for source/destination-specific methods would make these distinctions explicit and enforceable. Until that refactor happens, we should at minimum establish the convention that Display is not a stable interface.
Immediate action
For PR #888, the two test usages of .to_string() on TransactionHash should be changed to .encode_hex::<String>() to make the intent explicit. This unblocks the logging improvements without waiting for the larger architectural refactor.
Going forward, we should audit for any production code using .to_string() on hash types for machine interfaces and migrate those to encode_hex().
Related: #640 (order susceptible fields), #888 (improved logging)