Release/v1.5.0 #4846
Merged
Release/v1.5.0 #4846
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 reverts commit b8f5c6e.
This reverts commit e49c565.
Replace tx.bitcoinSerialize().length
Goal: Have a more accurate fee calculation
Segwit for the trade protocol
Segwit fee estimation
Completes 29f23fe
Complete Segwit fee estimation
Renaming: Use transaction virtual size
Recently, some Monero traders were complaining the XMR/BTC price on the Bisq Price Index was off from most exchanges. Indeed, it seems HitBTC is trading at a -10% divergence for some reason, I guess they have low liquidity or some other reason, but after taking a look we are currently using HitBTC for the following assets: * AEON - only 2 trades ever, last traded March 2019 * EMC - never traded * GRIN - looks like only fake trades, last traded Jan 2019 * PART - only 7 trades ever, last traded August 2020 * XRC - last traded December 2019 * XMR - obviously very important for Bisq So I feel it's worth it to drop HitBTC as a data provider from Bisq to make the Monero traders happy and make the Monero price more accurate.
These are failing on the tip of release/1.5.0 currently due to extra validation added to PersistenceManager, causing the build to fail upon merging upstream. Add missing PersistenceManager.shutDown calls to the tearDown methods of the affected tests to fix.
Make sure witness data is stripped from the seller's prepared deposit tx, in addition to ScriptSig data, to prevent the buyer from being able to publish it prematurely (before having signed the delayed payout tx).
Include a new 'delayedPayoutTxSellerSignature' field with the prepared delayed payout tx sent to the buyer, in DelayedPayoutTxSignatureRequest. This will allow the buyer to compute the final, signed delayedPayoutTx as early as possible and withhold their deposit tx witness from the seller until they know they have a valid delayedPayoutTx, preventing its premature publishing in the fully segwit case. (To be done in a later commit - for now just save the seller's delayedPayoutTx signature.) As part of this, run the SellerSignsDelayedPayoutTx trade task at an earlier step (just after payout tx creation) to make its signature available to the seller ASAP. Also rename 'delayedPayoutTxSignature' to 'delayedPayoutTxBuyerSignature' in DelayedPayoutTxSignatureResponse.
Improve validation of the buyer's delayed payout tx (both before & after they get the final DepositTxAndDelayedPayoutTxMessage from the peer), by finalising it independently of the seller. This is now possible since their 2-of-2 signature is included in the DelayedPayoutSignatureRequest. Check that the final delayedPayoutTx received from the seller matches it byte-for-byte (which actually makes its receipt redundant now). This also fixes an apparent security bug, where the final validation of the delayedPayoutTx appears to skip any kind of signature check (only a deposit tx hash check, which is still necessary). Finally, optimistically check the deposit tx against the input of the prepared delayedPayoutTx received from the seller, in the case that the former is non-malleable (that is, the fully segwit case) and thus has a stable ID given by the hash of the buyer's preparedDepositTx.
Strip all input witnesses from the depositTx message fields sent from the buyer, until the last (DelayedPayoutTxSignatureResponse) message is sent, where they can be bundled in as an extra field. Since the witness data doesn't affect the final deposit tx id, the seller does not need to know it until actually publishing the tx. In the (fully) segwit case, this allows the buyer to prevent the seller from publishing the deposit tx until the buyer has a valid, fully signed delayedPayoutTx. Provide the final witness data in an extra 'depositTx' field in DelayedPayoutTxSignatureResponse, which the seller can merge with his depositTx witness block (for his own input signatures).
Disallow non-P2WH depositTx inputs from the taker, while continuing to allow them from the maker, so that offers created pre-v1.5.0 can still be taken. (After some time, those inputs could be disallowed too.) This is mainly to prevent mass blackmail attacks, where more victims' money could be locked up than the DAO could possibly compensate them all for. (This is probably only an attractive attack for a buyer anyway, at least with the earlier commits.)
Make sure to use the segwit version of Script.correctlySpends in TradeWalletService.finalizeDelayedPayoutTx, which requires the input value and witness to be passed explicitly (as the latter holds the actual signature). This was causing BuyerFinalizesDelayedPayoutTx to fail to do any kind of signature check. Also refactor the method slightly and remove a redundant call to WalletService.checkScriptSig (which does the same thing as TransactionInput.verify) in the branch used by the seller.
Do some extra sanity checks like tx.outputSum < tx.inputSum, to rule out any edge cases where an invalid delayed payout tx might still arise.
…bilities Fix remaining blackmail vulnerabilities
and do not read or write persisted data. We had recently a case where a user downgraded from 1.4.2 to 1.3.9 and this caused failed trades and the wallet funds have been missing due to some complexities of the wallet wegwit upgrade. The fund could be recovered but it took quite some effort. As downgrade is never tested and can lead to all kind of weird bugs we should prevent that users accidentally can do it. If there is valid reason to downgrade they can remove the version file.
To persist in the very last moment before exit might cause problems on some OS. We do not have confirmed that this might be an issue but to be on the safe side we add a 1 sec. delay between persistence completed and exit.
Upgrade bitcoinj to commit 60b4f2f
Check if user has downgraded to an older version
…d-delay-to-exit # Conflicts: # core/src/main/java/bisq/core/app/BisqExecutable.java
Add 1 sec delay before calling exit
Update resources for v1.5.0
….5.0 # Conflicts: # core/src/main/java/bisq/core/provider/fee/FeeService.java # core/src/main/resources/i18n/displayStrings.properties # seednode/src/main/java/bisq/seednode/SeedNodeMain.java
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.
No description provided.