Update LIP0052 - improve code readability, correct/improve `getCollectionID`, `getChainID` and `lock` functions, typos

[mjerkov]

Motivation

Several issues in LIP 0052 have been noticed:

1. getCollectionID function enforces that an NFT must be stored before returning the corresponding collectionID. However, in the foreign-chain part of the execution of the cross-chain ccm, there is a call to the isNFTSupported function, which in turn calls getCollectionID function. This is a problem, since at that point the NFT is not yet stored on the foreign chain, and the execution would fail.
2. module argument of lock function is not checked against a possible nft input value, equal to the current value of NFT_NOT_LOCKED. This results in an unexpected behaviour of not locking the module.
3. Checks whether an NFT is escrowed or locked should be factored out to separate functions with self-explanatory names. Thus, when these checks are encountered elsewhere in the pseudocode, their purpose would be clear.
4. Several typos and/or code quality improvements are recommended.
5. In crossChainNFTTransferParamsSchema for the cross-chain transfer command, the property messageFeeTokenID is not needed, since it is does not appear anywhere in the execution.

Current Specifications

1. In getCollectionID, there is a check which ensures that NFT should be stored in the NFT store:

 def getCollectionID(nftID: NFTID) -> CollectionID:
     if NFTStore[nftID] does not exist:
         raise Exception("NFT substore entry does not exist")
     return nftID[`LENGTH_CHAIN_ID`:(`LENGTH_CHAIN_ID` + `LENGTH_COLLECTION_ID`)]

2. In lock, there is no check that module should not be equal to NFT_NOT_LOCKED.
3. In several functions, there are checks to ensure that an NFT is/isn't escrowed, e.g.

if len(getNFTOwner(nftID)) == LENGTH_CHAIN_ID:

Furthermore, in several functions there are checks to ensure that an NFT is/isn't locked, e.g.

if getLockingModule(nftID) != NFT_NOT_LOCKED:

4. In pattern definition for lockingModule property of userStoreSchema there is a typo (extra ] at the end of pattern expression):

"lockingModule": {
            "dataType": "string",
            "minLength": MIN_LENGTH_MODULE_NAME,
            "maxLength": MAX_LENGTH_MODULE_NAME,
            "pattern": "^[a-zA-Z0-9]*$]",
            "fieldNumber": 1
        }

Furthermore, the specification of the getCollectionID function has unnecessary "`" characters:

nftID[`LENGTH_CHAIN_ID`:(`LENGTH_CHAIN_ID` + `LENGTH_COLLECTION_ID`)]

Also, the return statement of the getAttributes function has unnecessary parentheses:

return(entry['attributes'])

5. Currently, messageFeeTokenID is a property of crossChainNFTTransferParamsSchema.

Proposed Specifications

1. In getCollectionID, remove the check whether NFT is stored in the NFT store:

  def getCollectionID(nftID: NFTID) -> CollectionID:
      return nftID[`LENGTH_CHAIN_ID`:(`LENGTH_CHAIN_ID` + `LENGTH_COLLECTION_ID`)]

2. In lock, add the check to ensure that the value of the moduleargument is not equal to NFT_NOT_LOCKED.
3. Define dedicated functions isNFTEscrowed and isNFTLocked that perform the mentioned checks and refactor the code accordingly.
4. Remove the extra symbols.
5. Remove messageFeeTokenID from crossChainNFTTransferParamsSchema and update the code accordingly. The following check in the verify function of the cross-chain transfer command now becomes obsolete:

if messageFeeTokenID != Interoperability.getMessageFeeTokenID(receivingChainID):
    raise Exception("Mismatching message fee Token ID")

Affected LIPs

0052