Skip to content

on-chain NFT governance module#281

Open
RadNi wants to merge 39 commits intostagingfrom
amirhossein/nft-governance-on-chain
Open

on-chain NFT governance module#281
RadNi wants to merge 39 commits intostagingfrom
amirhossein/nft-governance-on-chain

Conversation

@RadNi
Copy link
Copy Markdown

@RadNi RadNi commented Nov 22, 2021

Test --quickcheck-replay=223823 fails.

cstml and others added 6 commits November 15, 2021 19:15
…-cases into amirhossein/nft-governance-on-chain

� Conflicts:
�	mlabs/src/Mlabs/NFT/Contract/Init.hs
�	mlabs/src/Mlabs/NFT/Governance/Types.hs
�	mlabs/src/Mlabs/NFT/Governance/Validation.hs
�	mlabs/src/Mlabs/NFT/Types.hs
�	mlabs/test/Test/NFT/Script/Values.hs
@RadNi RadNi self-assigned this Nov 22, 2021
t4ccer and others added 9 commits November 22, 2021 18:01
…ossein/nft-governance-on-chain

# Conflicts:
#	mlabs/src/Mlabs/NFT/Contract/Init.hs
#	mlabs/src/Mlabs/NFT/Governance/Types.hs
#	mlabs/src/Mlabs/NFT/Types.hs
…-cases into amirhossein/nft-governance-on-chain

� Conflicts:
�	mlabs/src/Mlabs/Data/LinkedList.hs
�	mlabs/src/Mlabs/NFT/Contract/Aux.hs
�	mlabs/src/Mlabs/NFT/Contract/Buy.hs
�	mlabs/src/Mlabs/NFT/Contract/CloseAuction.hs
�	mlabs/src/Mlabs/NFT/Contract/Fees.hs
�	mlabs/src/Mlabs/NFT/Governance/Types.hs
�	mlabs/src/Mlabs/NFT/Types.hs
@RadNi RadNi changed the title WIP: on-chain NFT governance module on-chain NFT governance module Nov 24, 2021
@RadNi
Copy link
Copy Markdown
Author

RadNi commented Nov 24, 2021

Close #272

case act of
InitialiseGov ->
True
False -- FIXME Why should anybody for good reason use it as the redeemer?!
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You need to mint a token for the head when the list is initialised.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This token must be in the same eUTXO with the Unique Token to be able to find (on-chain) what the correct minting Policy CurrencySymbol is.

Copy link
Copy Markdown
Author

@RadNi RadNi Nov 25, 2021

Choose a reason for hiding this comment

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

The off-chain implementation is currently a bit different

    mintListHead :: NftAppInstance -> GenericContract NftAppInstance
    mintListHead appInstance = do
      let -- Unique Token
          uniqueToken = appInstance'AppAssetClass appInstance
          uniqueTokenValue = assetClassValue uniqueToken 1
          emptyTokenName = TokenName PlutusTx.Prelude.emptyByteString
      let -- Script Head Specific Information
          headDatum :: DatumNft = nftHeadInit appInstance
          headPolicy = mintPolicy appInstance
          proofTokenValue = Value.singleton (scriptCurrencySymbol headPolicy) emptyTokenName 1
          initRedeemer = asRedeemer Initialise
      let -- Gov App Head Specific information
          govHeadDatum :: GovDatum = govHeadInit
          govHeadPolicy = govMintPolicy appInstance
          govScr = govScript uniqueToken
          govProofTokenValue = Value.singleton (scriptCurrencySymbol govHeadPolicy) emptyTokenName 1
          govInitRedeemer = asRedeemer InitialiseGov

          -- NFT App Head
          (lookups, tx) =
            ( mconcat
                [ Constraints.typedValidatorLookups txPolicy
                , Constraints.mintingPolicy headPolicy
                , Constraints.mintingPolicy govHeadPolicy
                ]
            , mconcat
                [ Constraints.mustPayToTheScript headDatum (proofTokenValue <> uniqueTokenValue)
                , Constraints.mustPayToOtherScript (validatorHash govScr) (toDatum govHeadDatum) (govProofTokenValue <> uniqueTokenValue)
                , Constraints.mustMintValueWithRedeemer initRedeemer proofTokenValue
                , Constraints.mustMintValueWithRedeemer govInitRedeemer govProofTokenValue
                ]
            )
      void $ Contract.submitTxConstraintsWith @NftTrade lookups tx
      Contract.logInfo @Hask.String $ printf "Forged Script Head & Governance Head for %s" (Hask.show appInstance)
      return appInstance

Creating the list head and minting the head token are happening at the same transaction. So we are not actually spending the list head when minting the head token. Therefore, the InitialiseGov is not being used as the redeemer to the script validator.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you look at

govInitRedeemer = asRedeemer InitialiseGov

The redeemer used at initialisation is the InitialiseGov redeemer. It is the only redeemer that should allow the minting of a token with Head datum.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

But the govInitRedeemer is only used for the minting policy here. Not for the script validator.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

They are locking separate utxos, with separate logic.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

#281 (comment)

Anything that gets the AppSymbol should be making use of :

getNftAppSymbol :: UniqueToken -> GenericContract NftAppSymbol

This makes sure that the correct AppSymbol is retrieved through the correct path (by looking at the Head).

There are some contracts which might need to be deprecated, as they do not use this.

&& freeGovBurnt == listGovBurnt
&& headOutVal `geq` (headInVal <> (negate . lovelaceValueOf $ fee))

nodeInsertedWithCorrectValues
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add comment with description of behavior for each major test function.

@cstml cstml requested a review from zygomeb November 26, 2021 17:54
@@ -36,19 +77,282 @@ instance ValidatorTypes GovManage where

-- | Minting policy for GOV and xGOV tokens.
mkGovMintPolicy :: NftAppInstance -> GovAct -> ScriptContext -> Bool
Copy link
Copy Markdown

@cstml cstml Nov 26, 2021

Choose a reason for hiding this comment

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

Add a general test that verifies that the Minting Policy and the ListGovToken in the Head have the same CurrencySymbol. This way we know that our Minting Policy only mints for the correct List/Script.

This can be a separate issue.

Copy link
Copy Markdown
Author

@RadNi RadNi Nov 29, 2021

Choose a reason for hiding this comment

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

In MintGov and ProofAndBurn action cases we look for the head datum in the inputs.
The function that returns head datum is inputsWithHeadDatum.

    inputsWithHeadDatum :: [(GovDatum, TxOut)]
    inputsWithHeadDatum =
      filter
        ( \(datum', txO) ->
            datumIsHead (gov'list datum') && tokenPresent txO
        )
        $ getInputDatumsWithTx @GovDatum ctx

If you check it out, one of the checks that it performs is tokenPresent which itself checks the existence of our proofToken

    symbol = ownCurrencySymbol ctx
    asset tn = AssetClass (symbol, TokenName tn)
    proofToken = asset emptyByteString
    
    tokenPresent txO =
      assetClassValueOf (txOutValue txO) proofToken == 1
        && assetClassValueOf (txOutValue txO) uniqueToken >= 1

Therefore it's already checked that the head datum contains the listGov token with our own CurrencySymbol

traceIfFalse "NFT-Gov-Mint: Only allowed to mint a proof token" checkOnlyProofMinted
&& traceIfFalse "NFT-Gov-Mint: uniquetoken and proofToken are not sent to the right address" checkProofTokenToRightAddress
&& traceIfFalse "NFT-Gov-Mint: The unique token is not present" checkUniqueTokenIsPresent
MintGov ->
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Head must be present in this transaction, see previous comment about determining the correct CurrencySymbol.

&& traceIfFalse "NFT-Gov-Mint: Only allowd to mint/burn listGov and freeGov" checkOnlyListGovFreeGovMinted
&& traceIfFalse "NFT-Gov-Mint: The minted amount of listGov must be equal to the fee." checkListGovEqualToFee
Proof -> traceError "NFT-Gov-Mint: Not allowed to mint using Proof as the redeemer."
ProofAndBurn ->
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Head must be present in this transaction,

Changing the Fee contract to reflect the InitializeGov action on the list head.
Adopting some tests to accept the new governance module
RadNi and others added 13 commits November 30, 2021 16:24
Off-chain only
…utus-use-cases into amirhossein/nft-governance-on-chain"

This reverts commit b2a8911, reversing
changes made to 56d2b59.
…s-use-cases into amirhossein/nft-governance-on-chain

� Conflicts:
�	mlabs/test/Test/NFT/Contract.hs
�	mlabs/test/Test/NFT/Trace.hs
…-cases into amirhossein/nft-governance-on-chain

� Conflicts:
�	mlabs/mlabs-plutus-use-cases.cabal
�	mlabs/src/Mlabs/NFT/Contract/Gov/Fees.hs
�	mlabs/src/Mlabs/NFT/Contract/Gov/Query.hs
�	mlabs/src/Mlabs/NFT/Contract/Init.hs
�	mlabs/test/Test/NFT/Contract.hs
�	mlabs/test/Test/NFT/Init.hs
�	mlabs/test/Test/NFT/QuickCheck.hs
�	mlabs/test/Test/NFT/Trace.hs
Merging with flake version.
Copy link
Copy Markdown

@cstml cstml left a comment

Choose a reason for hiding this comment

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

Most of the changes are correct. I would have to have a conversation to validate the updated approach on Gov to be able to be sure it is all correct.

Comment on lines +98 to +102
-- TODO: Should this check be here? It would disallow minting differnt currencies,
-- and check for propper minting of gov tokens is below anyway
&& traceIfFalse "NFT-Gov-Mint: Only allowd to mint/burn listGov and freeGov" checkOnlyListGovFreeGovMinted
-- TODO: Isnt it implied by next check?
&& traceIfFalse "NFT-Gov-Mint: The minted amount of listGov must be positive." checkListGovPositive
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I am not sure about these two checks

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