From 110d8f3f5d219f6773e0a12804fea593d471ab36 Mon Sep 17 00:00:00 2001 From: Bo Yao Date: Tue, 15 Nov 2022 14:53:00 +0800 Subject: [PATCH 1/2] NFT standard refund account storage based on actual size --- .../lib/non_fungible_token/impl.js | 22 +++++++---- .../lib/non_fungible_token/utils.d.ts | 6 +-- .../lib/non_fungible_token/utils.js | 12 +----- .../src/non_fungible_token/impl.ts | 39 ++++++++++++++----- .../src/non_fungible_token/utils.ts | 22 +---------- 5 files changed, 47 insertions(+), 54 deletions(-) diff --git a/near-contract-standards/lib/non_fungible_token/impl.js b/near-contract-standards/lib/non_fungible_token/impl.js index c9496e120..784b4e8c1 100644 --- a/near-contract-standards/lib/non_fungible_token/impl.js +++ b/near-contract-standards/lib/non_fungible_token/impl.js @@ -1,6 +1,6 @@ -import { UnorderedMap, LookupMap, near, UnorderedSet, assert, NearPromise, bytes, serialize } from "near-sdk-js"; +import { UnorderedMap, LookupMap, near, UnorderedSet, assert, NearPromise, bytes, serialize, } from "near-sdk-js"; import { TokenMetadata } from "./metadata"; -import { refund_approved_account_ids, refund_deposit, refund_deposit_to_account, assert_at_least_one_yocto, bytes_for_approved_account_id, assert_one_yocto, refund_approved_account_ids_iter, } from "./utils"; +import { refund_storage_deposit, refund_deposit, refund_deposit_to_account, assert_at_least_one_yocto, assert_one_yocto, } from "./utils"; import { NftMint, NftTransfer } from "./events"; import { Token } from "./token"; const GAS_FOR_RESOLVE_TRANSFER = 15000000000000n; @@ -106,11 +106,12 @@ export class NonFungibleToken { const next_approval_id_by_id = expect_approval(this.next_approval_id_by_id); const approved_account_ids = approvals_by_id.get(token_id) ?? {}; const approval_id = next_approval_id_by_id.get(token_id) ?? 1n; - const old_approval_id = approved_account_ids[account_id]; + let old_approved_account_ids_size = JSON.stringify(approved_account_ids).length; approved_account_ids[account_id] = approval_id; + let new_approved_account_ids_size = JSON.stringify(approved_account_ids).length; approvals_by_id.set(token_id, approved_account_ids); next_approval_id_by_id.set(token_id, approval_id + 1n); - const storage_used = old_approval_id === null ? bytes_for_approved_account_id(account_id) : 0; + const storage_used = new_approved_account_ids_size - old_approved_account_ids_size; refund_deposit(BigInt(storage_used)); if (msg) { return NearPromise.new(account_id).functionCall("nft_on_approve", bytes(serialize({ token_id, owner_id, approval_id, msg })), 0n, near.prepaidGas() - GAS_FOR_NFT_APPROVE); @@ -127,15 +128,20 @@ export class NonFungibleToken { const predecessorAccountId = near.predecessorAccountId(); assert(predecessorAccountId === owner_id, "Predecessor must be token owner."); const approved_account_ids = approvals_by_id.get(token_id); + const old_approved_account_ids_size = JSON.stringify(approved_account_ids).length; + let new_approved_account_ids_size; if (approved_account_ids[account_id]) { delete approved_account_ids[account_id]; - refund_approved_account_ids_iter(predecessorAccountId, [account_id]); if (Object.keys(approved_account_ids).length === 0) { approvals_by_id.remove(token_id); + new_approved_account_ids_size = + JSON.stringify(approved_account_ids).length; } else { approvals_by_id.set(token_id, approved_account_ids); + new_approved_account_ids_size = 0; } + refund_storage_deposit(predecessorAccountId, new_approved_account_ids_size - old_approved_account_ids_size); } } nft_revoke_all({ token_id }) { @@ -149,7 +155,7 @@ export class NonFungibleToken { assert(predecessorAccountId === owner_id, "Predecessor must be token owner."); const approved_account_ids = approvals_by_id.get(token_id); if (approved_account_ids) { - refund_approved_account_ids(predecessorAccountId, approved_account_ids); + refund_storage_deposit(predecessorAccountId, JSON.stringify(approved_account_ids).length); approvals_by_id.remove(token_id); } } @@ -418,7 +424,7 @@ export class NonFungibleToken { } else { if (approved_account_ids) { - refund_approved_account_ids(previous_owner_id, approved_account_ids); + refund_storage_deposit(previous_owner_id, JSON.stringify(approved_account_ids).length); } return true; } @@ -426,7 +432,7 @@ export class NonFungibleToken { if (this.approvals_by_id) { const receiver_approvals = this.approvals_by_id.get(token_id); if (receiver_approvals) { - refund_approved_account_ids(receiver_id, receiver_approvals); + refund_storage_deposit(receiver_id, JSON.stringify(receiver_approvals).length); } if (approved_account_ids) { this.approvals_by_id.set(token_id, approved_account_ids); diff --git a/near-contract-standards/lib/non_fungible_token/utils.d.ts b/near-contract-standards/lib/non_fungible_token/utils.d.ts index bd1e39720..dd2715520 100644 --- a/near-contract-standards/lib/non_fungible_token/utils.d.ts +++ b/near-contract-standards/lib/non_fungible_token/utils.d.ts @@ -1,9 +1,5 @@ import { Bytes, AccountId } from "near-sdk-js"; -export declare function bytes_for_approved_account_id(account_id: AccountId): number; -export declare function refund_approved_account_ids_iter(account_id: AccountId, approved_account_ids: AccountId[]): void; -export declare function refund_approved_account_ids(account_id: AccountId, approved_account_ids: { - [approvals: AccountId]: bigint; -}): void; +export declare function refund_storage_deposit(account_id: AccountId, storage_released: number): void; export declare function refund_deposit_to_account(storage_used: bigint, account_id: AccountId): void; /** Assumes that the precedecessor will be refunded */ export declare function refund_deposit(storage_used: bigint): void; diff --git a/near-contract-standards/lib/non_fungible_token/utils.js b/near-contract-standards/lib/non_fungible_token/utils.js index 34a89b41f..68931c013 100644 --- a/near-contract-standards/lib/non_fungible_token/utils.js +++ b/near-contract-standards/lib/non_fungible_token/utils.js @@ -1,19 +1,9 @@ import { near, assert } from "near-sdk-js"; -export function bytes_for_approved_account_id(account_id) { - // The extra 4 bytes are coming from Borsh serialization to store the length of the string. - return account_id.length + 4 + 8; -} -export function refund_approved_account_ids_iter(account_id, approved_account_ids) { - const storage_released = approved_account_ids - .map(bytes_for_approved_account_id) - .reduce((a, b) => a + b); +export function refund_storage_deposit(account_id, storage_released) { const promise_id = near.promiseBatchCreate(account_id); near.promiseBatchActionTransfer(promise_id, BigInt(storage_released) * near.storageByteCost()); near.promiseReturn(promise_id); } -export function refund_approved_account_ids(account_id, approved_account_ids) { - refund_approved_account_ids_iter(account_id, Array.from(Object.keys(approved_account_ids))); -} export function refund_deposit_to_account(storage_used, account_id) { const required_cost = near.storageByteCost() * storage_used; const attached_deposit = near.attachedDeposit(); diff --git a/near-contract-standards/src/non_fungible_token/impl.ts b/near-contract-standards/src/non_fungible_token/impl.ts index e79355662..6ae5b70fa 100644 --- a/near-contract-standards/src/non_fungible_token/impl.ts +++ b/near-contract-standards/src/non_fungible_token/impl.ts @@ -12,15 +12,13 @@ import { } from "near-sdk-js"; import { TokenMetadata } from "./metadata"; import { - refund_approved_account_ids, + refund_storage_deposit, refund_deposit, refund_deposit_to_account, assert_at_least_one_yocto, IntoStorageKey, Option, - bytes_for_approved_account_id, assert_one_yocto, - refund_approved_account_ids_iter, } from "./utils"; import { NftMint, NftTransfer } from "./events"; import { NonFungibleTokenResolver } from "./core/resolver"; @@ -199,15 +197,18 @@ export class NonFungibleToken const next_approval_id_by_id = expect_approval(this.next_approval_id_by_id); const approved_account_ids = approvals_by_id.get(token_id) ?? {}; const approval_id = next_approval_id_by_id.get(token_id) ?? 1n; - const old_approval_id = approved_account_ids[account_id]; + const old_approved_account_ids_size = + JSON.stringify(approved_account_ids).length; approved_account_ids[account_id] = approval_id; + const new_approved_account_ids_size = + JSON.stringify(approved_account_ids).length; approvals_by_id.set(token_id, approved_account_ids); next_approval_id_by_id.set(token_id, approval_id + 1n); const storage_used = - old_approval_id === null ? bytes_for_approved_account_id(account_id) : 0; + new_approved_account_ids_size - old_approved_account_ids_size; refund_deposit(BigInt(storage_used)); if (msg) { @@ -242,15 +243,24 @@ export class NonFungibleToken ); const approved_account_ids = approvals_by_id.get(token_id); + const old_approved_account_ids_size = + JSON.stringify(approved_account_ids).length; + let new_approved_account_ids_size; + if (approved_account_ids[account_id]) { delete approved_account_ids[account_id]; - refund_approved_account_ids_iter(predecessorAccountId, [account_id]); - if (Object.keys(approved_account_ids).length === 0) { approvals_by_id.remove(token_id); + new_approved_account_ids_size = + JSON.stringify(approved_account_ids).length; } else { approvals_by_id.set(token_id, approved_account_ids); + new_approved_account_ids_size = 0; } + refund_storage_deposit( + predecessorAccountId, + new_approved_account_ids_size - old_approved_account_ids_size + ); } } @@ -270,7 +280,10 @@ export class NonFungibleToken const approved_account_ids = approvals_by_id.get(token_id); if (approved_account_ids) { - refund_approved_account_ids(predecessorAccountId, approved_account_ids); + refund_storage_deposit( + predecessorAccountId, + JSON.stringify(approved_account_ids).length + ); approvals_by_id.remove(token_id); } @@ -708,7 +721,10 @@ export class NonFungibleToken } } else { if (approved_account_ids) { - refund_approved_account_ids(previous_owner_id, approved_account_ids); + refund_storage_deposit( + previous_owner_id, + JSON.stringify(approved_account_ids).length + ); } return true; } @@ -718,7 +734,10 @@ export class NonFungibleToken if (this.approvals_by_id) { const receiver_approvals = this.approvals_by_id.get(token_id); if (receiver_approvals) { - refund_approved_account_ids(receiver_id, receiver_approvals); + refund_storage_deposit( + receiver_id, + JSON.stringify(receiver_approvals).length + ); } if (approved_account_ids) { this.approvals_by_id.set(token_id, approved_account_ids); diff --git a/near-contract-standards/src/non_fungible_token/utils.ts b/near-contract-standards/src/non_fungible_token/utils.ts index 39359faa5..60d9562ad 100644 --- a/near-contract-standards/src/non_fungible_token/utils.ts +++ b/near-contract-standards/src/non_fungible_token/utils.ts @@ -1,17 +1,9 @@ import { near, assert, Bytes, AccountId } from "near-sdk-js"; -export function bytes_for_approved_account_id(account_id: AccountId): number { - // The extra 4 bytes are coming from Borsh serialization to store the length of the string. - return account_id.length + 4 + 8; -} - -export function refund_approved_account_ids_iter( +export function refund_storage_deposit( account_id: AccountId, - approved_account_ids: AccountId[] + storage_released: number ): void { - const storage_released = approved_account_ids - .map(bytes_for_approved_account_id) - .reduce((a, b) => a + b); const promise_id = near.promiseBatchCreate(account_id); near.promiseBatchActionTransfer( promise_id, @@ -20,16 +12,6 @@ export function refund_approved_account_ids_iter( near.promiseReturn(promise_id); } -export function refund_approved_account_ids( - account_id: AccountId, - approved_account_ids: { [approvals: AccountId]: bigint } -) { - refund_approved_account_ids_iter( - account_id, - Array.from(Object.keys(approved_account_ids)) - ); -} - export function refund_deposit_to_account( storage_used: bigint, account_id: AccountId From b7b01087b1379b1e51659c1810998c8e171fedfd Mon Sep 17 00:00:00 2001 From: Bo Yao Date: Tue, 15 Nov 2022 15:29:34 +0800 Subject: [PATCH 2/2] fix tests --- .../__tests__/standard-nft/test_approval.ava.js | 4 ++-- .../lib/non_fungible_token/impl.js | 14 +++++++------- .../src/non_fungible_token/impl.ts | 15 +++++++-------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/examples/__tests__/standard-nft/test_approval.ava.js b/examples/__tests__/standard-nft/test_approval.ava.js index 91bb428e3..8ceaf33b5 100644 --- a/examples/__tests__/standard-nft/test_approval.ava.js +++ b/examples/__tests__/standard-nft/test_approval.ava.js @@ -141,7 +141,7 @@ test("Simple approve", async (t) => { msg: null, }, { - attachedDeposit: "450000000000000000000", + attachedDeposit: "550000000000000000000", } ); t.assert(res.result.status.SuccessValue); @@ -165,7 +165,7 @@ test("Approve call", async (t) => { account_id: approvalReceiver.accountId, msg: "return-now", }, - { attachedDeposit: "450000000000000000000", gas: "300 Tgas" } + { attachedDeposit: "550000000000000000000", gas: "300 Tgas" } ); t.is(res, "cool"); diff --git a/near-contract-standards/lib/non_fungible_token/impl.js b/near-contract-standards/lib/non_fungible_token/impl.js index 784b4e8c1..2a6217d17 100644 --- a/near-contract-standards/lib/non_fungible_token/impl.js +++ b/near-contract-standards/lib/non_fungible_token/impl.js @@ -106,9 +106,9 @@ export class NonFungibleToken { const next_approval_id_by_id = expect_approval(this.next_approval_id_by_id); const approved_account_ids = approvals_by_id.get(token_id) ?? {}; const approval_id = next_approval_id_by_id.get(token_id) ?? 1n; - let old_approved_account_ids_size = JSON.stringify(approved_account_ids).length; + const old_approved_account_ids_size = serialize(approved_account_ids).length; approved_account_ids[account_id] = approval_id; - let new_approved_account_ids_size = JSON.stringify(approved_account_ids).length; + const new_approved_account_ids_size = serialize(approved_account_ids).length; approvals_by_id.set(token_id, approved_account_ids); next_approval_id_by_id.set(token_id, approval_id + 1n); const storage_used = new_approved_account_ids_size - old_approved_account_ids_size; @@ -128,14 +128,14 @@ export class NonFungibleToken { const predecessorAccountId = near.predecessorAccountId(); assert(predecessorAccountId === owner_id, "Predecessor must be token owner."); const approved_account_ids = approvals_by_id.get(token_id); - const old_approved_account_ids_size = JSON.stringify(approved_account_ids).length; + const old_approved_account_ids_size = serialize(approved_account_ids).length; let new_approved_account_ids_size; if (approved_account_ids[account_id]) { delete approved_account_ids[account_id]; if (Object.keys(approved_account_ids).length === 0) { approvals_by_id.remove(token_id); new_approved_account_ids_size = - JSON.stringify(approved_account_ids).length; + serialize(approved_account_ids).length; } else { approvals_by_id.set(token_id, approved_account_ids); @@ -155,7 +155,7 @@ export class NonFungibleToken { assert(predecessorAccountId === owner_id, "Predecessor must be token owner."); const approved_account_ids = approvals_by_id.get(token_id); if (approved_account_ids) { - refund_storage_deposit(predecessorAccountId, JSON.stringify(approved_account_ids).length); + refund_storage_deposit(predecessorAccountId, serialize(approved_account_ids).length); approvals_by_id.remove(token_id); } } @@ -424,7 +424,7 @@ export class NonFungibleToken { } else { if (approved_account_ids) { - refund_storage_deposit(previous_owner_id, JSON.stringify(approved_account_ids).length); + refund_storage_deposit(previous_owner_id, serialize(approved_account_ids).length); } return true; } @@ -432,7 +432,7 @@ export class NonFungibleToken { if (this.approvals_by_id) { const receiver_approvals = this.approvals_by_id.get(token_id); if (receiver_approvals) { - refund_storage_deposit(receiver_id, JSON.stringify(receiver_approvals).length); + refund_storage_deposit(receiver_id, serialize(receiver_approvals).length); } if (approved_account_ids) { this.approvals_by_id.set(token_id, approved_account_ids); diff --git a/near-contract-standards/src/non_fungible_token/impl.ts b/near-contract-standards/src/non_fungible_token/impl.ts index 6ae5b70fa..86f3d7332 100644 --- a/near-contract-standards/src/non_fungible_token/impl.ts +++ b/near-contract-standards/src/non_fungible_token/impl.ts @@ -198,10 +198,10 @@ export class NonFungibleToken const approved_account_ids = approvals_by_id.get(token_id) ?? {}; const approval_id = next_approval_id_by_id.get(token_id) ?? 1n; const old_approved_account_ids_size = - JSON.stringify(approved_account_ids).length; + serialize(approved_account_ids).length; approved_account_ids[account_id] = approval_id; const new_approved_account_ids_size = - JSON.stringify(approved_account_ids).length; + serialize(approved_account_ids).length; approvals_by_id.set(token_id, approved_account_ids); @@ -244,15 +244,14 @@ export class NonFungibleToken const approved_account_ids = approvals_by_id.get(token_id); const old_approved_account_ids_size = - JSON.stringify(approved_account_ids).length; + serialize(approved_account_ids).length; let new_approved_account_ids_size; if (approved_account_ids[account_id]) { delete approved_account_ids[account_id]; if (Object.keys(approved_account_ids).length === 0) { approvals_by_id.remove(token_id); - new_approved_account_ids_size = - JSON.stringify(approved_account_ids).length; + new_approved_account_ids_size = serialize(approved_account_ids).length; } else { approvals_by_id.set(token_id, approved_account_ids); new_approved_account_ids_size = 0; @@ -282,7 +281,7 @@ export class NonFungibleToken if (approved_account_ids) { refund_storage_deposit( predecessorAccountId, - JSON.stringify(approved_account_ids).length + serialize(approved_account_ids).length ); approvals_by_id.remove(token_id); @@ -723,7 +722,7 @@ export class NonFungibleToken if (approved_account_ids) { refund_storage_deposit( previous_owner_id, - JSON.stringify(approved_account_ids).length + serialize(approved_account_ids).length ); } return true; @@ -736,7 +735,7 @@ export class NonFungibleToken if (receiver_approvals) { refund_storage_deposit( receiver_id, - JSON.stringify(receiver_approvals).length + serialize(receiver_approvals).length ); } if (approved_account_ids) {