-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Delist untraded assets #1861
Delist untraded assets #1861
Conversation
I wanted to do that but on my prio list there is so much more so I am not sure if I will have time. Maybe @blabno find time to delist not traded coins. I postes at a proposal the stats and added code to easily print it out. Mid term plan was to have that automated. I think removing would already work but we should have some flexibility to re-enable them (by fee payment). Have it on my todo list for the missing dao features, but as said i keep it rather low prio... |
@ManfredKarrer wrote:
Sure. I could possibly do it too, just want to make sure we do it correctly.
That's right. Here they are: bisq-network/proposals#41 (comment). That's a complete list (as of the date of the posting) of which coins should be delisted. Is it as simple as just removing the
Can you provide a link to that code, and indicate how to run it if it's not obvious? |
It should be enough to remove the entry from meta-inf/services.
Sent from ProtonMail mobile
…-------- Original Message --------
On 3 Nov 2018, 17:09, Chris Beams wrote:
***@***.***(https://github.com/ManfredKarrer) wrote:
> Maybe ***@***.***(https://github.com/blabno) find time to delist not traded coins.
Sure. I could possibly do it too, just want to make sure we do it correctly.
> I posted at a proposal the stats
That's right. Here they are: [bisq-network/proposals#41 (comment)](bisq-network/proposals#41 (comment)). That's a complete list (as of the date of the posting) of which coins should be delisted.
Is it as simple as just removing the Asset implementations, though? Is there anything else we would need to do to put together a proper backward compatible PR? We talked about implementing delisting under the DAO as logical instead of physical deletion, but that's for the future. For right now, i.e. 0.9.0, I just want to delist this stuff in the simplest way possible while not breaking compatibility.
> [...] and added code to easily print it out
Can you provide a link to that code, and indicate how to run it if it's not obvious?
—
You are receiving this because you were mentioned.
Reply to this email directly, [view it on GitHub](#1861 (comment)), or [mute the thread](https://github.com/notifications/unsubscribe-auth/AAaT2FYLUmelJtJT_dVyfMeifxVylaofks5urb-wgaJpZM4YMxg8).
|
Right; when asking if it's "enough" to just remove the implementations, I was referring more to compatibility concerns at the trade protocol level and across versions. Just don't want to do a naive removal of code / entries in META-INF. Perhaps it is just that simple, but wanted to double-check first. |
@cbeams @blabno Ah actually removing code can break users accounts who have used that altcoin. So we need to mark them as hidden. I think I implemented that already (AssetService.class but its kind of WIP). I will have a look on that a bit later. Being busy atm with bonds... I think we just can enable the automatic delisting and add a re-listing feature later... |
The code for printing the inactive assets is in TradeStatisticsManager.checkTradeActivity(). The log at the end are set to debug level, if u change that to info u will get an up to date list. |
- Remove assets from META-INF/services/bisq.asset.Asset - Preserve asset types but mark as @deprecated - @ignore asset tests Preserving the types is important from a compatibility perspective. Users who have traded these assets in the past, however few there may be, need to be able to classload the asset type(s) in order to avoid errors when browsing through their trade portfolio history.
I've implemented the necessary changes and converted this issue into a pull request. @ManfredKarrer, could you please review? @blabno, for now we'll need to start adding newly listed assets to |
Thanks @cbeams ! I need to test what happens if a user has used a coin in his altcoin accounts and if we remove it from code. Will update after that... |
Perfect, thanks. |
@cbeams Why is Koto and Obsidian not removed? Both have been added before 11. May. EDIT: Ah I see they are removed from the registry but code is still there. |
Here the current lists with changed parmeter for required volume to 0.001 BTC. Assets to remove: Insufficiently traded assets:Not traded assets: New assets (in warming up phase): Sufficiently traded assets (17): |
I will merge it as it is now. Still wondering why there are some files left with deprecated annotation. And we should also remove the 3 remaining assets, but that can be done before release with an updated check. |
Hey guys! We were waiting the release with our coin (CHA) to start using Bisq. The merge was this, but we wont see our CHA in the list with AltCoins from the BisQ software downloaded from the download page. Please, could you help us with this issue? That's why we havn't made any trade. |
Would it be possible to list the Nano(NANO) again? When it was listed, much of the community did not know about Bisq. Official project URL: https://nano.org |
Per https://docs.bisq.network/exchange/howto/list-asset.html#inactive-assets-will-be-de-listed:
@ManfredKarrer, can you put together a PR for this for 0.9.0? In subsequent releases, perhaps @blabno can keep an eye on this and put together similar PRs that follow the template you lay out in this first one.
And can you also include a description as to how you create the list of assets to be delisted based on the trade statistics? Again, this would be so that others can easily follow suit (and possibly automate it) in the future.