Implementing review: most aspects corrected, tests working

Author: Tomas-Silva2187

packages/cactus-plugin-satp-hermes/src/main/solidity/contracts/SATPWrapperContract.sol

@@ -1,9 +1,10 @@
 // SPDX-License-Identifier: GPL-3.0
-pragma solidity 0.8.20;
+pragma solidity ^0.8.20;
 
 import "@openzeppelin/contracts/access/Ownable.sol";
 import "./ITraceableContract.sol";
 import "@openzeppelin/contracts/utils/Strings.sol";
+import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
 
 /**
  * @dev Enum for the supported token types.
@@ -12,11 +13,11 @@ enum TokenType { UNSPECIFIED, ERC20, ERC721, ERC1155, NONSTANDARD_FUNGIBLE, NONS
 /**
  * @dev Enum for the supported interaction types.
  */
-enum InteractionType { MINT, BURN, ASSIGN, CHECKPERMITION, LOCK, UNLOCK }
+enum InteractionType { MINT, BURN, ASSIGN, CHECKPERMITION, LOCK, UNLOCK, APPROVE }
 /**
  * @dev Enum representing the supported variable types used for contract-to-contract calls.
  */
-enum VarType {CONTRACTADDRESS, TOKENTYPE, TOKENID, OWNER, AMOUNT, BRIDGE, RECEIVER, UNIQUEDESCRIPTOR}
+enum AssetParameterIdentifier {CONTRACTADDRESS, TOKENTYPE, TOKENID, OWNER, AMOUNT, BRIDGE, RECEIVER, UNIQUE_DESCRIPTOR}
 
 
 /**
@@ -50,7 +51,7 @@ struct Token {
 struct InteractionSignature {
     InteractionType interactionType;
     string[] functionsSignature;
-    VarType[][] variables;
+    AssetParameterIdentifier[][] variables;
     bool available;
 }
 
@@ -66,7 +67,9 @@ error TokenNotUnlocked(string tokenId);
 
 error InsuficientAmountLocked(string tokenId, uint256 amount);
 
-error TokenTypeNotSupported(string tokenId);
+error TokenNotSupported(string tokenId);
+
+error TokenAlreadyLocked(string tokenId);
 
 
 /**
@@ -78,7 +81,7 @@ error TokenTypeNotSupported(string tokenId);
  *
  * @notice Ensure that the contract is deployed and configured correctly before interacting with it.
  */
-contract SATPWrapperContract is Ownable, ITraceableContract{
+contract SATPWrapperContract is Ownable, ITraceableContract, IERC721Receiver{
 
     /**
      * Maping of token IDs to Token structs.
@@ -105,6 +108,7 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
     event Mint(string indexed tokenId, uint256 amount);
     event Burn(string indexed tokenId, uint256 amount);
     event Assign(string indexed tokenId, address receiver_account, uint256 amount);
+    event Approve(string indexed tokenId, address spender, uint256 amount);
 
 
     /**
@@ -174,7 +178,7 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
             }
         }
         else {
-            revert TokenTypeNotSupported(tokenId);
+            revert TokenNotSupported(tokenId);
         }
         deleteFromArray(tokens[tokenId].tokenId);
         delete tokens[tokenId];
@@ -203,8 +207,14 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
             }
             else if (tt == TokenType.ERC721 || tt == TokenType.NONSTANDARD_NONFUNGIBLE) {
                 // When dealing with non-fungible tokens, the "amount" is interpreted as the token unique descriptor.
+                if(tokens[tokenId].amount != 0) {
+                    revert TokenAlreadyLocked(tokenId);
+                }
                 tokens[tokenId].amount = assetAttribute;
             }
+            else {
+                revert TokenNotSupported(tokenId);
+            }
             emit Lock(tokenId, assetAttribute);
             return true;
         }
@@ -239,9 +249,10 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
             tokens[tokenId].amount = 0;
             emit Unlock(tokenId, assetAttribute);
             return true;
-        }     
-
-        revert TokenNotUnlocked(tokenId);
+        }
+        else {
+            revert TokenNotSupported(tokenId);
+        }
     } 
 
     /**
@@ -292,7 +303,7 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
             return true;
         }   
         else {
-            revert TokenTypeNotSupported(tokenId);
+            revert TokenNotSupported(tokenId);
         }
     }
 
@@ -325,9 +336,23 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
             return true;
         }
         else {
-            revert TokenTypeNotSupported(tokenId);
+            revert TokenNotSupported(tokenId);
         }
-    }   
+    }
+
+    /**
+     * @notice REQUIRED by OpenZeppelin: Supports the use of safe functions for ERC721 tokens.
+     * @return success A boolean indicating if the account has the bridge role.
+     */
+    function onERC721Received(
+    address,
+    address,
+    uint256,
+    bytes calldata
+) external pure override returns (bytes4) {
+    return this.onERC721Received.selector;
+}
+
 
     /**
      * Gets all the token IDs.
@@ -407,7 +432,7 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
         for (uint i = 0; i < tokensInteractions[tokenId][interactionType].functionsSignature.length; i++) {
             bytes4 functionSelector = bytes4(keccak256(abi.encodePacked(tokensInteractions[tokenId][interactionType].functionsSignature[i])));
 
-            bytes memory encodedParams = encodeDynamicParams(functionSelector, encodeParams(tokensInteractions[tokenId][interactionType].variables[i], tokenId, receiver, assetAttribute));
+            bytes memory encodedParams = encodeDynamicParams(functionSelector, AssetParameterIdentifierEncoder(tokensInteractions[tokenId][interactionType].variables[i], tokenId, receiver, assetAttribute));
 
             (bool callSuccess, ) = tokens[tokenId].contractAddress.call(encodedParams);
             if (!callSuccess) {
@@ -438,22 +463,22 @@ contract SATPWrapperContract is Ownable, ITraceableContract{
      * @param receiver The address of the receiver account.
      * @param assetAttribute The asset attribute of tokens to be encoded.
      */
-    function encodeParams(VarType[] memory variables, string memory tokenId, address receiver, uint256 assetAttribute)  internal view returns (bytes[] memory){
+    function AssetParameterIdentifierEncoder(AssetParameterIdentifier[] memory variables, string memory tokenId, address receiver, uint256 assetAttribute)  internal view returns (bytes[] memory){
         bytes[] memory dynamicParams = new bytes[](variables.length);
         for (uint i = 0; i < variables.length; i++) {
-            if (variables[i] == VarType.BRIDGE) {
+            if (variables[i] == AssetParameterIdentifier.BRIDGE) {
                 dynamicParams[i] = abi.encode(address(this));
-            } else if (variables[i] == VarType.TOKENID) {
+            } else if (variables[i] == AssetParameterIdentifier.TOKENID) {
                 dynamicParams[i] = abi.encode(tokenId);
-            } else if (variables[i] == VarType.AMOUNT) {
+            } else if (variables[i] == AssetParameterIdentifier.AMOUNT) {
                 dynamicParams[i] = abi.encode(assetAttribute);
-            } else if (variables[i] == VarType.OWNER) {
+            } else if (variables[i] == AssetParameterIdentifier.OWNER) {
                 dynamicParams[i] = abi.encode(tokens[tokenId].owner);
-            } else if (variables[i] == VarType.CONTRACTADDRESS) {
+            } else if (variables[i] == AssetParameterIdentifier.CONTRACTADDRESS) {
                 dynamicParams[i] = abi.encode(tokens[tokenId].contractAddress);
-            } else if (variables[i] == VarType.RECEIVER) {
+            } else if (variables[i] == AssetParameterIdentifier.RECEIVER) {
                 dynamicParams[i] = abi.encode(receiver);
-            } else if (variables[i] == VarType.UNIQUEDESCRIPTOR) {
+            } else if (variables[i] == AssetParameterIdentifier.UNIQUE_DESCRIPTOR) {
                 dynamicParams[i] = abi.encode(assetAttribute);
             } else {
                 revert("Variable not supported");

packages/cactus-plugin-satp-hermes/src/main/solidity/generated/Context.sol/Context.json

@@ -1 +1 @@
-{"abi":[],"bytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"deployedBytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"methodIdentifiers":{},"rawMetadata":"{\"compiler\":{\"version\":\"0.8.20+commit.a1b79de6\"},\"language\":\"Solidity\",\"output\":{\"abi\":[],\"devdoc\":{\"details\":\"Provides information about the current execution context, including the sender of the transaction and its data. While these are generally available via msg.sender and msg.data, they should not be accessed in such a direct manner, since when dealing with meta-transactions the account sending and paying for execution may not be the actual sender (as far as an application is concerned). This contract is only required for intermediate, library-like contracts.\",\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"node_modules/@openzeppelin/contracts/utils/Context.sol\":\"Context\"},\"evmVersion\":\"paris\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"ipfs\"},\"optimizer\":{\"enabled\":true,\"runs\":1000},\"remappings\":[\":@openzeppelin/=node_modules/@openzeppelin/\",\":ds-test/=node_modules/ds-test/src/\",\":forge-std/=node_modules/forge-std/src/\",\":hardhat/=node_modules/hardhat/\",\":openzeppelin-solidity/=node_modules/openzeppelin-solidity/\"]},\"sources\":{\"node_modules/@openzeppelin/contracts/utils/Context.sol\":{\"keccak256\":\"0x493033a8d1b176a037b2cc6a04dad01a5c157722049bbecf632ca876224dd4b2\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://6a708e8a5bdb1011c2c381c9a5cfd8a9a956d7d0a9dc1bd8bcdaf52f76ef2f12\",\"dweb:/ipfs/Qmax9WHBnVsZP46ZxEMNRQpLQnrdE4dK8LehML1Py8FowF\"]}},\"version\":1}","metadata":{"compiler":{"version":"0.8.20+commit.a1b79de6"},"language":"Solidity","output":{"abi":[],"devdoc":{"kind":"dev","methods":{},"version":1},"userdoc":{"kind":"user","methods":{},"version":1}},"settings":{"remappings":["@openzeppelin/=node_modules/@openzeppelin/","ds-test/=node_modules/ds-test/src/","forge-std/=node_modules/forge-std/src/","hardhat/=node_modules/hardhat/","openzeppelin-solidity/=node_modules/openzeppelin-solidity/"],"optimizer":{"enabled":true,"runs":1000},"metadata":{"bytecodeHash":"ipfs"},"compilationTarget":{"node_modules/@openzeppelin/contracts/utils/Context.sol":"Context"},"evmVersion":"paris","libraries":{}},"sources":{"node_modules/@openzeppelin/contracts/utils/Context.sol":{"keccak256":"0x493033a8d1b176a037b2cc6a04dad01a5c157722049bbecf632ca876224dd4b2","urls":["bzz-raw://6a708e8a5bdb1011c2c381c9a5cfd8a9a956d7d0a9dc1bd8bcdaf52f76ef2f12","dweb:/ipfs/Qmax9WHBnVsZP46ZxEMNRQpLQnrdE4dK8LehML1Py8FowF"],"license":"MIT"}},"version":1},"id":1}
+{"abi":[],"bytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"deployedBytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"methodIdentifiers":{},"rawMetadata":"{\"compiler\":{\"version\":\"0.8.20+commit.a1b79de6\"},\"language\":\"Solidity\",\"output\":{\"abi\":[],\"devdoc\":{\"details\":\"Provides information about the current execution context, including the sender of the transaction and its data. While these are generally available via msg.sender and msg.data, they should not be accessed in such a direct manner, since when dealing with meta-transactions the account sending and paying for execution may not be the actual sender (as far as an application is concerned). This contract is only required for intermediate, library-like contracts.\",\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"node_modules/@openzeppelin/contracts/utils/Context.sol\":\"Context\"},\"evmVersion\":\"paris\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"ipfs\"},\"optimizer\":{\"enabled\":true,\"runs\":1000},\"remappings\":[\":@openzeppelin/=node_modules/@openzeppelin/\",\":ds-test/=node_modules/ds-test/src/\",\":forge-std/=node_modules/forge-std/src/\",\":hardhat/=node_modules/hardhat/\",\":openzeppelin-solidity/=node_modules/openzeppelin-solidity/\"]},\"sources\":{\"node_modules/@openzeppelin/contracts/utils/Context.sol\":{\"keccak256\":\"0x493033a8d1b176a037b2cc6a04dad01a5c157722049bbecf632ca876224dd4b2\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://6a708e8a5bdb1011c2c381c9a5cfd8a9a956d7d0a9dc1bd8bcdaf52f76ef2f12\",\"dweb:/ipfs/Qmax9WHBnVsZP46ZxEMNRQpLQnrdE4dK8LehML1Py8FowF\"]}},\"version\":1}","metadata":{"compiler":{"version":"0.8.20+commit.a1b79de6"},"language":"Solidity","output":{"abi":[],"devdoc":{"kind":"dev","methods":{},"version":1},"userdoc":{"kind":"user","methods":{},"version":1}},"settings":{"remappings":["@openzeppelin/=node_modules/@openzeppelin/","ds-test/=node_modules/ds-test/src/","forge-std/=node_modules/forge-std/src/","hardhat/=node_modules/hardhat/","openzeppelin-solidity/=node_modules/openzeppelin-solidity/"],"optimizer":{"enabled":true,"runs":1000},"metadata":{"bytecodeHash":"ipfs"},"compilationTarget":{"node_modules/@openzeppelin/contracts/utils/Context.sol":"Context"},"evmVersion":"paris","libraries":{}},"sources":{"node_modules/@openzeppelin/contracts/utils/Context.sol":{"keccak256":"0x493033a8d1b176a037b2cc6a04dad01a5c157722049bbecf632ca876224dd4b2","urls":["bzz-raw://6a708e8a5bdb1011c2c381c9a5cfd8a9a956d7d0a9dc1bd8bcdaf52f76ef2f12","dweb:/ipfs/Qmax9WHBnVsZP46ZxEMNRQpLQnrdE4dK8LehML1Py8FowF"],"license":"MIT"}},"version":1},"id":2}

packages/cactus-plugin-satp-hermes/src/main/solidity/generated/IERC721Receiver.sol/IERC721Receiver.json

@@ -0,0 +1 @@
+{"abi":[{"type":"function","name":"onERC721Received","inputs":[{"name":"operator","type":"address","internalType":"address"},{"name":"from","type":"address","internalType":"address"},{"name":"tokenId","type":"uint256","internalType":"uint256"},{"name":"data","type":"bytes","internalType":"bytes"}],"outputs":[{"name":"","type":"bytes4","internalType":"bytes4"}],"stateMutability":"nonpayable"}],"bytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"deployedBytecode":{"object":"0x","sourceMap":"","linkReferences":{}},"methodIdentifiers":{"onERC721Received(address,address,uint256,bytes)":"150b7a02"},"rawMetadata":"{\"compiler\":{\"version\":\"0.8.20+commit.a1b79de6\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[{\"internalType\":\"address\",\"name\":\"operator\",\"type\":\"address\"},{\"internalType\":\"address\",\"name\":\"from\",\"type\":\"address\"},{\"internalType\":\"uint256\",\"name\":\"tokenId\",\"type\":\"uint256\"},{\"internalType\":\"bytes\",\"name\":\"data\",\"type\":\"bytes\"}],\"name\":\"onERC721Received\",\"outputs\":[{\"internalType\":\"bytes4\",\"name\":\"\",\"type\":\"bytes4\"}],\"stateMutability\":\"nonpayable\",\"type\":\"function\"}],\"devdoc\":{\"details\":\"Interface for any contract that wants to support safeTransfers from ERC721 asset contracts.\",\"kind\":\"dev\",\"methods\":{\"onERC721Received(address,address,uint256,bytes)\":{\"details\":\"Whenever an {IERC721} `tokenId` token is transferred to this contract via {IERC721-safeTransferFrom} by `operator` from `from`, this function is called. It must return its Solidity selector to confirm the token transfer. If any other value is returned or the interface is not implemented by the recipient, the transfer will be reverted. The selector can be obtained in Solidity with `IERC721Receiver.onERC721Received.selector`.\"}},\"title\":\"ERC721 token receiver interface\",\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"node_modules/@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol\":\"IERC721Receiver\"},\"evmVersion\":\"paris\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"ipfs\"},\"optimizer\":{\"enabled\":true,\"runs\":1000},\"remappings\":[\":@openzeppelin/=node_modules/@openzeppelin/\",\":ds-test/=node_modules/ds-test/src/\",\":forge-std/=node_modules/forge-std/src/\",\":hardhat/=node_modules/hardhat/\",\":openzeppelin-solidity/=node_modules/openzeppelin-solidity/\"]},\"sources\":{\"node_modules/@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol\":{\"keccak256\":\"0x7f7a26306c79a65fb8b3b6c757cd74660c532cd8a02e165488e30027dd34ca49\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://d01e0b2b837ee2f628545e54d8715b49c7ef2befd08356c2e7f6c50dde8a1c22\",\"dweb:/ipfs/QmWBAn6y2D1xgftci97Z3qR9tQnkvwQpYwFwkTvDMvqU4i\"]}},\"version\":1}","metadata":{"compiler":{"version":"0.8.20+commit.a1b79de6"},"language":"Solidity","output":{"abi":[{"inputs":[{"internalType":"address","name":"operator","type":"address"},{"internalType":"address","name":"from","type":"address"},{"internalType":"uint256","name":"tokenId","type":"uint256"},{"internalType":"bytes","name":"data","type":"bytes"}],"stateMutability":"nonpayable","type":"function","name":"onERC721Received","outputs":[{"internalType":"bytes4","name":"","type":"bytes4"}]}],"devdoc":{"kind":"dev","methods":{"onERC721Received(address,address,uint256,bytes)":{"details":"Whenever an {IERC721} `tokenId` token is transferred to this contract via {IERC721-safeTransferFrom} by `operator` from `from`, this function is called. It must return its Solidity selector to confirm the token transfer. If any other value is returned or the interface is not implemented by the recipient, the transfer will be reverted. The selector can be obtained in Solidity with `IERC721Receiver.onERC721Received.selector`."}},"version":1},"userdoc":{"kind":"user","methods":{},"version":1}},"settings":{"remappings":["@openzeppelin/=node_modules/@openzeppelin/","ds-test/=node_modules/ds-test/src/","forge-std/=node_modules/forge-std/src/","hardhat/=node_modules/hardhat/","openzeppelin-solidity/=node_modules/openzeppelin-solidity/"],"optimizer":{"enabled":true,"runs":1000},"metadata":{"bytecodeHash":"ipfs"},"compilationTarget":{"node_modules/@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol":"IERC721Receiver"},"evmVersion":"paris","libraries":{}},"sources":{"node_modules/@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol":{"keccak256":"0x7f7a26306c79a65fb8b3b6c757cd74660c532cd8a02e165488e30027dd34ca49","urls":["bzz-raw://d01e0b2b837ee2f628545e54d8715b49c7ef2befd08356c2e7f6c50dde8a1c22","dweb:/ipfs/QmWBAn6y2D1xgftci97Z3qR9tQnkvwQpYwFwkTvDMvqU4i"],"license":"MIT"}},"version":1},"id":1}