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-0112? | Observe script type #749

Merged
merged 22 commits into from
May 14, 2024

Conversation

colll78
Copy link
Contributor

@colll78 colll78 commented Jan 23, 2024

We propose to introduce a new Plutus scripts type Observe in addition to those currently available (spending, certifying, rewarding, minting, drep). The purpose of this script type is to allow arbitrary validation logic to be decoupled from any ledger action. Since observe validators are decoupled from actions, you can run them in a transaction without needing to perform any associated action (ie you don't need to consume a script input, or mint a token, or withdraw from a staking script just to execute this validator).


Rendered Proposal on Fork

@colll78 colll78 marked this pull request as draft January 23, 2024 01:02
@Ryun1 Ryun1 added the Category: Plutus Proposals belonging to the 'Plutus' category. label Jan 23, 2024
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.

I'm in favour of this. There is clearly a need; the existing workaround is gross; this is a clean and orthogonal addition that also mirrors other functionality (e.g. requiredSigners).

cc @Quantumplation , I think you have also been thinking about this

cip-observe-script-type/README.md Outdated Show resolved Hide resolved
Comment on lines 90 to 99
Scripts are passed information about transactions via the script context.
We propose to augment the script context to include information about the observation scripts that are executed in the transaction.

Changing the script context will require a new Plutus language version in the ledger to support the new interface.
The change is: a new field is added to the script context which represents the list of observation scripts that must validate the transaction.
The observation scripts in the list are represented by their hash.

The interface for old versions of the language will not be changed.
Scripts with old versions cannot be spent in transactions that include observation scripts, attempting to do so will be a phase 1 transaction validation failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Scripts are passed information about transactions via the script context.
We propose to augment the script context to include information about the observation scripts that are executed in the transaction.
Changing the script context will require a new Plutus language version in the ledger to support the new interface.
The change is: a new field is added to the script context which represents the list of observation scripts that must validate the transaction.
The observation scripts in the list are represented by their hash.
The interface for old versions of the language will not be changed.
Scripts with old versions cannot be spent in transactions that include observation scripts, attempting to do so will be a phase 1 transaction validation failure.

@rphair
Copy link
Collaborator

rphair commented Jan 23, 2024

@colll78 we're not adding this for discussion in today's CIP meeting, where new proposals are normally introduced in Triage, because of the Draft status (as also with #735). Please feel free to leave it in this state as long as you need... then we will proceed to meeting discussion as well as more rigorous community review (though leaving it here for public comment now is also welcome).

@michaelpj's positive feedback is also noted but generally we wouldn't try to solicit expert opinions about this while it's still in Draft state.

@Quantumplation
Copy link
Contributor

Quantumplation commented Jan 23, 2024

Yea, I've been thinking about this for a while, and @colll78 and I have discussed it before. It's clearly the "right" way to express this concept, if this was greenfield work.

My only reservations are that it adds more complexity to the ledger, for largely aesthetic / very small micro-optimization benefits, without any big material advantage over the stake script approach.

I'd much rather have @colll78's output redeemers proposal, for example.

Still, if the ledger team feels confident in that added complexity being small / low risk, and it doesn't come at the cost of other potential features that would have higher value, I wouldn't push this CIP away from the lunch table 😅

colll78 and others added 2 commits January 23, 2024 08:28
Co-authored-by: Ryan Williams <44342099+Ryun1@users.noreply.github.com>
Co-authored-by: Michael Peyton Jones <me@michaelpj.com>
@colll78 colll78 marked this pull request as ready for review January 23, 2024 14:34
@colll78
Copy link
Contributor Author

colll78 commented Jan 23, 2024

Yea, I've been thinking about this for a while, and @colll78 and I have discussed it before. It's clearly the "right" way to express this concept, if this was greenfield work.

My only reservations are that it adds more complexity to the ledger, for largely aesthetic / very small micro-optimization benefits, without any big material advantage over the stake script approach.

I'd much rather have @colll78's output redeemers proposal, for example.

Generally, from developers in the community that I have discussed these with, most share @Quantumplation's opinion that output redeemers is a much more pressing and valuable proposal given that for the problems it addresses every solution currently employed has serious drawbacks (breaks composability, heavily bloats the ledger, bloats the UTxO set, not generalized, etc).

Where-as for this CIP, the withdraw-zero design pattern, although awkward, obscure and not optimal does provide equivalent functionality without any glaring drawbacks. Still given that nearly every major DApp in the ecosystem is relying on this withdraw-zero trick, clearly this is a desired functionality for DApp developers, and having it rely on some obscure mechanic about zero amount not being filtered from the txInfoWdrl arbitrarily enforcing staking script execution is clearly not healthy for the ecosystem (and it is inconsistent with how zero amount values are filtered from the txInfoMint so the same cannot be used to execute minting validators without any ledger action).

@rphair
Copy link
Collaborator

rphair commented Jan 23, 2024

@colll78 @Quantumplation @michaelpj Due to last minute status change this has been added to Triage (not time for full technical review; just an introduction) at today's meeting: https://hackmd.io/@cip-editors/80

@micahkendall
Copy link

micahkendall commented Jan 23, 2024

This would greatly simplify explaining the pattern to new developers because we wouldn't need to explain it in terms of a hack or an unexpected feature of the ledger, as you have to with the withdraw-0 trick. There are many scenarios even with prominent developers where we have this long back and forth on why we're using stake withdrawal validation to export logic.

Additionally, where we use withdraw-0, we aren't sure of the future of this in V3 and whether there will be adjustments to prevent us using this. This makes depending on the pattern long-term (despite how necessary it is) troublesome.

@michaelpj
Copy link
Contributor

Yes, I think it's definitely desirable to express this properly. It would be bad to be prevented from removing the nonsensical 0 withdrawal case because dapp developers rely on it. Better to have a route that's designed to do that.

@Quantumplation
Copy link
Contributor

@michaelpj well, just to be clear, we and many other dApps now are depending on that 0 withdrawal trick, so I think we're stuck with that now. Removing that capability would constitute a really rough breaking change, unless you had some mechanism to "fake" the withdrawals for old plutus versions or something.

@rphair rphair changed the title CIP-???? | Observe script type CIP-0112? | Observe script type Jan 23, 2024
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Agreed to promote at today's CIP meeting 🚀

cip-observe-script-type/README.md Outdated Show resolved Hide resolved
@Quantumplation
Copy link
Contributor

Note that much of Phil's performance concern disappears if we get arrays and use them for many of the script context fields 😅

@michaelpj
Copy link
Contributor

I'm not sure if that's true? If you're searching through a list you potentially have to look at all the elements, which is O(n) anyway, and I thought his worry was the extra overhead on looking at each element.

@colll78
Copy link
Contributor Author

colll78 commented Feb 5, 2024

If you're searching through a list you potentially have to look at all the elements, which is O(n) anyway, and I thought his worry was the extra overhead on looking at each element.

In practice, you don't need to look through each element, because we know what the list looks like at the time of transaction construction we can just pass in the index where the item we are looking for should be and fail if it is not there. This way, only assuming transactions are built correctly, it will succeed. Transaction determinism (we know the exact state of the transaction we are trying to submit) is exactly why O(1) index lookup data-structures are so incredibly powerful. Because we can lookup anything in O(1) by providing the onchain code with the index (via redeemer) to where the element you want to find is supposed to be. In this case, if the validator is looking for the signature PubKeyCredential "7c7bfa6b888fb3e600d4d9505b4fbca905df8ac58ed623b7170ab12a" then we provide the validator with the index where it is located in the txInfoObservations and then check that it indeed is at that index all of which can be done in O(1).

https://github.com/Anastasia-Labs/design-patterns/blob/main/UTXO-INDEXERS.md

So my conerns aren't relevant if we get O(1) index lookup. Otherwise, while I still believe this will be an efficiency bottleneck, if placing it into one field is what the ledger / plutus team want I am happy to change the CIP to reflect that.

then let's have a general discussion about representing lists of sum types as multiple lists.

That's fair. In general yes, that is something that I would want (ie separate script inputs from pub key inputs).

@colll78
Copy link
Contributor Author

colll78 commented Feb 24, 2024

Updated the CIP to remove the new field from the CDDL instead opting to promote the existing required_signatures as per the feedback above. Also updated the CIP to reflect that native scripts are allowed to be used as observers; although given that based on the feedback this is desired, the current method of achieving this (include native script inputs in reference inputs) should probably be removed from future versions in the same way that withdraw zero will be.

@peter-mlabs
Copy link

Would love to see this come to fruition!

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

The feedback from Plutus reps at the CIP editor's meeting today was positive: particularly that the IOG Plutus Team would prefer this approach to what @SebastienGllmt had proposed in candidate CIP-0038 (#309).

@colll78 I'm ready to ✅ this after you can please remove the template comments from the document:

cip-observe-script-type/README.md Outdated Show resolved Hide resolved
colll78 and others added 2 commits March 19, 2024 10:41
Co-authored-by: Robert Phair <rphair@cosd.com>
@colll78
Copy link
Contributor Author

colll78 commented Mar 20, 2024

The feedback from Plutus reps at the CIP editor's meeting today was positive: particularly that the IOG Plutus Team would prefer this approach to what @SebastienGllmt had proposed in candidate CIP-0038 (#309).

@colll78 I'm ready to ✅ this after you can please remove the template comments from the document:

Done ✅, thank you!

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

as per #749 (review)... thanks @colll78. We can mark this Last Check for next meeting if reviewed favourably by other editors.

@Ryun1 @Crypto2099 I was thinking the Path to Active looks emaciated compared to other Plutus proposals but since the Plutus Team (cc @michaelpj) is looking kindly on its implementation then that may be OK.

@rphair
Copy link
Collaborator

rphair commented Apr 21, 2024

OK no objection from @michaelpj in last month regarding CIP-0035 compliance... so marking this Last Check as proposed above & @Ryun1 @Crypto2099 let's try to maintain some Plutus category visibility for this one.

@rphair rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label Apr 21, 2024
@rphair
Copy link
Collaborator

rphair commented Apr 25, 2024

p.s... on agenda as such for CIP meeting review in 5 days' time: https://hackmd.io/@cip-editors/87

@rphair rphair requested review from Ryun1 and Crypto2099 April 25, 2024 18:19
rphair and others added 2 commits April 30, 2024 22:41
Co-authored-by: Ryan <44342099+Ryun1@users.noreply.github.com>
@rphair rphair requested a review from Ryun1 May 4, 2024 04:55
@rphair rphair merged commit e10f16c into cardano-foundation:master May 14, 2024
Copy link
Contributor

@GeorgeFlerovsky GeorgeFlerovsky left a comment

Choose a reason for hiding this comment

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

I wholeheartedly endorse this CIP in its current form.

I also support the changes to the initial draft that have emerged from the PR discussion thread, so far.

My vote is for separate fields in TxInfo for signatures and observer scripts. Whereas, for CDDL, I'm fine with either a combined field or separate fields — which ever Ledger/Plutus teams prefer.

@GeorgeFlerovsky
Copy link
Contributor

GeorgeFlerovsky commented Jun 29, 2024

One interesting corollary of this CIP (if I understand it correctly) is that we will now be able to attach conditional consent to a partially drafted tx, instead of waiting to absolutely consent to the finalized tx.

This would be quite useful in multi-party interactive tx drafting because the party finalizing the transaction body doesn't necessarily have to go back to all the counter-parties for signatures before submitting.

Concretely, Alice can conditionally consent by attaching her personalized script to the required observers, with the redeemer set to a (Signature, Instruction) that the script interprets as her authorized intent.

One of the native/spending/minting/staking scripts would require Alice's personalized script to be in the tx observers, so that other drafters can't remove her conditions.

(Alice's instructions should probably include some nonce tied to the transaction to prevent replay/double satisfaction, but I'm sure that can be figured out)

We can already do something similar with signed tuples in spending/minting/staking scripts, but I think observer scripts will give us more general/novel/clean versions of this pattern.

(CAUTION: conditional consent is non-revocable)

@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Jul 23, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.