first comments

1 files changed, 94 insertions, 6 deletions

diff --git a/design-documents/037-wallet-transactions-lifecycle.rst b/design-documents/037-wallet-transactions-lifecycle.rst
index 24896a34..d383f564 100644
--- a/design-documents/037-wallet-transactions-lifecycle.rst
+++ b/design-documents/037-wallet-transactions-lifecycle.rst
@@ -29,7 +29,8 @@ There are some other distinctions for pending transactions:
 * long-polling vs exponential backoff: A pending transaction is either waiting
   on an external service by making a long-polling request or by repeating requests
   with exponential back-off.
-* ``lastError``: A pending transaction is either clean (i.e. the external service successfully
+* ``lastError``: A pending transaction is either clean (i.e. the network interaction
+  is literally active in transmission or the external service successfully
   communicated that it is not ready yet) or has a ``lastError``, which is a ``TalerErrorDetails``
   object with details about what happened during the last attempt to proceed
   with the transaction.
@@ -47,10 +48,25 @@ indicates errors the wallet experienced while taking active steps to abort the t
 ``aborted``: Similar to a ``done`` transaction, but the transaction was successfully aborted
 instead of successfully finished.
 
+``suspended``: Similar to a ``aborted`` transaction, but the transaction was could be
+resumed and may then still succeed.
+
 ``failed``: Similar to ``done``, but the transaction could not even be aborted successfully.
 
 ``kyc-required``: The transaction can't proceed because the user needs to actively
-finish a KYC process.
+finish a KYC process. 
+
+``aml-required``: The transaction can't proceed because the user needs to wait for
+the exchange operator to conclude an AML investigation.
+
+There are two key distinctions for AML-required transactions:
+
+* pending: the staff at the exchange is running its investigation. The user is not
+  expected to take any action and should just wait for the investigation to conclude.
+
+* frozen: the staff at the exchange decided that the account needed to be frozen.
+  The user should contact the exchange provider's customer service department and
+  seek resolution (possibly through the courts) to avoid loosing the funds for good.
 
 ``deleted``: A ``deleted`` state is always a final state.  We only use this
 state for illustrative purposes. In the implementation, the data associated
@@ -66,7 +82,7 @@ Transitions are actions or other events.
 completely deletes the transaction in the database.  Depending on the type of
 transaction, some of the other data *resulting* from the transaction might
 still survive deletion. For example, deleting a withdrawal transaction does not
-delete withdrawn coins.
+delete already successfully withdrawn coins.
 
 ``[action:retry]``: Retrying a transaction *(1.)* stops ongoing longpolling
 requests for the transaction *(2.)* resets the retry timeout *(3.)* re-runs the
@@ -80,15 +96,33 @@ states: ``pending(*)``, ``kyc-required(*)``, ``updating(*)``, ``aborting(*)``.
 ``[action:abort]``: Aborting a transaction either directly stops processing for the transaction and puts it in an ``aborted`` state
 or starts the necessary steps to actively abort the transaction (e.g. to avoid losing money) and puts it in an ``aborting`` state.
 
-``[action:resume]``: Some aborted transactions may be resumed.  Whether resuming is possible depends on the transaction type.
+``[action:suspend]``: Suspends a pending transaction, stopping any associated network activities, but with a chance of trying
+again at a later time. This could be useful if a user needs to save battery power or bandwidth and an operation is expected
+to take longer (such as a backup, recovery or very large withdrawal operation).
+
+``[action:resume]``: Suspended transactions may be resumed, placing them back into a pending state.
 
 ``[action:abort-force]``: Directly puts an ``aborting`` transaction into the ``failed`` state.
 
+
+Whether aborting or resuming is possible depends on the transaction type, and usually only one
+of the two choices should be offered.
+
+
 Transaction Type: Withdrawal
 ----------------------------
 
 XXX: What if available denominations change?  Does this require a user re-approval if fees
 change due to this?
+CG: I think the answer can be "no", for two reasons: the wallet MUST pick denominations
+to withdraw with the "most long-term" withdraw window (i.e. active denominations that have
+the longest available withdraw durations). So in 99.9% of all cases, this will just succeed
+as a sane exchange will have a reasonable duration overlap, and in the 0.1% of cases it's
+really the user's fault for going offline in the middle of the operation. Plus, even in those
+0.1% of cases, it is highly unlikely that the fee would actually change: again 99% of key
+roatations can be expected to be there to rotate the key, and not to adjust the withdraw fee.
+And in the 1:1M case that the fee does *increase*, it's again unlikely to matter much to the
+user. So special-casing this and testing this is IMO just not worth it.
 
 * ``pending(bank-register-reserve)``
 
@@ -105,7 +139,7 @@ change due to this?
   exchange.
 
   * ``[poll-success] => pending(exchange-wait-reserve)``
-  * ``[action:abort] => aborting(wallet-to-bank)
+  * ``[action:abort] => aborting(wallet-to-bank)``
 
 * ``pending(exchange-wait-reserve)``
 
@@ -115,9 +149,15 @@ change due to this?
 
 * ``pending(withdrawing-coins)``
 
+  * ``[action:suspend] => suspended``
   * ``[processed-success] => done``
   * ``[processed-kyc-required] => kyc-required``
 
+* ``suspended``
+
+  * ``[action:resume] => pending``
+  * ``[action:abort] => aborted(after-wired)``
+    
 * ``kyc-required``
 
   * ``[poll-success] => pending(withdrawing-coins)``
@@ -140,30 +180,65 @@ change due to this?
 
 * ``done``
 
+  * ``[action:delete] => deleted``
+
+* ``deleted``
+
+  Withdrawn coins are preserved, as is reserve information for recoup.
+  So this mostly removes the entry from the visible transaction history.
+  Only once all coins were spent, the withdraw is fully removed.
+
+
 Transaction Type: Payment to Merchant
 -------------------------------------
 
 XXX: Also consider re-selection when the wallet accidentally double-spends coins
 or the selected coins have expired.  Do we ask the user in this case?
 
+CG: I think no. We correct our balance (after all, we got a proof of
+double-spending) and try other coins. If we do not have enough money left, we
+abort and simply inform the user that their balance was insufficient to make
+the payment after all (very sorry...).
+
+Note that the case of selected coins having expired shouldn't really happen,
+as the wallet should have noticed that when is started up, tried to refresh,
+and if that already failed should have update the balance with a transaction
+history entry saying something like "coins expired, offline too long" or
+something like that.
+
 * ``pending(download-proposal)``
 
   Initial state. Download (claim) the proposal from the merchant.
 
   XXX: Also consider repurchase detection here?
 
+  CG: Well, we could mention that this is a possible transition from
+  ``pending(download-proposal)`` to ``deleted`` with a side-effect
+  of transitioning the UI into a ``pending(repurchase-session-reset)``
+  on a different transaction (which before was in ``done``).
+
 * ``pending(proposed)``
 
   Let the user accept (or refuse) the payment.
 
   * ``[action:pay-accept] => pending(submit-payment)``
+  * ``[action:abort] => deleted`` -- user explicitly decides not
+    to proceed
+  * ``[action:expired] => deleted`` -- when the offer expires
+    before the user decides to make the payment! (We can keep
+    pending contracts even in a 'pending transaction' list to
+    allow the user to choose to not proceed, but then this
+    transition would clean up that list).
 
 * ``pending(submit-payment)``
 
   * ``[action:abort] => aborting(refund)``
   * ``[processed-success(auto-refund-enabled)] => pending(paid-auto-refund-check)``
   * ``[processed-error(expired)] => aborting(refresh)`` XXX: If the order is expired but the payment
-    succeeded partially before, do we still try an abort-refund?
+    succeeded partially before, do we still try an abort-refund? CG: YES, but of course
+    we probably should use the ``expired`` transition above a few seconds before the
+    offer *actually* expires to avoid this problem in 99.9% of real-world scenarios
+    ("prevent last-second payments client-side")
 
 * ``pending(submit-payment-replay)``
 
@@ -177,6 +252,7 @@ or the selected coins have expired.  Do we ask the user in this case?
 
   * ``[action:check-refund] => pending(paid-check-refund)``
   * ``[action:pay-replay] => pending(submit-payment-replay)``
+  * ``[action:delete] => deleted``
 
 * ``aborting(refund)``
 
@@ -233,6 +309,12 @@ Transaction Type: Refresh
 XXX: If we have to adjust the refund amount (because a coin has fewer funds on
 it than we expect), what is the resulting state of the whole refresh?
 
+CG: first the pending balance is decreased by the reduced amount, and then of
+course the final balance. The coin transaction responsible for the reduction
+in funds is historic (and we don't have details), so that just changes the total
+available balance in the wallet, but without an associated history entry (as we
+cannot give details).
+
 * ``pending``
 
   * ``[processed-success] => done``
@@ -264,6 +346,12 @@ Transaction Type: Deposit
 
 XXX: Handle expired/invalid coins in the coin selection. Does this require user approval if fees changed?
 
+CG: Again, expired coins should never happen. If deposit fees *increase* due
+to a double-spend detection during payment, we might want to have an
+_optional_ dialog ("Balance reduced by X as wallet state was not up-to-date
+(did you restore from backup?).  Consequently, the fees for this transactions
+increased from Y to Z.  [Abort] [Continue] + checkbox: [X] Do not ask again."
+
 * ``pending(initial)``
 
   The wallet deposits coins with the exchange.