Prevent that a wrong tx is set as deposit tx #4883
Merged
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
We check if the txIds of the inputs matches our maker fee tx and taker fee tx and if the depositTxAddress we use for the confidence lookup is use as an output address. This prevents that past txs which have the our depositTxAddress as input or output (deposit or payout txs) could be interpreted as our deposit tx. This happened because if a bug which caused re-use of the Multisig address entries and if both traders use the same key for multiple trades the depositTxAddress would be the same. We fix that bug as well but we also need to avoid that past already used addresses might be taken again (the Multisig flag got reverted to available in the address entry). - Add check to swapTradeEntryToAvailableEntry to not swap MULTI_SIG entries. - Remove swap for MULTI_SIG entries at resetAddressEntriesForPendingTrade - Add check to swapToAvailable to not swap MULTI_SIG entries. - Remove swaps for MULTI_SIG entries - Add setCoinLockedInMultiSigAddressEntry method - Make coinLockedInMultiSig final and remove setter but use it in constructor. - Rename getCoinLockedInMultiSig to getCoinLockedInMultiSigAsCoin We use an immutable list when operating on AddressEntry so changes on the object would not be reflected in the list. The only mutable field (beside non critical cache fields) is the keyPair. Might be good to refactor that as well at some point. - Add setCoinLockedInMultiSigAddressEntrymethods - Apply API changes: -- resetCoinLockedInMultiSigAddressEntry -- setCoinLockedInMultiSigAddressEntry -- renamed methods
This was referenced Dec 3, 2020
|
ACK (re-tested). ☑️ |
ghost
approved these changes
Dec 3, 2020
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.
Fixes #4873
Beside what is mentioned in #4873 I found another (minor) issue. We use an immutable list for address entries but the coinLockedInMultiSig value was set in the object via a setter. This change would be lost when accessed as when one open the wallet tool with cmd+j (after take offe the value was 0 there instead of the locked up funds). As the field is only used for lookup for locked funds from failed trades the bug was not very visible.
coinLockedInMultiSig is now a final field set by the constructor.