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

MTG 1164 Apply showZeroBalance only for NFTs #368

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ßó4'p) Ÿ%¦ GáëÎX·2G¿%Šù£< y‹
Binary file not shown.
Binary file not shown.
1 change: 0 additions & 1 deletion integration_tests/src/mpl_core_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ async fn test_mpl_core_get_collection() {
}

#[tokio::test]
#[ignore = "TODO: must recheck snapshot incompatibility"]
#[serial]
#[named]
async fn test_mpl_core_get_assets_by_authority() {
Expand Down
49 changes: 49 additions & 0 deletions integration_tests/src/regular_nft_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,55 @@ async fn test_regular_nft_collection() {
insta::assert_json_snapshot!(name.clone(), response);
}

#[tokio::test]
#[serial]
#[named]
async fn test_search_by_owner_with_show_zero_balance() {
let name = trim_test_name(function_name!());
let setup = TestSetup::new_with_options(
name.clone(),
TestSetupOptions { network: Some(Network::Mainnet), clear_db: true },
)
.await;

let seeds: Vec<SeedEvent> = seed_token_mints([
"HxhWkVpk5NS4Ltg5nij2G671CKXFRKPK8vy271Ub4uEK", // mint for fungible acc
]);
index_seed_events(&setup, seeds.iter().collect_vec()).await;

let seeds: Vec<SeedEvent> = seed_accounts([
"3rzjtWZcZyvADaT5rrkRwGKWjnuzvK3PDedGMUwpnrrP", // empty token acc from NFT (3yMfqHsajYFw2Yw6C4kwrvHRESMg9U7isNVJuzNETJKG)
"94eSnb5qBWTvxj3gqP6Ukq8bPhRTNNVZrE7zR5yTZd9E", // fungible token with zero balance
]);

index_seed_events(&setup, seeds.iter().collect_vec()).await;

let seeds: Vec<SeedEvent> = seed_nfts([
"BFjgKzLNKZEbZoDrESi79ai8jXgyBth1HXCJPXBGs8sj", // NFT wallet has
"3yMfqHsajYFw2Yw6C4kwrvHRESMg9U7isNVJuzNETJKG", // NFT wallet used to have
]);

index_seed_events(&setup, seeds.iter().collect_vec()).await;

let request = r#"
{
"page": 1,
"limit": 500,
"ownerAddress": "3VvLDXqJbw3heyRwFxv8MmurPznmDVUJS9gPMX2BDqfM",
"tokenType": "all",
"options": {
"showNativeBalance": true, "showZeroBalance": true
}
}
"#;

let mutexed_tasks = Arc::new(Mutex::new(JoinSet::new()));

let request: SearchAssets = serde_json::from_str(request).unwrap();
let response = setup.das_api.search_assets(request, mutexed_tasks.clone()).await.unwrap();
insta::assert_json_snapshot!(name, response);
}

#[tokio::test]
#[serial]
#[named]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: integration_tests/src/general_scenario_tests.rs
assertion_line: 101
assertion_line: 83
expression: response
snapshot_kind: text
---
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: integration_tests/src/general_scenario_tests.rs
assertion_line: 124
assertion_line: 102
expression: response
snapshot_kind: text
---
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
---
source: integration_tests/src/regular_nft_tests.rs
assertion_line: 250
expression: response
snapshot_kind: text
---
{
"total": 2,
"limit": 500,
"page": 1,
"items": [
{
"interface": "Custom",
"id": "HxhWkVpk5NS4Ltg5nij2G671CKXFRKPK8vy271Ub4uEK",
"content": {
"$schema": "https://schema.metaplex.com/nft1.0.json",
"json_uri": "",
"files": [],
"metadata": {
"name": "Hxro (Wormhole)",
"symbol": "HXRO"
},
"links": {}
},
"authorities": [
{
"address": "BCD75RNBHrJJpW4dXVagL5mPjzRLnVZq4YirJdjEYMV7",
"scopes": [
"full"
]
}
],
"compression": {
"eligible": false,
"compressed": false,
"data_hash": "",
"creator_hash": "",
"asset_hash": "",
"tree": "",
"seq": 0,
"leaf_id": 0
},
"grouping": [],
"royalty": {
"royalty_model": "creators",
"target": null,
"percent": 0.0,
"basis_points": 0,
"primary_sale_happened": false,
"locked": false
},
"creators": [],
"ownership": {
"frozen": false,
"delegated": false,
"delegate": null,
"ownership_model": "token",
"owner": "3VvLDXqJbw3heyRwFxv8MmurPznmDVUJS9gPMX2BDqfM"
},
"supply": {
"print_max_supply": 0,
"print_current_supply": 0,
"edition_nonce": 252
},
"mutable": true,
"burnt": false,
"lamports": 5616720,
"executable": false,
"metadata_owner": "metaqbxxUerdq28cj1RbAWkYQm3ybzjb6a8bt518x1s",
"rent_epoch": 18446744073709551615,
"token_info": {
"balance": 0,
"supply": 69421234056477841,
"decimals": 8,
"token_program": "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA",
"associated_token_address": "94eSnb5qBWTvxj3gqP6Ukq8bPhRTNNVZrE7zR5yTZd9E",
"mint_authority": "BCD75RNBHrJJpW4dXVagL5mPjzRLnVZq4YirJdjEYMV7"
}
},
{
"interface": "V1_NFT",
"id": "BFjgKzLNKZEbZoDrESi79ai8jXgyBth1HXCJPXBGs8sj",
"content": {
"$schema": "https://schema.metaplex.com/nft1.0.json",
"json_uri": "",
"files": [],
"metadata": {
"name": "Degen Ape",
"symbol": "DAPE",
"token_standard": "NonFungible"
},
"links": {}
},
"authorities": [
{
"address": "3VvLDXqJbw3heyRwFxv8MmurPznmDVUJS9gPMX2BDqfM",
"scopes": [
"full"
]
}
],
"compression": {
"eligible": false,
"compressed": false,
"data_hash": "",
"creator_hash": "",
"asset_hash": "",
"tree": "",
"seq": 0,
"leaf_id": 0
},
"grouping": [],
"royalty": {
"royalty_model": "creators",
"target": null,
"percent": 0.0,
"basis_points": 0,
"primary_sale_happened": false,
"locked": false
},
"creators": [],
"ownership": {
"frozen": false,
"delegated": false,
"delegate": null,
"ownership_model": "single",
"owner": "3VvLDXqJbw3heyRwFxv8MmurPznmDVUJS9gPMX2BDqfM"
},
"supply": {
"print_max_supply": 0,
"print_current_supply": 0,
"edition_nonce": 255
},
"mutable": true,
"burnt": false,
"lamports": 5616720,
"executable": false,
"metadata_owner": "metaqbxxUerdq28cj1RbAWkYQm3ybzjb6a8bt518x1s",
"rent_epoch": 18446744073709551615,
"token_info": {
"balance": 1,
"supply": 1,
"decimals": 0,
"token_program": "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA",
"associated_token_address": "3oiDhCQMDzxj8NcLfRBCQj3R9mQkE1DnDZfrNbAgruQk",
"mint_authority": "565tK79EKhQSYybx52awrrFhD4m2ty198rrUBmiqq7xo",
"freeze_authority": "565tK79EKhQSYybx52awrrFhD4m2ty198rrUBmiqq7xo"
}
}
]
}
3 changes: 3 additions & 0 deletions migrations/12_fungible_and_assets_indexes.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CREATE INDEX fungible_tokens_fbt_owner_fbt_asset_fbt_balance_idx ON fungible_tokens (fbt_owner, fbt_asset, fbt_balance);

CREATE INDEX assets_ast_owner_type_ast_pubkey_idx ON assets_v3 (ast_owner_type, ast_pubkey);
Copy link
Contributor

Choose a reason for hiding this comment

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

new line please

Copy link
Contributor

Choose a reason for hiding this comment

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

you may also want to drop assets_v3_owner_type as it becomes obsolete

56 changes: 46 additions & 10 deletions postgre-client/src/asset_filter_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ impl PgClient {
if group_clause_required {
query_builder.push(" GROUP BY ast_pubkey, ast_slot_created, ast_slot_updated ");
}
query_builder.push(") ");

// Add ORDER BY clause
let direction = match (&order.sort_direction, order_reversed) {
(AssetSortDirection::Asc, true) | (AssetSortDirection::Desc, false) => " DESC ",
(AssetSortDirection::Asc, false) | (AssetSortDirection::Desc, true) => " ASC ",
};

// the function with a side-effect that mutates the query_builder
if let Some(owner_address) = &filter.owner_address {
Expand All @@ -113,17 +118,37 @@ impl PgClient {
TokenType::Fungible
| TokenType::NonFungible
| TokenType::RegularNFT
| TokenType::CompressedNFT => {},
| TokenType::CompressedNFT => {
query_builder.push(")");
},
TokenType::All => {
query_builder.push(" UNION ");
// For this type of query we do union and apply limit with ordering for both parts of a query.
// It allows us to speed up the query for queries with wallets which has lots of assets.
// Not the cleanest approach.
// TODO: this may be improved
query_builder.push(" ORDER BY ");
query_builder.push(order.sort_by.to_string().replace("ast_", ""));
query_builder.push(direction);

query_builder.push(" LIMIT ");
if let Some(page_num) = page.filter(|&p| p > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as below

let lim = (page_num - 1) * limit;
query_builder.push_bind(lim as i64);
} else {
query_builder.push_bind(limit as i64);
}

query_builder.push(")");

query_builder.push(" UNION ALL ");
query_builder.push(" ( ");
query_builder.push(
"SELECT
ast_pubkey,
ast_slot_created,
ast_slot_updated
FROM assets_v3
JOIN fungible_tokens ON ast_pubkey = fungible_tokens.fbt_asset
JOIN fungible_tokens ON ast_pubkey = fungible_tokens.fbt_asset AND ast_owner_type = 'token'
andrii-kl marked this conversation as resolved.
Show resolved Hide resolved
WHERE ast_supply > 0 AND fbt_owner = ",
);
query_builder.push_bind(owner_address);
Expand All @@ -137,18 +162,29 @@ impl PgClient {
" AND assets_v3.ast_is_collection_verified IS DISTINCT FROM FALSE",
);
}

query_builder.push(" ORDER BY ");
query_builder.push(order.sort_by.to_string());
query_builder.push(direction);

query_builder.push(" LIMIT ");
if let Some(page_num) = page.filter(|&p| p > 1) {
let lim = (page_num - 1) * limit;
Copy link
Contributor

Choose a reason for hiding this comment

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

The condition is wrong. it's page_num*limit. You want to have 2000 records pre filtered for the second page. Also add a test for the second page of pagination of this request. The correct way to do it is you always get a 1-based page and multiply it by the limit

query_builder.push_bind(lim as i64);
} else {
query_builder.push_bind(limit as i64);
}

query_builder.push(")");
},
}
} else {
query_builder.push(")");
}
} else {
query_builder.push(")");
}

// Add ORDER BY clause
let direction = match (&order.sort_direction, order_reversed) {
(AssetSortDirection::Asc, true) | (AssetSortDirection::Desc, false) => " DESC ",
(AssetSortDirection::Asc, false) | (AssetSortDirection::Desc, true) => " ASC ",
};

query_builder.push(" ORDER BY ");
query_builder.push(order.sort_by.to_string().replace("ast_", ""));
query_builder.push(direction);
Expand Down
Loading