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

Offers with invalid maker fees are shown live on the Bisq offer book resulting in failed trades and lost fees for the taker #6603

Closed
darawhelan opened this issue Mar 10, 2023 · 4 comments · Fixed by #6615

Comments

@darawhelan
Copy link

Description

When a maker with an out of sync bitcoin wallet creates a trade to buy or seller bitcoin if their maker transaction fails due to a problem with their wallet the trade is still published live in the Bisq offer book.

This results in a taker coming along taking the offer only for the trade to fail and them to lose miner and trade fees.

I believe their was a filter in place previously that prevented this by checking the offers to validate the maker transactions before publishing the offer to the Bisq offer book.

For examples of these types of failed trades please see:

4 days ago: bisq-network/support#1352
2 weeks ago: bisq-network/support#1348

Both of these trades are avoidable if the trades would have been validated before being shown on Bisq.

Version

v1.9.9

Steps to reproduce

  • Make an offer with an out of sync bitcoin wallet
  • You will not pay any trade fees or mining fees
  • Your offer will be shown live on Bisq for another user to take

Expected behaviour

  • Make an offer with an out of sync bitcoin wallet
  • You will not pay any trade fees or mining fees
  • Your offer will not be shown live on Bisq for another user to take because it will fail the validation test

Actual behaviour

Offers are shown in the offer book that will fail when taken.

@boring-cyborg
Copy link

boring-cyborg bot commented Mar 10, 2023

Thanks for opening your first issue here!

Be sure to follow the issue template. Your issue will be reviewed by a maintainer and labeled for further action.

@ghost
Copy link

ghost commented Mar 13, 2023

The check for missing tx & trade fee is disabled since the start of year by the central filter disableMempoolValidation=true. It could be re-enabled after the upcoming release which includes PRs #6494 and #6425.

Since its been disabled quite a while, a re-test of the feature should be done first. I'll try to do this in the coming week.

Ping @bisq-network/bisq-maintainers as a reminder.

@ghost
Copy link

ghost commented Mar 17, 2023

Checked open offers with the following issues:

  • The validation code assumes a BSQ tx not in the database is simply unconfirmed and lets it pass. This is because BSQ txs are all unknown until they are confirmed. But if the offer was made from an out of sync SPV wallet it will never confirm. The issue could be fixed by checking the blockheight the offer was made at, and raising an error if it is old and yet the BSQ tx is not known.
  • A couple of BTC fee receivers were rejected as "unknown". They are previous reimbursement addresses so those offers were placed a couple of years ago. Not sure what the best solution is given its just 2 offers.

@darawhelan
Copy link
Author

  • The validation code assumes a BSQ tx not in the database is simply unconfirmed and lets it pass. This is because BSQ txs are all unknown until they are confirmed. But if the offer was made from an out of sync SPV wallet it will never confirm. The issue could be fixed by checking the blockheight the offer was made at, and raising an error if it is old and yet the BSQ tx is not known.

Sounds like a good solution to me.

  • A couple of BTC fee receivers were rejected as "unknown". They are previous reimbursement addresses so those offers were placed a couple of years ago. Not sure what the best solution is given its just 2 offers.

Could these addresses be added to the filter? Likely the odd maker fee will still go to these addresses and some users might have offers were these addresses were used to pay the maker fee that are offline at present.

Failing that maybe it is best not to display these older offers and instead 'force' the makers to create new offers to be displayed in the address book.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant