bisq-network / projects

@bisq-network project management
https://bisq.wiki/Project_management
9 stars 2 forks source link

Remove the need for Bisq to trust Bitcoin Average by querying exchange APIs directly and calculating our own weighted average for fiat and altcoin prices #35

Closed wiz closed 4 years ago

wiz commented 4 years ago

_This is a Bisq Network project. Please familiarize yourself with the project management process._

Approved in https://github.com/bisq-network/proposals/issues/163 and fixes https://github.com/bisq-network/bisq/issues/1074

Screen Shot 2020-08-10 at 1 32 14

Description

This project will remove the need for Bisq network to trust BitcoinAverage as a data oracle and provide fiat and altcoin prices, by implementing our own weighted average price index for fiat and altcoin prices calculated from querying 10 or more Bitcoin Exchange APIs directly.

Rationale

Currently the Bisq Pricenode Operators all subscribe to expensive Bitcoin Average API subscription plans, which is a large recurring expense for the Ops team budget. It's a no-brainer to do this project financially, since we will no longer have to pay these recurring monthly expenses for API subscriptions, but much more importantly in terms of our goals to improve reliability and censorship-resistance, this project will remove a very centralized TTP and CPOF for Bisq by decentralizing the data sources. If BitcoinAverage were to suddenly ban Bisq, we would be in trouble.

Criteria for delivery

When all the items in Part 1 below are completed, we can consider this project to have been delivered.

Measures of success

After we have shut down all Bisq Pricenodes using Bitcoin Average, and Bisq is running stable using the new decentralized providers, we can consider this project to be successful.

Risks

The new code could crash our Pricenodes. The new code could calculate prices incorrectly. The pricenodes might disagree on the price for a certain fiat or crypto asset, causing network issues. More risks probably exist which we cannot know.

Tasks

Part 1: Implementation

Part 2: Documentation

Part 3: Testing

Part 4: Migration

Estimates for budget allocation

Dev: $5000 Ops: $3000

sqrrm commented 4 years ago

I like this idea in principle although I'm not familiar with the different feeds and how much work it would be to use all the different data feeds. It is certainly a weakness to rely on BitcoinAverage for all our market data.

I do see a risk in inconsistent price feeds though using an aggregate of feeds from different exchanges calculated at each node. We have seen people abusing the tolerance margin introduced to allow for difference in price feed before.

wiz commented 4 years ago

I'm not familiar with the different feeds and how much work it would be to use all the different data feeds.

Since @cd2357 worked on the Pricenode PR that implemented https://github.com/bisq-network/projects/issues/27, I've asked him to work on the PR for this project as well. I understand he already has a WIP that enables Pricenodes to directly query the 10+ centralized exchange APIs directly without adding any jar deps to Bisq. Since the jar dep library that Bisq already uses to query Exchange APIs has now added support for over 80 exchanges, it's not much work to add these new exchanges: https://github.com/knowm/XChange

I do see a risk in inconsistent price feeds though using an aggregate of feeds from different exchanges calculated at each node.

While this is possible, in practice I don't think it will be an issue as we have price thresholds to resolve this. In any case, it will require lots of testing and a slow phase-in to the new system.

We have seen people abusing the tolerance margin introduced to allow for difference in price feed before.

Yes, but AFAIK the only cases of this in the past were caused by BitcoinAverage flaws, which would be resolved by this proposal. For example, BA used LocalBitcoins for SEK prices instead of the largest Bitcoin Exchange in Sweden called Safello, which made Bisq vulnerable to someone trading a small amount of LocalBitcoins to move the Bisq price and buy Bitcoin below market: https://bisq.community/t/btc-sek-is-way-off/6558

sqrrm commented 4 years ago

The issue I'm talking about is for buyer and seller to agree on price. We have a very narrow allowance to avoid someone tries to game it. If we have multiple sources it might not be possible to have such a narrow spread, or it won't be possible to take offers.

I'm not sure how a phase-in would work considering the buyer and seller has to come to the same price, or they won't be able to trade. If they have different sources it's much more likely that they will disagree on price and taking the trade will fail.

cd2357 commented 4 years ago

My understanding is that

There are about 6 pricenodes now which basically run the same code and should provide the same exchange rate data (within some margin of error, since the pricenodes could poll BitcoinAverage within a few seconds from each other, so they could get slightly different data).

The Bisq clients then poll these pricenodes (one of them? several of them? not sure) and for each currency pair, the exchange rate with the latest timestamp gets used in the client.

So if I understand this correctly, the only ways two Bisq clients can have massively different exchange rates is either:

I guess this is possible now as well, but Bisq defends against this by using this narrow allowance (which you mentioned @sqrrm ) for an acceptable exchange rate difference between traders. So even if a Bisq client tries to game the system and use favorable exchange rates at the expense of someone they trade with, the protocol would not allow a big difference between their rates.

Again, assuming my understanding is correct, what this project proposes is to have the pricenodes use more providers to get exchange rate data from multiple exchanges. Then each pricenode would aggregate the data and calculate some kind of "weighted average" (prioritizing values from exchanges with more liquidity, or the more established ones, or the more reputable, etc; specific logic TBD). When Bisq clients poll the pricenodes, instead of getting the BitcoinAverage rates, they would get these weighted average rates.

And if all 6 Bisq pricenodes use the same code, meaning

then they should basically report very similar exchange rates to the Bisq clients.

Basically it's a similar situation to now, cause the pricenodes could report slightly different BitcoinAverage rates (cause they query BitcoinAverage at different times).

So I think this risk exists only to the same degree that it already exists now, and the narrow allowance check in the Bisq clients already mitigate against it.

But please correct me if I'm wrong, it's quite a complex system so it's easy to overlook smth.

wiz commented 4 years ago

As the ops team lead, I am assigning this project to @cd2357 and I have allocated budget for his compensation for work under this project from the ops team budget.

wiz commented 4 years ago

Now that @cd2357's PR seems to be working well, we can proceed to have the pricenode operators setup new instances for testing. Can all the @bisq-network/pricenode-operators please make a fresh Ubuntu 20.04 VM and download the pricenode installation script from @cd2357's PR branch and replace these 4 lines then run it locally:

BISQ_REPO_URL=https://github.com/cd2357/bisq
BISQ_REPO_NAME=bisq
BISQ_REPO_TAG=xchange-integration-introduce-aggregate-rates
BISQ_LATEST_RELEASE=xchange-integration-introduce-aggregate-rates

After you get your new Tor V3 onion, please comment here and after we have several new pricenodes running we can begin full testing of the new pricenode code:

Also @bisq-network/pricenode-operators be advised this project is a 🚨 high risk change 🚨 and that we might call for a rollback (or upgrade to the new, reverted version) in short order after the migration.

alexej996 commented 4 years ago

I just installed the service and it seems to be running. The onion is ro7nv73awqs3ga2qtqeqawrjpbxwarsazznszvr6whv7tes5ehffopid.onion

mrosseel commented 4 years ago

aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd.onion

wiz commented 4 years ago

Okay, now that we have 5 pricenodes online the staging environment is ready for full testing. The branch I'll be using to test is https://github.com/bisq-network/bisq/pull/4403

devinbileck commented 4 years ago

My node is now running with the following onion address: devinpndvdwll4wiqcyq5e7itezmarg7rzicrvf6brzkwxdm374kmmyd.onion

wiz commented 4 years ago
Screen Shot 2020-08-10 at 1 32 14

@sqrrm @cbeams Okay I think we are ready for review. Please see the following PRs: https://github.com/bisq-network/bisq/pull/4315 https://github.com/bisq-network/bisq/pull/4403 https://github.com/bisq-network/bisq/pull/4406

wiz commented 4 years ago

I've been doing a lot of testing and found no issues so far. As for the migration to repoint old V2 onions to the new pricenodes, I would say after the above 3 PRs are merged and there are no issues found, once v1.3.8 gets pre-released the pricenode operators should all coordinate a time for everyone to switch at the same time. When there are no issues from the migration, we can safely release v1.3.8 with the new V3 onions hard-coded.

@ripcurlx what's the tentative schedule for the v1.3.8 release timeline?

Emzy commented 4 years ago

I would like to change my HS to: emzypricpidesmyqg2hc6dkwitqzaxrqnpkdg3ae2wef5znncu2ambqd

wiz commented 4 years ago

lol ok. will amend PR.

wiz commented 4 years ago

@devinbileck are you planning to do full release testing for v1.3.8 ? obviously we will want to emphasize anything that uses prices

wiz commented 4 years ago

@m52go Since the new Bisq Price Indices are different from Bitcoin Average prices, in the case of BTC/USD about $100 or 1%, we should probably give some advance warning to users about the migration and @ripcurlx maybe an in-app broadcast message reminding users of the change a few days/hours/minutes in advance. What do you think?

💰Bisq Pricenode Data Check on Sun Aug 9 21:05:01 UTC 2020

xc3nh4juf2hshy7e.onion - BTC/USD: $11685.37
ceaanhbvluug4we6.onion - BTC/USD: $11685.37
44mgyoe2b6oqiytt.onion - BTC/USD: $11685.37
62nvujg5iou3vu3i.onion - BTC/USD: $11685.22
wizpriceje6q5tdrxkyiazsgu7irquiqjy2dptezqhrtu7l2qelqktid - BTC/USD: $11584.77
emzypricpidesmyqg2hc6dkwitqzaxrqnpkdg3ae2wef5znncu2ambqd - BTC/USD: $11584.719
aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd - BTC/USD: $11582.67
devinpndvdwll4wiqcyq5e7itezmarg7rzicrvf6brzkwxdm374kmmyd - BTC/USD: $11585.20
ro7nv73awqs3ga2qtqeqawrjpbxwarsazznszvr6whv7tes5ehffopid - BTC/USD: $11582.83
wiz commented 4 years ago

Since the Bisq Pricenodes were all migrated to the new code, and Bisq is no longer using Bitcoin Average as a price oracle, calculating our own weighted averages (documented at https://bisq.wiki/Bisq_Price_Indices), I am closing this project as was:delivered - example of pricenode monitoring logs follow:

💰Bisq Pricenode Price Check on Thu Aug 20 09:05:02 UTC 2020 BTC/USD:

xc3nh4juf2hshy7e 11809 USD
ceaanhbvluug4we6 11809 USD
44mgyoe2b6oqiytt 11809 USD
62nvujg5iou3vu3i 11809 USD
gztmprecgqjq64zh 11809 USD
wizpriceje6q5tdr 11809 USD
emzypricpidesmyq 11808 USD
aprcndeiwdrkbf4f 11809 USD
devinpndvdwll4wi 11809 USD
ro7nv73awqs3ga2q 11809 USD

BTC/EUR:

xc3nh4juf2hshy7e 9950 EUR
ceaanhbvluug4we6 9951 EUR
44mgyoe2b6oqiytt 9951 EUR
62nvujg5iou3vu3i 9951 EUR
gztmprecgqjq64zh 9952 EUR
wizpriceje6q5tdr 9952 EUR
emzypricpidesmyq 9951 EUR
aprcndeiwdrkbf4f 9951 EUR
devinpndvdwll4wi 9951 EUR
ro7nv73awqs3ga2q 9951 EUR

BTC/GBP:

xc3nh4juf2hshy7e 9003 GBP
ceaanhbvluug4we6 9004 GBP
44mgyoe2b6oqiytt 9003 GBP
62nvujg5iou3vu3i 9003 GBP
gztmprecgqjq64zh 9003 GBP
wizpriceje6q5tdr 9003 GBP
emzypricpidesmyq 9003 GBP
aprcndeiwdrkbf4f 9003 GBP
devinpndvdwll4wi 9003 GBP
ro7nv73awqs3ga2q 9003 GBP

BTC/BRL:

xc3nh4juf2hshy7e 65467 BRL
ceaanhbvluug4we6 65457 BRL
44mgyoe2b6oqiytt 65469 BRL
62nvujg5iou3vu3i 65468 BRL
gztmprecgqjq64zh 65469 BRL
wizpriceje6q5tdr 65469 BRL
emzypricpidesmyq 65457 BRL
aprcndeiwdrkbf4f 65457 BRL
devinpndvdwll4wi 65457 BRL
ro7nv73awqs3ga2q 65468 BRL

BTC/CAD:

xc3nh4juf2hshy7e 15565 CAD
ceaanhbvluug4we6 15565 CAD
44mgyoe2b6oqiytt 15565 CAD
62nvujg5iou3vu3i 15565 CAD
gztmprecgqjq64zh 15565 CAD
wizpriceje6q5tdr 15565 CAD
emzypricpidesmyq 15565 CAD
aprcndeiwdrkbf4f 15565 CAD
devinpndvdwll4wi 15565 CAD
ro7nv73awqs3ga2q 15565 CAD

BTC/AUD:

xc3nh4juf2hshy7e 16482 AUD
ceaanhbvluug4we6 16484 AUD
44mgyoe2b6oqiytt 16484 AUD
62nvujg5iou3vu3i 16482 AUD
gztmprecgqjq64zh 16481 AUD
wizpriceje6q5tdr 16481 AUD
emzypricpidesmyq 16482 AUD
aprcndeiwdrkbf4f 16485 AUD
devinpndvdwll4wi 16484 AUD
ro7nv73awqs3ga2q 16478 AUD

BTC/JPY:

xc3nh4juf2hshy7e 1248777 JPY
ceaanhbvluug4we6 1248774 JPY
44mgyoe2b6oqiytt 1248755 JPY
62nvujg5iou3vu3i 1248748 JPY
gztmprecgqjq64zh 1248748 JPY
wizpriceje6q5tdr 1248748 JPY
emzypricpidesmyq 1248777 JPY
aprcndeiwdrkbf4f 1248788 JPY
devinpndvdwll4wi 1248755 JPY
ro7nv73awqs3ga2q 1248748 JPY

BTC/SEK:

xc3nh4juf2hshy7e 102632 SEK
ceaanhbvluug4we6 102632 SEK
44mgyoe2b6oqiytt 102632 SEK
62nvujg5iou3vu3i 102629 SEK
gztmprecgqjq64zh 102629 SEK
wizpriceje6q5tdr 102629 SEK
emzypricpidesmyq 102632 SEK
aprcndeiwdrkbf4f 102632 SEK
devinpndvdwll4wi 102632 SEK
ro7nv73awqs3ga2q 102632 SEK

BTC/XMR:

xc3nh4juf2hshy7e 0.007942267142857142 BTC
ceaanhbvluug4we6 0.007942571428571429 BTC
44mgyoe2b6oqiytt 0.007942085714285714 BTC
62nvujg5iou3vu3i 0.007942942857142856 BTC
gztmprecgqjq64zh 0.007942942857142856 BTC
wizpriceje6q5tdr 0.007942942857142856 BTC
emzypricpidesmyq 0.00794241 BTC
aprcndeiwdrkbf4f 0.007942571428571429 BTC
devinpndvdwll4wi 0.007942085714285714 BTC
ro7nv73awqs3ga2q 0.007942267142857142 BTC

BTC/ZEC:

xc3nh4juf2hshy7e 0.006584737142857143 BTC
ceaanhbvluug4we6 0.006583867142857143 BTC
44mgyoe2b6oqiytt 0.006585022857142857 BTC
62nvujg5iou3vu3i 0.006584737142857143 BTC
gztmprecgqjq64zh 0.006584737142857143 BTC
wizpriceje6q5tdr 0.006584737142857143 BTC
emzypricpidesmyq 0.006584737142857143 BTC
aprcndeiwdrkbf4f 0.006583867142857143 BTC
devinpndvdwll4wi 0.0065851657142857135 BTC
ro7nv73awqs3ga2q 0.006584737142857143 BTC

BTC/ETH:

xc3nh4juf2hshy7e 0.03466955625 BTC
ceaanhbvluug4we6 0.034673680625 BTC
44mgyoe2b6oqiytt 0.034674243125000004 BTC
62nvujg5iou3vu3i 0.03466999375 BTC
gztmprecgqjq64zh 0.03467068125 BTC
wizpriceje6q5tdr 0.03467068125 BTC
emzypricpidesmyq 0.034670305625 BTC
aprcndeiwdrkbf4f 0.034673680625 BTC
devinpndvdwll4wi 0.034674618125 BTC
ro7nv73awqs3ga2q 0.03467105625 BTC

BTC/ETC:

xc3nh4juf2hshy7e 0.0005866 BTC
ceaanhbvluug4we6 0.0005866 BTC
44mgyoe2b6oqiytt 0.0005866 BTC
62nvujg5iou3vu3i 0.0005866 BTC
gztmprecgqjq64zh 0.0005866 BTC
wizpriceje6q5tdr 0.0005866 BTC
emzypricpidesmyq 0.0005866 BTC
aprcndeiwdrkbf4f 0.0005866 BTC
devinpndvdwll4wi 0.0005866 BTC
ro7nv73awqs3ga2q 0.0005866028571428571 BTC

BTC/DOGE:

xc3nh4juf2hshy7e 2.94085e-07 BTC
ceaanhbvluug4we6 2.94085e-07 BTC
44mgyoe2b6oqiytt 2.94085e-07 BTC
62nvujg5iou3vu3i 2.94085e-07 BTC
gztmprecgqjq64zh 2.94085e-07 BTC
wizpriceje6q5tdr 2.94085e-07 BTC
emzypricpidesmyq 2.94085e-07 BTC
aprcndeiwdrkbf4f 2.94085e-07 BTC
devinpndvdwll4wi 2.94085e-07 BTC
ro7nv73awqs3ga2q 2.94085e-07 BTC

Thanks a lot to @cd2357 @sqrrm @cbeams and @bisq-network/pricenode-operators for all the hard work for this project, another one for the ops team win list! :tada:

cbeams commented 4 years ago

It appears this change has rolled out quite smoothly. Congrats, all!