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

feat(contract-standards): add events to FT contract standard #723

Merged
merged 5 commits into from
Feb 1, 2022

Conversation

itegulov
Copy link
Contributor

As minting and burning are not a part of FT contract standard yet, these events are not being emitted

Copy link
Member

@ChaoticTempest ChaoticTempest left a comment

Choose a reason for hiding this comment

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

Looks pretty straightforward to me, but I'll leave it to @austinabell to have the final say since he might point out some serialization stuff I might not know of

@@ -7,17 +7,17 @@
//! This is an extension of the events format (nep-297):
//! <https://github.com/near/NEPs/blob/master/specs/Standards/EventsFormat.md>
//!
//! The three events in this standard are [`NftMintData`], [`NftTransferData`], and [`NftBurnData`].
//! The three events in this standard are [`NftMint`], [`NftTransfer`], and [`NftBurn`].
Copy link
Member

Choose a reason for hiding this comment

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

Nice, thanks for the catching this!

Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

Awesome!

@austinabell austinabell merged commit 0e89031 into master Feb 1, 2022
@austinabell austinabell deleted the daniyar/standards/ft-events branch February 1, 2022 01:41
@austinabell
Copy link
Contributor

Hey @frol and/or @telezhnaya I should have pinged you guys to make sure this all matches up with what gets picked up by the indexer. Do you guys mind having a quick look before I release this?

@frol
Copy link
Collaborator

frol commented Feb 12, 2022

@austinabell Sorry for the delay. This implementation is compatible with the NEP, and I have even checked how it works with the FT example: near-examples/FT#106

}

#[test]
fn nft_transfers() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: it should be tf_transfers

Copy link
Contributor

Choose a reason for hiding this comment

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

ft_transfers 🙂

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.

Thank you!

}

#[test]
fn nft_burns() {
Copy link
Contributor

Choose a reason for hiding this comment

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

ft_burns

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh it's merged 😅 I just noticed that
Created PR with the fix #735

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.

5 participants