hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

Potential Gas Limit issues in the Router Contract Functions Due to a High Number of Partitions of a Condition ID #30

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: @MatinR1 Twitter username: MatinRezaii1 Submission hash (on-chain): 0xd3ae86f978ae649ff859aeb45bc3f3e20b0f458c9a140075dabaeda37765eb64 Severity: medium

Description: Description\ The Router contract's mergePositions(), and splitPositions() functions iterate over all the partitions corresponding to the outcome slots of a condition ID. In scenarios where a market's condition ID has a large number of partitions, these functions are prone to hitting the block gas limit, resulting in transactions that fail to execute. This not only hinders the market's and user's ability to interact with the contract as intended but also may lead to situations where unwrapping and transferring the tokens fail.

https://github.com/seer-pm/demo/blob/6e5db716e44e251fcee6abd7c1f6a8d6e36db910/contracts/src/Router.sol#L54

        for (uint256 j = 0; j < partition.length; j++) {    // Potential Gas Limit hitting
            uint256 tokenId = getTokenId(collateralToken, parentCollectionId, conditionId, partition[j]);

            (IERC20 wrapped1155, bytes memory data) = market.wrappedOutcome(j);

            // wrap to erc20.
            conditionalTokens.safeTransferFrom(address(this), address(wrapped1155Factory), tokenId, amount, data);

            // transfer the ERC20 back to the user.
            wrapped1155.transfer(msg.sender, amount);
        }

https://github.com/seer-pm/demo/blob/6e5db716e44e251fcee6abd7c1f6a8d6e36db910/contracts/src/Router.sol#L71 https://github.com/seer-pm/demo/blob/6e5db716e44e251fcee6abd7c1f6a8d6e36db910/contracts/src/Router.sol#L112

Attachments

  1. Revised Code File (Optional)

Implement Batch Processing: Consider adding functionality that allows users to specify subsets of partitions for processing within a single transaction. By operating on smaller batches, users can avoid the gas limit issue while still being able to perform the desired actions on all the partitions of a condition ID over several transactions

clesaege commented 1 month ago

First, creating markets is way more gas intensive than the other operations. Here is an example with 30 outcomes (creation, splitting). So it's unlikely that we could end up with markets which can be created but can't be handled by the router.

Second, the router is there to simplify operations and do lot of things at once (in particular the transition between ERC20 and ERC1115). People could split/redeem/merge by calling the underlying functions directly.

MatinR1 commented 1 month ago

@clesaege Thanks for your comment. I agree that the router simplifies the operations here, but the core issue exists. Theoretically, as the outcomes and thus loop length increase, the merging and splitting face gas limit issues. Also, considering the docs and paper, the outcomes can be up to 255 and there is no evidence to take the outcomes equal to 30 here.

clesaege commented 1 month ago

But you can't make more outcomes than what you can do with the block gas limit. Since splitting is cheaper than creating the markets (by more than 2X factor), you can't make markets that you can't split.

Also, considering the docs and paper, the outcomes can be up to 255

Where did you see that outcomes could be up to 255?

Theoretically, as the outcomes and thus loop length increase, the merging and splitting face gas limit issues.

As per contest rules, are excluded: