CLI bug fix: show trade's contract volume, not moving offer volume #5704
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a bug fix for the CLI's displayed fiat trade cost value, which should be trade.volume, not offer.volume. Offer volume varies with BTC volatility, and the CLI should be showing the trade.volume value instead, frozen when the contract is made.
This is a bug fix for the CLI's displayed altcoin trade amount value, which should be trade.volume, not offer.volume. It has been hidden by the stability of the BSQ price, and exposed while testing API support for XMR trades.
Fixes problem simlar to other CLI output changes in this PR. CLI's 'gettrade' output should show contracted trade.price instead of moving offer.price value for price margin based altcoin offers. This fixed bug did not affect any fiat or BSQ trades, only price margin based XMR trades, and API support for XMR trading has not yet been released.
ghubstan
added a commit
to ghubstan/bisq
that referenced
this pull request
Sep 18, 2021
ghubstan
added a commit
to ghubstan/bisq
that referenced
this pull request
Sep 21, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a an API CLI
gettradebug that could have resulted in loss of funds (explained below).There are two minor changes:
Show correct, frozen
Buyer Cost(trade volume, not offer volume) in CLI console.This corrects the CLI's displayed fiat trade cost value, which should be trade.volume, not offer.volume. Offer volume varies with BTC volatility, and the CLI should be showing the trade.volume value instead, frozen when the contract was made.
Show correct, frozen altcoin
Amount(trade volume, not offer volume) in CLI console.This corrects the CLI's displayed altcoin trade amount value, which should be trade.volume, not offer.volume. The bug has been hidden by the stability of the BSQ price, and exposed while testing API support for XMR trades.
The bug could have resulted in loss of funds for traders who used the API to buy BTC in the following scenario:
The CLI's
gettradecommand has been showing the moving offer volume in theBuyer Costcolumn -- howmuch the buyer pays. This value varies with BTC price if the offer is market-price-margin based.
The CLI should be showing the constant trade volume, frozen at the time a trade contract is written.
This bug does not affect BSQ trading because all BSQ trades are at fixed prices.
This bug would have affected XMR trading if the fix was not applied before XMR trading support in the API is released.
To check to see if you paid too much or too little, you could use the desktop app to view your trade history,
and match the correct payment amounts to your bank transfer records.