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.

Sign up for GitHub

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

Jump to bottom

Reference implementation for proposed Multi-token spec (NEP-245) #776

Closed
wants to merge 3 commits into from
Closed

Reference implementation for proposed Multi-token spec (NEP-245) #776

wants to merge 3 commits into from

Conversation

jriemann
Copy link

@jriemann jriemann commented Apr 1, 2022

This is a reference implementation of the current draft Multi-token spec (NEP-245) - near/NEPs#245 . I am sending this out now to be reviewed in conjunction with that PR so that reviewers can start looking it over.

The work is based on the repo started by @i3ima who deserves credit for getting the ball rolling - https://github.com/i3ima/nep-246 . I have filled in the gaps and it should now be close to the draft spec.

  • Core MT contract
  • Approvals
  • Enumeration
  • Metadata

Not fully complete yet:

  • simulation tests for the XCCs soon. I have included boilerplate but am still learning how to convey what I need to test.
  • Logic for re-granting approvals in case of *transfer_call failures.

@jriemann jriemann mentioned this pull request Apr 1, 2022
Merged
@zcstarr
Copy link
Contributor

zcstarr commented Apr 1, 2022

@austinabell this is the token standard I mentioned a while ago coming down the pipeline. PTAL when you get a chance @jriemann is doing the development on this one so feel free to ping him, I'm around to talk spec things if needed .

@austinabell
Copy link
Contributor

@jlogelin will likely review the logical components and how they match the spec. I'll be more than happy to review this once I have some time also

austinabell
austinabell reviewed Apr 1, 2022
View reviewed changes
examples/multi-token/tests/workspaces.rs Outdated

#[tokio::test]
async fn test_total_supply() -> anyhow::Result<()> {
// TODO: Write simulation tests involving cross-contract calls (mt_transfer_call, etc).
Copy link
Contributor

Choose a reason for hiding this comment

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

Note, you'll probably use workspaces tests based on what's above, but just a warning to not invest time into sim tests (near-sdk-sim) as it will be deprecated soon in favor of workspaces.

Copy link
Author

Choose a reason for hiding this comment

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

Gotcha

@jriemann
Copy link
Author

jriemann commented Apr 4, 2022

@telezhnaya pinging you since you were active on the NEP-245 review & this is related. Could you take a look as well, if you have time?
This is a large PR but a good amount of it is boilerplate code similar to the existing NFT stuff. If any areas are unclear, let me know.

telezhnaya
telezhnaya reviewed Apr 5, 2022
View reviewed changes
Copy link
Contributor

@telezhnaya telezhnaya left a comment

Choose a reason for hiding this comment

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

Could you please add tests that we produce valid event logs on each mint/transfer/burn?

near-contract-standards/src/multi_token/core/receiver.rs Outdated
/// Take some action after receiving a multi-token's
///
/// ## Requirements:
/// * Contract MUST restrict calls to this function to a set of whitelisted NFT
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// * Contract MUST restrict calls to this function to a set of whitelisted NFT
/// * Contract MUST restrict calls to this function to a set of whitelisted MT

near-contract-standards/src/multi_token/events.rs Outdated
enum Nep245EventKind<'a> {
MtMint(&'a [MtMint<'a>]),
MtTransfer(&'a [MtTransfer<'a>]),
// NftBurn(&'a [NftBurn<'a>]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// NftBurn(&'a [NftBurn<'a>]),

@zcstarr zcstarr mentioned this pull request Jun 24, 2022
Open
uncle-T0ny added a commit to uncle-T0ny/near-sdk-rs that referenced this pull request Sep 26, 2022
@uncle-T0ny
update near#776
5535b54
@frol frol mentioned this pull request Oct 12, 2022
Open
@uncle-T0ny uncle-T0ny mentioned this pull request Oct 27, 2022
Draft
@uint
Copy link
Contributor

uint commented Jul 11, 2023

Closing as work seems to have been continued here: #950

@uint uint closed this Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinabell austinabell austinabell left review comments

@telezhnaya telezhnaya telezhnaya left review comments

@frol frol Awaiting requested review from frol frol will be requested when the pull request is marked ready for review frol is a code owner

@dj8yfo dj8yfo Awaiting requested review from dj8yfo dj8yfo will be requested when the pull request is marked ready for review dj8yfo is a code owner

@ruseinov ruseinov Awaiting requested review from ruseinov ruseinov will be requested when the pull request is marked ready for review ruseinov is a code owner

@akorchyn akorchyn Awaiting requested review from akorchyn akorchyn will be requested when the pull request is marked ready for review akorchyn is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jriemann @zcstarr @austinabell @uint @telezhnaya