fix(nft): fix notes for 1.0.6-beta

[laruh]

• Address was added in nft and transaction objects:
  solves notes in chore(release): v1.0.6-beta #1871
• typos were fixed:
  solves notes in chore(release): v1.0.6-beta #1871
• use guard: Arc<AsyncMutex<()>> from struct NftCtx to lock nft functions which uses db:
  solves mm2 error from update_nft request return error on the first start of browser app in incognito tab and send errors to console #1909
• use IndexedDB Cursor collect method:
  fixes runtime cursor error in wasm Uncaught Error: closure invoked recursively or after being dropped related to IdbCursor::continue_ and IdbIndex::open_cursor_with_range_and_direction
  also error from update_nft request return error on the first start of browser app in incognito tab and send errors to console #1909

[laruh]

@Alrighttt @ozkanonur please review Address type for nft addresses and prepared stmt.

[shamardy]

Thank you for the fixes! First review iteration!

[shamardy]

I think instead of a guard you should have the nft_cache_db in the Nft context

komodo-defi-framework/mm2src/coins/lp_coins.rs

Line 2577 in 806ac63

nft_cache_db: ConstructibleDb::new(ctx).into_shared(),

then it can be initialized only once when from_ctx first called and then retrieved on subsequent calls and this is done using from_ctx

komodo-defi-framework/mm2src/mm2_core/src/mm_ctx.rs

Line 608 in eaed80e

pub fn from_ctx<T, C>(

like how it's used in CoinsContext

komodo-defi-framework/mm2src/coins/lp_coins.rs

Line 2562 in 806ac63

Ok(try_s!(from_ctx(&ctx.coins_ctx, move || {

This is not a blocker since guard works right now, but it's better code structure since now we have CoinsContext and NftCtx and nft_cache_db is in CoinsContext. I think you can remove the need to build storage in every nft function also following from the above.

[laruh]

I think instead of a guard you should have the nft_cache_db in the Nft context

without mutex guard, we will have the same mm2 problem due to race condition.

{"mmrpc":"2.0","error":"DB error ErrorSaving(\"Error uploading an item: \\\"DomException { obj: Object { obj: JsValue(ConstraintError: Unable to add key to index 'chain_tx_hash_index': at least one key does not satisfy the uniqueness requirements.\\\\nundefined) } }\\\"\")","error_path":"nft.wasm_storage.object_store","error_trace":"nft:115] wasm_storage:362] object_store:42]","error_type":"DbError","error_data":"ErrorSaving(\"Error uploading an item: \\\"DomException { obj: Object { obj: JsValue(ConstraintError: Unable to add key to index 'chain_tx_hash_index': at least one key does not satisfy the uniqueness requirements.\\\\nundefined) } }\\\"\")","id":null}

PS: I also caught this problem in native target. Sql propagates the error that you are trying to add item with identical primary key.

I mean if we just move nft_cache_db to NftCtx, we will have the same situation. Instead of CoinsContext::from_ctx(ctx) we will just have NftCtx::from_ctx(&ctx) here

Also dont forget, that nft_cache_db is for wasm target, for native we use sqlite_connection from mmctx.

We need to lock the whole function before using storage and sending the moralis request. So I believe it doesn't mater, where we have nft_cache_db or build storage, we need to prevent the access to storage and sending identical moralis request before previous RPC is done.

But yeah, it is a good idea, bcz nft_cache_db should be a part of context of related feature. I didnt notice it.

I think you can remove the need to build storage in every nft function also following from the above.

yep, I have it in my plans. Actually I was trying to move the process of NftBuilder creation and NftStorage building to NftCtx as in this example, but I faced with problem during creating of Box with type impl NftListStorageOps + NftTxHistoryStorageOps. Moreover traits have type Error: NftStorageError. So I postponed this idea until the next refactoring.

[laruh]

moved nft_cache_db field to struct NftCtx

[Alrighttt]

LGTM

[smk762]

Testing NFT database successful:

On first load, nft list and history empty until update_nft called.
Ddid some transfers, from external and to self, and each time no change to list/history, until update called. No errors encountered.
On self send, saw block number go up in list as expected. No errors encountered.
After logging out and then in again, previous stored list/history was visible without needing to call update.