Skip to content
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

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
6 tasks done
wiz opened this issue Jun 10, 2020 · 18 comments
Assignees
Labels
has:approval bisq.wiki/Project_management#Approval has:budget bisq.wiki/Project_management#Budgeting to:Improve Reliability was:delivered bisq.wiki/Project_management#Closing_as_delivered

Comments

@wiz
Copy link
Member

wiz commented Jun 10, 2020

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

Approved in bisq-network/proposals#163 and fixes bisq-network/bisq#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

  • After we have a working setup with several providers + weighted average per currency pair, begin full testing Update all Pricenodes with new Tor V3 onions bisq#4403
  • Based on testing results, add new providers if necessary, and tweak average weights if necessary, etc. - deemed not necessary

Part 4: Migration

Estimates for budget allocation

Dev: $5000
Ops: $3000

@wiz wiz added a:proposal bisq.wiki/Project_management#Proposal needs:triage bisq.wiki/Project_management#Triage labels Jun 10, 2020
@sqrrm
Copy link
Member

sqrrm commented Jun 10, 2020

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
Copy link
Member Author

wiz commented Jun 10, 2020

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 #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
Copy link
Member

sqrrm commented Jun 10, 2020

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
Copy link

cd2357 commented Jun 10, 2020

My understanding is that

  • the pricenodes
    • have providers which query different exchanges or other endpoints (like BitcoinAverage), that return the latest exchange rates for certain currency pairs
    • schedule such providers to run at different intervals, based on API rate limits
    • expose an API to the Bisq clients, which present the current / latest known exchange rates
  • the Bisq clients
    • query the pricenodes regularly for price data

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:

  • if they change the client code locally and use some custom exchange rate, bypassing the data from the pricenodes, or
  • if they provide a custom pricenode as a commandline argument when starting Bisq
    • they would control the pricenode, so they could get whatever rates they want

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

  • the same providers
  • the same logic for aggregating the values (weighted average params, or simply a normal average from all available providers)

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
Copy link
Member Author

wiz commented Jun 10, 2020

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
Copy link
Member Author

wiz commented Aug 7, 2020

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:

  • @wiz -> wizpriceje6q5tdrxkyiazsgu7irquiqjy2dptezqhrtu7l2qelqktid
  • @Emzy -> emzypricpidesmyqg2hc6dkwitqzaxrqnpkdg3ae2wef5znncu2ambqd
  • @devinbileck -> devinpndvdwll4wiqcyq5e7itezmarg7rzicrvf6brzkwxdm374kmmyd
  • @mrosseel -> aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd
  • @alexej996 -> ro7nv73awqs3ga2qtqeqawrjpbxwarsazznszvr6whv7tes5ehffopid

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
Copy link
Member

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

@mrosseel
Copy link
Member

mrosseel commented Aug 8, 2020

aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd.onion

wiz added a commit to wiz/bisq that referenced this issue Aug 8, 2020
Pending merge of bisq-network#4315 and rough consensus to proceed with migration plan in bisq-network/projects#35

wizpriceje6q5tdrxkyiazsgu7irquiqjy2dptezqhrtu7l2qelqktid (@wiz)
emzyprwrcz22h2fzhsbqd3deoe5hkqdm5yfy4geuqcdqqicgoor65iad (@Emzy)
6b7jpqiy2ejq3m7jskem6cuverg7xelhhuf3d2nvooucayoeapo2m3qd (@devinbileck)
aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd (@mrosseel)
ro7nv73awqs3ga2qtqeqawrjpbxwarsazznszvr6whv7tes5ehffopid (@alexej996)
@wiz
Copy link
Member Author

wiz commented Aug 8, 2020

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 bisq-network/bisq#4403

@devinbileck
Copy link
Member

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

wiz added a commit to wiz/bisq that referenced this issue Aug 9, 2020
Pending merge of bisq-network#4315 and rough consensus to proceed with migration plan in bisq-network/projects#35

wizpriceje6q5tdrxkyiazsgu7irquiqjy2dptezqhrtu7l2qelqktid (@wiz)
emzyprwrcz22h2fzhsbqd3deoe5hkqdm5yfy4geuqcdqqicgoor65iad (@Emzy)
devinpndvdwll4wiqcyq5e7itezmarg7rzicrvf6brzkwxdm374kmmyd (@devinbileck)
aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd (@mrosseel)
ro7nv73awqs3ga2qtqeqawrjpbxwarsazznszvr6whv7tes5ehffopid (@alexej996)
@wiz
Copy link
Member Author

wiz commented Aug 9, 2020

Screen Shot 2020-08-10 at 1 32 14

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

@wiz
Copy link
Member Author

wiz commented Aug 9, 2020

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
Copy link

Emzy commented Aug 9, 2020

I would like to change my HS to:
emzypricpidesmyqg2hc6dkwitqzaxrqnpkdg3ae2wef5znncu2ambqd

@wiz
Copy link
Member Author

wiz commented Aug 9, 2020

lol ok. will amend PR.

wiz added a commit to wiz/bisq that referenced this issue Aug 9, 2020
Pending merge of bisq-network#4315 and rough consensus to proceed with migration plan in bisq-network/projects#35

wizpriceje6q5tdrxkyiazsgu7irquiqjy2dptezqhrtu7l2qelqktid (@wiz)
emzypricpidesmyqg2hc6dkwitqzaxrqnpkdg3ae2wef5znncu2ambqd (@Emzy)
devinpndvdwll4wiqcyq5e7itezmarg7rzicrvf6brzkwxdm374kmmyd (@devinbileck)
aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd (@mrosseel)
ro7nv73awqs3ga2qtqeqawrjpbxwarsazznszvr6whv7tes5ehffopid (@alexej996)
@wiz
Copy link
Member Author

wiz commented Aug 9, 2020

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

@wiz
Copy link
Member Author

wiz commented Aug 9, 2020

@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 wiz added has:approval bisq.wiki/Project_management#Approval has:budget bisq.wiki/Project_management#Budgeting to:Improve Reliability was:delivered bisq.wiki/Project_management#Closing_as_delivered labels Aug 20, 2020
@wiz
Copy link
Member Author

wiz commented Aug 20, 2020

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! 🎉

@cbeams
Copy link
Member

cbeams commented Aug 24, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has:approval bisq.wiki/Project_management#Approval has:budget bisq.wiki/Project_management#Budgeting to:Improve Reliability was:delivered bisq.wiki/Project_management#Closing_as_delivered
Projects
None yet
Development

No branches or pull requests

8 participants