Conversation
PR SummaryMedium Risk Overview Introduces the Written by Cursor Bugbot for commit e644014. This will update automatically on new commits. Configure here. |
There was a problem hiding this comment.
can we rename file TIP20OFT to be consistent?
Larger ask, can we reframe this as a TIP20MintBurnOFTAltAdapter? Naming is gross, but consistent.
There was a problem hiding this comment.
Larger ask, can we reframe this as a TIP20MintBurnOFTAltAdapter? Naming is gross, but consistent.
I don't see the need for MintBurn in the name if the mint burn is for the underlying Tip20, and it is not following the patter in MintBurnOFTAdapter when a _minterBurner is defined.
I don't think it should be an adapter either. Adapters are used for existent ERC20s that can't mint, so they lock tokens (like a pool). This is not the case, it is just an oft that instead of having the ERC20 embedded has an underlying tip20 token.
Why isMintBurnOFTAdapter is called adapter? I think this is misleading; adapters should be unique per mesh, because they control all mesh liquidity.
Checking the docs I see it says
Unlike the standard OFTAdapter which locks/unlocks tokens, this adapter burns tokens on the source chain and mints them on the destination chain. (1)
But I see it more like and OFT that, unlike the standard OFT, which has the ERC20 token embedded, it has an underlying ERC20 where the OFT is the minter._
All StargateOFTs are like that since we use Circle implementation for USDC and Tether implementation for USDT.
I can rename it as you suggested, to be aligned with the documentation, but I find it misleading.
There was a problem hiding this comment.
Adapter actually is just naming convention for when the token itself is not the OFT contract. It doesn't specify the debit or credit mechanism, and this was an oversight of the original adapter code (also why we're renaming to explicitly denote lock/unlock, burn/mint in the future).
There was a problem hiding this comment.
My point is that, from a Stargate perspective, adapters are the pool-like contracts; the ones that hold the mesh liquidity. OFTs do not hold shared liquidity; they simply manage the token itself, whether embedded or underlying.
This was also my understanding when I went through the LayerZero documentation some time ago. It made sense to me, since only a single adapter should be allowed in a mesh. Otherwise, liquidity would be fragmented across multiple chains, which could result in tokens not flowing correctly.
Under this interpretation, an OFT mesh would be of a set of OFT contracts deployed across chains, with at most one Adapter responsible for managing the shared liquidity.
Adding Mint/Burn to contract name makes sense, but ultimately all OFTs call OFT::token::mint/burn. The only difference is that, in some cases, the token being minted or burned belongs to another contract address rather than the OFT itself.
The real distinction, in my view, is between lock/unlock and mint/burn mechanisms. Which, from my pov was what differentiated Adapters from OFTs. This is a significant difference that needs to be clearly defined and carefully considered, due to the liquidity fragmentation risks mentioned earlier. Defining adapters as OFTs that use lock/unlock seems potentially dangerous for me
There was a problem hiding this comment.
Adapters adapt an existing token to the OFT interface. Whether it locks/unlocks or mints/burns doesn't matter for naming. We already ship MintBurnOFTAdapter in oft-evm/contracts/ which burns on debit and mints on credit. No pool, no locking.
For now let's just focus on consistency with what's already published in the package. We can sort out the naming with the console team when we work on the new OFT package.
There was a problem hiding this comment.
The risk is in double spend of escrowed funds so OFTs that "lock" tokens in escrow are the risk and the main thing to communicate to customers.
St0rmBr3w
left a comment
There was a problem hiding this comment.
Can we add a changeset via pnpm changeset?
|
changeset added e644014 |
No description provided.