SCP-2880: Transaction outputs should contain a minimum of 2 Ada. #76

koslambrou · 2021-11-01T09:08:16Z

Enforce constraint from the real Cardano network where each transaction output should contain a minimum amount (here 2) of Ada.

Changed the transaction validation code in Ledger.Index to check this minimum deposit.
Changed the transaction construction code in Ledger.Constraints.OffChain to add enough Ada for the minimum deposit to all transaction outputs of the unbalanced tx.
Changed the emulator Wallet handler to correctly handle this constraints when balancing a transaction.
Adapted the contracts in plutus-use-cases to work with this new constraint.
The command cabal run plutus-use-cases-scripts – ./tmp transactions -p ./plutus-use-cases/scripts/protocol-parameters.json does not produce warnings anymore.

Pre-submit checklist:

Branch
- Tests are provided (if possible)
- Commit sequence broadly makes sense
- Key commits have useful messages
- Relevant tickets are mentioned in commit messages
- Formatting, materialized Nix files, PNG optimization, etc. are updated
PR
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested

plutus-ledger/src/Ledger/Constraints/OnChain.hs

koslambrou · 2021-11-01T09:23:08Z

plutus-ledger/src/Ledger/Index.hs

+maxFee :: Ada
+maxFee = Ada.lovelaceOf 1_000_000


This is currently used for testings purposes in plutus-use-cases

koslambrou · 2021-11-01T09:23:47Z

plutus-use-cases/src/Plutus/Contracts/Future.hs

-testAccounts :: FutureAccounts
-testAccounts =
-    let con = setupTokens @() @FutureSchema @FutureError
-        fld = Folds.instanceOutcome con (Trace.walletInstanceTag (Wallet.knownWallet 1))
-        getOutcome (Folds.Done a) = a
-        getOutcome e              = Haskell.error $ "not finished: " <> Haskell.show e
-    in
-    either (Haskell.error . Haskell.show) (getOutcome . S.fst')
-        $ Freer.run
-        $ Freer.runError @Folds.EmulatorFoldErr
-        $ Stream.foldEmulatorStreamM fld
-        $ Stream.takeUntilSlot 10
-        $ Trace.runEmulatorStream def setupTokensTrace
-


This was moved to the test file.

plutus-ledger/src/Ledger/Constraints/OffChain.hs

koslambrou · 2021-11-01T13:47:50Z

plutus-ledger/src/Ledger/Constraints/OnChain.hs

@@ -40,10 +41,12 @@ checkOwnOutputConstraint
 checkOwnOutputConstraint ctx@ScriptContext{scriptContextTxInfo} OutputConstraint{ocDatum, ocValue} =
    let hsh = V.findDatumHash (Datum $ toBuiltinData ocDatum) scriptContextTxInfo
        checkOutput TxOut{txOutValue, txOutDatumHash=Just svh} =
-            txOutValue == ocValue && hsh == Just svh
+               Ada.fromValue txOutValue >= Ada.fromValue ocValue


Revert to old implementation when adjusting constraints using manually function as said in another comment.

Added another condition below to make it more precise. However, I can't revert it to the old implementation because of the way I currently adjusted the StateMachine (the new output constraint in mkStep). Here's why. If 10 Ada was locked in the script address, the user creates a tx to redeem 9 Ada (uses mustPayToTheScript of 1Ada), and calls adjustUnbalancedTx manually to adjust the tx outputs, then there's a discrepancy between the output constraint value and the tx output value.

What I meant during the review was a function that adjusts the OutputConstraints, instead of adjusting the unbalanced tx. I'm not sure if that's still a useful thing to do.

Ahhh ok. Mmm... I'll check that out. Might be better this way than to adjust unbalanced tx

Tried to write a function which adjusts the TxConstraints instead of UnbalancedTx, but it doesn't seem possible. I had the constraint mustSpendAtLeast <Guess token> in the GameStateMachine contract. In this scenario, there's a tx output which contains the Guess token, but I couldn't modify the TxConstraints such that a 2Ada would be associated with the token.

plutus-use-cases/src/Plutus/Contracts/Auction.hs

plutus-use-cases/src/Plutus/Contracts/Governance.hs

koslambrou · 2021-11-03T09:52:15Z

plutus-use-cases/src/Plutus/Contracts/Auction.hs

+                        , stateValue = Value.noAdaValue oldStateValue
+                                    <> Ada.toValue (Ada.fromValue oldStateValue - highestBid)
+                                    <> Ada.toValue newBid -- and lock the new bid in the script output


This new stateValue calculation is more precise, and works better with the StateMachine now that it adjustes the Ada per tx output.

koslambrou · 2021-11-03T12:19:07Z

plutus-contract/src/Plutus/Contract/StateMachine.hs

+      logWarn @Text $ "Plutus.Contract.StateMachine.runInitialise: "
+                    <> "Found a transaction output value with less than the minimum amount of Ada. Adjusting ..."


Not sure about that warning

koslambrou · 2021-11-03T12:24:05Z

@sjoerdvisscher @j-mueller What's mostly left is maybe reorganize some of the functions in Plutus.Contract.Request since user can now manually adjust an UnbalancedTx. For now, only mkTxConstraints return an UnbalancedTx so we can balance it afterwards. Any suggestions are welcome.

sjoerdvisscher · 2021-11-03T12:39:00Z

plutus-contract/src/Plutus/Contract/StateMachine/OnChain.hs

                                    [ OutputConstraint
                                        { ocDatum = newData
                                          -- Check that the thread token value is still there
                                        , ocValue = newValue <> threadTokenValueInner threadToken (ownHash ptx)
                                        }
-                                    ]
+                                    | not (isZero newValue)]


Can you explain this one?

This was needed because of the GameStateMachine contract.

Here's an use case example which explains the mod:

Wallet1 locks 8 Ada to the script address

Wallet2 call the guess endpoint with the correct secret and wants the full 8 Ada.

In GameStateMachine.transition, the (Locked, Guess) transition will generate a new stateValue of mempty (8 locked Ada - 8 requested Ada).

Then, StateMachine.runStep for the (Locked, Guess) transition is executed and it creates an unbalanced tx containing an output of mempty Value to the script address.

Wallet.Emulator.Wallet will validate the balanced transaction and report the error GameSMError (SMCContractError (WalletError (ValidationError (ScriptFailure (EvaluationError ["L1","Ld","S5","PT5"] "CekEvaluationFailure")))))

The validation that is used is part of StateMachine.OnChain.mkValidator that I modified above. It will call Ledger.Constraints.OnChain.checkOwnOutputConstraint. Since the new stateValue is mempty, getContinuingOutputs ctx return an empty, list therefore checkOwnOutputConstraint returns False (because of List.any, thus the validation fails.

Therefore, my mod is simply to not create an output constraint is the new value is empty.

But now it doesn't check for the thread token anymore. And what happens if the value is larger than zero but less than 2 Ada?

But now it doesn't check for the thread token anymore.

You're right. I should replace it with not (isZero (newValue <> threadTokenValueInner thread (ownHash ptx)). I think this is valid. Note that the runStep and mkStep function will adjust the new state value to contain min of 2Ada is it's not empty. Also note, that I adjusted the mkStep function as well. Great job find that.

And what happens if the value is larger than zero but less than 2 Ada?

So if Wallet1 locks 8 Ada, and Wallet2 correctly guesses the secret and want 7Ada, then the new state value will be 1Ada. mkStep will generate an adjusted contraint which adds the missing ada (output of 2Ada + thread token), and runStep will adjust the outputs of created unbalanced tx.
Makes sense?

Alright, I managed to find a way to revert this change. What I needed to do was adapt the GameStateMachine contract such that when the locked funds are 0, then we change the state to final, so there will be no more txs.

plutus-contract/src/Wallet/API.hs

sjoerdvisscher · 2021-11-05T10:21:25Z

plutus-contract/src/Plutus/Contract/StateMachine.hs

+                                      -- Add the thread token value back to the output
+                                    , ocValue = newStateValue <> Ada.toValue missingLovelace
+                                  }
+                                | not isFinal && not (Value.isZero newStateValue) ]


Ah, this is what I was missing. I don't think we want to throw away the outputconstraint when the value is zero, since we still want to propagate the ocDatum.

Then what do we do if the output value is zero? Simply adjusting it to 2 Ada doesn't make much sense, since that would mean that I could never use all funds in a script address.

And similarly it would not be possible to use all the funds but 1 Ada in a script address right? Maybe we need to discuss this with people who have thought about this more...

So currently, we need to adapt contracts like GameStateMachine such that if the locked funds are 0, then we need to transition of a final state. Of course, if want all the funds - 1, then the adjusting function will change that to funds - 2.

sjoerdvisscher

Looks good and and impressive job! (Although I have a nagging feeling this won't be the last of it, since it is all quite fiddly)

koslambrou · 2021-11-11T12:58:39Z

Looks good and and impressive job! (Although I have a nagging feeling this won't be the last of it, since it is all quite fiddly)

Thanks :)

True, it certainly feels fiddly.

… transaction outputs should contain a minimum amount (here 2) of Ada. * Changed the transaction validation code in Ledger.Index to check this minimum deposit. * Changed the transaction construction code in Ledger.Constraints.OffChain to add enough Ada for the minimum deposit to all transaction outputs of the unbalanced tx. * Changed the emulator Wallet handler to correctly handle this constraints when balancing a transaction. * Adapted the contracts in plutus-use-cases to work with this new constraint. * The command `cabal run plutus-use-cases-scripts – ./tmp transactions -p ./plutus-use-cases/scripts/protocol-parameters.json` does not produce warnings anymore.