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

debug: improve Debug impl for Ed25519 VerificationKeys #1272

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

hdevalence
Copy link
Collaborator

Since the ed25519_consensus::VerificationKey is no longer used, the VerificationKeys go back to having raw debug output that's hard to read when it appears in logs. This commit restores the hex-formatting previously inherited from ed25519_consensus, allowing nice output like (in use in pd downstream):

2023-02-24T03:34:57.002766Z DEBUG abci:EndBlock{height=187}:end_block:staking:build_tendermint_validator_updates: updates=[Update { pub_key: Ed25519(7423aa71eb844cdde4dc0b8c72ef1225931dc54c21a102cbe2fded2645d4e814), power: Power(50000000000) }]

In the future, another option could be to have the key be base64-encoded, to match Tendermint's data structures, but this is a less obtrusive change (not requiring a base64 encoder) to restore functionality first.

Since the `ed25519_consensus::VerificationKey` is no longer used, the
`VerificationKey`s go back to having raw debug output that's hard to read when
it appears in logs.  This commit restores the hex-formatting previously
inherited from `ed25519_consensus`, allowing nice output like (in use in `pd`
downstream):

```
2023-02-24T03:34:57.002766Z DEBUG abci:EndBlock{height=187}:end_block:staking:build_tendermint_validator_updates: updates=[Update { pub_key: Ed25519(7423aa71eb844cdde4dc0b8c72ef1225931dc54c21a102cbe2fded2645d4e814), power: Power(50000000000) }]
```

In the future, another option could be to have the key be base64-encoded, to
match Tendermint's data structures, but this is a less obtrusive change (not
requiring a base64 encoder) to restore functionality first.
@codecov-commenter
Copy link

Codecov Report

Merging #1272 (ad91ae2) into main (0c37d3a) will increase coverage by 0.0%.
The diff coverage is 100.0%.

❗ Current head ad91ae2 differs from pull request most recent head 7325b10. Consider uploading reports for the commit 7325b10 to get more accurate results

@@          Coverage Diff          @@
##            main   #1272   +/-   ##
=====================================
  Coverage   64.3%   64.3%           
=====================================
  Files        250     250           
  Lines      21618   21626    +8     
=====================================
+ Hits       13915   13923    +8     
  Misses      7703    7703           
Impacted Files Coverage Δ
tendermint/src/crypto/ed25519/verification_key.rs 96.2% <100.0%> (+1.5%) ⬆️
testgen/src/validator.rs 86.7% <0.0%> (-0.8%) ⬇️
light-client-verifier/src/types.rs 38.4% <0.0%> (ø)
testgen/src/header.rs 84.0% <0.0%> (+0.5%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@xla xla left a comment

Choose a reason for hiding this comment

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

🦾

@plaidfinch plaidfinch merged commit a889347 into informalsystems:main Feb 24, 2023
@plaidfinch plaidfinch deleted the ed25519-debug branch February 24, 2023 22:29
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.

4 participants