tokenName is encoded TWICE in `deployERC20Positions()` function

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x904a93417822e9e5ddf3bbf96590835aaf32db4edc016e6fea4a3dde6c2a3590 Severity: low

Description: Description\

The following function is used to deploy the ERC20 positions and its called in createNewMarketParams() function. To calculate, _data, it takes tokenNames which is passed as argument as string and token decimal which is 18 decimal for ERC20 tokens.

    function deployERC20Positions(
        bytes32 parentCollectionId,
        bytes32 conditionId,
        uint256 outcomeSlotCount,
        string[] memory tokenNames
    ) internal returns (IERC20[] memory wrapped1155, bytes[] memory data) {
        uint256 invalidResultIndex = outcomeSlotCount - 1;

        wrapped1155 = new IERC20[](outcomeSlotCount);
        data = new bytes[](outcomeSlotCount);

        for (uint256 j = 0; j < outcomeSlotCount; j++) {
            bytes32 collectionId = conditionalTokens.getCollectionId(parentCollectionId, conditionId, 1 << j);
            uint256 tokenId = conditionalTokens.getPositionId(collateralToken, collectionId);

            require(j == invalidResultIndex || bytes(tokenNames[j]).length != 0, "Missing token name");

            bytes memory _data = abi.encodePacked(
                toString31(j == invalidResultIndex ? "SER-INVALID" : tokenNames[j]),
@>                toString31(j == invalidResultIndex ? "SER-INVALID" : tokenNames[j]),
                uint8(18)
            );

            IERC20 _wrapped1155 = wrapped1155Factory.requireWrapped1155(address(conditionalTokens), tokenId, _data);

            wrapped1155[j] = _wrapped1155;
            data[j] = _data;
        }
    }

The issue is that, while encoding the _data, it incorrecty encoded the tokenNames twice which would produce incorrect data.

This data will be stored in createNewMarketParams() function where its stored as argument for struct Market.ConditionalTokensParams.

        return (
            Market.ConditionalTokensParams({
                conditionId: conditionId,
                questionId: questionId,
                parentCollectionId: parentCollectionId,
                parentOutcome: params.parentOutcome,
                parentMarket: params.parentMarket,
                wrapped1155: wrapped1155,
@>                data: data
            }),

Impact\ Incorrect data encoding in deployERC20 positions would lead to incorrect data stored in ConditionalTokensParams struct. This data would return incorrectly across contracts where it will be called/referred.

Recommendations\ Consider removing the additional code to perform correct _data encoding combining only tokenName and decimals.

    function deployERC20Positions(
        bytes32 parentCollectionId,
        bytes32 conditionId,
        uint256 outcomeSlotCount,
        string[] memory tokenNames
    ) internal returns (IERC20[] memory wrapped1155, bytes[] memory data) {
        uint256 invalidResultIndex = outcomeSlotCount - 1;

        wrapped1155 = new IERC20[](outcomeSlotCount);
        data = new bytes[](outcomeSlotCount);

        for (uint256 j = 0; j < outcomeSlotCount; j++) {
            bytes32 collectionId = conditionalTokens.getCollectionId(parentCollectionId, conditionId, 1 << j);
            uint256 tokenId = conditionalTokens.getPositionId(collateralToken, collectionId);

            require(j == invalidResultIndex || bytes(tokenNames[j]).length != 0, "Missing token name");

            bytes memory _data = abi.encodePacked(
                toString31(j == invalidResultIndex ? "SER-INVALID" : tokenNames[j]),
-                toString31(j == invalidResultIndex ? "SER-INVALID" : tokenNames[j]),
                uint8(18)
            );

            IERC20 _wrapped1155 = wrapped1155Factory.requireWrapped1155(address(conditionalTokens), tokenId, _data);

            wrapped1155[j] = _wrapped1155;
            data[j] = _data;
        }
    }

[greenlucid]

It's passed twice because it's providing name and symbol for the erc20

[clesaege]

Yep, @greenlucid is correct.