[bisq-network/bisq] [WIP] PriceNode: Add support for multiple ExchangeRateProviders (#4315)

Chris Beams notifications at github.com
Thu Jul 23 13:32:10 UTC 2020


Disclaimer: I've taken about 15 minutes to go through everything you've posted above. I haven't been hands on with it, and I haven't scrutinized it closely enough to really poke holes in it, but I wanted to get back sooner than later. With that said, in general, the thinking and approach looks great; I'm impressed by the amount of effort you've put into it already.

One question I don't believe you addressed above: what actually happens in the failure mode of the price tolerance being out of range, i.e. over 1% difference? How do users know what went wrong, and how, if at all, can they recover? The answer to this question shouldn't be much different than the status quo, i.e. before applying this patch, but it's just a failure mode I've never personally seen. Do we give people good error messages, etc? It could be argued that that's out of scope for this change, but since this change considerably increases the likelihood of this failure mode occurring, we should make sure the failure is a graceful as it can be.

So I don't have much more detailed feedback, other than that this seems directionally correct. Ideally, you'd find someone to pair up with this on and really complete the testing together in real-time, as opposed to just asking someone to review the plan. Two sets of eyes / minds would probably really help in a situation like this (and would probably be more fun less tedious than doing it alone). Any volunteers, @bisq-network/bisq-devs? Maybe pairing up on this is something you could bring up on the next dev call (I haven't been tracking the plan for those calls, just an idea).

Another note would be that once whatever diligence is done on the testing front and it seems reasonable to roll this out, I'd suggest having everyone involved (pricenode operators, bisq maintainers) primed and ready to roll back as soon as we start seeing something unexpected. For example, make sure the changes in this PR are as easy to revert as possible (multiple commits are fine—you can revert a whole merge, just want to make sure it doesn't have any extra stuff that might make the revert complicated). And make sure that the pricenode operators know that this upgrade is a relatively risky one and that we might call for a rollback (or upgrade to the new, reverted version) in short order.

I hope that helps, I realize it's all pretty high level and perhaps obvious, but that's about as much time as I have to spend on this for now.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/4315#issuecomment-663008626
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200723/e2d37e99/attachment.html>


More information about the bisq-github mailing list