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(NFT): add events to standard #627

Merged
merged 48 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
a6e16e8
feat(NFT): add events to standard
willemneal Nov 4, 2021
3238e4a
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Nov 8, 2021
b65081a
feat: add last two events and add tests
willemneal Nov 8, 2021
19151a1
fix: fmt
willemneal Nov 8, 2021
4b8f0aa
fix: use near indexer's types
willemneal Nov 10, 2021
d27974f
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Nov 30, 2021
c32de7a
feat: add new methods; address feedback and move file up a directory
willemneal Dec 1, 2021
a32852b
chore: update wasm binaries
willemneal Dec 1, 2021
1ddc497
fix: fmt
willemneal Dec 1, 2021
d4a4c8b
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Dec 8, 2021
344d483
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Dec 13, 2021
30a2be5
feat: use Cow and generics to avoid copying strings & remove Display
willemneal Dec 13, 2021
00e0bf5
feat: rename to emit
willemneal Dec 13, 2021
bc403d5
feat: refactor emit functions to use &str instead of generic
willemneal Dec 13, 2021
b6bc5f3
fix: fmt
willemneal Dec 13, 2021
8fc2c80
feat: remove emit_* and add must_use attribute
willemneal Dec 14, 2021
62694c9
add the annoying wasm files
willemneal Dec 14, 2021
0726e8e
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Dec 14, 2021
f9fb90f
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Dec 16, 2021
2f2c0a2
feat: make nft data constructors generic & add from method to AccountId
willemneal Dec 16, 2021
7202200
feat: use event logs in standard
willemneal Dec 16, 2021
0917ab5
fix: fmt
willemneal Dec 16, 2021
62112ad
fix: duplicate import
willemneal Dec 16, 2021
4d494e8
fix: emit event for internal_mint
willemneal Dec 17, 2021
a01075c
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Dec 17, 2021
63beb49
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Dec 20, 2021
c184d9e
Merge branch 'master' into feat/event_logs
willemneal Dec 22, 2021
79a319e
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Dec 23, 2021
3524ffd
feat: ensure log when `nft_transfer_call`s receiver fails
willemneal Jan 12, 2022
c0ef525
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Jan 12, 2022
db7d5d4
fix: fmt
willemneal Jan 12, 2022
83a3b75
fix:wasm
willemneal Jan 12, 2022
38656b0
Merge branch 'master' into feat/event_logs
willemneal Jan 13, 2022
740a594
feat: add test to show that gas check is sufficient
willemneal Jan 13, 2022
9d7474c
fix: test correctly failed; fixed to actually not have enough gas
willemneal Jan 13, 2022
9c561e0
Merge branch 'master' into feat/event_logs
willemneal Jan 13, 2022
7187af3
Merge branch 'master' into feat/event_logs
willemneal Jan 18, 2022
3459d27
Merge branch 'master' into feat/event_logs
willemneal Jan 18, 2022
1404f55
Update near-contract-standards/src/event.rs
willemneal Jan 20, 2022
1746487
chore: add more documentation and edit CHANGELOG
willemneal Jan 20, 2022
9d0d28d
feat: switch to `&'a str` since the event will have shorter lifetime
willemneal Jan 20, 2022
1ce258b
fix: update wasm
willemneal Jan 20, 2022
64c1c6f
chore: add more documentation to core_impl
willemneal Jan 20, 2022
d004203
Update near-contract-standards/src/event.rs
willemneal Jan 20, 2022
179faf1
fix: annoying wasm files
willemneal Jan 20, 2022
bce9abe
Merge branch 'master' into feat/event_logs
willemneal Jan 20, 2022
ae405da
Merge remote-tracking branch 'origin/master' into feat/event_logs
willemneal Jan 21, 2022
173f6f7
Merge branch 'feat/event_logs'
austinabell Jan 25, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions near-contract-standards/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@ NEAR smart contracts standard library.

[dependencies]
near-sdk = { path = "../near-sdk", version = "=4.0.0-pre.4" }
serde = "1"
serde_json = "1"
serde_with = "1"
174 changes: 174 additions & 0 deletions near-contract-standards/src/non_fungible_token/event.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
use std::fmt::Display;

use serde::{Deserialize, Serialize};
use serde_with::skip_serializing_none;

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "standard")]
#[serde(rename_all = "snake_case")]
pub enum NearEvent {
Nep171(Nep171Event),
}
frol marked this conversation as resolved.
Show resolved Hide resolved

#[derive(Serialize, Deserialize, Debug)]
pub struct Nep171Event {
pub version: String,
#[serde(flatten)]
pub event_kind: Nep171EventKind,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "event", content = "data")]
#[serde(rename_all = "snake_case")]
#[allow(clippy::enum_variant_names)]
frol marked this conversation as resolved.
Show resolved Hide resolved
pub enum Nep171EventKind {
NftMint(Vec<NftMintData>),
NftTransfer(Vec<NftTransferData>),
NftBurn(Vec<NftBurnData>),
}

#[skip_serializing_none]
#[derive(Serialize, Deserialize, Debug)]
frol marked this conversation as resolved.
Show resolved Hide resolved
pub struct NftMintData {
pub owner_id: String,
pub token_ids: Vec<String>,
pub memo: Option<String>,
}

#[skip_serializing_none]
#[derive(Serialize, Deserialize, Debug)]
pub struct NftTransferData {
pub authorized_id: Option<String>,
pub old_owner_id: String,
pub new_owner_id: String,
pub token_ids: Vec<String>,
pub memo: Option<String>,
}

#[skip_serializing_none]
#[derive(Serialize, Deserialize, Debug)]
pub struct NftBurnData {
pub authorized_id: Option<String>,
pub owner_id: String,
pub token_ids: Vec<String>,
pub memo: Option<String>,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may consider using Cow<'a, str> instead of Strings to avoid unnecessary clones when users would need to generate the events without giving the ownership over the strings. Yet, that might make the API harder to follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah again this is a copy/paste and I agree that we should try to minimize the cost of these logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still not sure how to fix this. It is annoying with to_strings everywhere.


impl Display for NearEvent {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&format!("EVENT_JSON:{}", self.to_json_string()))
}
}

use Nep171EventKind::*;
willemneal marked this conversation as resolved.
Show resolved Hide resolved

impl NearEvent {
pub fn new_171(version: String, event_kind: Nep171EventKind) -> Self {
NearEvent::Nep171(Nep171Event { version, event_kind })
}

pub fn new_171_v1(event_kind: Nep171EventKind) -> Self {
NearEvent::new_171("1.0.0".to_string(), event_kind)
}

pub fn nft_burn(data: Vec<NftBurnData>) -> Self {
NearEvent::new_171_v1(NftBurn(data))
}
pub fn nft_transfer(data: Vec<NftTransferData>) -> Self {
NearEvent::new_171_v1(NftTransfer(data))
}

pub fn nft_mint(data: Vec<NftMintData>) -> Self {
NearEvent::new_171_v1(NftMint(data))
}

pub(crate) fn to_json_string(&self) -> String {
serde_json::to_string(self).unwrap()
}

pub fn log(&self) {
near_sdk::env::log_str(&self.to_string());
}

pub fn log_nft_mint(owner_id: String, token_ids: Vec<String>, memo: Option<String>) {
NearEvent::nft_mint(vec![NftMintData { owner_id, token_ids, memo }]).log();
}
pub fn log_nft_transfer(
old_owner_id: String,
new_owner_id: String,
token_ids: Vec<String>,
memo: Option<String>,
authorized_id: Option<String>,
) {
NearEvent::nft_transfer(vec![NftTransferData {
authorized_id,
old_owner_id,
new_owner_id,
token_ids,
memo,
}])
.log();
}

pub fn log_nft_burn(
owner_id: String,
authorized_id: Option<String>,
token_ids: Vec<String>,
memo: Option<String>,
) {
NearEvent::nft_burn(vec![NftBurnData { owner_id, authorized_id, token_ids, memo }]).log();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This API has a few problems:

  1. The usage of these methods will be error-prone to use and hard to reason about:

    NearEvent::log_nft_transfer("frol.near", "willem.near", vec!["dacha".to_string()], Some("qq".to_string()), Some("alex.near"))
    NearEvent::log_nft_burn("willem.near", Some("alex.near"), vec!["dacha".to_string()], Some("qq".to_string()))

    (You already mixed up the order of authorized_id in transfer and burn methods)

  2. These methods promote non-batched events and thus we promote using more gas on events than necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Yeah oops, this was from just following the ordering from the struct.

  2. I agree, my inital thoughts were that they were the common case so it should be easy to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed 1 and added methods to log data vectors.

}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn nft_mint() {
let owner_id = "bob".to_string();
let token_ids = vec!["0", "1"].iter().map(|t| t.to_string()).collect();
let mint_log = NftMintData { owner_id, token_ids, memo: None };
let event_log = NearEvent::nft_mint(vec![mint_log]);
assert_eq!(
serde_json::to_string(&event_log).unwrap(),
r#"{"standard":"nep171","version":"1.0.0","event":"nft_mint","data":[{"owner_id":"bob","token_ids":["0","1"]}]}"#
);
}

#[test]
fn nft_burn() {
let owner_id = "bob".to_string();
let token_ids = vec!["0", "1"].iter().map(|t| t.to_string()).collect();
let log = NearEvent::nft_burn(vec![NftBurnData {
owner_id,
authorized_id: None,
token_ids,
memo: None,
}])
.to_json_string();
assert_eq!(
log,
r#"{"standard":"nep171","version":"1.0.0","event":"nft_burn","data":[{"owner_id":"bob","token_ids":["0","1"]}]}"#
);
}

#[test]
fn nft_transfer() {
let old_owner_id = "bob".to_string();
let new_owner_id = "alice".to_string();
let token_ids = vec!["0", "1"].iter().map(|t| t.to_string()).collect();
let log = NearEvent::nft_transfer(vec![NftTransferData {
old_owner_id,
new_owner_id,
authorized_id: None,
token_ids,
memo: None,
}])
.to_json_string();
assert_eq!(
log,
r#"{"standard":"nep171","version":"1.0.0","event":"nft_transfer","data":[{"old_owner_id":"bob","new_owner_id":"alice","token_ids":["0","1"]}]}"#
);
}
}
3 changes: 3 additions & 0 deletions near-contract-standards/src/non_fungible_token/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ pub use utils::*;

pub use self::core::NonFungibleToken;
pub use macros::*;

// Events that can be logged
pub mod event;