Migrate Bisq pricenodes from earn.com fee estimation API to our own self-hosted mempool fee estimation API service

wiz commented 4 years ago

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

Description

This is a project proposal to migrate Bisq pricenodes away from using the earn.com fee estimation API in favor of an open-source self-hosted mempool fee estimation API backend, that we can decentralize further by having all Bisq btcnode operators run.

Rationale

Whenever the Bitcoin market sees major price action, trading volume suddenly spikes, which results in a huge rush of new transactions into the Bitcoin mempool, and "next block" fees rise very quickly:

This sudden rise in mempool fees causes a major issue in Bisq whereby any trades taken during the time when the mempool fees start to rise use far too low of a fee, and they do not get confirmed for several hours or even days later, until the mempool clears out. The cause of this issue is that Bisq currently uses Earn.com fee estimation API which is slow and not well maintained. The https://mempool.space backend uses realtime data from Bitcoin nodes, so it is extremely fast to return realtime fee estimates, compared to the Earn.com API which is always lagging behind:

This issue was first reported by @ManfredKarrer in https://github.com/bisq-network/bisq/issues/1077 but at the time, a better mempool fee estimation API service did not exist. But now we have a great open-source and self-hosted solution, the mempool project. Of course I am bias, since I am a maintainer/contributor to this project, but IMO it really is the best mempool project for Bitcoin and we have worked very hard making it so.

Recently during the past few days we have seen this issue reproduce again, and now users are complaining in support that their deposit TX have still not confirmed even after waiting for 12 hours. So therefore I propose that we pull the trigger on ditching Earn.com API once and for all.

Criteria for delivery

After all Bisq pricenodes upgrade to the new code that will query our federated btcnode mempool fee estimation backend service, we can consider it delivered.

Measures of success

After all Bisq nodes start receiving the self-hosted mempool fee data, and users no longer experience this issue due to sudden fee spikes, we can consider it a success.

Risks

I've been running the mempool backend and hosting https://mempool.space for over 6 months, and it has never crashed, so I feel it is quite stable to run in production. However, we could keep Earn.com API as a backup data source to mitigate the risk of any issues with our own self-hosted API service.

Tasks

[x] Modify the Bisq pricenode code to use the mempool fee estimation backend
[x] Have all Bisq btcnode operators are running the mempool fee estimation backend
[x] Have all Bisq pricenodes upgrade to the new code that will query our federated btcnode mempool fee estimation backend service

Estimates

Dev: $2000 USD Ops: $1000 USD

wiz commented 4 years ago

This issue is reproducing now since the mempool suddenly surged again. The mempool backend adjusts fee estimates in realtime, but Earn.com is lagging behind hard...

cd2357 commented 4 years ago

Have you considered using the estimatesmartfee call from Bitcoin Core?

ifarnung commented 4 years ago

Please everyone take a look at this proposal, this is a very serious issue both in terms of "too low" estimates in rising mempools and WAY "too high" fees in shrinking mempools.

The mempool just emptied out and Bisq is still forcing users to pay 97 sats per byte. I, for one, won't be trading until the fees are fixed or lag catches up to reality. Bisq is relying on Earn.com for that...

cd2357 commented 4 years ago

If my understanding is correct, all Bisq nodes have access to Bitcoin Core (either to a local node, or through a service interface exposed by a full node).

In that case, why not just use Bitcoin Core's mechanism for fee estimation? They already expose it through their API: estimatesmartfee. The fee estimation call can take different parameters: for example, the caller can indicate in how many blocks they wish to have the tx confirmed, or the estimation mode (conservative or not).

Using this API in Bitcoin Core would achieve 2 things for Bisq:

one less dependency on a 3rd party, centralized service (earn.com API)
a more accurate and flexible fee estimation algorithm

I've already used it in other projects via the Java interface exposed by bitcoin-rpc-client, seems to be a good match for Bisq's use-case.

eigentsmis commented 4 years ago

100% agree with this proposal.

Furthermore, I think it would be a great idea to allow offer makers to set their own custom fee when creating an offer. It could of course offer a hardcoded min limit (just like the 2 sat/byte min limit when withdrawing funds from the bisq wallet). On the other end, for the offer taker, there should be a way (warning popup maybe) of letting them know something like: "Before confirming the taking of this offer please be advised that the deposit transactions will take about X more hours to confirm". This feature could even start as a "hidden" feature for power-users only to test out any potential bugs or game theoretical issues arising. (ie. offer a certain hidden key shortcut in the offer creation screen) I believe implementing something like this, on top of moving away from Earn.com, will further entice more users/offers onto the Bisq market.

Just my 2 BSQ sats...

ifarnung commented 4 years ago

@eigentsmis I agree with your second idea as I would like to set my own mining fees, but I would put that on the back burner until a better estimator is found for the majority of trades.

cbeams commented 4 years ago

If my understanding is correct, all Bisq nodes have access to Bitcoin Core (either to a local node, or through a service interface exposed by a full node).

In that case, why not just use Bitcoin Core's mechanism for fee estimation? They already expose it through their API: estimatesmartfee.

Bisq nodes are connected to one or more Bitcoin Core nodes via the Bitcoin p2p protocol. They have no access to these nodes' RPC interfaces, and estimatesmartfee is an RPC. It would be possible to give Bisq access to one's own personal Bitcoin Core node's RPC API, but it would not be a general, works-out-of-the-box solution for all Bisq users.

cd2357 commented 4 years ago

Thanks for the explanation @cbeams

Until a better solution for the fee estimation is found, I prepared a workaround (which was actually already requested a few times) that allows users to manually override the fee estimation with a custom fee (https://github.com/bisq-network/bisq/pull/4231).

Initial tests look promising (custom fee can be set to anything between 2 and 5000 sats/byte), but I may well have overlooked smth, so the change is up for review.

cbeams commented 4 years ago

The current situation is pretty extreme, I agree. At time of writing, estimatesmartfee is recommending fee rates between 4 and 6 sats/byte:

$ bitcoin-cli estimatesmartfee 1 ECONOMICAL
{
  "feerate": 0.00004439,
  "blocks": 2
}
$ bitcoin-cli estimatesmartfee 1 CONSERVATIVE
{
  "feerate": 0.00065398,
  "blocks": 2
}

while earn.com is reporting 110 sats/byte as the "fastest and cheapest" rate:

Based on that value, Bisq is currently requiring 94 sats/byte. Clearly way too much with the mempool having been relatively uncongested for many hours.

I see a couple alternative options here to what @wiz proposes.

The first is that we switch from using the earn.com API to using the whatthefee.com API. We considered doing this in late 2017 when @FelixWeis was first standing up the service, but never did it in the end, and fees have been pretty low for the most part since. This change would be the lowest-risk, because it's just swapping out one API for another. No new moving parts to establish, no role duties to modify, etc. Even if we do go ahead with @wiz's proposal, I would still propose that we do this first (and it doesn't have to be whatthefee; any reputable estimation service with a web API would be up for discussion).

For reference, here are https://whatthefee.io/ current estimations:

The second alternative I'd offer is to make running a (possibly pruned) full node a requirement for pricenode operators such that the pricenode can call directly into the full node's estimatesmartfee RPC. This would be the most decentralized / censorship resistant approach, as it would eliminate the use of a centralized API altogether. It would require more significant changes to the pricenode code, but nothing too difficult. The main hurdle / burden would be having operators need to run their own node. With that said, it's not so different than @wiz's proposal that we run a federation of mempool.space services. Those operators would need to run full nodes too, and the only thing these would be used for / used by is the pricenodes. @wiz, I'm not sure what advantages mempool.space has over getting estimates via estimatesmartfee; can you comment?

wiz commented 4 years ago

@wiz I'm not sure what advantages mempool.space has over getting estimates via estimatesmartfee; can you comment?

@softsimon is the author of this algorithm so I'll ask him to respond as to why it's better than using the one in core, he is the best person to explain it

softsimon commented 4 years ago

I'm not sure what advantages mempool.space has over getting estimates via estimatesmartfee; can you comment?

I am not quite sure how estimatesmartfee operates, but mempool.space uses a fully mempool based fee estimation while most traditional estimators are based on probability. I personally and many people have reached out and told me they mostly get lower fees by checking mempool space for the next block fee. The mempool based estimation is very dynamic and updates as new blocks and transactions come in so if there's several blocks in a row within a short time span that will clear out high fee transactions, the fee suggestions will immediately drop. One downside currently is that it's not able to predict accurately into the future above 2-3 blocks (20-30 minutes) away.

wiz commented 4 years ago

This change would be the lowest-risk, because it's just swapping out one API for another.

Replacing earn.com with another TTP / CPOF is not an acceptable solution IMO. If we wanted to do that we could simply query the mempool.space API at https://mempool.space/api/v1/fees/recommended which is the same as https://bitcoinfees.earn.com/api/v1/fees/recommended but this would just make everyone trust my node, which is why I'm proposing that all pricenodes run their own open-source backends for local fee estimation in the spirit of decentralization. AFAIK all pricenode operators already operate bitcoin nodes, so this is very easy to arrange.

softsimon commented 4 years ago

https://bitcoinfees.earn.com/api/v1/fees/recommended {"fastestFee":110,"halfHourFee":108,"hourFee":94}

https://mempool.space/api/v1/fees/recommended {"fastestFee":1,"halfHourFee":1,"hourFee":1}

So. The mempool is almost empty, not even a half block full in queue, yet earn.com recommends 110 to get into the next block. 1 is enough the next 10 minutes to get into the next block if you actually just look at the mempool.

cd2357 commented 4 years ago

Fee estimation has been a known issue since 2017: https://github.com/bisq-network/bisq/issues/1077

And its the 3rd most commented issue in Bisq, ever. So there is lots of interest from users.

Looks straightforward enough to fix + would directly improve every Bisq trade in every market, for both sellers and buyers.

Its especially bad for Bisq contributors who want to convert their BSQ to fiat, cause they have to go through 2 trades (BSQ-BTC then BTC-fiat), so they pay the unnecessarily higher (100x) miner fees twice.

In addition, every Bisq trade has 3 on-chain bitcoin transactions. So for every trade, both parties pay each 300x the normal mining fee (instead of just 3x, one per tx).

Why is it not prioritised? (not listed on either Critical Bugs or Master Projects boards)

I can help with dev if necessary.

ericrpatton commented 4 years ago

Would it help if a dedicated funding mechanism was set up specifically for resolving this issue? I would be more than willing to kick in some funds toward it.

cbeams commented 4 years ago

I'm working on a PR now to switch to the mempool.space API as a stopgap measure.

cd2357 commented 4 years ago

I'm working on a PR now to switch to the mempool.space API as a stopgap measure.

You read my mind :)

I went ahead and already prepared a PR in the meantime: https://github.com/bisq-network/bisq/pull/4235

Please have a look, it might be good enough and you might not need to do any extra work.

cbeams commented 4 years ago

Please have a look

Yep, reviewing it now, per https://github.com/bisq-network/bisq/pull/4235#issuecomment-624109739.

cbeams commented 4 years ago

@wiz, I see that you created https://github.com/bisq-network/proposals/issues/219 to establish a new Mempool API Operator role, but before we go ahead with that, I'd like to further vet this proposal.

First, I want to address the risk of querying N fee estimation services vs 1. Yes, this is good for decentralization, but it also introduces the risk of getting divergent fee estimates from different mempool.space nodes. Each service is talking to its own full node with its own mempool, and for a variety of reasons may report different estimates. So long as they are within a reasonable range of one another, that's fine, but if we start getting minority reports that are very high or very low, we will have a more difficult time tracking down and solving these problems. Imagine having to analyze users' logs after the fact to see which pricenode (and therefore which mempool.space node) they were talking to.

To mitigate this risk, I would suggest writing a simple script that periodically polls N different (existing) mempool.space services to see how their fee estimates diverge over time. Getting the logs from running this for a week or so would give us a sense what to expect. Ideally, these mempool services would be running against bitcoin nodes that use the same bitcoin.conf.

Going back to the suggestion to create a mempool operator role, I'd actually suggest that running such a service and its backing fullnode become part of the normal duties of running a pricenode. I don't see the advantage in having a separate group of operators manage mempool nodes if the only thing we use them for is a backend for our pricenodes. Indeed, we would want all those mempool and bitcoind nodes to be configured in precisely the same way to avoid divergent results as much as possible.

If we're agreed that this would be the way to go, then we should ask all @bisq-network/pricenode-operators if they're willing to run these services, and if they are, then they should just set them up now, and we can run the script I talked about above against them to make sure that divergent results aren't going to be a problem. When we've de-risked that, then we could go ahead with the implementation.

Which brings us to the tasks section, and what's missing from it. As mentioned in https://github.com/bisq-network/bisq/pull/4235#pullrequestreview-405917893, the pricenode code will need to be modified to allow for configuring a specific mempool.space backend. Indeed this will be a required argument, as there is no one, correct fallback value.

So, to summarize:

[ ] we should do whatever makes sense to de-risk running against N fee estimation backends vs 1. I've suggested how we can do this, but other ideas / arguments are welcome.
[ ] a task should be added to capture necessary modifications to the pricenode to accommodate this change
[ ] a task should be added to update pricenode operator role documentation to include running a mempool.space node and a bitcoin core node
[ ] a task should be added to define a common bitcoin.conf for that bitcoin core node (and a common mempool.space config if appropriate)
[ ] a task should be added to, before making any switch in production, poll all pricenode operators' mempool backends over a period of time to ensure they return fee estimate values in a tight (enough) band with one another. (again, this is based on my suggestion for how to de-risk moving to N fee estimation backends)

wiz commented 4 years ago

querying N fee estimation services introduces the risk of getting divergent fee estimates from different mempool API nodes

You're right, and this is actually the reason why we need to separate pricenodes from mempool nodes - all pricenodes should query all mempool nodes, and average the returned values. The more mempool nodes we have, the more they will average out in the event one is off for some reason and converge on consensus.

For example, take the following script:

#!/usr/bin/env zsh
pricenodes=()
pricenodes+=xc3nh4juf2hshy7e # emzy
pricenodes+=ceaanhbvluug4we6 # mrosseel
pricenodes+=44mgyoe2b6oqiytt # devinbileck
pricenodes+=62nvujg5iou3vu3i # alexej996
#pricenodes+=gztmprecgqjq64zh # wiz

for i in $pricenodes
do
    echo -n "$i - suggested fees: "
    torsocks curl -s http://$i.onion/getFees | jq .dataMap.btcTxFee
done

It currently outputs the following suggested fees:

% ./pricenode-check
xc3nh4juf2hshy7e - suggested fees: 16
ceaanhbvluug4we6 - suggested fees: 16
44mgyoe2b6oqiytt - suggested fees: 16
62nvujg5iou3vu3i - suggested fees: 16

This weighted average, combined a with good monitoring system that runs the same script and sends alerts for any deviations to us should be sufficient enough IMO. I can add this service check to my monitoring system which will graph the results over time. I can also set it to graph the average deviation from mean of each mempool node.

I don't see the advantage in having a separate group of operators manage mempool nodes if the only thing we use them for is a backend for our pricenodes

I'd prefer having it separate for the simple reason that not all pricenode operators will run mempool nodes, and vice-versa. For example, it's easy to have existing bitcoin node operators also become mempool node operators, and some may do this, but it is not so easy for existing pricenode operators to also become mempool node operators because they would need to setup a bitcoin node for this if they don't already have one.

the pricenode code will need to be modified to allow for configuring a specific mempool.space backend

Instead of doing it that way, I'd like to have a hard-coded list of our 5 nodes, similar to how Bisq nodes have a hard-coded list of seednodes and pricenodes and bitcoin nodes, so that every pricenode will query all mempool nodes and average the results together. I've assigned this task to @cd2357 and he is working on it now. We will document it along with the other github issues you want created.

bisq-network / projects