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-0076? | Hash-Checked Data #363

Closed
wants to merge 5 commits into from

Conversation

zygomeb
Copy link
Contributor

@zygomeb zygomeb commented Oct 25, 2022

The aim of this CIP is to extend the use of node as a source of truth and as a result bring cost-savings to on-chain computation.

Related work: CIP 42


(draft proposal in branch)

Copy link
Contributor

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

This was a considered alternative to CIP-42, but was rejected because it confuses the meaning of txInfoData. It currently reflects the corresponding field in the transaction, this would pollute it with ad-hoc data.


Hashing data on-chain is quite expensive from a computational perspective. Currently, however, we are forced to use the `serializeData` primitive combined with a hashing function of our choice. It would be computationally beneficial to let the node hash the data before running the smart contracts, giving us only a guarantee that this mapping is correct, just as we do with Datums presently.

Therefore we propose extending the ScriptContext from just a mapping between DatumHash and Datum to a more universal mapping between BuiltinByteString and BuiltinData.
Copy link
Contributor

Choose a reason for hiding this comment

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

the ScriptContext is not a mapping. I think you mean the txInfoData field of TxInfo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this sentence refers to the one field in ScriptContext. Although at the time of writing I was unsure whether or not to include the other way of doing this via including a new field txInfoAdditionalData to contain these which, to the other thread, would make it backwards compatible.

@L-as
Copy link
Contributor

L-as commented Oct 28, 2022

Relevant: IntersectMBO/cardano-ledger#2649

@KtorZ
Copy link
Member

KtorZ commented Nov 8, 2022

I think this proposal is rather about removing the current restriction in the ledger rules that prevent one from adding extraneous unreferenced data to the transaction, than it is about changing anything in Plutus per-se (though, one will likely impact the other I agree).

@SebastienGllmt
Copy link
Contributor

Hmm, you could make the same argument for data that had to be signed with different signing algorithms for other chains

CIP-AnyDataIsDatum/README.md Outdated Show resolved Hide resolved
@KtorZ KtorZ changed the title CIP-???? | Hash-Checked Data CIP-0076? | Hash-Checked Data Nov 8, 2022
@KtorZ
Copy link
Member

KtorZ commented Nov 8, 2022

As discussed in today's editors meeting, @kevinhammond to reach to relevant people at IOG from the ledger team to assess the feasibility and consequences of adjusting the ledger rules in that direction.


### Extending the ScriptContext

We change the field `txInfoData` to `[(BuiltinByteString, BuiltinData)]`. Nominally, this changes nothing as the `DatumHash` and `Datum` types are equivalent to these. Importantly however, we allow any pair to be pushed to this list, and the node will hash the `BuiltinData` to verify that it matches the `BuiltinByteString`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you suggesting we only support Blake2b 256? (ie the hash function currently used for Data). Above it says "hashing function of our choice", but here it looks like you are suggesting just keeping with the status quo.

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 believe that we should have more discussion around how it would be possible to have an arbitrary hashing algorithm used here -- but I'd be happy to have an MVP here of just the blake256 as for the purpose of what this aims to achieve it is entirely sufficient.

@JaredCorduan
Copy link
Contributor

JaredCorduan commented Nov 8, 2022

I don't see any explanation in this CIP regarding how this extra data is supplied. A new field in the transaction body?

Or is the idea just to relax the restrictions on the auxiliary data (and keep to Blake2b 256)?

@JaredCorduan
Copy link
Contributor

My two lovelace, just as a random human (ie, just me speaking without any authority, etc):

I empathize with the desire to relax the constraints mentioned in IntersectMBO/cardano-ledger#2649. This would let folks be able to utilize the node for random Blake2b 256 hashes inside of Plutus. Of course, this is more work for nodes that no one is really going to be paid for (except in some very indirect way of creating user-value).

I also empathize with @michaelpj in wanting to keep some coherency to the meaning of txInfoData.

As for the "feasibility and consequences" of the design (assuming all my assumptions above are valid regarding what exactly is being proposed), it's a matter of tweaking a rule in the next ledger era and deciding that we are okay with changing the meaning of txInfoData.

@michaelpj
Copy link
Contributor

I think this proposal is rather about removing the current restriction in the ledger rules that prevent one from adding extraneous unreferenced data to the transaction, than it is about changing anything in Plutus per-se

I don't agree with this. The purpose of this CIP is to work around a problem in Plutus. I don't think there's any reason to do it apart from that.


My objections are:

  1. I generally think it's a bad idea to work around problems in Plutus by making ad-hoc changes in the ledger. We should fix the problem at its root. Otherwise we open ourselves up to a never-ending sequence of such proposals, as we've already seen here with people saying "oh, but what if I want other hash functions? can we change the ledger some more?". Let's just not even start. We have a system for running user-specified logic including hash functions as part of transaction validation: it's Plutus scripts!
  2. As Jared points out, who pays for this? The costs for hashing things in Plutus are calibrated based on real-world performance, and the overhead is not so high. So if it is the case that moving this into the ledger leads to significant cost savings... that must mean that the ledger is doing significantly more work for you, and that work is unaccounted for. Putting it another way, what we really care about is our transaction validation time budget. Moving work from scripts into the ledger is a false saving, it means the user doesn't get billed, but it's still eating into our time budget!
  3. It muddies the meaning of txInfoData. It's not just a case of "removing restrictions", it makes the field less meaningful and harder to work with.
  4. Adding another field is also bad: transaction body fields are expensive, require serialization changes, bloat the ledger spec, etc. I think the bar should be high for them.

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>
@KtorZ KtorZ added the Category: Plutus Proposals belonging to the 'Plutus' category. label Mar 18, 2023
@rphair
Copy link
Collaborator

rphair commented Aug 19, 2024

@zygomeb I'm closing this as Abandoned because of both the 1 year+ without updates and the unfavourable response from both the Ledger & Plutus teams as they were at the time. There has been lots of Plutus reform in the meantime especially with what is going into Chang and perhaps the problem definition has changed since then. In any case this could be reopened with further work & advocacy, if you are still interested.

@rphair rphair closed this Aug 19, 2024
@rphair rphair added the State: Likely Abandoned Close if confirmed abandoned (long waiting). label Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Plutus Proposals belonging to the 'Plutus' category. State: Likely Abandoned Close if confirmed abandoned (long waiting).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants