atomicDEX-API icon indicating copy to clipboard operation
atomicDEX-API copied to clipboard

fix(nft): add token_id field to the tx history primary key, fix balance

Open laruh opened this issue 1 year ago • 6 comments

fixes https://github.com/KomodoPlatform/komodo-defi-framework/issues/2208

also in this commit https://github.com/KomodoPlatform/komodo-defi-framework/pull/2209/commits/2bdee4f85e23fef6b8b85d3545d1f972047da220 fixes withdarw_erc1155 decimal issue. ERC1155 "balanceOf" method started returning exact Nft amount owned by wallet without any scaling, so we dont need to use self.decimals shift when we convert U256 type to BigDecimal

laruh avatar Sep 03 '24 02:09 laruh

@borngraced can you please review the wasm / indexeddb changes?

shamardy avatar Sep 23 '24 09:09 shamardy

@borngraced can you please review the wasm / indexeddb changes?

ps right now it doesnt work as expected.

Screenshot 2024-09-23 at 16 42 34 Screenshot 2024-09-23 at 16 43 15

We use web-sys to access Web APIs available in the browser. Despite the fact that web-sys functions are sync, the actuall process in browser can be async (that what happens with IndexedDB). I suppose thats why I got A mutation operation was attempted on a database that did not allow mutations. error. In another branch Im working on async versions of on_upgrade_needed and with_table function to see will it work.

UPD: I also suggest to check for wasm db clients which support migrations.

laruh avatar Sep 23 '24 09:09 laruh

@borngraced can you please review the wasm / indexeddb changes?

ps right now it doesnt work as expected.

Screenshot 2024-09-23 at 16 42 34 Screenshot 2024-09-23 at 16 43 15

We use web-sys to access Web APIs available in the browser. Despite the fact that web-sys functions are sync, the actuall process in browser can be async (that what happens with IndexedDB). I suppose thats why I got A mutation operation was attempted on a database that did not allow mutations. error. In another branch Im working on async versions of on_upgrade_needed and with_table function to see will it work.

UPD: I also suggest to check for wasm db clients which support migrations.

Have checked the cursor implementation example on how async and channels are used?

borngraced avatar Sep 23 '24 10:09 borngraced

@borngraced can you please review the wasm / indexeddb changes?

ps right now it doesnt work as expected. Screenshot 2024-09-23 at 16 42 34 Screenshot 2024-09-23 at 16 43 15 We use web-sys to access Web APIs available in the browser. Despite the fact that web-sys functions are sync, the actuall process in browser can be async (that what happens with IndexedDB). I suppose thats why I got A mutation operation was attempted on a database that did not allow mutations. error. In another branch Im working on async versions of on_upgrade_needed and with_table function to see will it work. UPD: I also suggest to check for wasm db clients which support migrations.

Have checked the cursor implementation example on how async and channels are used?

yes, in the end cursor usage requires to call "await" somewhere to wait for the end of the process. When we init table only build function is async

    async fn init(db_id: DbIdentifier) -> InitDbResult<Self> {
        let inner = IndexedDbBuilder::new(db_id)
            .with_version(...)
            .with_table::<...>()
            .build()
            .await?;

        Ok(Self { inner })
    }

The other idea is that, the current migration fails, bcz it started earlier than upgrade permission was granted in build function https://github.com/KomodoPlatform/komodo-defi-framework/blob/e8be55680d6c8e2a28eef26b00e22c5b89969e76/mm2src/mm2_db/src/indexed_db/drivers/builder.rs#L87-L91

UPD: may be it worth trying to reuse our cursor driver implementation withour making with_table async

laruh avatar Sep 23 '24 10:09 laruh

@borngraced can you please review the wasm / indexeddb changes?

ps right now it doesnt work as expected. Screenshot 2024-09-23 at 16 42 34 Screenshot 2024-09-23 at 16 43 15 We use web-sys to access Web APIs available in the browser. Despite the fact that web-sys functions are sync, the actuall process in browser can be async (that what happens with IndexedDB). I suppose thats why I got A mutation operation was attempted on a database that did not allow mutations. error. In another branch Im working on async versions of on_upgrade_needed and with_table function to see will it work. UPD: I also suggest to check for wasm db clients which support migrations.

Have checked the cursor implementation example on how async and channels are used?

yes, in the end cursor usage requires to call "await" somewhere to wait for the end of the process. When we init table only build function is async

    async fn init(db_id: DbIdentifier) -> InitDbResult<Self> {
        let inner = IndexedDbBuilder::new(db_id)
            .with_version(...)
            .with_table::<...>()
            .build()
            .await?;

        Ok(Self { inner })
    }

The other idea is that, the current migration fails, bcz it started earlier than upgrade permission was granted in build function

https://github.com/KomodoPlatform/komodo-defi-framework/blob/e8be55680d6c8e2a28eef26b00e22c5b89969e76/mm2src/mm2_db/src/indexed_db/drivers/builder.rs#L87-L90

UPD: may be it worth trying to reuse our cursor driver implementation withour making with_table async

I will dedicate some time to this later today/tomorrow morning and get back to you for sure!

borngraced avatar Sep 23 '24 11:09 borngraced

@borngraced can you please review the wasm / indexeddb changes?

ps right now it doesnt work as expected. Screenshot 2024-09-23 at 16 42 34 Screenshot 2024-09-23 at 16 43 15 We use web-sys to access Web APIs available in the browser. Despite the fact that web-sys functions are sync, the actuall process in browser can be async (that what happens with IndexedDB). I suppose thats why I got A mutation operation was attempted on a database that did not allow mutations. error. In another branch Im working on async versions of on_upgrade_needed and with_table function to see will it work. UPD: I also suggest to check for wasm db clients which support migrations.

Have checked the cursor implementation example on how async and channels are used?

yes, in the end cursor usage requires to call "await" somewhere to wait for the end of the process. When we init table only build function is async

    async fn init(db_id: DbIdentifier) -> InitDbResult<Self> {
        let inner = IndexedDbBuilder::new(db_id)
            .with_version(...)
            .with_table::<...>()
            .build()
            .await?;

        Ok(Self { inner })
    }

The other idea is that, the current migration fails, bcz it started earlier than upgrade permission was granted in build function https://github.com/KomodoPlatform/komodo-defi-framework/blob/e8be55680d6c8e2a28eef26b00e22c5b89969e76/mm2src/mm2_db/src/indexed_db/drivers/builder.rs#L87-L90

UPD: may be it worth trying to reuse our cursor driver implementation withour making with_table async

I will dedicate some time to this later today/tomorrow morning and get back to you for sure!

@laruh have you seen this yet? https://github.com/KomodoPlatform/komodo-defi-framework/blob/35e92394928825c337f246f8e19fbfab1f65c4a8/mm2src/mm2_main/src/lp_swap/saved_swap.rs#L274-L340

borngraced avatar Sep 24 '24 00:09 borngraced

@laruh have you seen this yet?

https://github.com/KomodoPlatform/komodo-defi-framework/blob/35e92394928825c337f246f8e19fbfab1f65c4a8/mm2src/mm2_main/src/lp_swap/saved_swap.rs#L274-L340

You cant change db schema outside onupgradeneeded.

The get_current_migration and migrate_swaps_data functions were used, when we only needed to add new index to MySwapsFiltersTable https://github.com/KomodoPlatform/komodo-defi-framework/blob/ce183766480abafb7275a10f384f308c60711118/mm2src/mm2_main/src/lp_swap/swap_wasm_db.rs#L112

So you could without problems fill empty new index outside onupgradeneeded request. While in the current issue, we need to update the existing index. create new one, copy data from old to the new, and delete old index. It should be done during on_upgrade_needed, as this process requires table schema changes.

Im planing to try two ways:

  • Call db_request.set_onupgradeneeded(Some(onupgradeneeded_closure.as_ref().unchecked_ref())) after table build
  • Make fn on_upgrade_needed async in TableSignature trait, and do the necessary nft tx history table changes inside on_upgrade_needed Note: we have two fn on_upgrade_needed implementations for IdbDatabaseBuilder and from TableSignature trait

laruh avatar Oct 04 '24 12:10 laruh

@borngraced @shamardy All is good on IndexedDB side in our on_upgrade_needed function implementations from TableSignature trait. Found out that when adding a new unique index during a database upgrade, IndexedDB automatically updates it with the existing records, eliminating the need for manual re-indexing.

We have pretty good our own representation of https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API with low-level Rust wrappers over basic JS structures, so I suggest to make this crate as a separate repo (during kdf refactoring) and deploy on crate.io. So we can present komodo-idb-rs crate. There is small number of developed indexeddb libs for rust, which is good opportunity for us.

I remember about the need to create an issue about new db clients suggestion for sql part, so also will add komodo idb crate note to it.

laruh avatar Oct 14 '24 10:10 laruh

Created two issues about sql db migration tools https://github.com/KomodoPlatform/komodo-defi-framework/issues/2243 and separate indexed db repo https://github.com/KomodoPlatform/komodo-defi-framework/issues/2244

laruh avatar Oct 15 '24 11:10 laruh