-
Notifications
You must be signed in to change notification settings - Fork 20
[Dijkstra] batch-aware collectP2ScriptsWithContext
#1015
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
[Dijkstra] batch-aware collectP2ScriptsWithContext
#1015
Conversation
d2e681c to
0e6ef2c
Compare
6ee6860 to
90a7192
Compare
0e6ef2c to
20498dc
Compare
90a7192 to
bb90078
Compare
20498dc to
0eb3c68
Compare
bb90078 to
66ac197
Compare
0eb3c68 to
58fc737
Compare
66ac197 to
1134bff
Compare
58fc737 to
d9fd7b5
Compare
1134bff to
4a690a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the Dijkstra-era script validation infrastructure to support batch-wide phase-2 evaluation. The key change is making collectP2ScriptsWithContext batch-aware by introducing dual UTxO views: one for spending input inspection (pinned to the initial snapshot) and one for reference script/datum lookup (which will eventually use an evolving view).
- Introduced separate
ScriptPurpose.lagda.mdmodule to houseScriptPurpose,TxInfo, andSubTxInfotypes - Added
txInfoForPurposefunction that conditionally includes sub-transaction context for guard scripts at the top level - Updated
collectP2ScriptsWithContextto accept two UTxO parameters and thread them appropriately through script lookup and context construction
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Ledger/Dijkstra/Specification/Script/ScriptPurpose.lagda.md | New module defining ScriptPurpose data type and TxInfo record with txInfoSubTxs field for guard scripts |
| src/Ledger/Dijkstra/Specification/Abstract.lagda.md | Imports ScriptPurpose module and adds valContext function to AbstractFunctions interface |
| src/Ledger/Dijkstra/Specification/Script/Validation.lagda.md | Removes inline type definitions (moved to ScriptPurpose), updates collectP2ScriptsWithContext to use dual UTxO views, adds txInfoForPurpose for purpose-specific context construction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d9fd7b5 to
f2f9003
Compare
b0da3a6 to
a60af70
Compare
c070f60 to
ce58826
Compare
a60af70 to
e5e912f
Compare
ce58826 to
8418eed
Compare
e5e912f to
1f7ea3b
Compare
1f7ea3b to
4c2836d
Compare
carlostome
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a minor comment (TB discussed)
| : ScriptPurpose → ScriptHash | ||
| → Maybe (P2Script × List Data × ExUnits × CostModel) | ||
| toScriptInput sp sh = | ||
| do s ← lookupScriptHash sh tx utxoSpend₀ utxoRefView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not particularly happy about the namings utxoSpend₀ utxoRefView.
In utxoSpend₀ the subscript 0 suggests (to me) is a snapshot of the original utxo while the suffix RefView in utxoRefView suggest the same.
For simplicity I'd suggest keeping utxo for the utxo in the state and maybe utxo₀ for the snapshot before subtransactions are applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that sounds good. The idea behind the current names is that the subscript 0 refers to the origin utxo, while RefView is the evolving one that the reference scripts see... but I'm fine with your proposed naming scheme, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll rename utxoSpend₀ → utxo₀ (pre-batch snapshot used for spending-input inspection + realizedInputs) and keep utxoRefView (ref-input lookup view, potentially evolving to include earlier-prefix outputs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see an obvious way to avoid having two utxo arguments in the script functions defined near the bottom of the Transactions module. I agree it would be nicer if we could bind these utxos to a UTxOEnv and a UTxOState, but the functions are defined in the Transaction module and used in the Scripts.Validation module, before the UTxOEnv and UTxOState types are even defined.
a636905 to
9b4798b
Compare
+ Remove contradiction between "ref inputs may refer to earlier tx outputs in the batch" vs "all inputs must exist before applying any tx in the batch." The new text punts the exact constraint to the UTxO rules (where it belongs). + Fix Plutus bullet (old "nor earlier versions" reads like "no Plutus at all"). + Align fees with current Agda (`txFee : InTopLevel …`), but leave room for later CIP-driven updates.
1. Introduce subTx info type (using an alias for `TxInfo` for now). 2. Extend `TxInfo` with field `txInfoSubTxs : Maybe (List SubTxInfo)`. 3. Define a purpose-built builder: + Top-level Guard scripts ⇒ `txInfoSubTxs = just (...)` + Everything else ⇒ `txInfoSubTxs = nothing` + SubTx scripts ⇒ always `nothing` (even for `Guard` at sub level)
…split UTxO views - Update collectP2ScriptsWithContext to take both utxoSpend₀ (initial snapshot) and utxoRefView (for reference lookups), preparing for batch-aware collection and single phase-2 eval. - Use txInfoForPurpose to construct validation context (TxLevel-indexed) instead of Conway's language-indexed txInfo. Dijkstra txInfo is indexed by TxLevel, not by Plutus language.
01f1a15 to
bff46ee
Compare
bff46ee to
ca47549
Compare
Description
Stacked PR. (This PR should be rebased on and target
masterif PR #1014 is merged first.)This PR closes issue #1006 by starting the Dijkstra-era refactor needed for batch-wide phase-2 evaluation.
This PR handles the collector refactor; the single call site is implemented in PR #1021.
Key change:
collectP2ScriptsWithContextnow takes two UTxO views:utxo₀= the snapshot UTxO "before any of the batch transactions are applied" (this is explicitly a CIP-0118 requirement).utxoRef= the view used for reference-input lookup, which is allowed to include outputs from earlier subtransactions in the batch (also explicitly CIP-0118).It also fixes context construction to use Dijkstra's
TxLevel-indexedtxInfoviatxInfoForPurpose, rather than Conway'stxInfo (language p2s) ...pattern.Motivation
Dijkstra batches need to:
UTXOS,This PR is an incremental step: it makes P2 collection ready to be called per-subTx with the correct UTxO parameters.
Changes
{ℓ : TxLevel}and(utxo₀ utxoRef : UTxO)arguments tocollectP2ScriptsWithContext.credsNeeded ℓ utxo₀ ...so spend-side input inspection remains snapshot-based.lookupScriptHashandtxOutToP2Script.txInfoForPurpose ℓ utxo₀ tx sp(TxLevel-indexed).Follow-ups (next commits / next PR)
utxoRefper subTx prefix.evalP2Scriptscall site atUTXOS.Copilot-generated Description
This pull request introduces significant refactoring and abstraction improvements to the Dijkstra specification, focusing on transaction structure, script validation, and the handling of scripts and guards. The changes add new type classes for modular access to transaction components, refactor script and UTxO handling for clarity and extensibility, and move or abstract several definitions to improve code organization. These updates lay the groundwork for more robust, maintainable, and extensible formalizations of transaction and script validation logic.
Major abstraction and type class improvements:
Introduced a suite of new type classes (e.g.,
HasTxBody,HasTxWitnesses,HasRedeemers,HasCollateralInputs,HasTxFees,HasSubTransactions,HasTopLevelGuards,HasIndexedOutputs,HasMintedValue,HasGuards,HasScripts) to provide modular, level-dependent and level-independent accessors for transaction components, replacing direct record field access throughout the codebase. [1] [2] [3] [4] [5] [6] [7]Added corresponding type class instances for
Tx,TxBody, andTxWitnesses, ensuring seamless and consistent access to transaction data at both top-level and sub-level contexts. [1] [2] [3] [4]Script and UTxO handling refactor:
Refactored script and UTxO handling functions (e.g.,
refScripts,txscripts,lookupScriptHash,getSubTxScripts,getTxScripts) to use the new type classes and clarified the distinction between initial and evolving UTxO snapshots (utxo₀andutxoRef), improving readability and correctness.Updated documentation and function signatures to reflect the new UTxO handling conventions and to clarify the use of two UTxO arguments for reference and spending inputs.
Script validation and context improvements:
Moved the definition of
ScriptPurposeandTxInfoto a new module (ScriptPurpose.lagda.md), and updated imports accordingly, centralizing script purpose logic. [1] [2] [3]Added the
valContextfunction to theAbstractFunctionsrecord, and used it in script validation to provide context data to scripts, improving the expressiveness and correctness of script evaluation. [1] [2]Correctness and consistency fixes:
Updated the
credsNeededMinusCollateralfunction to use the newMintedValueOfaccessor instead of the removedValueOf, ensuring correct calculation of credentials needed for minting.Standardized variable naming and usage (e.g.,
utxo₀,utxoRef) across script validation and helper functions for consistency. [1] [2] [3] [4]Guard and credential handling improvements:
GuardsOfandTopLevelGuardsOfaccessors, streamlining the logic for CIP-0118 compliance.Checklist
CHANGELOG.md