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

NFT as an account (NFTAA) #2396

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

NFT as an account (NFTAA) #2396

wants to merge 5 commits into from

Conversation

Roman-24
Copy link

@Roman-24 Roman-24 commented Sep 15, 2024

Project Abstract

NFT as an Account (NFTAA) is an innovative project aimed at revolutionizing the use of NFTs within the blockchain ecosystem, particularly in the Substrate based networks like Polkadot and Kusama. The project seeks to leverage the unique properties of NFTs to function as proxy accounts, enhancing interoperability, security, and flexibility across various blockchain networks. By enabling NFTs to hold assets, stake, and interact with other blockchain components similarly to standard accounts, NFTAA introduces a novel approach to blockchain interactions.

Grant level

  • Level 1: Up to $10,000, 2 approvals
  • Level 2: Up to $30,000, 3 approvals
  • Level 3: Unlimited, 5 approvals (for >$100k: Web3 Foundation Council approval)

Application Checklist

  • The application template has been copied and aptly renamed (project_name.md).
  • I have read the application guidelines.
  • Payment details have been provided (Polkadot AssetHub (DOT, USDC & USDT) address in the application and bank details via email, if applicable).
  • I understand that an agreed upon percentage of each milestone will be paid in vested DOT, to the Polkadot address listed in the application.
  • I am aware that, in order to receive a grant, I (and the entity I represent) have to successfully complete a KYC/KYB check.
  • The software delivered for this grant will be released under an open-source license specified in the application.
  • The initial PR contains only one commit (squash and force-push if needed).
  • The grant will only be announced once the first milestone has been accepted (see the announcement guidelines).
  • I prefer the discussion of this application to take place in a private Element/Matrix channel. My username is: @_______:matrix.org (change the homeserver if you use a different one)

@github-actions github-actions bot added the admin-review This application requires a review from an admin. label Sep 15, 2024
Copy link
Contributor

github-actions bot commented Sep 15, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@Roman-24
Copy link
Author

I have read and hereby sign the Contributor License Agreement.

fix table, delete eth addr
Copy link
Collaborator

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

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

Thanks for the application. Just one quick feedback here: The default deliverables 0a-0d of the template are mandatory for all milestones and deliverable 0e at least for the last one.

Add 0 requirements to milestones 2 and 3
@Roman-24
Copy link
Author

@Noc2 thank you for your feedback. Our application has been modified and default deliverables 0a-0d have been added to milestones 2 and 3. The point about the article in milestone 3 has been modified from 3a to 0e to make it consistent with the templates. We believe that our application now complies with all the necessary formalities.

@Roman-24 Roman-24 requested a review from Noc2 September 16, 2024 12:30
@semuelle semuelle self-assigned this Sep 23, 2024
@github-actions github-actions bot added the stale label Oct 8, 2024
@semuelle semuelle removed the stale label Oct 8, 2024
Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

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

Thank you for the application and the long wait, @Roman-24.

This looks like an interesting idea. However, I'm a bit wary of another NFT pallet. We have supported several NFT implementations, each with their own twist, but few have actually been integrated into parachains or seen much adoption. Can you elaborate what your intention is here? From the looks of it, this is more of an "applied research" exercise than anything.

@Roman-24
Copy link
Author

Roman-24 commented Oct 9, 2024

@semuelle Thank you for your feedback.

Our goal is to expand the boundaries of how web3 uses NFTs today. It's an extension of how current NFTs are used, adding a whole new set of capabilities. Another feature addresses the problem of current account illiquidity. If you want to change the owner of an account (or, for example, a locked staking), there is no secure way to do it (except by sharing private keys (unsecure thing)). In our proposal, we are primarily focusing on the use case within staking, but there are several other use cases, which we have addressed in the "Business Overview" subsection. The implementation of this idea makes sense and is supported by the fact that there is a similar proposal for Ethereum, which we have discussed in the "Differences and Distinctions (NFTAA vs. EIP-6551)" paragraph.

As mentioned in the proposal, we have already conducted research on this topic, along with some prototypes. Therefore, we now want to move towards a real implementation that will provide value to real users.
We would also like to point to the ongoing cross-chain NFT development, which in the future, combined with NFTAA, could enable sendable accounts across the entire Polkadot Paraverse.

semuelle
semuelle previously approved these changes Oct 10, 2024
Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

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

we now want to move towards a real implementation that will provide value to real users.

Thanks for the clarification, @Roman-24. My question was more geared towards understanding whether you are planning to deploy this yourself or if you have parachain teams interested in it. But either way, it looks like an interesting project to me, so I'm giving my approval and will share it with the rest of the committee.

In the meantime, could you fill out the KYB form (assuming you are applying as a company, otherwise please use this form)?

@semuelle semuelle added ready for review The project is ready to be reviewed by the committee members. admin-review This application requires a review from an admin. and removed admin-review This application requires a review from an admin. labels Oct 10, 2024
@Roman-24
Copy link
Author

We have completed the KYB and therefore verification is in process. We are glad you like our idea and thank you for your support @semuelle. We will definitely communicate with the parachains, we believe we have a good network for that.

Copy link
Member

@PieWol PieWol left a comment

Choose a reason for hiding this comment

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

Thanks for your interest in the Grants Program! :)
I left some comments and am looking forward to your statements and answers.

applications/nftaa.md Show resolved Hide resolved
applications/nftaa.md Show resolved Hide resolved
applications/nftaa.md Show resolved Hide resolved
Comment on lines +77 to +78
The second approach involves using smart contracts. For this proposal, we prefer the pallet approach due to its
integration capabilities and performance benefits.
Copy link
Member

Choose a reason for hiding this comment

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

According to my understanding you are forced to set up this solution on a pallet level to integrate this solution with other pallets. This wouldn't work this smart contracts. Especially not now while Kusama and Polkadot don't support smart contracts yet. So sounds like the right decision.

Copy link
Author

Choose a reason for hiding this comment

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

We are glad that you agree with our decision of tech details.
According to our information there are some possibilities to do smart contracts, but we think it's only possible on some parachains (https://wiki.polkadot.network/docs/build-smart-contracts).
Anyway, if there will be NFTAA pallet in the future it will be possible to add chain extensions and bring this functionality to smart contract layer.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see anything about chain-extensions mentioned in the doc and I think this is a deprecated feature. Would be great if you can check on this. Also I think if you were to realize this on the smart-contract level then you can only have this logic be applied on the smart contract level. You wouldn't be able to influence the chain level. Please correct me if I'm wrong.

Copy link
Author

Choose a reason for hiding this comment

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

It looks like you are correct, anyway what is stated in the proposition remains valid because to integrate this solution to other pallets, our preferred way is through the pallet approach. But definitely, we will need to freshen up our state of the art in our minds based on our chain extension not knowledge :))))

Comment on lines +86 to +87
Attributes are crucial for enterprise use cases, allowing NFTAAs to be marked as entities such as companies or departments
within a corporate structure.
Copy link
Member

Choose a reason for hiding this comment

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

Why are attributes crucial?

Copy link
Author

Choose a reason for hiding this comment

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

From our point of view, this is an opportunity to differentiate between already existing unique NFTs inside one company. To better manage and govern the orgs represented by NFTAAs or to add additional information to the NFT that is needed to implement the usecase we are writing about.

Copy link
Member

Choose a reason for hiding this comment

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

We already have the identity pallet which is able to represent additional information linked to an account. Given that this is the established way of linking information to an account I suggest to re-evaluate this strategy. This NFT attribute approach would introduce unnecessary overhead to existing projects that aggregate data linked to accounts. e.g. Polkassembly.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for this comment and I'm sure it can be used since NFTAA will be like an account so it shouldn't be a problem to join. However, in our case we were thinking more of the technical attributes but thus the detail will only be clear when implemented. We just wanted to have the "corridor" more open so we don't promise something which could be potentially improved.

Regarding security, there are at least two differences between NFTAA and TBA. The first is the possibility of generating the address of TBA sooner without having the deployed contract (as the create2 function for creating contracts is deterministic, i.e., you can calculate the future contract address).
In this case, if the upcoming smart contract is badly developed (does not contain an execute function for byte calls to other contracts) or is not upgradeable, it can happen that assets or liquidity sent to the TBA will not be withdrawable and become locked forever (even the NFT bounded in TBA can be sent by a mistake (if not safeguarded) to the address of TBA. Therefore you lock yourself from it).
The problem is that TBA uses only the address of any contract you want in the registry contract and does not look for any details on how the contract looks or if it is already deployed. In our case, the account form is pre-defined in the factory pallet, and even after deployment, we guarantee upgradeability as it is a proxy account.
The second security issue is potential fraud, which is [described in the EIP 6551 specification](https://eips.ethereum.org/EIPS/eip-6551\#fraud-prevention) that you withdraw assets from the TBA and at the same time sell the NFT to someone who thought that they would also have the assets in the TBA. In our case, we provide the security measures for not withdrawing the assets from the NFTAA while also selling the NFT, which is bound to the NFTAA.
Copy link
Member

Choose a reason for hiding this comment

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

Will these security measures be included in your currently developed pallet? I don't see it in the milestones.

Copy link
Author

Choose a reason for hiding this comment

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

We will try to address this problem within existing features, but at this point we cannot exclude the possibility that this mechanism may need to be improved later.

Copy link
Member

Choose a reason for hiding this comment

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

How will this be addressed?

Copy link
Author

Choose a reason for hiding this comment

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

for example, one possibility is: If NFTAA is listed you will not be able to sell assets owned by NFTAA

applications/nftaa.md Show resolved Hide resolved
Comment on lines +357 to +366
| Number | Deliverable | Specification |
|-------:|-------------------------|------------------------------------------------------------------------------------------------------------------------------------|
| **0a.** | License | MIT |
| **0b.** | Documentation | Inline documentation of code, as well as startup configuration with all necessary commands, included in repository |
| **0c.** | Testing and Testing Guide | Core functions will be fully covered by comprehensive unit tests to ensure functionality and robustness. In the guide, we will describe how to run these tests. |
| **0d.** | Docker | We will provide a Dockerfile(s) that can be used to test all the functionality delivered with this milestone. |
| 2a. | nftaa_check | We will add functionalities to read and check if NFTAA exists for a given account |
| 2b. | nftaa_collections | We will add functionalities to read existing collections and manage NFTAAs if they are in a collection. |
| 2c. | nftaa_stake/unstake | The functionality needed to do stake/unstake operations on behalf of NFTAA, its like deposit/withdrawal functions |
| 2d. | nftaa_increase/decrease | Functions needed to increase or decrease stake by NFTAA, this is needed for full compatibility with traditional methods of staking |
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused by this milestone. Previously you described your project as a solution to use the NFTAAs as virtual keyless accounts which are able to do anything a regular account can. Yet this milestone sounds like you are putting in a lot of work to make your solution compatible with the existing pallets for staking. If this is indeed necessary I think you should try to find a more generic solution which doesn't create this overhead for every pallet you want to support with NFTAAs.

Copy link
Author

Choose a reason for hiding this comment

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

We don't want to do this for every pallet and we certainly want our solution to be generic, but the case of staking is special and therefore we want to have these functionalities explicitly ready. But it still applies NFTAAs are virtual keyless accounts which are able to do anything a regular account can.

Copy link
Member

Choose a reason for hiding this comment

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

What makes staking special?

Copy link
Author

Choose a reason for hiding this comment

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

We believe staking is an ideal use case to showcase how NFTAA operates because it's a concept that many users are already familiar with, making it easier to grasp the potential of NFTAA. Additionally, staking plays a crucial role in maintaining network security, making it a fitting example to demonstrate the robustness and functionality of NFTAA in a practical environment.

Copy link
Member

Choose a reason for hiding this comment

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

I know what staking ist. I just don't understand your answer. If you are planning to make your solution generic as in "this NFTAA can be used just like a regular account", then why do you need to put in any work to make your solution compatible with the staking pallet. I hope you understand where my issue is here. Staking is already supported for regular accounts. With you adding just another layer on top of accounts with this form of account abstraction I don't understand why you would need to modify anything below this account level. E.g. How individual pallets work with accounts.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe we don't fully understand your concerns. In our eyes, as mentioned, staking is a use case to show NFTAA in some scenario. The NFTAA will be generic, but without showcasing its potential, noone will use it, or know about it. We invest focus into development to help users as much as we can to use NFTAA in any kind of use case they think is a good fit. And as staking is a well-known word to everyone, therefore we decided to put as a showcase in the proposal staking. That was the only reason, the proposal is meant for "non-fungible tokens as accounts" not "non-fungible tokens as accounts for staking".

We prefer to have staking explicitly mentioned in the proposal, but do you prefer to remove these explicitly mentioned functionalities?

applications/nftaa.md Outdated Show resolved Hide resolved
applications/nftaa.md Outdated Show resolved Hide resolved
@Roman-24
Copy link
Author

@PieWol thank you so much for your effort to review our proposal.

We've tried to answer your questions as best we can. If there is still something that is not clear we will be happy to continue the discussion.

@keeganquigley keeganquigley removed the admin-review This application requires a review from an admin. label Oct 17, 2024
@github-actions github-actions bot added the admin-review This application requires a review from an admin. label Oct 21, 2024
@Roman-24
Copy link
Author

@PieWol We have modified the milestone changes to say that we will integrate to existing applications. You can see the changes here

@keeganquigley keeganquigley removed the admin-review This application requires a review from an admin. label Oct 22, 2024
@Roman-24
Copy link
Author

Would there be anything we can do to help move things forward? Please feel free to let me know if there’s anything needed from our side. Thank you

@PieWol
Copy link
Member

PieWol commented Oct 25, 2024

Hey @Roman-24 ,
sorry for the delay here. I'll get back to your comments today and I'm sure the rest of the team will revisit your application soon. There is nothing missing on your side. Thanks.

@PieWol PieWol added the changes requested The team needs to clarify a few things first. label Oct 25, 2024
Copy link
Member

@PieWol PieWol left a comment

Choose a reason for hiding this comment

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

Thanks for your comments. I answered or marked the conversations as resolved. Looking forward to your answers to all unresolved comments. I appreciate the time and effort you are putting into this to explain your project and that you are incorporating our feedback 🙏

@Roman-24
Copy link
Author

Thank you for reviewing and resolving the conversations. We appreciate your feedback and have addressed all unresolved comments. Your insights were valuable in refining the project, and we’re grateful for your collaboration.

@semuelle semuelle assigned PieWol and unassigned semuelle Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes requested The team needs to clarify a few things first. ready for review The project is ready to be reviewed by the committee members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants