Skip to content

Add oft tip 20 implementation and tests#1933

Open
clauBv23 wants to merge 4 commits intomainfrom
oft-tip-20
Open

Add oft tip 20 implementation and tests#1933
clauBv23 wants to merge 4 commits intomainfrom
oft-tip-20

Conversation

@clauBv23
Copy link
Contributor

No description provided.

@cursor
Copy link

cursor bot commented Feb 18, 2026

PR Summary

Medium Risk
Adds new token debit/credit semantics (transfer-then-burn and mint-on-credit) that directly affect token supply and custody; also appears to have a potentially broken test import path (OFTTIP20.sol vs TIP20OFT.sol) that could mask issues if not corrected.

Overview
Adds a new TIP20OFT variant that supports bridging TIP20-style tokens by transferring tokens into the OFT then burning via ITIP20Minter.burn on send, and minting via ITIP20Minter.mint on receive (with zero-address credits redirected to 0xdead).

Introduces the ITIP20Minter interface plus Foundry tests and mocks validating approval-required debits, slippage reverts, credits/minting behavior, and an end-to-end send flow; includes a changeset bumping @layerzerolabs/oft-alt-evm as a patch.

Written by Cursor Bugbot for commit e644014. This will update automatically on new commits. Configure here.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 18, 2026

🧪 E2E Test Status

E2E tests are non-blocking and validate real blockchain interactions. Failures may occur due to network issues, RPC rate limits, or external service downtime.

Test Runs (Newest First):

  • Run #6720 - Failed - 2026-02-18 21:06 (UTC)
  • Run #6717 - Failed - 2026-02-18 17:17 (UTC)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename file TIP20OFT to be consistent?

Larger ask, can we reframe this as a TIP20MintBurnOFTAltAdapter? Naming is gross, but consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@St0rmBr3w St0rmBr3w Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@St0rmBr3w St0rmBr3w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a changeset via pnpm changeset?

@clauBv23
Copy link
Contributor Author

changeset added e644014

@clauBv23 clauBv23 requested a review from St0rmBr3w February 18, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments