Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement helpers for SNIP-21, SNIP-22, and SNIP-23 interfaces #20

Open
reuvenpo opened this issue May 26, 2021 · 8 comments
Open

Implement helpers for SNIP-21, SNIP-22, and SNIP-23 interfaces #20

reuvenpo opened this issue May 26, 2021 · 8 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@reuvenpo
Copy link
Contributor

reuvenpo commented May 26, 2021

Extend the methods available under secret_toolkit::snip20 to also include the new interface specified in:
SNIP-21, SNIP-22, and SNIP-23.

Alternatively, implement new modules called snip21, snip22, and snip23 which include versions of the modified interfaces that are aware of the new features, as well as support for the new message types.

NOTE: at the time of writing SNIP-23 is not finalized yet so it's a lower priority.

UPDATE 2022-01-19:

We ended up including all new SNIP-2x functionality in the snip20 module. As far as I'm concerned we can still reshuffle this as discussed in this thread and release a new version, but for now let's experiment with it and see what feedback we get.

@reuvenpo reuvenpo added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels May 26, 2021
@floAr
Copy link

floAr commented May 27, 2021

A quick question to this: Would the new modules only include the additions and changes to snip20 or would you rather have it to mirror everything?

Concrete example: If I call Transfer on a snip21 do I import it from ::snip20 or ::snip21 ?

@reuvenpo
Copy link
Contributor Author

Good question, that's the discussion that i'd like to have here.

We could perhaps provide an alternative implementations to some things under snip21, e.g. a Transfer implementation with the memo, and then pub use (reexport) the interfaces that didn't change. That way we don't have to implement things twice, but users can still access everything they need from one module.

The question though then becomes: does snip22 include snip21? Since snip22 is completely orthogonal to snip20 and snip21 we could have it export only the new messages that it defines, but then there's a certain asymmetry between them. Is that ok? I think it's a fine decision to make for now.

(I'm gonna explain that in the relevant PRs in a few minutes but) We're going to shelve snip23 (allowance improvements) for now, but if we were to include it, how would we represent it in the toolkit? just like snip21, it adds new messages AND extends existing ones. What should/would the snip23 module export?

@floAr
Copy link

floAr commented May 27, 2021

I would personally prefer asymmetry in the packages. So the functionally resides in the package it is defined in and I compose it in my contracts. That will mean that I potentially have multiple crates to import but it also makes it very clear which functionality I am using when writing the contract.
For something like the snip23 it would make sense to reexport functions and then I as a developer can choose which one I want to import and use. Here I am not sure how well intellisense works for Rust but in C# it would give me the tooltip like 'You are missing function Transfer, do you want to import it from ::snip20, ::snip22 or snip:33

@reuvenpo
Copy link
Contributor Author

Yeah, at least in CLion if you use a symbol you didn't import, it offers several options, and pastes in the appropriate import line next to your other imports.

@reuvenpo
Copy link
Contributor Author

reuvenpo commented May 27, 2021

So just to make sure, in your opinion should snip21 export all the symbols from snip20, or not?

@floAr
Copy link

floAr commented May 27, 2021

I think it should not export unchanged symbols. This is based on my following understanding (and please correct me if I am wrong here): Snip2X implementations are composable functionality- So my token could be based on snip20 and implement snip22 and 21 as well. Or potentially only 22 and so forth.
If we re-export snip20 from snip22 I would expect to have similar structure for snip21 and a similar one for snip21+ snip22 so the number of permutations rises exponentially with future improvements.
Instead I would suggest that I have a base functionality in snip20 and I can choose to pick a modern replacement from another crate to use it instead. This does still not solve the problem how I would solve the problem if two snips improve on the same symbol, but for the current moment this seems to be the clearest way for me.

@reuvenpo
Copy link
Contributor Author

Yup, your understanding is correct and makes sense to me.
I believe that if we make another SNIP that touches the same parts of the interface that a previous SNIP has changed, we'll either say that it requires the previous SNIP, or the fields it adds will be optional so if a contract isn't returning a field you need, you'll just get a None somewhere and will have to deal with it.

@reuvenpo
Copy link
Contributor Author

We ended up including all new SNIP-2x functionality in the snip20 module. As far as I'm concerned we can still reshuffle this as discussed in this thread and release a new version, but for now let's experiment with it and see what feedback we get.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants