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

[AIP-5] Soulbound and Freezing Token Standard #16

Closed
wants to merge 6 commits into from

Conversation

JacobADevore
Copy link

No description provided.

@sherry-x sherry-x requested review from sherry-x and davidiw December 8, 2022 23:02
aips/aip-4.md Outdated

Modifications required to the current [Aptos-Token](https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/framework/aptos-token/sources/token.move) implementation:

- Append “soulbound: bool” too *Tokens* and *Collections.*
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the usage of soulbound collections?

Copy link
Author

Choose a reason for hiding this comment

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

After further thought, I do not believe Collections requires the soulbound property. Good call.

Choose a reason for hiding this comment

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

what is the usage of soulbound collections?

  1. Gift cards
  2. Birthday Wishes
  3. Lifetime community memberships

These are a few that i can think at this moment but there're more.

aips/aip-4.md Outdated
- Validate before any transfer execution to ensure the token isn’t soulbound.

```rust
/// Executed when a specific token_id is set or updated to soulbound, according to the parameter "bounded".
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 mean a token can mutate between a soulbound and non-soulbound token? In my mind, soulbound should not be transferrable after minting to its owner

Copy link
Author

Choose a reason for hiding this comment

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

No, the soulbound property should never be mutatable. The validation checks the existing transfer functions to ensure that the current token is not soulbound. This validation is needed as soulbound and non-soulbound tokens can coexist.

aips/aip-4.md Outdated
@@ -0,0 +1,78 @@
# AIP-4: Multi-Composable Token Standard
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for submitting the PR. please also create a discussion issue like this one #15. It is easier to keep the discussion in an issue

Copy link
Author

Choose a reason for hiding this comment

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

Sure thing - #18

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @JacobADevore could you please add the header like other AIPs? Let's make this one AIP-5. once you updated, we can land this draft and iterate on it

Copy link
Author

Choose a reason for hiding this comment

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

Hey @JacobADevore could you please add the header like other AIPs? Let's make this one AIP-5. once you updated, we can land this draft and iterate on it

Sure thing! Does that suffice?

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to update the file name too

Copy link
Author

Choose a reason for hiding this comment

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

Got it! Sorry about that.

Copy link
Contributor

@davidiw davidiw left a comment

Choose a reason for hiding this comment

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

Really exciting to have our first proposal from the community. I look forward to working with you more on this topic :)

aips/aip-4.md Outdated

Modifications required to the current [Aptos-Token](https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/framework/aptos-token/sources/token.move) implementation:

- Append “soulbound: bool” too *Tokens*
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

will look into it.

aips/aip-4.md Outdated
Modifications required to the current [Aptos-Token](https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/framework/aptos-token/sources/token.move) implementation:

- Append “soulbound: bool” too *Tokens*
- Minting of soulbound tokens can only be handled through *“claims”*. After a soulbound NFT has been claimed it is locked to the receiver with the only valid operation being *“burn”,* if applicable to the token’s configuration*.*
Copy link
Contributor

Choose a reason for hiding this comment

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

the biggest issue I see is that soulbound would increase operating costs for everyone, but I'm not sure by how much. Every withdraw operation would go up by the cost to check to see if it is set.

Copy link
Author

Choose a reason for hiding this comment

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

agreed. looking into.

aips/aip-4.md Outdated

- Append “soulbound: bool” too *Tokens*
- Minting of soulbound tokens can only be handled through *“claims”*. After a soulbound NFT has been claimed it is locked to the receiver with the only valid operation being *“burn”,* if applicable to the token’s configuration*.*
- Validate before any transfer execution to ensure the token isn’t soulbound.
Copy link
Contributor

Choose a reason for hiding this comment

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

simply verify the withdraw

Copy link
Author

Choose a reason for hiding this comment

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

Really exciting to have our first proposal from the community. I look forward to working with you more on this topic :)

Thank you @davidiw for all the amazing feedback here!!

aips/aip-4.md Outdated

### Suggested I**mplementation Timeline**

To be determined.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is one of those things where you should try a couple implementations and come back with the pros and cons. It isn't clear to me that there is a perfect solution here.

Copy link
Author

Choose a reason for hiding this comment

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

Will do, working through it with @areshand :)

@JacobADevore JacobADevore changed the title AIP-4: Multi-Composable Token Standard [AIP-5] Multi-Composable Token Standard Dec 20, 2022
@JacobADevore JacobADevore changed the title [AIP-5] Multi-Composable Token Standard [AIP-5] Soulbound and Freezing Token Standard Dec 22, 2022
@ghost
Copy link

ghost commented Jan 9, 2023

0x3::token is really gas heavy enough, why we should keep change that?
I recommend an another approach: clone 0x3::token standard to be 0x3::soulbound_token, so you don't need do a verify for withdraw/deposit.

@alnoki
Copy link
Contributor

alnoki commented Jan 22, 2023

Comment moved to #18

@davidiw
Copy link
Contributor

davidiw commented Apr 16, 2023

@ghost
Copy link

ghost commented Apr 16, 2023

Why should be so hard? just do move_to(&signer, YourTokenName {}) then make it able to query from another contract, next to frozen the contract then, the SBT is made out.

Of course, by this method, the user need to interact the contract, but this contract has been frozen and open source, 100% safe,

@davidiw
Copy link
Contributor

davidiw commented Apr 17, 2023

Why should be so hard? just do move_to(&signer, YourTokenName {}) then make it able to query from another contract, next to frozen the contract then, the SBT is made out.

Of course, by this method, the user need to interact the contract, but this contract has been frozen and open source, 100% safe,

@e99243506bigplay, I agree, the goal of this AIP was to make this inline and a part of the existing core code, which would have ramifications to every token. If someone wants to push a TokenV1 AIP that supports soul bound as you proposed, I think that would be acceptable from a framework perspective -- the question is would it be good for @JacobADevore and would other ecosystem partners adopt.

@JacobADevore
Copy link
Author

Why should be so hard? just do move_to(&signer, YourTokenName {}) then make it able to query from another contract, next to frozen the contract then, the SBT is made out.
Of course, by this method, the user need to interact the contract, but this contract has been frozen and open source, 100% safe,

@e99243506bigplay, I agree, the goal of this AIP was to make this inline and a part of the existing core code, which would have ramifications to every token. If someone wants to push a TokenV1 AIP that supports soul bound as you proposed, I think that would be acceptable from a framework perspective -- the question is would it be good for @JacobADevore and would other ecosystem partners adopt.

Hey @davidiw, I think it would be great if you can extend or fork your https://github.com/aptos-labs/aptos-core/tree/main/aptos-move/move-examples/token_objects project and show an example of how soul bound NFTs would come to life with the new object pattern.

@davidiw
Copy link
Contributor

davidiw commented Jul 14, 2023

Why should be so hard? just do move_to(&signer, YourTokenName {}) then make it able to query from another contract, next to frozen the contract then, the SBT is made out.
Of course, by this method, the user need to interact the contract, but this contract has been frozen and open source, 100% safe,

@e99243506bigplay, I agree, the goal of this AIP was to make this inline and a part of the existing core code, which would have ramifications to every token. If someone wants to push a TokenV1 AIP that supports soul bound as you proposed, I think that would be acceptable from a framework perspective -- the question is would it be good for @JacobADevore and would other ecosystem partners adopt.

Hey @davidiw, I think it would be great if you can extend or fork your https://github.com/aptos-labs/aptos-core/tree/main/aptos-move/move-examples/token_objects project and show an example of how soul bound NFTs would come to life with the new object pattern.

Take a look here:
https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/framework/aptos-token-objects/sources/aptos_token.move#L189

Any objections to closing this and focusing on TokenV2? There are already many wallets that support it: Petra, Fewcha, Rise, and more coming soon :)

@thepomeranian
Copy link
Collaborator

Closing this for now due to inactivity and no actionable items

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.

7 participants