search

interledger / rafiki

An open-source, comprehensive Interledger service for wallet providers, enabling them to provide Interledger functionality to their users.
https://rafiki.dev/
Apache License 2.0
233 stars 83 forks source link

feat(2589): soft delete unused asset #2699

Closed lengyel-arpad85 closed 4 months ago

lengyel-arpad85 commented 4 months ago

Soft delete asset including the Admin frontend part, by adding a deletedAt date and filtering get & getAll calls for deletedAt being NULL This approach was adopted instead of hard delete since TigerBeetle has no support for deleting rows

Checklist

netlify[bot] commented 4 months ago

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
Latest commit bdc5e1b1ac74df203af21ef24c15586970dd5077
Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/66525ff288f2810008a91358
lengyel-arpad85 commented 4 months ago

My main question is, what should we do when we delete an asset, then try to re-create another one with the same code and scale (via createAsset mutation)?

Right now, we will get a "duplicate asset" error, but that might be a bit confusing given the asset isn't visible in the UI. Maybe we can just set deletedAt to be null again, and return the asset?

yes, that might work, but if the scale is different from what we had before, do we just update the scale to the new one ?

mkurapov commented 4 months ago

@arpad-lengyel

yes, that might work, but if the scale is different from what we had before, do we just update the scale to the new one ?

An asset is unique on scale and code together, so that would signify a new/different asset

lengyel-arpad85 commented 4 months ago

@arpad-lengyel

yes, that might work, but if the scale is different from what we had before, do we just update the scale to the new one ?

An asset is unique on scale and code together, so that would signify a new/different asset

@mkurapov It's an unused asset, so in theory it should not matter what the previous scale was, it was never used, so it won't have an effect on anything, no ?

mkurapov commented 4 months ago

It's an unused asset, so in theory it should not matter what the previous scale was, it was never used, so it won't have an effect on anything, no ?

Right, however I would say to be safe, we should treat e.g. USD/9 vs USD/2 as completely different assets (even if they have the same asset code). This is reflected in the DB as well:

https://github.com/interledger/rafiki/blob/8835414f0f478b30ab7ee80a54e8af39ac3b1904/packages/backend/migrations/20210422194130_create_assets_table.js#L15

lengyel-arpad85 commented 4 months ago

It's an unused asset, so in theory it should not matter what the previous scale was, it was never used, so it won't have an effect on anything, no ?

Right, however I would say to be safe, we should treat e.g. USD/9 vs USD/2 as completely different assets (even if they have the same asset code). This is reflected in the DB as well:

https://github.com/interledger/rafiki/blob/8835414f0f478b30ab7ee80a54e8af39ac3b1904/packages/backend/migrations/20210422194130_create_assets_table.js#L15

yes, you are right, I've corrected the condition in my code