feat(wallet): send a cancel message to a receiver when it responds to a canceled transaction #3976
Conversation
SWvheerden
changed the title
SWvheerden
force-pushed
the
sw_cancels_stale
branch
from
ff03c2c
to
c17ef24
Compare
stringhandler
changed the title
| @@ -1682,6 +1687,21 @@ where | |||
| Some(s) => s, | |||
| } | |||
| }, | |||
| Err(TransactionStorageError::ValueNotFound(_)) => { | |||
There was a problem hiding this comment.
Is this correct? Feels like it might send a false positive
There was a problem hiding this comment.
I was looking at this code, and added this extra.
My reasoning was, we don't know about this tx for whatever reason.
For example, our storage crashed or something. Now someone is asking about the tx and we don't know about, so rather send them a cancel than just ignore the request and they don't have to continue looking at the tx.
There was a problem hiding this comment.
I believe Err(_) => return Err(TransactionServiceError::TransactionDoesNotExistError) (or Err(TransactionStorageError::ValueNotFound(key))) when receiving a finalized transaction should result in a recovery attempt for those outputs.
Reasoning: The sender has already broadcast this transaction, so the receiver can attempt to recover the funds.
There was a problem hiding this comment.
Any error there could result in lost funds. We need to do some recovery here to get those funds. At this point, the sender cannot cancel the tx, so we should try and do a recovery to recover that tx as we should have created the keys from our master.
SWvheerden
force-pushed
the
sw_cancels_stale
branch
from
c17ef24
to
51a4cab
Compare
| @@ -313,3 +313,21 @@ Feature: Wallet Transactions | |||
| And I wait 5 seconds | |||
| Then I restart wallet WALLET_RECV | |||
| Then I wait for wallet WALLET_RECV to have at least 1000000 uT | |||
|
|
|||
| @critical | |||
| Scenario: Wallet should cancel stale transactions | |||
There was a problem hiding this comment.
| Scenario: Wallet should cancel stale transactions | |
| Scenario: Wallet should cancel stale cancelled transactions |
There was a problem hiding this comment.
The focus must be to do this automatically only for cancelled transactions; this is not clear reading the name of the test
Description
This will send a cancel message to a receiving node when it responds to an already canceled transaction.
Motivation and Context
This catches an edge case where the sender can cancel a stale transaction when the receiver is offline, but when it comes online it tries to continue the transaction.
How Has This Been Tested?
Included new cucumber test.