tokamak-network/crossTrade

J_10 [protocol discussion]

usgeeus opened this issue · 7 comments

usgeeus commented

Configuration

  • severity : low
  • confidence : low

Description

From what I heard in the seminar, we only support a few tokens, including ETH,
In L2FastWithrdraw.sol:requestFW, allowing _l1token to be passed as a parameter seems to make it more complicated to analyze the vulnerability, both on the front end and in terms of FWing _l1token with an unrelated _l2token.

Recommendation

Why not create a _l2token ↔ _l1token mapping and manage the mapping with the onlyOwner function????

zzooppii commented

It was initially applied like this, but the format was changed because there was still a responsibility for someone to manage l1token <-> l2token.

  • 👍1
suahnkim commented

I think we can have a discussion around this point.

we discussed a permissioned way (onlyOwner) that registers, but we did not want to restrict which tokens can be FastWithdrawn.

we also discussed using l1token storage (token address on L1) as a parameter to automatically set the L1 token address on L2, but i think there were some concerns using that.

Can we have a discussion about this tomorrow? (May 24th)

  • 👍2
zzooppii commented

If you make call tomorrow, i can discuss.

zzooppii commented

Basically, some tokens are registered using mapping, and requests for tokens that do not have mapping registered are also allowed, but we decided to add a warning message.

zzooppii commented

Changing this will result in a lot of structural changes, so we will revise it as development progresses next week.

usgeeus commented

Solved