tweag / cooked-validators

MIT License
39 stars 11 forks source link

Signer `Tweaks` #179

Closed carlhammann closed 1 year ago

carlhammann commented 1 year ago

The work on the double satisfaction vulnerability from PR #177 and the new version of the auction contract from PR #161 made me realise that it's now possible to demonstrate the vulnerability using the automatic doubleSatAttack, if we can have Tweaks that change the list of signers of the transaction. This latter capability is the contribution of this PR. Defining

newtype Tweak a = Tweak {getTweak :: MockChainSt -> TxSkel -> NE.NonEmpty Wallet -> [(TxSkel, NE.NonEmpty Wallet, a)]

allows us to have functions like ensureSignersTweak :: [Wallet] -> Tweak [Wallet], which makes sure that the modified transaction(s) are signed by the given list of wallets. The rest of the attack API is unchanged.

Additionally, this PR

mmontin commented 1 year ago

I am a bit skeptical with this change because it seems that further uncovered vulnerabilities will yield yet other changes to the type of Tweak. In that case, we take a list of wallets to add to the list of possible signers (bring them in scope) so that we can eventually sign the transaction with these wallets. It looks like some kind of a change in the context to me, which we wanted to originally avoid by deciding not to live in MonadMockChain and having a custom monad instead. Do we really want to add custom arguments whenever needed or are we confident that this new type brings everything to the table for further attacks? On another note, will we still need these 2 layers of signatures for transactions?

carlhammann commented 1 year ago

I am a bit skeptical with this change because it seems that further uncovered vulnerabilities will yield yet other changes to the type of Tweak. [...] It looks like some kind of a change in the context to me, which we wanted to originally avoid by deciding not to live in MonadMockChain and having a custom monad instead.

So am I. The value in incroporating these changes would be that they document the state of affairs. Also, there's a case to be made that, with the current design of MonadMockChain, there isn't anything left to add. Famous last words, I know, but let me explain: If Tweaks are about changing a transaction, they are mediately also about changing the evolution of the state of the world, through transaction validation. Transaction validation depends on

We use the function hasValidationErrors in the crucial step of transaction validation here, which needs exactly these data. The protocol parameters should not be touched by single-transaction tweaks, the state of the world should only be interacted with through transaction validation itself, the transaction itself can already be changed by the old definition of Tweak. It's just that we forgot about signatures so far, because the need never arose.

However, all of this is pretty contingent on the current implementation of the direct monad, so I'm also happy if we distill this PR into one or more issues to be considered in our Plutus V2 rewriting. What does @0xd34df00d think?

On another note, will we still need these 2 layers of signatures for transactions?

I think we will still need it (but I might be wrong), and I'll open an issue to discuss that question.

mmontin commented 1 year ago

If Tweaks are about changing a transaction, they are mediately also about changing the evolution of the state of the world

Although I don't fully understand the meaning of "mediately", doesn't that sentence mean that we could indeed live in MonadMockChain?

It seems to me that every change we make goes in that direction. The point here is not for me to push my own ideas, just that this work seems to get closer and closer to those.

carlhammann commented 1 year ago

I meant that the idea of Tweaks, as they stand at the moment, can be described as changing the evolution of the state of the world only by validating one transaction differently. That's still considerably less than the full capability of MonadMockChain. I do agree however that we're heading in that direction.

At the moment, the reason why I'm hesitant to go all the way (even if there would be many possible applications, I'm sure) is the LTL implementation, which for me is conceptually based around the idea that it's a single transaction validation that changes. I confused myself thoroughly when I thought about questions like this a few weeks ago. I wouldn't attempt anything like that without a really good semantical foundation.

carlhammann commented 1 year ago

Here is the issue about SignedBy vs. signingWith I promised above: #180

mmontin commented 1 year ago

It seems to me that the only "problematic" interaction point with the chain is the validation process, which is validateTxSkel. So instead of going uphill and trying to add features to Tweak up until we reach MonadMockChain without transaction validation (and execution) maybe we could go downhill and come up with a monad that is basically a mock chain without the ability to execute transactions. I have always thought that this specific primitive should live somewhere else than the rest. Would it make sense in our hierarchy of monads ? What about signatures and time primitives ? All in all I think there's some thinking to do before merging this. We can either discuss it live or pursue here.

carlhammann commented 1 year ago

This feature was now implemented (as a byproduct) by #210.