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

CIP-0043 | New Plutus Core built-in dataHash #222

Closed
wants to merge 1 commit into from

Conversation

L-as
Copy link
Contributor

@L-as L-as commented Feb 15, 2022

Replaces #218

Rendered

@L-as L-as changed the title dataHash CIP New Plutus Core built-in dataHash Feb 15, 2022
Copy link
Contributor

@ch1bo ch1bo left a comment

Choose a reason for hiding this comment

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

As one of the authors of the proposed CIP-36 (#218) which this would want to replace, I left some comments on where we (Hydra) would need additional specification analogous to our PR.

We were considering this approach as well (or even a much more specific to Hydra variant), but we felt that sha3_256 . serialiseBuiltinData (or a different hash algorithm of choice) is straight-forward to write, does not add any additional cost, is more flexible and does not increase the scope of CIP-36 unnecessarily.

## Abstract

We propose adding a new built-in `dataHash` for calculating the hash of `Data` on-chain,
in such a way that it is coherent with `txInfoData`.
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to consider this as a replacement for #218, we would require quite some additional specification on how the builtinData is actually serialized before it is hashed. Mere "in such a way that it is coherent with txInfoData" is not sufficient for our use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you explain this? How is this not sufficient?

can access by having the concrete `Data` be passed in the redeemer (whether it's a minting policy
or validator).

It is also possible to verify signatures of (hashes of) `Data` with this built-in.
Copy link
Contributor

Choose a reason for hiding this comment

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

This would require having the same implementation of dataHash off-chain to produce the signature or a sufficiently detailed specification on how the Data is serialized (and hashed) such that the system creating the signature can follow the spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do already have a corresponding off-chain dataHash that is also coherent wrt. txInfoData.

Hashing using a different algorithm isn't something you need in practice,
because no existing protocol hashes Plutus's `Data` with something other
than `blake2b_256`. If in a future version of Plutus the hashing algorithm
were to be changed, then `dataHash` would also be changed accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

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

While this may be true for the Data provided by the ledger, it is not necessarily true for "user-supplied" Data. Furthermore, using a different hashing algorithm than blake2b_256 is not unrealistic. For example, in Hydra we are using sha2_256 right now! Although we could pick the used hash algorithm ourselves in our use case, other situations where some specific hashing scheme might be dictated by other, off-chain systems are perfectly thinkable and would not be able to use this builtin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure what you mean. You are saying that users could want to choose a different hashing algorithm, but without specifying a reason as to why. Can you give a realistic use case that would be prevented by this?


Another important point to note is that `dataHash` is potentially more
efficient on large `Data`, since the hash can be computed without storing
the entire serialisation of the `Data` in memory.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is something which should be detailed in the "Path to active" section as the most straight-forward blake2b_256 . serialiseData implementation (and thus easiest to develop and maintain) would not have that property. Only a hand-crafted serialize + hash routine might have these resource benefits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fundamentally disagree with this. This is a core property of dataHash. According to CIP-35, this is where it ought to be. You can make bad implementations of anything.


Implementation-wise, it should also be simpler than `serialiseData`, since
there are no considerations wrt. what the format of the serialisation should be,
because it's never exposed to users.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unfortunately not the case and tying the semantics of how the data is serialized and then hashed within this primitive to "whatever is available in off-chain dataHash" is not sufficient for non-Haskell use cases and potential source of regressions. As commented above, a specification of the actual binary representation for Data is required.

Copy link
Contributor Author

@L-as L-as Feb 17, 2022

Choose a reason for hiding this comment

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

...non-Haskell use cases? If you want to replicate this hashing algorithm in e.g. Rust, you just use whatever hashing algorithm Plutus already uses for txInfoData. How do you think https://github.com/input-output-hk/jormungandr would be implemented?

@L-as
Copy link
Contributor Author

L-as commented Feb 17, 2022

Do note #218 (comment)

@KtorZ KtorZ changed the title New Plutus Core built-in dataHash CIP-42? | New Plutus Core built-in dataHash Mar 17, 2022
@KtorZ
Copy link
Member

KtorZ commented Apr 5, 2022

The Plutus team decided to adopt #218 as a new built-in, making this proposal now redundant.

For the sake of keeping a track record, we may still want to merge this proposal as rejected.

@michaelpj
Copy link
Contributor

The primary reason for adopting #218 over this was the argument from increased composability.

@L-as
Copy link
Contributor Author

L-as commented Apr 7, 2022

What should I change concretely? status to rejected?

@KtorZ KtorZ changed the title CIP-42? | New Plutus Core built-in dataHash CIP-43 | New Plutus Core built-in dataHash May 11, 2022
@KtorZ KtorZ changed the title CIP-43 | New Plutus Core built-in dataHash CIP-0043 | New Plutus Core built-in dataHash May 11, 2022
@KtorZ KtorZ added tentative CIP State: Likely Deprecated Close if confirmed deprecated (or long waiting). and removed Candidate CIP labels May 11, 2022
@KtorZ
Copy link
Member

KtorZ commented Jun 7, 2022

@L-as mind if we close this PR or are you willing to mark it as rejected and have it merged to keep track of the discussion?

@L-as L-as closed this Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Likely Deprecated Close if confirmed deprecated (or long waiting).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants