Closed carlhammann closed 1 year ago
I edited my review to add nothing less than the positive feedback. Please, excuse me for the hiccup.
So, let me summarise the state of this PR:
xAttack
to x
.AddConstraints
module to PushPopConstraints
.mkAccumLAttack
that rely on optics to build attacks is not clear enough.
doubleSatAttack
understandable now? In particular, is it clear what it does with TryCombinations
?sameConstraints
is a sensible "semantic" equality relation on Constraints
?Attack.AddConstraints
respect this equality?I think that my last question is interesting enough to warrant its own issue, which I opened: #165. For the purposes of this PR, I think we should leave sameConstraints
as it is.
In my perception, after talking to many of you, all unanswered questions are now recorded as issues, and only these two remain:
doubleSatAttack
understandable now? @facundominguez and @lucaspena were the ones who were, rightfully, most unhappy with it.Attack
type become? I quite like Tweak
: It sounds like a small adjustment, which arguably is what most of the "xAttack
" functions do, and a combination of tweaks with malicious intent is an attack.Is the documentation of the doubleSatAttack understandable now?
I think is much better. Thanks.
Do we have a better word than "Attack"?
Tweak is good. Should it be plural? An attack can produce more than one modification, right?
A long name would be TransactionModifications
which could be compressed to TxMods
perhaps.
This PR defines
and uses the fact that this type is a monad with branching and failure capabilities to rework our attack language. A good first example to see this in action is probably the simplest of the "big" attacks: The token duplication attack.
This is not yet completely finished, in particular:
Cooked.Attack.AddConstraints
are heavily biased towards input constraints, because that's what I needed so far. This should be made more symmetric.Cooked.Tx.Constraints.Optics
However, it's ready for feedback and review!
If merged, this PR will close #143 .