tokamak-network / tokamak-bridge

An interface designed to bridge assets between Titan and Ethereum chains.
https://app.bridge.tokamak.network
6 stars 0 forks source link

[Cross Trade - Suggestion]: Use storage for registered token pairs for gas saving #204

Closed blackcow1987 closed 4 weeks ago

blackcow1987 commented 1 month ago

What is your recommendation for?

No response

Explain your suggestion

When registerToken is performed in the L2 contract, you can reduce the size of the calldata added to the rollup by issuing an ID for the l1/l2 token pair and changing the contract to use that ID.

    function registerToken(
        address _l1token,
        address _l2token,
        uint256 _l1chainId
    )
        external
        onlyOwner
    {
        bytes32 id = keccak256(abi.encode(_l1chainId, _l1token, _l2token));
        require(registeredPair[id].l1Token == address(0), "already registerToken");
        registeredPair[id] = RequestedPair({
            l1token: _l1token,
            l2token: _l2token,
            chainId: _endChainId,
        });
    }

    ...

    function requestRegisteredToken(
        bytes32 _id,
        uint256 _totalAmount,
        uint256 _ctAmount,
    )
        external
        payable
        onlyEOA
        nonZero(_totalAmount)
        nonZero(_ctAmount)
        nonReentrant
    {
        require(registeredToken[id].l1token != address(0), "not register token");
monica-tokamak commented 1 month ago

@zzooppii

zzooppii commented 1 month ago

This is a good way to reduce the size of calldata to the function.

However, I know that the gas cost is higher for loading values ​​from storage than for using input values.

So I think we need to discuss whether to apply this part.

Thank you for your good opinion.

blackcow1987 commented 1 month ago

storage cost is expensive then input data, but in L2 these data is stored in L1 through rollup. So I think it is important to minimize overall calldata.

zzooppii commented 1 month ago

We will discuss the relevant part.

Thank you for the good feedback!

zzooppii commented 4 weeks ago

@blackcow1987 We will not change it right now, but we will consider supporting that part in the future.

Thank you.