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

Add support for PubKey into PubKeyHash conversion on-chain #5369

Open
uhbif19 opened this issue Jun 2, 2023 · 15 comments
Open

Add support for PubKey into PubKeyHash conversion on-chain #5369

uhbif19 opened this issue Jun 2, 2023 · 15 comments
Labels
enhancement Low priority Doesn't require immediate attention status: triaged

Comments

@uhbif19
Copy link

uhbif19 commented Jun 2, 2023

Describe the feature you'd like

Tx signatures only can be checked using PubKeyHash.
But datum signatures only can be checked using PubKey.

So naturally we would use PubKey to check both of them, but we cannot find a way to convert it to PubKeyHash on-chain.

Describe alternatives you've considered

For now we including the pair of PubKey and its hash, and rely on off-chain code to check that they do match.

This hack is described in our spec:

https://github.com/mlabs-haskell/hydra-auction/blob/staging/doc/on_chain_spec.md#parameters-and-state

@uhbif19 uhbif19 changed the title Add support PubKey into PubKeyHash conversion on-chain Add support for PubKey into PubKeyHash conversion on-chain Jun 2, 2023
@github-actions github-actions bot added the status: needs triage GH issues that requires triage label Jun 2, 2023
@effectfully
Copy link
Contributor

@michaelpj could you please take a look at this one?

@effectfully
Copy link
Contributor

effectfully commented Jun 13, 2023

Unfortunately, Michael is going to be on leave for a while. I've been jumping around asking people if there's a way to convert a PubKey to a PubKeyHash on-chain, but nobody seems to have an answer to this question. I've asked the question in one more internal channel, let's see if we can get an answer.

I need to triage the issue somehow, so I'm going to triage it with "Low priority" (given that there's a sensible workaround) as I'm not sure what else I can do about it right now...

@effectfully effectfully added Low priority Doesn't require immediate attention status: triaged and removed status: needs triage GH issues that requires triage labels Jun 13, 2023
@uhbif19
Copy link
Author

uhbif19 commented Jun 14, 2023

@effectfully

Looking on previous such issues (byte operations or something like that), I think this is up for CIP, cuz changes user primops list. Is that right?

given that there's a sensible workaround

There is no "sensible" workaround, AFAIK. Leaving it to off-chain as we do have different security guaranties, same as any other oracle implementation.

@effectfully
Copy link
Contributor

The Plutus Tools team pointed me to how a PubKey gets converted to a PubKeyHash. That conversion function uses Blake2b_224, but we only have Blake2b_256 on-chain, so it doesn't seem possible to me to convert a PubKey to a a PubKeyHash without adding a new builtin.

Does this make sense to you?

Adding new builtins is generally low-priority, hence I'm going to keep this issue as is for now, but we'll need to discuss with the team if adding Blake2b_224 to the set of builtins is cost-effective (it probably is, but it's not me who decides).

@effectfully
Copy link
Contributor

Looking on previous such issues (byte operations or something like that), I think this is up for CIP, cuz changes user primops list. Is that right?

Yeah, that's what I think too, and if you or somebody else created a CIP we'd highly appreciate that.

There is no "sensible" workaround, AFAIK. Leaving it to off-chain as we do have different security guaranties, same as any other oracle implementation.

I see, thank you, I agree with your assessment.

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented Jun 29, 2023

This inconsistency between PubKey and PubKeyHash support in Cardano and Plutus is an obstacle to the developer experience on Cardano.

  • The signatories of a transaction are provided as a list of pub key hashes [PubKeyHash], which means that if you want to know if Alice signed a particular transaction, you need to know her PubKeyHash.
  • The built-in Plutus on-chain function to check a signature for an arbitrary piece of data uses PubKey, which means that if you want to know if Alice signed a particular piece of data provided to a transaction context (e.g. via a redeemer), you need to know her PubKey.
  • There is no built-in Plutus on-chain function to convert a PubKey to a PubKeyHash. As you mentioned, this would require implementing the Blake2b_224 hashing function to built-ins (which already contain the related Blake2b_256 hashing function).

The upshot of this is that whenever we're building applications where we care about which both the transactions and data that our users sign, we need to keep track of both their PubKey and PubKeyHash, even though the latter is derived from the former.

@effectfully
Could you please explain how you arrived at the "Low Priority" status for this issue? I realize that signing data is a fairly new mechanism relative to signing transactions, but it is becoming more and more relevant and widely used in modern applications built on Cardano, particularly in the L2/Hydra/sidechain streams.

@uhbif19
Copy link
Author

uhbif19 commented Jun 29, 2023

we need to keep track of both their PubKey and PubKeyHash, even though the latter is derived from the former

It is not only that, but overall need to check that correspondence off-chain. In some cases it is possible to do this not making on-chain security worse (like in hydra-auction project), while it still complicates security reasoning. In some cases I believe it is not possible at all (as I believe in case of voting on Hydra in POCRE project), at least without changing interaction design from signing datum into signing transactions (which is worse, cuz does not support re-submiting tx on side of delegate server).

@effectfully
Copy link
Contributor

@GeorgeFlerovsky

@effectfully
Could you please explain how you arrived at the "Low Priority" status for this issue?

I already did:

Adding new builtins is generally low-priority

but let me elaborate.

Firstly, "low-priority" doesn't mean that we don't care or are not going to do it, it only means that we don't need to track it closely in the nearest future possible (unlike bugs or serious but easily fixable inefficiencies for example).

Secondly, any extension to the language needs to go through an approval of multiple people, often from different teams, and is generally a non-trivial process, particularly for crypto primitives that members of the Plutus team aren't familiar with (and we don't have any crypto experts among us). What this means is that in order to add any crypto primitive we need to reach out to a crypto expert and ask them to scrutinize it carefully, as well as review the implementation by a member of the Plutus team, plus a change to the specification. It's a lot of work, so before we commit to that, it is ideal for us to make sure that this is indeed something that we want, so that we don't end up spending a lot of time on something that isn't very helpful. And the way we ensure that is by having a CIP that we can read and ask other people to read, including various stakeholders who make the final decision.

Basically, it's a complicated and time consuming process, which is why having a CIP is a prerequisite for starting it. Sometimes we feel confident and not overloaded already, so we create a CIP ourselves. Other times we wait for the community to demonstrate us the need for a builtin by submitting a CIP. And even in this case it can take a while, e.g. the bitwise operations CIP that was approved long ago is still not done despite there being a PR. We're a fairly small team responsible for lots of different things working in a setting with an extremely high cost of failure, so some things just don't move as fast as everyone would like to.

Hope this explains why I labeled the ticket as "low-priority".

@GeorgeFlerovsky and @uhbif19 thanks a lot for the examples, those are going to be very helpful once we start discussing the possibility of adding the builtin.

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented Jun 29, 2023

@effectfully Thank you so much for the detailed reply. Indeed, a thorough discussion via CIP seems warranted here — adding a new crypto primitive is important to get right. 🙂

We'll go ahead and start that process. Incidentally, we may have some crypto experts available on our end who may be able to advise on this.

@effectfully
Copy link
Contributor

We'll go ahead and start that process. Incidentally, we may have some crypto experts available on our end who may be able to advise on this.

@GeorgeFlerovsky thank you a ton for considering this, it would be invaluable for us. Any kind of help from the community is always highly appreciated and I personally insist that in the Plutus team we need to prioritize things that folks helped us with, because at that point it's very clear that the community does care about the specific functionality.

@uhbif19
Copy link
Author

uhbif19 commented Feb 18, 2024

@effectfully Am I understanding correctly, that with this PR, this issue can be done with new primops?

@effectfully
Copy link
Contributor

@effectfully Am I understanding correctly, that with this PR, this issue can be done with new primops?

I don't know, sorry.

@kwxm do you happen to know? If not, any idea who might?

@GeorgeFlerovsky
Copy link

@effectfully Am I understanding correctly, that with this PR, this issue can be done with new primops?

I believe so, but only with Plutus V3, which won't be on mainnet until Conway.

@uhbif19
Copy link
Author

uhbif19 commented Feb 20, 2024

@effectfully @GeorgeFlerovsky Are there any reference for Plutus/blockchain versions compability?

@effectfully
Copy link
Contributor

@uhbif19 does this help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Low priority Doesn't require immediate attention status: triaged
Projects
None yet
Development

No branches or pull requests

3 participants