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

[Aptos Framework][Token] fully support deletion of TokenData and Token if reach 0 #3367

Merged
merged 1 commit into from
Aug 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions aptos-move/framework/aptos-stdlib/sources/simple_map.move
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/// 1) Keys point to Values
/// 2) Each Key must be unique
/// 3) A Key can be found within O(Log N) time
/// 4) The data is stored as a sorted by Key
/// 4) The data is stored as sorted by Key
/// 5) Adds and removals take O(N) time
module aptos_std::simple_map {
use std::error;
Expand Down Expand Up @@ -119,7 +119,7 @@ module aptos_std::simple_map {
let right = length;

while (left != right) {
let mid = (left + right) / 2;
lightmark marked this conversation as resolved.
Show resolved Hide resolved
let mid = left + (right - left) / 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

left + ((right - left) / 2) = left + right/2 - left /2 = left/2 + right/2 = (left + right) / 2

Why bother?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see kevin's comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please help me fix the ts tests tmr...

Copy link
Contributor

Choose a reason for hiding this comment

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

thoughtful lol. then how about left/2 + right/2?

Copy link
Contributor Author

@lightmark lightmark Aug 24, 2022

Choose a reason for hiding this comment

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

this cannot ensure the correctness because of division rounding towards 0
if left = 1 and right = 3, left/2 + right/2 = 1.... but you want 2...

let potential_key = &vector::borrow(&map.data, mid).key;
if (comparator::is_smaller_than(&comparator::compare(potential_key, key))) {
left = mid + 1;
Expand Down
49 changes: 33 additions & 16 deletions aptos-move/framework/aptos-token/sources/token.move
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ module aptos_token::token {
const ENO_TOKEN_IN_TOKEN_STORE: u64 = 19;
const ENON_ZERO_PROPERTY_VERSION_ONLY_ONE_INSTANCE: u64 = 20;
const EUSER_NOT_OPT_IN_DIRECT_TRANSFER: u64 = 21;

const EWITHDRAW_ZERO: u64 = 22;

//
// Core data structures for holding tokens
Expand Down Expand Up @@ -505,6 +505,11 @@ module aptos_token::token {
id: TokenId,
amount: u64,
): Token acquires TokenStore {
// It does not make sense to withdraw 0 tokens.
assert!(amount > 0, error::invalid_argument(EWITHDRAW_ZERO));
// Make sure the account has sufficient tokens to withdraw.
assert!(balance_of(account_addr, id) >= amount, error::invalid_argument(EINSUFFICIENT_BALANCE));

let token_store = borrow_global_mut<TokenStore>(account_addr);
event::emit_event<WithdrawEvent>(
&mut token_store.withdraw_events,
Expand All @@ -519,12 +524,12 @@ module aptos_token::token {
table::contains(tokens, id),
error::not_found(EBALANCE_NOT_PUBLISHED),
);
// balance > amount and amount > 0 indirectly asserted that balance > 0.
let balance = &mut table::borrow_mut(tokens, id).amount;
if (id.property_version == 0) {
if (*balance > amount) {
*balance = *balance - amount;
Token{ id, amount, token_properties: property_map::empty() }
} else {
// only 1 token per property_version > 0. we can directly extract from owner's token store
table::remove(tokens, id)
}
}
Expand Down Expand Up @@ -781,8 +786,6 @@ module aptos_token::token {
amount: u64
) acquires Collections, TokenStore {
let token_id = create_token_id_raw(creators_address, collection, name, property_version);
let owner_addr = signer::address_of(owner);
assert!(balance_of(owner_addr, token_id) >= amount, EINSUFFICIENT_BALANCE);
let creator_addr = token_id.token_data_id.creator;
assert!(
exists<Collections>(creator_addr),
Expand All @@ -795,21 +798,35 @@ module aptos_token::token {
error::not_found(ETOKEN_NOT_PUBLISHED),
);

let token_data = table::borrow_mut(
&mut collections.token_data,
token_id.token_data_id,
);

let token = withdraw_token(owner, token_id, amount);
token_data.supply = token_data.supply - token.amount;
let Token { id: _, amount: burned_amount, token_properties: _ } = token;

let token_store = borrow_global_mut<TokenStore>(owner_addr);

// Burn the tokens.
let Token { id: _, amount: burned_amount, token_properties: _ } = withdraw_token(owner, token_id, amount);
let token_store = borrow_global_mut<TokenStore>(signer::address_of(owner));
event::emit_event<BurnTokenEvent>(
&mut token_store.burn_events,
BurnTokenEvent { id: token_id, amount: burned_amount},
);

// Decrease the supply correspondingly by the amount of tokens burned.
let token_data = table::borrow_mut(
&mut collections.token_data,
token_id.token_data_id,
);
token_data.supply = token_data.supply - burned_amount;

// Delete the token_data if supply drops to 0.
if (token_data.supply == 0) {
let TokenData {
maximum: _,
largest_property_version: _,
supply: _,
uri: _,
royalty: _,
name: _,
description: _,
default_properties: _,
mutability_config: _,
} = table::remove(&mut collections.token_data, token_id.token_data_id);
};
}

public fun create_token_id(token_data_id: TokenDataId, property_version: u64): TokenId {
Expand Down
6 changes: 3 additions & 3 deletions ecosystem/typescript/sdk/src/aptos_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ test(

// Transfer Token from Alice's Account to Bob's Account
await tokenClient.getCollectionData(alice.address().hex(), collectionName);
let aliceBalance = await tokenClient.getTokenBalanceForAccount(alice.address().hex(), tokenId);
let aliceBalance = await tokenClient.getTokenForAccount(alice.address().hex(), tokenId);
expect(aliceBalance.amount).toBe("1");

const entryFunctionPayload = new TxnBuilderTypes.TransactionPayloadEntryFunction(
Expand Down Expand Up @@ -452,10 +452,10 @@ test(
const transaction = await client.getTransactionByHash(transactionRes.hash);
expect((transaction as any)?.success).toBe(true);

aliceBalance = await tokenClient.getTokenBalanceForAccount(alice.address().hex(), tokenId);
aliceBalance = await tokenClient.getTokenForAccount(alice.address().hex(), tokenId);
expect(aliceBalance.amount).toBe("0");

const bobBalance = await tokenClient.getTokenBalanceForAccount(bob.address().hex(), tokenId);
const bobBalance = await tokenClient.getTokenForAccount(bob.address().hex(), tokenId);
expect(bobBalance.amount).toBe("1");
},
30 * 1000,
Expand Down
10 changes: 5 additions & 5 deletions ecosystem/typescript/sdk/src/token_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,34 +65,34 @@ test(

// Transfer Token from Alice's Account to Bob's Account
await tokenClient.getCollectionData(alice.address().hex(), collectionName);
let aliceBalance = await tokenClient.getTokenBalanceForAccount(alice.address().hex(), tokenId);
let aliceBalance = await tokenClient.getTokenForAccount(alice.address().hex(), tokenId);
expect(aliceBalance.amount).toBe("1");
const tokenData = await tokenClient.getTokenData(alice.address().hex(), collectionName, tokenName);
expect(tokenData.name).toBe(tokenName);

await ensureTxnSuccess(
tokenClient.offerToken(alice, bob.address().hex(), alice.address().hex(), collectionName, tokenName, 1),
);
aliceBalance = await tokenClient.getTokenBalanceForAccount(alice.address().hex(), tokenId);
aliceBalance = await tokenClient.getTokenForAccount(alice.address().hex(), tokenId);
expect(aliceBalance.amount).toBe("0");

await ensureTxnSuccess(
tokenClient.cancelTokenOffer(alice, bob.address().hex(), alice.address().hex(), collectionName, tokenName),
);
aliceBalance = await tokenClient.getTokenBalanceForAccount(alice.address().hex(), tokenId);
aliceBalance = await tokenClient.getTokenForAccount(alice.address().hex(), tokenId);
expect(aliceBalance.amount).toBe("1");

await ensureTxnSuccess(
tokenClient.offerToken(alice, bob.address().hex(), alice.address().hex(), collectionName, tokenName, 1),
);
aliceBalance = await tokenClient.getTokenBalanceForAccount(alice.address().hex(), tokenId);
aliceBalance = await tokenClient.getTokenForAccount(alice.address().hex(), tokenId);
expect(aliceBalance.amount).toBe("0");

await ensureTxnSuccess(
tokenClient.claimToken(bob, alice.address().hex(), alice.address().hex(), collectionName, tokenName),
);

const bobBalance = await tokenClient.getTokenBalanceForAccount(bob.address().hex(), tokenId);
const bobBalance = await tokenClient.getTokenForAccount(bob.address().hex(), tokenId);
expect(bobBalance.amount).toBe("1");
},
30 * 1000,
Expand Down
18 changes: 14 additions & 4 deletions ecosystem/typescript/sdk/src/token_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ export class TokenClient {
/**
* Queries token balance for the token creator
*/
async getTokenBalance(
async getToken(
creator: MaybeHexString,
collectionName: string,
tokenName: string,
Expand All @@ -318,7 +318,7 @@ export class TokenClient {
collection: collectionName,
name: tokenName,
};
return this.getTokenBalanceForAccount(creator, {
return this.getTokenForAccount(creator, {
token_data_id: tokenDataId,
property_version,
});
Expand Down Expand Up @@ -347,7 +347,7 @@ export class TokenClient {
* }
* ```
*/
async getTokenBalanceForAccount(account: MaybeHexString, tokenId: TokenTypes.TokenId): Promise<TokenTypes.Token> {
async getTokenForAccount(account: MaybeHexString, tokenId: TokenTypes.TokenId): Promise<TokenTypes.Token> {
const tokenStore: { type: Gen.MoveStructTag; data: any } = await this.aptosClient.getAccountResource(
account,
"0x3::token::TokenStore",
Expand All @@ -360,6 +360,16 @@ export class TokenClient {
key: tokenId,
};

return this.aptosClient.getTableItem(handle, getTokenTableItemRequest);
try {
return await this.aptosClient.getTableItem(handle, getTokenTableItemRequest);
} catch (error) {
if (error.status === 404) {
return {
id: tokenId,
amount: "0",
};
}
return error;
}
}
}
6 changes: 5 additions & 1 deletion ecosystem/typescript/sdk/src/token_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ export interface TokenId {
property_version: string;
}

/** server will return string for u64 */
type U64 = string;

export interface Token {
id: TokenId;
amount: number;
/** server will return string for u64 */
amount: U64;
}