-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix: BTC maker/taker fee validation #6814
Conversation
To clarify: Bisq validates fees paid in Trade Step 2, prior to payment. In this case the app was started up with an existing trade and the validation was allowed to occur too early, before the DAO was ready, causing the erroneous warning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit
In this PR, you check with canRequestBeMade()
in isServiceSupported
whether the request can be made or not. This should cover the change from your previous PR (#6810) too. Therefore, we can safely remove mempoolService.canRequestBeMade(offerPayload)
in the TriggerPriceService
. Is this correct or am I missing something?
TriggerPriceService:
OfferPayload offerPayload = offer.getOfferPayload().orElseThrow();
if (openOffer.getMempoolStatus() < 0 &&
mempoolService.canRequestBeMade(offerPayload)) {
mempoolService.validateOfferMakerTx(offerPayload, (txValidator -> {
openOffer.setMempoolStatus(txValidator.isFail() ? 0 : 1);
}));
}
Here, mempoolService.validateOfferMakerTx(...)
, calls validateOfferMakerTx(...)
, and validateOfferMakerTx()
calls isServiceSupported()
before creating the mempool request.
MempoolService:
public void validateOfferMakerTx(TxValidator txValidator, Consumer<TxValidator> resultHandler) {
if (txValidator.getIsFeeCurrencyBtc() != null && txValidator.getIsFeeCurrencyBtc()) {
if (!isServiceSupported()) {
UserThread.runAfter(() -> resultHandler.accept(txValidator.endResult("mempool request not supported, bypassing", true)), 1);
return;
}
MempoolRequest mempoolRequest = new MempoolRequest(preferences, socks5ProxyProvider);
validateOfferMakerTx(mempoolRequest, txValidator, resultHandler);
} else {
// using BSQ for fees
UserThread.runAfter(() -> resultHandler.accept(txValidator.validateBsqFeeTx(true)), 1);
}
}
If TriggerPriceService was simplified to only call So I think the validation has to be done after the system becomes ready (i.e. the way it is now). |
@jmacxx Thank you very much for the explanation! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Fixes #6813
It is another edge case similar to #6810, so protect from false flagging a trade if the DAO is not yet ready.