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

Formal spec update #2353

Merged
merged 1 commit into from
Jul 20, 2021
Merged

Formal spec update #2353

merged 1 commit into from
Jul 20, 2021

Conversation

polinavino
Copy link
Contributor

@polinavino polinavino commented Jun 24, 2021

Comments that have come up over the last couple of weeks (including Sebastian's)
First attempt at "script evaluation is deterministic" property
JIRA ticket CAD-3056

Not in here (coming up in next PR):

  • some renaming
  • txInfo
  • Datums only for inputs or outputs check

alonzo/formal-spec/properties.tex Outdated Show resolved Hide resolved
\begin{property}[Script interpreter arguments are fixed (deterministic script evaluation)]
\label{prop:fixed-inputs}

For this property to hold, we make an important assumption about the Plutus script
Copy link
Contributor

Choose a reason for hiding this comment

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

This paragraph is vacuously true because we're talking about mathematical functions instead of program code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I will leave it in as a note and make the function vs code distinction

alonzo/formal-spec/properties.tex Show resolved Hide resolved
e\vdash s\trans{ledger}{tx}s', \\
e'\vdash u\trans{ledger}{tx'}u', \\
\txbody{tx} = \txbody{tx'}, \\
(\fun{languages}~(\fun{txwits}~{tx}))~\restrictdom~\fun{costmdls}~(\field_{pp}(s))~=~(\fun{languages}~(\fun{txwits}~{tx'}))~\restrictdom\fun{costmdls}~(\field_{pp}(s')), \\
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be a precondition, but a conclusion from the wppHash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah! I forgot to remove this after i figure that too

in order to proceed to phase-2, wherein $\fun{evalScripts}$ is run
\end{itemize}

\item The second argument is a list. We will show that it must be the same list for the $e, s, tx, s'$
Copy link
Contributor

Choose a reason for hiding this comment

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

This proof seems to be mostly a repetition of the properties already discussed in the main text. I think this should be more formal instead. Also, I don't think it's necessary to remind the reader about what a function does here. They probably need to refer to its definition anyway if they don't remember.

@polinavino polinavino changed the title Formal spec update [WIP] - Formal spec update Jun 24, 2021
@polinavino polinavino changed the title [WIP] - Formal spec update Formal spec update Jul 5, 2021
@polinavino polinavino requested a review from WhatisRT July 6, 2021 13:24
& \fun{isNonNativeScriptAddress} \in \Tx \to \Addr \to \Bool \\
& \fun{isNonNativeScriptAddress}~tx~a = \\
& \fun{isTwoPhaseScriptAddress} \in \Tx \to \Addr \to \Bool \\
& \fun{isTwoPhaseScriptAddress}~tx~a = \\
Copy link
Contributor

Choose a reason for hiding this comment

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

this is certainly an improvement, I appreciate it, now the code and the spec agree. unfortunately 😬 we may have to change it again, just a heads up, sorry

Copy link
Contributor

@WhatisRT WhatisRT left a comment

Choose a reason for hiding this comment

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

Looks good in general, but I found some small issues and typos


\begin{lemma}
For all environments $e$, transactions $tx$ and states $s, s'$ such that
\begin{equation*}
e\vdash s\trans{ledger}{tx}s',
e\vdash s\trans{ledger}{tx}s'~\wedge~\fun{collateralPercent}~(\field_{pp}~(s))~\geq~100
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit awkward, why not use the φ function from above and multiply minfee below with collateralPercent instead of requiring this condition?

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 think the usefulness of this lemma comes from the idea that we want to establish some conditions under which we can guarantee that the transaction pays the minfee

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we care about the minfee specifically, only that it pays a fee if we want it to. But the statement using φ is more general, and so this can be a corollary

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'll try this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the collateralPercent is smaller than 100, we dont have anything to bound the fee with. I dont think it is useful to say anything along the lines of "depending on collateralPercent, and assuming one of the fee params is > 0,
0 < totalCollateralBalance < minfee <= txfee OR
0 < minfee < totalCollateralBalance .... etc. and consider every other case, then for fee params = 0 case too

I think this property is clear and useful for testing as is


The following holds :

\[\forall ~tp,~tp~\in~\fun{collectTwoPhaseScriptInputs} ~\field_{pp}(s)~\var{tx}~ \Utxo(s)\]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think using actual equality would be cleaner here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, hm - never mind. The return type of collectTwoPhaseScriptInputs is a list, and that the order of this list does not matter for evalScripts is discussed in the corollary that follows

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I forgot that it's a list. Don't we have a toSet function that we already use? Then we could say that they are equal as sets. The issue I'm having with introducing a name here is that it sort of suggests that that element could be used elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yeah, toSet would make sense


The $\fun{collectTwoPhaseScriptInputs}$
function (see \ref{fig:functions:script2})
makes a list where each entry contains a Plutus script
Copy link
Contributor

Choose a reason for hiding this comment

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

'constructs' is probably nicer than 'makes'

alonzo/formal-spec/properties.tex Outdated Show resolved Hide resolved
alonzo/formal-spec/properties.tex Outdated Show resolved Hide resolved
\end{corollary}

\begin{proof}
Let us consider the use of arguments of $\fun{evalScripts}$ (see Figure \ref{fig:functions:script2}).
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the proof just be a reference to the previous lemma?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

evalScripts now gets an additional tx argument and runs all the native scripts, so I had to discuss that in this proof. It worked out that way because of how the code ended up and there wasnt a clearly better way to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I missed that. But I don't think the way it's written right now works: the list passed to evalScripts only contains 2 phase scripts (that's even in the type). I'm very confused by this change...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is weird for sure - I changed the type to be Script instead of just phase-1 scripts to match the code. In the code, the filtering is done at the collectTwoPhaseScriptInputs stage, so the (s, _, _, _) option with s being a phase-1 script never actually gets included in sLst (even though the evaluation for them is defined correctly in evalScripts).

The reason for all this craziness is that scripts are of just one type, Script (with two constructors), so the type-level distinction is not possible. So, each function has to behave correctly on both kinds of scripts even if we know some are filtered out.

alonzo/formal-spec/properties.tex Outdated Show resolved Hide resolved
alonzo/formal-spec/utxo.tex Show resolved Hide resolved
alonzo/formal-spec/utxo.tex Outdated Show resolved Hide resolved
@polinavino polinavino requested a review from WhatisRT July 7, 2021 18:40
and the argumets passed to the interpreter for its evaluation.

To show that $\fun{collectTwoPhaseScriptInputs}$ returns a list containing
the same elements for both $tx$ and $tx'$,
we observe that
the set of elements from which this list is produced via $\fun{toList}$
is generated using the following functions, as well as set comprehension
and list operations. We observe that these functions are deterministic
and list operations (which are known to be deterministic).
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should be talking about functions being deterministic. We're doing a mathematical model, so there's no such thing as a non-deterministic function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah true

a given transaction), and the script purpose from the script hash and purpose pair.

\item $\fun{valContext}$ and $\fun{txInfo}$ :
The $\fun{valContext}$ function is assumed deterministic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

@polinavino polinavino requested a review from WhatisRT July 18, 2021 21:31
@@ -348,7 +348,7 @@ \subsection{Plutus Script Validation}
inputs. Specifically, each tuple in the list contains :

\begin{itemize}
\item the script;
\item the script, only if it is a non-native one;
Copy link
Contributor

Choose a reason for hiding this comment

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

2-phase instead?

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 used phase-2 and phase-1, it seemed nicer to me (eg. this script is run in phase 2, so it's a phase-2 script). But I can change.

Copy link
Contributor

@WhatisRT WhatisRT left a comment

Choose a reason for hiding this comment

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

Sorry for holding this up for this long, but I think we can merge it now!

@polinavino polinavino merged commit 0143575 into master Jul 20, 2021
@iohk-bors iohk-bors bot deleted the polina/formal-fix branch July 20, 2021 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants