Onchain Security enhancements & offchain tests #92
Conversation
…mable assets in registration
There was a problem hiding this comment.
Copilot reviewed 46 out of 66 changed files in this pull request and generated no comments.
Files not reviewed (20)
- cabal.project: Language not supported
- src/programmable-tokens/lib/SmartTokens/Contracts/ExampleTransferLogic.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/Contracts/Issuance.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/Contracts/IssuanceCborHex.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/Contracts/ProgrammableLogicBase.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/Contracts/ProtocolParams.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/Core/Scripts.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/LinkedList/Common.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/LinkedList/MintBlacklist.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/LinkedList/MintDirectory.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/Types/Constants.hs: Language not supported
- src/programmable-tokens/lib/SmartTokens/Types/PTokenDirectory.hs: Language not supported
- src/programmable-tokens/programmable-tokens.cabal: Language not supported
- src/regulated-stablecoin/exe/export-smart-tokens/Main.hs: Language not supported
- src/regulated-stablecoin/lib/Wst/AppError.hs: Language not supported
- src/regulated-stablecoin/lib/Wst/Cli.hs: Language not supported
- src/regulated-stablecoin/lib/Wst/Cli/Command.hs: Language not supported
- src/regulated-stablecoin/lib/Wst/Offchain/BuildTx/DirectorySet.hs: Language not supported
- src/regulated-stablecoin/lib/Wst/Offchain/BuildTx/IssuanceCborHexRef.hs: Language not supported
- src/regulated-stablecoin/lib/Wst/Offchain/BuildTx/ProgrammableLogic.hs: Language not supported
There was a problem hiding this comment.
Currently we require that new registrtaions of programmable tokens are allowed only if minting is allowed. However it seems to me that the issuance script does not require tokens to be registered to mint.
I think we should also enforce that the issuance script requires a proof that the token is registered either before or in the same tx as minting. i.e. first mint of a programmable token should be made together with its registration.
- I think in the same spirit that we don't want non-programmable assets to be registered and hijacked after being moved to programmableBaseAddresses, the transfer and thirdPartyTransfer logics for a programmable token should be registered prior to any minting of that token. Right now it is possible to mint programmable tokens, distribute them and only later register them in the directory node.
- Additionally, given that we don't allow changing of a smart tokens spending logics we should make sure that this is enforced throughout the lifetime of a programmable token, not just after registration.
I think we should be able to do this without cyclic dependency by requiring an extra directoryNodeCS parameter in the issuance script.
There was a problem hiding this comment.
This is sensible. I'll add this change to this PR.
| _psingleMintWithCredential :: Term (s :: S) (PAsData PRedeemer :--> AssocMap.PMap 'AssocMap.Unsorted PScriptPurpose PRedeemer :--> PBool) | ||
| _psingleMintWithCredential = | ||
| red <- plet pscriptContext'redeemer | ||
| -- PMintPToken is used to register a new programmable token in the directory and to mint the programmable token upon registration and |
There was a problem hiding this comment.
Remove or update comment. PMintPToken no longer exists.
| let purposeConstrPair = pfstBuiltin # x | ||
| purposeConstrIdx = pfstBuiltin # (pasConstr # pforgetData purposeConstrPair) |
There was a problem hiding this comment.
constructor index extraction can be refactored as haskell level functions and moved out of the go, go1, go2 loops.
| purposeConstrIdx = pfstBuiltin # (pasConstr # pforgetData purposeConstrPair) | ||
| in pif | ||
| (pnot # (purposeConstrIdx #== 0)) | ||
| (self # xs) |
There was a problem hiding this comment.
(self # xs) seems unreachable here.
We check in the issuance script that the purpose is minting therefore there must be at least 1 mint redeemer.
We should be able to safely start the search with go1.
There was a problem hiding this comment.
Yes, there must be at-least one mint redeemer, but the purpose of go is to skip spending redeemers, (self # xs) is indeed reachable if the transaction executing spending validators because the redeemers list is sorted according to:
So the SpendingPurpose entries appear first in the txInfoRedeemers list even though the constructor index of the spending purpose is 1, and the constructor index of the minting purpose is 0.
There was a problem hiding this comment.
Ahh, I see. That's fine then.
| ] | ||
| ) | ||
|
|
||
| -- | Check that exactly one redeemer with the mintingLogic credential is present in the transaction. |
There was a problem hiding this comment.
| -- | Check that exactly one redeemer with the mintingLogic credential is present in the transaction. | |
| -- | Check that exactly one `PMinting` redeemer with the mintingLogic credential is present in the transaction. |
| ) | ||
|
|
||
| -- | Check that exactly one redeemer with the mintingLogic credential is present in the transaction. | ||
| psingleMintWithCredential :: Term (s :: S) (PAsData PRedeemer :--> AssocMap.PMap 'AssocMap.Unsorted PScriptPurpose PRedeemer :--> PBool) |
There was a problem hiding this comment.
This is minor but we should document somewhere that user minting scripts should not use any mint redeemers that equal to the our encoding of PSmartTokenMintingAction (Term s PCredential) in their application specific code.
We could also use some arbitrary large constructor index for the encoding of the credential to reduce likelihood of collision.
| @@ -49,7 +49,7 @@ PlutusTx.makeIsDataIndexed ''DirectoryNodeAction | |||
|
|
|||
| data PDirectoryNodeAction (s :: S) | |||
| = PInit | |||
| | PInsert { pkeyToInsert :: Term s (PAsData PByteString) } | |||
| | PInsert { pkeyToInsert :: Term s (PAsData PByteString), phashedParam :: Term s (PAsData PByteString) } | |||
There was a problem hiding this comment.
I would suggest changing phashedParam to pmintingLogicCred.
Otherwise some comments/documentation explaining where phashedParam is coming from can also suffice.
| case compile NoTracing (mkProgrammableLogicMinting # pconstant progLogicCred) of | ||
| Right compiledScript -> compiledScript | ||
| Left err -> error $ "Failed to compile issuer script: " <> show err | ||
| dummyIssuerInstanceCborHex = encodeSerialiseCBOR $ applyArguments issuerScriptBase [toData placeholderMintingLogic] |
There was a problem hiding this comment.
Not sure how relevant it is here, but the API in aiken-design-patterns requires the prefix to be single CBOR encoded, so that it can prepend it with version for achieving the correct hash on-chain.
There was a problem hiding this comment.
This PR includes end-to-end tests demonstrating that this format does indeed work, so I believe encodeSerialiseCBOR must be single encoding, as we do append the version prefix onchain as follows:
_papplyHashedParameter ::
Term s PByteString
-> Term s PByteString
-> Term s (PAsData PByteString)
-> Term s PByteString
_papplyHashedParameter prefix postfix hashedParam =
pblake2b_224 # (scriptHeader <> postfix)
where
_versionHeader :: Term s PByteString
_versionHeader = unsafeEvalTerm NoTracing (pintegerToByteString # pmostSignificantFirst # 1 # _plutusVersion)
_plutusVersion :: Term s PInteger
_plutusVersion = pconstant 3
scriptHeader :: Term _ PByteString
scriptHeader = _versionHeader <> prefix <> (pserialiseData # pforgetData hashedParam)
There was a problem hiding this comment.
Ah I see, the generated .txt file is what I looked at, which seems double encoded. Does that matter?
There was a problem hiding this comment.
That was garbage that I generated during testing, and I forgot to cleanup the export module after doing so, thanks for reminding me! Deleting those files and the other vestiges from testing in the export module in my next commit.
Modified the onchain code to enforce that the programmable token registration logic (insert directory node) correctly enforces that any asset that is inserted to the registry must be a programmable asset (a parameterized instance of the issuance minting policy)
Modified the offchain code and tests accordingly.