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

Feature Request: Show Separate Proof-of-Work Hash #1965

Closed
davecgh opened this issue May 19, 2023 · 11 comments · Fixed by #1970
Closed

Feature Request: Show Separate Proof-of-Work Hash #1965

davecgh opened this issue May 19, 2023 · 11 comments · Fixed by #1970

Comments

@davecgh
Copy link
Member

davecgh commented May 19, 2023

With the upcoming blake3 consensus change, assuming it passes, the block hash and the proof-of-work hash will no longer be the same.

In light of this, I think the explorer should add a separate field that displays the PoW hash. I suppose it's a matter of taste of whether you prefer the consistency of showing it for all blocks even though it will be the same for all of them prior to the activation of the agenda, or the compactness of only showing the new field when the associated agenda is active.

The relevant code to calculate the new hash was added in decred/dcrd#3115. In short, there are new PoWHashV1 and PoWHashV2 methods on the wire.MsgBlock (and wire.BlockHeader) that return the respective PoW hash variant that the relevant consensus validation logic uses. For PoWHashV1, it will obviously match BlockHash, but having the separate methods makes it clear the PoW hash is no longer tied to the block hash.

Here are a couple of mockups I hacked together for potential placement:

  1. Under the block hash on the left:

image

  1. Above the Merkle Root on the right:

image

@xaur
Copy link

xaur commented Jun 18, 2023

I think 2nd version looks better for most users who will want the block hash. PoW hash belongs to the technical details section.

@davecgh
Copy link
Member Author

davecgh commented Jul 17, 2023

Perhaps useful for testing purposes:

Final testnet blake256 PoW block: 1170047 (000000b396bfeaa6ae6fa9e3cee441d7215191630bdaa9b979a872985caed727)
First testnet blake3 PoW block: 1170048 (c7da7b548a2a9463dc97adb48433c4ffff18c3873f7e2ae99338a990dae039f0)

@ukane-philemon
Copy link
Collaborator

I'd like to take this up if no one was already on it. Thanks @davecgh for the mock. I agree with @xaur, the second design looks better.

@chappjc
Copy link
Member

chappjc commented Jul 18, 2023

I'd like to take this up if no one was already on it. Thanks @davecgh for the mock. I agree with @xaur, the second design looks better.

OK. But if you haven't started it yet, I was planning to knock this out.
In any case, we just need it on the page, not added to the DB.

@ukane-philemon
Copy link
Collaborator

But if you haven't started it yet, I was planning to knock this out.

np, you can.

@chappjc
Copy link
Member

chappjc commented Jul 18, 2023

Actually, please take this if you have time. Some other things have come up for me.

@ukane-philemon
Copy link
Collaborator

Okay, I will.

@davecgh
Copy link
Member Author

davecgh commented Jul 18, 2023

Out of curiosity, do you plan to always show it or only when the agenda is active? I have a bit of preference for the latter, since it seems redundant to show the same hash value twice, but either way works.

@chappjc
Copy link
Member

chappjc commented Jul 18, 2023

I envisioned only having it shown when it is different from the block hash.

BTW, has anyone every used those two merkel roots, at least from the frontend?

@davecgh
Copy link
Member Author

davecgh commented Jul 18, 2023

I did a little bit for convenience when doing some testing with the version 2 cfilters on multiple machines where I intentionally didn't want them to have direct access to dcrd in order to prove the model, but it wouldn't have been difficult to hit the API link and grab it from there or query from another box that has dcrd.

I expect it is also useful for people using the timestamp service since it references that as well (as it's necessary to prove inclusion).

e.g.

image

@chappjc
Copy link
Member

chappjc commented Jul 18, 2023

Gotcha. They should stay then. Was just considering reclaiming some real estate.

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 a pull request may close this issue.

4 participants