Implement transaction negotiation message resending in the wallet #2161
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
philipr-za
force-pushed
the
philip-tx-resending
branch
from
5d2bfc5 to
00ce339
Compare
sdbondi
reviewed
Aug 26, 2020
sdbondi
previously approved these changes
Aug 27, 2020
base_layer/wallet/src/transaction_service/protocols/transaction_receive_protocol.rs
Show resolved
Hide resolved
base_layer/wallet/src/transaction_service/protocols/transaction_receive_protocol.rs
Outdated
Show resolved
Hide resolved
philipr-za
force-pushed
the
philip-tx-resending
branch
2 times, most recently
from
e482002 to
b9c3931
Compare
philipr-za
force-pushed
the
philip-tx-resending
branch
from
b9c3931 to
11fced4
Compare
sdbondi
previously approved these changes
Sep 14, 2020
Currently the logic for sending transactions and negotiating them between parties is not very robust. Each stage in the negotiation is sent only once. When sending we first try to send the message directly and if that fails, or discovery needs to occur, the message is sent indirectly via Store and Forward (SAF). We hoped to get SAF 100% reliable but due to reasons laid out in tari-project#2137 it is not tenable. This PR adds in the logic to periodically resends the initial Transaction Sender Message and the Transaction Reply in order to illicit the next step in the negotiation protocol from the counterparty. The PR also adds the logic to respond to repeated messages but with a cool down period in which repeats will be ignored to protect against DoS attacks. The PR also adds in a new comms level message to inform the counterparty if a transaction protocol has been cancelled. Initially this is sent if the Sender cancels an in progress transaction so that the receiver knows to stop expecting it. This message is also sent if a Transaction Reply is received for a cancelled message to let the Received know about the cancellation. In order to stop a wallet resending these message indefinitely a transaction will be cancelled if it hasn’t resolved after a long timeout period (Default is being set to 3 days). A large portion of the PR are tests so its not as big as it looks.
philipr-za
force-pushed
the
philip-tx-resending
branch
from
11fced4 to
4919926
Compare
stringhandler
added a commit
that referenced
this pull request
Sep 18, 2020
Major Fixes since v0.5.4 * Many fixes and updates to tari_merge_mining_proxy * Implement transaction negotiation message resending in the wallet (#2161) Minor Fixes * Added basic RPC interface for DHT (#2188) * Refactor Tokio runtime out of wallet container (#2213) * Add period stats UI call (#2189) * Protect against possible panics in header/horizon sync (#2219) * Fix coinbase vulnerability (#2143) * Added a user configurable chain monitoring timeout (#2202) * Implement LMDB automatic resizing * Add `is_synced` flag to BaseNodeServiceResponse & update UTXO validator
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.
Description
Currently the logic for sending transactions and negotiating them between parties is not very robust. Each stage in the negotiation is sent only once. When sending we first try to send the message directly and if that fails, or discovery needs to occur, the message is sent indirectly via Store and Forward (SAF). We hoped to get SAF 100% reliable but due to reasons laid out in #2137 it is not tenable.
This PR adds in the logic to periodically resends the initial Transaction Sender Message and the Transaction Reply in order to illicit the next step in the negotiation protocol from the counterparty. The PR also adds the logic to respond to repeated messages but with a cool down period in which repeats will be ignored to protect against DoS attacks.
The PR also adds in a new comms level message to inform the counterparty if a transaction protocol has been cancelled. Initially this is sent if the Sender cancels an in progress transaction so that the receiver knows to stop expecting it. This message is also sent if a Transaction Reply is received for a cancelled message to let the Received know about the cancellation.
In order to stop a wallet resending these message indefinitely a transaction will be cancelled if it hasn’t resolved after a long timeout period (Default is being set to 3 days).
A large portion of the PR is tests and some code reorganization so its not as big as it looks.
Motivation and Context
Closes #2137
Closes #1804
How Has This Been Tested?
Tests are provided that test:
Types of changes
Checklist:
developmentbranch.cargo-fmt --allbefore pushing.