Skip to content

SEP-41: Add a token contract interface proposal#1402

Merged
leighmcculloch merged 18 commits intomasterfrom
token
Oct 12, 2023
Merged

SEP-41: Add a token contract interface proposal#1402
leighmcculloch merged 18 commits intomasterfrom
token

Conversation

@leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Sep 23, 2023

What

Add a Soroban token interface proposal.

Why

See the motivation section for why this specific proposal.

@leighmcculloch leighmcculloch marked this pull request as ready for review September 23, 2023 01:11
@namankumar
Copy link
Contributor

namankumar commented Sep 23, 2023

So my questions are all like poking holes in your amazing proposal. Thanks for taking the first stab which lets us discuss and iterate!

CIPs:

  1. Why the need for a separate entity? I think CIPs fit within the definition of SEPs so why shouldn't they fall under the SEP(Standard) track? Current SEP(Standard) don't seems verbose... they are only long because they contain implementation details. Interfaces/standards by definition shouldn't contain implementation, so SEP(Standard) for Soroban should be short and sweet.
  • Or do we think it'll be confusing for the community?
  • CIPs living on-chain is a good differentiator but ultimately, I believe most devs will import existing wasms before they add definition to the interface

Sections:

  1. Overall, the interface makes a lot of sense to me. Dependencies section I think can be used to point to not just prior interfaces but also wasms.

  2. Think it's going to be difficult to differentiate between Abstract and Summary. They sound very similar to me in the token standard example. Why not just have one?

  3. Agree about Usecases. I also think it's better to have a "Philosophy" / "Approach" section that explains the choice of the functions declared by the interface. I appreciate it goes against the idea of being unopinionated (and consequently, more interoperable) but unless the author conveys their view of the world, IMO it's hard for that world to come to life. We can limit the number of interfaces we define but still convey why we included these but not others.

  4. Can we add:
    5.1 Table of Contents: makes it easier to read.
    5.2 Versioning: iff Changelog doesn't have versions. Versioning/Changelog define a path to iterate the standard after it's been live in the ecosystem.
    5.3 Backward Compatibility: since we live in the world of wasms i.e. executables, it's best to be explicit about backward compatibility. That said, I appreciate that State Expiry may make the section Backward Compatibility unnecessary.
    5.4 Link to a page / discord channel for discussion seems useful though not necessary.

  5. Do we need a copyright section? ERC has it.

The Token Interface:

  1. Events are defined in comments vs. as functions -- comments are easy to miss. That said, I don't have a better suggestion either.
  2. We don't have totalSupply or balanceOf declared -- seem important for fungible tokens? I know most chains skip mint but what's the reason for skipping. How can we have a defi token without mint.
  3. Lastly, I want to emphasize that in the world of wasms, I believe devs will import wasms before they define an interface. So it's very valuable to have the Implementations section, though it gets a bit tricky with state expiry.

@leighmcculloch
Copy link
Member Author

  1. Why the need for a separate entity? I think CIPs fit within the definition of SEPs so why shouldn't they fall under the SEP(Standard) track? Current SEP(Standard) don't seems verbose... they are only long because they contain implementation details. Interfaces/standards by definition shouldn't contain implementation, so SEP(Standard) for Soroban should be short and sweet.
  • Or do we think it'll be confusing for the community?
  • CIPs living on-chain is a good differentiator but ultimately, I believe most devs will import existing wasms before they add definition to the interface

I think the audience of CIPs and SEPs are broadly different. I think folks in the CIP audience will find it easier to engage in a smaller focused place to define and share common contract interfaces.

I think the evolution story of CIPs and SEPs will be different too. I think once CIPs are adopted they'll be hard to change. Probably new features will be a new CIP. SEPs on the other hand do typically evolve, sometimes in significant ways.

I think there's some advantage to disconnecting on-chain vs off-chain interoperability. For example, define a contract interface in a CIP, then define a coordinated off-chain use of that CIP in a SEP.

🤔 But you raise a great point about CIPs living on chain naturally by their original implementer deploying a contract that implements it. For contracts that have an initial implementation that could work well. The token interface is unique in that it doesn't really have an initial implementation in wasm form. Tooling gets its interface from the soroban-sdk.

  1. Think it's going to be difficult to differentiate between Abstract and Summary. They sound very similar to me in the token standard example. Why not just have one?

👍🏻 I'll remove the abstract, it's redundant. Removed in 537cbc4.

  1. Can we add:
    5.1 Table of Contents: makes it easier to read.

It's not very obvious, but every markdown document on GitHub has a table of contents on the right side you can expand.

5.2 Versioning: iff Changelog doesn't have versions. Versioning/Changelog define a path to iterate the standard after it's been live in the ecosystem.

The document has versioning at the top and in the changelog.

5.3 Backward Compatibility: since we live in the world of wasms i.e. executables, it's best to be explicit about backward compatibility. That said, I appreciate that State Expiry may make the section Backward Compatibility unnecessary.

What do you see being included in that section?

5.4 Link to a page / discord channel for discussion seems useful though not necessary.

At the top, the preamble has a discussion link. It's currently TBD, but that's where it'll go.

  1. Do we need a copyright section? ERC has it.

I don't know. CAPs and SEPs do not have it. Can you take that question to the legal team? cc @tomerweller

  1. Events are defined in comments vs. as functions -- comments are easy to miss. That said, I don't have a better suggestion either.

👍🏻 That's a great point. While the contract interface has done a decent job of capturing the format of shared data, it hasn't done a great job of capturing the format of events. In fact, there's no events captured in the contract interface. If the contract interface was only consumed by other contracts that limitation would make sense since contracts don't consume events. But other tooling is code generated off the interface and if the events were included in the interface we could generate the types there too. I've opened an issue to track that:

  1. We don't have totalSupply or balanceOf declared -- seem important for fungible tokens? I know most chains skip mint but what's the reason for skipping. How can we have a defi token without mint. 9. Lastly, I want to emphasize that in the world of wasms, I believe devs will import wasms before they define an interface. So it's very valuable to have the Implementations section, though it gets a bit tricky with state expiry.

balanceOf from ERC-20 is the same as balance in the token interface.

totalSupply would be expensive to implement as it would require every invocation touching balances of a token to be serialized. Soroban will support concurrent execution. A token will be cheaper to execute / more scalable if its not encouraged to serialize its ops.

@namankumar
Copy link
Contributor

I think the audience of CIPs and SEPs are broadly different. I think folks in the CIP audience will find it easier to engage in a smaller focused place to define and share common contract interfaces.

I think the evolution story of CIPs and SEPs will be different too. I think once CIPs are adopted they'll be hard to change. Probably new features will be a new CIP. SEPs on the other hand do typically evolve, sometimes in significant ways.

I think the authors for both are different but the audience/consumers for both are ecosystem developers, no?

We have an opportunity to somewhat mold how CIPs might evolve: ERC223 was a significant improvement on ERC20 but it was never widely adopted due to the network effects of ERC20. If there had been a way to upgrade ERC20, the benefits of ERC223 might have percolated deeper. The notable exception is ERC72, which addresses a completely different use case. All that to say that the evolutionary arc of a SEP might be preferable for CIPs.

I think there's some advantage to disconnecting on-chain vs off-chain interoperability. For example, define a contract interface in a CIP, then define a coordinated off-chain use of that CIP in a SEP.

🤔 But you raise a great point about CIPs living on chain naturally by their original implementer deploying a contract that implements it. For contracts that have an initial implementation that could work well. The token interface is unique in that it doesn't really have an initial implementation in wasm form. Tooling gets its interface from the soroban-sdk.

I think CIPs would be a general way to formalize functionality that the ecosystem has achieved previously by hacking. It could also be functionality that has existed on other ecosystems and should be brought to Soroban. By formalizing, the ecosystem is converging on how the functionality should live on Soroban. In both cases, some implementation came before the actual CIP. If there are examples that contradict the above, just let me know.

5.3 Backward Compatibility: since we live in the world of wasms i.e. executables, it's best to be explicit about backward compatibility. That said, I appreciate that State Expiry may make the section Backward Compatibility unnecessary.

What do you see being included in that section?

Let's revisit after building consensus on the evolutionary story of CIPs.
But I think Github versioning will take care of it, so we might not need to be explicit.

balanceOf from ERC-20 is the same as balance in the token interface.
totalSupply would be expensive to implement as it would require every invocation touching balances of a token to be serialized. Soroban will support concurrent execution. A token will be cheaper to execute / more scalable if its not encouraged to serialize its ops.

Got it. So ERC-20 has some pitfalls that were fixed in later EIPs like 223 and others. We can add those benefits here but we can also add those later in favor of shipping this asap.

@namankumar
Copy link
Contributor

Surfacing enhancements to ERC-20 that we might want to add to the Soroban Token Standard.

  • ERC-223: prevent tokens from being sent to an invalid address.
  • ERC-777: more complete version of ERC20, also allows for events, hooks and custom behaviours. May be an overkill at this stage. Would love to hear more opinions.
  • ERC-747: lets wallets get the token's logo. Given that logo is critical to wallet UX, why not include it in the token interface.

@dmkozh
Copy link
Contributor

dmkozh commented Oct 6, 2023

Surfacing enhancements to ERC-20 that we might want to add to the Soroban Token Standard.

I just want to make it clear that we iterated on the token standard quite a lot and it is developed with Soroban features in mind (and some solutions to ERC-20 problems).

  • ERC-223: prevent tokens from being sent to an invalid address.

Not sure I understand this, how can one tell whether an address is valid or not? From reading the EIP, it doesn't seem to me like it's about invalid addresses, it seems more like one of the workarounds for atomic transfers, which we already support via Soroban auth framework.

  • ERC-777: more complete version of ERC20, also allows for events, hooks and custom behaviours. May be an overkill at this stage. Would love to hear more opinions.

Events are already in the standard. Hooks are not supported in Soroban, at least not in v1. Not sure what's the 'custom behaviors' feature about.

  • ERC-747: lets wallets get the token's logo. Given that logo is critical to wallet UX, why not include it in the token interface.

Given that SAC doesn't support logo, this seems like a possible standard extension. I think we'd like this standard to be compatible with SAC. Additional features like this one can be added as separate standards/extensions. On a side note, I'm not sure what the EIP does for logos.

@leighmcculloch
Copy link
Member Author

I just want to make it clear that we iterated on the token standard quite a lot and it is developed with Soroban features in mind

To add to this, the proposal in this PR is really just capturing existing interface rather than re-validating it. I should probably change it's status to "Final" in the doc.

But also, if we are going to make changes before this is final, now is the time?

@leighmcculloch
Copy link
Member Author

  • ERC-747: lets wallets get the token's logo. Given that logo is critical to wallet UX, why not include it in the token interface.

Given that SAC doesn't support logo, this seems like a possible standard extension. I think we'd like this standard to be compatible with SAC. Additional features like this one can be added as separate standards/extensions. On a side note, I'm not sure what the EIP does for logos.

Stellar supports logo finding today via SEP-1. Maybe that's not how we should do it moving forward, but that's how it works today. Probably no harm in continuing to do it, but we might also need to think about how to incorporate contract tokens into SEP-1.

@dmkozh
Copy link
Contributor

dmkozh commented Oct 6, 2023

Stellar supports logo finding today via SEP-1.

Yes, we do support it in off-chain way, which is why SAC can't support it for now. My point here is not that we shouldn't come up with some standard for the custom tokens, it's rather that we should have the 'base' interface compatible with SAC and then add extensions to it that aren't compatible with SAC (yet).

@namankumar
Copy link
Contributor

Not sure I understand this, how can one tell whether an address is valid or not? From reading the EIP, it doesn't seem to me like it's about invalid addresses, it seems more like one of the workarounds for atomic transfers, which we already support via Soroban auth framework.

A large problem in the EVM ecosystem is assets being sent to the wrong address, such that they are lost irreversibly. I don't want to have this problem on Soroban. ERC-223 attempts to mitigate it. What is our solution to the problem?

My point here is not that we shouldn't come up with some standard for the custom tokens, it's rather that we should have the 'base' interface compatible with SAC and then add extensions to it that aren't compatible with SAC (yet).

The interface (CIP-1) may have started from the SAC but do we want it to strictly adhere to SAC? Why?

The goal is to define an interface that'll be used by future tokens. So, at the very minimum, the interface should specify functionality that'll be useful to majority of the Soroban ecosystem. If majority of the ecosystem needs to add extensions to offer basic functionality (for example, logos), then what is point of the base interface?

@namankumar
Copy link
Contributor

I just want to make it clear that we iterated on the token standard quite a lot and it is developed with Soroban features in mind (and some solutions to ERC-20 problems).

That's great btw. Link me to the notes? Would love to understand the rationale.

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Oct 6, 2023

A large problem in the EVM ecosystem is assets being sent to the wrong address, such that they are lost irreversibly. I don't want to have this problem on Soroban.

Stellar has historically had the reverse problem. In the Stellar payment operations that exist today, recipient accounts must exist, and must have been setup with the asset enabled (it's called a trust line). This has introduced inefficiencies into interop flows that were difficult to overcome, to the point that we added claimable balances so that folks could send to addresses that both don't exist yet and don't have assets enabled yet.

With Soroban we've tried to address this by allowing sends to accounts/addresses that don't exist yet.

@dmkozh
Copy link
Contributor

dmkozh commented Oct 9, 2023

A large problem in the EVM ecosystem is assets being sent to the wrong address, such that they are lost irreversibly. I don't want to have this problem on Soroban. ERC-223 attempts to mitigate it. What is our solution to the problem?

I must admit that I don't understand how does EIP-223 solve this problem, or how this problem can be solved in general. What I can infer from it, is that they're trying to solve a problem of 'transfer, then do something' pattern, which is solved in Soroban in general via call tree auth. But this has nothing to do with 'wrong' addresses - nothing prevents your from sending your token to a valid address B instead of a valid address A, neither in Soroban, nor in ETH. Stellar does have a few more safeguards compared to ETH, e.g. strkeys are check-summed so at least the chance of sending to a completely incorrect address is low.

The interface (CIP-1) may have started from the SAC but do we want it to strictly adhere to SAC? Why?

Because SAC is likely going to be the most prevalent token implementation due to a huge number of benefits (cheap cost, verified implementation, classic interop, classic DEX/LP access etc.). If you develop a new token, it likely needs to be SAC compatible (or vice versa - if we think that some functionality has to belong to tokens, it should be in SAC).

offer basic functionality (for example, logos),

FWIW I don't see the need for logo functionality to be present in the interface (as interface is meant for cross-contract interaction and I doubt contracts ever need to fetch it directly) and I'm also not entirely sold on the idea of storing the logs on-chain in the first place. Off-chain mechanism works fine for classic. Similar mechanism can be developed for custom tokens - but it doesn't have to be a part of the common interface (arguably, the same can be said about the name/symbol methods - these probably are better achieved via metadata and they should be defined in terms of off-chain interaction).

That's great btw. Link me to the notes? Would love to understand the rationale.

Rationale for what? We've had several design discussions and iterations both on token interface and on auth in general. I don't think there is a single place to look though. CAP-46-06 details some design choices, but I don't think it captures everything.

@kalepail
Copy link
Contributor

In general I think this is a great idea however the naming is slightly confusing to me. In Ethereum EIPs are more like CAPs as they are proposals to improve Ethereum itself where ERC is more like what you're proposing here. I think having three different and similar yet not in the way you're thinking, will further confuse the ecosystem. I wonder if SRC (Soroban Request for Comment)

I also wonder if we can just rework SEPs to better support contract interface standards. Personally I think I'd be more keen to revamp the SEP system vs creating something new.

@namankumar
Copy link
Contributor

@leighmcculloch @dmkozh given that at least 65 tokens are already using the SAC and given the info in CAP-46-6, it makes sense to ship the token interface asap based on SAC; and add other capabilities to future proposals or extensions. I'll withdraw my last 3 comments from this issue and save them for later discussion.

@mootz12
Copy link

mootz12 commented Oct 10, 2023

The interface looks good to me. I still have my gripes with balance and spendable_balance (everyone is going to call balance with the expectation it represents the spendable amount).

Some general comments:

Events

I'd highly prefer if we could separate Events into their own section. This is vital for downstream systems to have expectations as to where and when an event will be emitted.

For the token contract this is more obvious, but for more complicated scenarios events will not match up 1 to 1 with functions so the event definitions will be hard to follow.

CIPs

Given I want a separate Events section, I think having a new proposal type is smart. I think the name chosen is a bit confusing tho, given "C" already has a definition as "Core" in CAP.

Going to suggest SIP (Stellar Interface Proposal) or (Soroban Interface Proposal). I also don't mind @tyvdh's SRC.

Lastly, it was fairly awkward writing https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0040.md as a SEP, so I think some direction should be taken as more Interfaces are bound to get proposed.

Ethereum ERC-20 Fixes

I'm mostly in agreement with @dmkozh here. The majority of the proposed fixes to ERC-20 don't really make much sense in Soroban. (Also no-matter what is proposed none if these have gotten any serious traction AFAIK)

ERC-223: XLM (and Stellar Classic) doesn't require or expect a tokensReceived function to be implemented on the receiver. This also means I would be required to load receiving contracts on all token transfers which is going to be a cost passed on to users and on the chain.

https://eips.ethereum.org/EIPS/eip-2612: I could see some value in adding permits. But, I don't know why a contract wouldn't use the auth-next abilities instead for user interactions.

cc: @leighmcculloch

@leighmcculloch
Copy link
Member Author

I'd highly prefer if we could separate Events into their own section.

We have an open issue to make events a first-class Rust type, such that they would be defined like contract types. If that happens, they'd be discussed clearly in their own portion of the interface. I think this would address your concern. The related issue is:

@mootz12
Copy link

mootz12 commented Oct 10, 2023

We have an open issue to make events a first-class Rust type, such that they would be defined like contract types. If that happens, they'd be discussed clearly in their own portion of the interface. I think this would address your concern.

This would be great!

Even without this, I still think an Events sections would be a significant improvement, to decouple event definitions from the interface as they are not necessarily related.

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Oct 10, 2023

@MonsieurNicolas made a point offline to me that we could have contract interfaces be a template within SEPs, and there's no reason we can't have multiple templates for proposals within SEPs.

After thinking about that, I see some benefits in going that route. For proposals that blur the lines between different types, it's easier if everything is a SEP with categories/labels, since a SEP might fit in multiple categories.

I'm inclined to rename CIP-1 to be a SEP-N like SEP-40, and then add an optional categories meta field to SEPs, and add the Category "Contract Interface" to SEP-N and SEP-40.

Thoughts @MonsieurNicolas @tyvdh @mootz12 @namankumar ?

@tomerweller
Copy link
Contributor

I'm inclined to rename CIP-1 to be a SEP-N like SEP-40, and then add an optional categories meta field to SEPs, and add the Category "Contract Interface" to SEP-N and SEP-40.

A category makes sense to me. Alternatively we need to Prefix Soroban SEPS appropriately ("Soroban Token Interface"). Regardless, the title is too generic right now.

Also ERC-20 is called "Token Standard", any thoughts on "Standard" vs. "Interface"?

@leighmcculloch
Copy link
Member Author

Also ERC-20 is called "Token Standard", any thoughts on "Standard" vs. "Interface"?

I think either works. Interfaces become standards when they're successful. SEP-40 referred to itself as the Oracle Consumer Interface, so I've followed suite for SEP-41 at the moment.

Should we prefix the title with Soroban:?

@leighmcculloch
Copy link
Member Author

@tomerweller I added a Soroban prefix to the title.

tomerweller
tomerweller previously approved these changes Oct 10, 2023
Copy link
Contributor

@tomerweller tomerweller left a comment

Choose a reason for hiding this comment

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

LGTM

@leighmcculloch
Copy link
Member Author

I'll keep the document in Draft status until the pubnet launch, then we can shift it to Final then.

namankumar
namankumar previously approved these changes Oct 10, 2023
Copy link
Contributor

@namankumar namankumar left a comment

Choose a reason for hiding this comment

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

🚀

@leighmcculloch leighmcculloch changed the title Add a token contract interface proposal SEP-41: Add a token contract interface proposal Oct 10, 2023
@silence48
Copy link
Contributor

silence48 commented Oct 11, 2023

This is looking great guys.

Even without this, I still think an Events sections would be a significant improvement, to decouple event definitions from the interface as they are not necessarily related.

I agree it would keep it less cluttered.

I just made a first draft of a NFT collection interface to be similar to ERC-1155. Based on a not yet finished implementation. #1406 Will love your feedback there.

I haven't defined the events here yet but it does seem like it would make sense to have them since knowing what events are emitted can be helpful for dapp and contract developers alike.

/// # Arguments
///
/// - `id` - The address for which a spendable balance is being queried.
fn spendable_balance(env: Env, id: Address) -> i128;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this interface need a spendable_balance (I think this was discussed recently but don't remember what came out of that).

Copy link
Member Author

Choose a reason for hiding this comment

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

The discussion and resolution is here:

The resolution was we could go both ways so leave it. But, I've lost count how many times this has come up, and every time the opinion is "it doesn't belong."

I think we could remove it with little impact.

cc @tomerweller

leighmcculloch and others added 2 commits October 11, 2023 15:37
Co-authored-by: Siddharth Suresh <siddharth@stellar.org>
Co-authored-by: Siddharth Suresh <siddharth@stellar.org>
@leighmcculloch leighmcculloch enabled auto-merge (squash) October 12, 2023 00:05
@leighmcculloch leighmcculloch merged commit 03a1aed into master Oct 12, 2023
@leighmcculloch leighmcculloch deleted the token branch October 12, 2023 00:11
Copy link
Contributor

@kalepail kalepail left a comment

Choose a reason for hiding this comment

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

This is great! Excited to see more of these. I do wonder if this should have a different name than the "Soroban Token Interface" though. There will likely be many "Soroban Token Interfaces" so specifying this as a fungible or standard or basic or base or some such nomenclature allows for more diversity later on.

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Oct 12, 2023

I do wonder if this should have a different name than the "Soroban Token Interface" though. There will likely be many "Soroban Token Interfaces" so specifying this as a fungible or standard or basic or base or some such nomenclature allows for more diversity later on.

If there becomes many token interfaces I don't think naming it the "Fungible Token Interface" will do much to distinguish because of many there's likely many fungible tokens. A highly generic name is more likely to set it apart because other tokens are likely to have specializations or target specific use cases they can be named after.

Also, any ambiguity can be addressed by calling it SEP-14?

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.

8 participants