User can set custom tx fee to override FeeService estimated fee #4231
Conversation
…ransaction fee The user preferences already contained an option to set a custom withdrawal tx fee. This commit simply extends the scope of that field, to apply to all callers of the FeeService. In summary, two things were done: 1. the FeeService logic was extended (if user set a custom fee, returns that; otherwise queries fee API endpoint for estimations, as usual) 2. respective fields, labels and methods have been renamed to reflect that (customWithdrawalTxFee -> customTxFee)
There was a problem hiding this comment.
NACK.
We allow users to set a custom withdrawal transaction fee rate because it's their problem if they set it too low and end up queued in the mempool for an extended period.
We do not allow users to set a custom trading transaction fee rate because it creates a problem for their trading counterparty if they set it too low. The whole trade will get held up in such a case, and that's unfair to the counterparty who used the default fee or chose a more reasonable custom value.
Nit: for future PRs, please follow the commit guidelines at bisq-network/style#9.
|
I'm not sure about extending the custom fee all over the place, but perhaps it could be used a bit more and only forcing higher fees for the most sensitive parts of the trade protocol. It's not obvious that's a good idea though. Any stuck tx adds to the support load, even if it's due to a user setting the custom fee too low. |
|
@cbeams I understand that your argument applies to offer-taking, but what about offer-making and proposals? |
Offer making is a different story as it's theoretically only the maker's problem, and they're not externalizing anything on their counterparty, but as @sqrrm mentions, in practice it's probably still going to land in support. Supporting RBF (replace-by-fee) in Bisq would help solve this as it would allow makers to self-serve solving the problem. And there's nothing we can do in support to help them anyway. We would have to be very clear that in the UI that changing the recommended transaction fee is to be done at the users' risk, that there's nothing we can do to help their transaction if it's stuck, etc. Would need to think about how offer publication happens as well. I think right now, the offer gets published to the offer book and becomes visible even before the first confirmation occurs. We wouldn't want that to be the case if this change were made, because again an unwitting taker could get hurt for no fault of their own if the maker fee tx gets stuck.
This would be less risky to allow it here. It's fully the problem of the proposer if the tx gets stuck, and we could be clear that there is no recourse for this, just do it at your own risk. With the above said, I don't see this as a priority now that we've solved the problem of too-high fee rate recommendations out of earn.com with @cd2357's PR at bisq-network/projects#27. As far as I'm aware, users haven't been complaining about the tx fee rate in Bisq except during a couple incidents where rates were way too high and got stuck there. This happened in 2017, just recently, and IIRC there was one other time too. If the new mempool.space service proves reliable in producing reasonable estimates, my preference would be to leave this area alone for now, and not add more complexity and risk leading to potentially more support incidents and poor user experiences. |
That is a rather charitable interpretation I'd say, considering that
So there are many stages to go through, and only the last group are "visible" to the Bisq team as having made a complaint. Its also a question of which metric to use when deciding how to prioritize limited dev resources:
I agree with @cbeams. From a "should Bisq merge this change" perspective, this PR is probably not necessary anymore, now that the fee estimation is more accurate. So I'll just go ahead and close the PR. However, looking from a "should Bisq eventually support this" sort of perspective, I'd say yes. After all, Bisq is open source and if the protocol allows such a change, sooner or later someone will patch their local client and do it, cause it would be in their best interest to save some sats on mining fees. |
The user preferences already contained an option to set a custom withdrawal tx fee.
With this change, the scope of that field has been extended to cover all callers of the FeeService, which means every place where a tx fee is needed. If the preferences field is unset, the FeeService behavior falls back to the existing one (uses fee estimates from the earn.com API).
In summary, two things were done in this commit:
I tried to keep the change as non-invasive as possible. From what I can tell, the change is minimal (consisting mostly of renamed fields and methods).
The protobuf change is backward compatible with previously serialized user preference protobufs, because the "custom fee" fields already existed before. Its only the metadata for addressing the fields that has changed, which does not end up in the serialization. Only the field values are serialized, which remained the same.
Several local tests were successful:
One known minor issue remains open, which is that of adjusting translations for other languages ("Withdrawal transaction fee" -> "Transaction fee"). Currently I have only changed the EN version.
Please let me know if you see any issues with the general direction or with the implementation.
Workaround for bisq-network/projects#27 until a better fee estimation service is used.