-
Notifications
You must be signed in to change notification settings - Fork 20
[Dijkstra] UTXOW rule #1018
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] UTXOW rule #1018
Conversation
d849d8f to
c9ee7c7
Compare
33250ed to
75365e1
Compare
ee7c200 to
3bcd752
Compare
44095d7 to
b4b1086
Compare
williamdemeo
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.
This looks great. I made some comments/change requests, but nothing too major. Otherwise, nice work, Carlos!
| globalScripts = ∅ -- TODO | ||
| globalData : DataHash ⇀ Datum | ||
| globalData = ∅ -- TODO |
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.
| globalData = ∅ -- TODO | |
| globalData = DatumMapOf tx -- TODO |
I think we should put something other than ∅ here for now, since otherwise the neededDataHashes ⊆ dom (Γ .globalData) fails for all nonempty neededDataHashes.
The suggestion above (DatumMapOf tx) should work if you put the following in Dijkstra/Specification/Transaction.lagda.md:
record HasDatumMap {a} (A : Type a) : Type a where
field DatumMapOf : A → DataHash ⇀ Datum
open HasDatumMap ⦃...⦄ public
instance
HasDatumMap-TxWitnesses : HasDatumMap (TxWitnesses txLevel)
HasDatumMap-TxWitnesses .DatumMapOf = TxWitnesses.txData
HasDatumMap-Tx : HasDatumMap (Tx txLevel)
HasDatumMap-Tx .DatumMapOf = DatumMapOf ∘ TxWitnessesOfSimilarly, we should probably put a non-empty placeholder in globalScripts. (Maybe something like witness scripts ∪ ref-input scripts?)
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 suggest we leave this to the corresponding issue.
| allScripts = | ||
| ( scripts -- (1) scripts from witnesses | ||
| ∪ mapPartial txOutToScript | ||
| ( range (utxo₀ ∣ txIns) -- (2) scripts from transaction inputs |
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 think we should document asap the important semantic choice of using utxo₀ in both UTXOW and SUBUTXOW to compute scripts/data needed from spending inputs:
range (utxo₀ ∣ txIns)which encodes the policy spending inputs are always resolved from the initial snapshot (utxo₀). (The "mempool-safety" / "no subtransaction spends newly created outputs" semantics.)
For now maybe just add one sentence above the relevant code blocks, something like,
"In SUBUTXOW, needed scripts/data for txIns are looked up in utxo₀ rather than the current s.utxo since subtransactions cannot spend outputs created earlier in the batch."
By the way, in my PR, 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), and I'll add something like the following above where they're introduced:
"utxo₀{.AgdaField} is the pre-batch snapshot used for spending-input inspection and TxInfo.realizedInputs{.AgdaField}; utxoRefView{.AgdaField} is the reference-lookup view (which may include earlier-prefix outputs)."
| let | ||
| open Tx tx | ||
| open TxBody txBody | ||
| open TxWitnesses txWitnesses | ||
| open UTxOEnv | ||
| utxo₀ = Γ .utxo₀ | ||
| utxo = s .UTxOState.utxo | ||
| witsKeyHashes : ℙ KeyHash | ||
| witsKeyHashes = mapˢ hash (dom vKeySigs) | ||
| allScripts : ℙ Script | ||
| allScripts = | ||
| ( scripts -- (1) scripts from witnesses | ||
| ∪ mapPartial txOutToScript | ||
| ( range (utxo₀ ∣ txIns) -- (2) scripts from transaction inputs | ||
| ∪ range (utxo ∣ refInputs) -- (3) scripts from reference inputs | ||
| ) | ||
| ∪ Γ .globalRefInputsScripts -- (4) scripts from global reference inputs | ||
| ) | ||
| p1Scripts : ℙ P1Script | ||
| p1Scripts = mapPartial toP1Script allScripts | ||
| p2Scripts : ℙ P2Script | ||
| p2Scripts = mapPartial toP2Script allScripts | ||
| neededScriptHashes : ℙ ScriptHash | ||
| neededScriptHashes = mapPartial (isScriptObj ∘ proj₂) (credsNeeded utxo₀ txBody) | ||
| neededVKeyHashes : ℙ KeyHash | ||
| neededVKeyHashes = mapPartial (isKeyHashObj ∘ proj₂) (credsNeeded utxo₀ txBody) | ||
| neededDataHashes : ℙ DataHash | ||
| neededDataHashes = mapPartial (λ txOut@(a , _ , d , _) → do sh ← isScriptObj (payCred a) | ||
| _ ← lookupHash sh p2Scripts | ||
| d >>= isInj₂) | ||
| (range (utxo₀ ∣ txIns)) |
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.
This block is essentially a repeat of the let..in block of SUBUTXOW. I wonder if it's worth factoring out this block and placing it in a parametrized module, e.g.,
module UTxOWContext
(ℓ : TxLevel)
(Γ : UTxOEnv)
(s : UTxOState)
(tx : Tx ℓ)
where
...(stuff inside your let..in block)...so that, in the definitions of the rules, we can just do:
let open UTxOWContext TxLevelSub Γ s stx inand
let open UTxOWContext TxLevelTop Γ s tx inThere 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 think there is a better opportunity here, that is, make the rule level-parametric. Since this is a matter of refactoring but doesn't change the intention I suggest we leave this as work for the near future.
| globalScripts : ℙ Script | ||
| globalScripts = ∅ -- TODO | ||
| globalData : DataHash ⇀ Datum | ||
| globalData = ∅ -- TODO |
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.
It's probably a good idea to document globalScripts and globalData above where they're first introduced, since they're central to the new Dijkstra semantics. Could you add something like the following above this code block?
globalScripts : ℙ Scriptdenotes the batch-wide script pool, available for resolving script hashes (witness scripts + reference scripts reachable via reference inputs, possibly including outputs created earlier in the batch).globalData : DataHash ⇀ Datumdenotes the batch-wide datum pool, available for resolving datum hashes (witness datums + datums reachable via reference inputs and/or other transactions in the batch).
Of course you should revise the prose if you have a better or different understanding of the roles played by these objects.
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 leave this to the corresponding issue.
williamdemeo
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.
This looks good. Let's get this merged once conflicts are resolved.
Co-authored-by: William DeMeo <[email protected]>
90149f4 to
43c1f6f
Compare
Description
Stacked PR (Please review PR #1014 before this one!)The branch of this PR should be rebased on master once PR #1015 is merged.This PR adapts and adds those preconditions present in Conway to the UTXOW rule.
Still TBD: #1024 #1025
Checklist
CHANGELOG.md