-
Notifications
You must be signed in to change notification settings - Fork 155
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
Add update mechanism and related STS rules #598
Conversation
0815970
to
0815ab6
Compare
Found some small issues, marked as |
08159b2
to
08156c1
Compare
instance Embed OCERT CHAIN where | ||
wrapFailed = OcertFailure | ||
let ls' = setIssueNumbers ls cs' -- TODO: is this cs or cs' ? | ||
-- (formal spec ignored cs') |
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 should be cs'
, good catch!
@@ -709,6 +709,7 @@ \subsection{Block Header Transition} | |||
} | |||
\\~\\ | |||
(\wcard,~\wcard,~\var{b_{prev}},~\wcard,~\var{es},~\var{ru},~\wcard,~\wcard)\leteq\var{nes} | |||
% TODO: is es = (_ _ _ pp) ? |
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.
yes, the protocol parameters are the fourth component of the epoch state. but I do not see what this is needed, we do not use the pp
in the rule.
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.
we extract the max header size and max block size from the paramters
@@ -852,6 +853,7 @@ \subsection{Verifiable Random Function} | |||
& ~~~~\where \\ | |||
& ~~~~~~~~~~\var{hk} \leteq \hashKey{(\bvkcold bhb)} \\ | |||
& ~~~~~~~~~~\var{ss} \leteq \slotToSeed{(\bslot{bhb})} \\ | |||
% TODO : is vk = bvkcold bhb ? |
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.
yes, good catch!
@@ -1381,7 +1383,8 @@ \subsection{Chain Transition} | |||
\eta_c' \\ | |||
\end{array}\right)} | |||
} \\~\\~\\ | |||
\var{ls'}\leteq\fun{setIssueNumbers}~\var{ls}~\var{cs} \\ | |||
\var{ls'}\leteq\fun{setIssueNumbers}~\var{ls}~\var{cs} \\ % TODO should | |||
% this be cs' ? |
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.
yes!
@@ -900,6 +900,8 @@ \subsection{Complete Epoch Boundary Transition} | |||
\\~\\~\\ | |||
\var{(\wcard,~\wcard,~\wcard,~(\var{pup},~\wcard,~\wcard,~\wcard))}\leteq\var{utxoSt'}\\ | |||
\var{pp_{new}}\leteq\fun{votedValue_{PParams}}~\var{pup}\\ | |||
% TODO: I think this isn't the right type, `pup` is PPUpdate which is not | |||
% a complete set of `PParams`. |
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 it is okay, though it maybe does not translate well to Haskell. The type of PParams
is a mapping of PPm
to Value
.
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.
ok, I see. So that's a place where the executable spec deviates a bit PParams
is a record with all values and the parameter update is a mapping. This comes in handy whenever a value of PParams
is read as the mapping would require checking for presence of the element. I will create an issue for that.
@@ -244,7 +244,7 @@ \section{Updates} | |||
& | |||
\dom{\var{aup}}\subseteq\dom{\var{dms}} | |||
\\ | |||
\var{aup'}\leteq\var{aup_s}\unionoverrideRight\var{aup} | |||
\var{aup'}\leteq\var{aup_s}\unionoverrideRight\var{aup} % TODO: not needed here? |
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.
yes, good catch! that was a copy past error 😬
@@ -327,7 +327,8 @@ \section{Updates} | |||
\begin{equation}\label{eq:update} | |||
\inference[Update] | |||
{ | |||
(\var{pup},~\var{aup})\leteq\var{up} | |||
(\var{pup},~\var{aup})\leteq\var{up} % TODO: duplicate of state names in |
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.
yes, good catch!
0815e08
to
0815e32
Compare
`LedgerState` still contains a field for the current `Ix` in the slot. This doesn't appear in the formal spec.
We allow redundant witnesses to support mutli-sig which might require more than one valid signature for transactions.
- remove unit tests that tests exactly that situation - remove `UnneededWitnesses` from expected failures
0815e32
to
0815b93
Compare
…_mechanism Add update mechanism and related STS rules
This is rebased onto current master with the tighter compilation error settings and includes all changes from #585.