Contracts: bring the actions README in line with the code

The introduction still described a /setup endpoint that no longer exists
and called view and revoke future work. Describe the lifecycle as it is:
setup is a step inside new and sign, not a call, and all four actions
have owner API methods and commands.

Prune the TODO and test lists to what is still open, and record what the
contract flows do not support.

libwallet/src/contract/actions/README.md

@@ -2,7 +2,15 @@
 
 ### API endpoints
 
-We introduce 3 new API endpoints `/new, /setup, /sign` each corresponding to a specific action on a contract. In the future we'll add the ability to also `view` a contract and to `revoke` it.
+Four owner API methods, each with a matching `grin-wallet contract` subcommand: `new`,
+`sign`, `view` and `revoke`. There is no `setup` endpoint; setup is a step rather than a
+call, run by `new` for the initiator and by `sign` for the signer if it hasn't happened
+yet. So a two party contract is `new`, `sign`, `sign`, and a self-spend is `new`, `sign`.
+
+`view` summarises a slate: participants, signatures so far, the net change it suggests or
+the one we already agreed to, and whether the transaction has confirmed. `revoke`
+double-spends an input of a contract we signed but that hasn't confirmed, and is
+idempotent, so an interrupted revoke can be run again.
 
 ### Rust implementation
 
@@ -22,7 +30,7 @@ return slate;
 ```
 
 We only allow contribution of custom inputs/outputs when we're doing the setup phase. Once the setup phase is done,
-we no longer allow any customization of inputs. This means that the customization can only happen at contract setup phase which is the first time we see the contract.Additionally, if we customize output selection, we immediately pick the inputs/outputs which means it's an early lock. These are however not added to the slate until we reach the 'sign' phase of the contract. Counterparties don't need to see our inputs/outputs before that. This means we always add inputs/outputs only when we have to and never before.
+we no longer allow any customization of inputs. This means that the customization can only happen at contract setup phase which is the first time we see the contract. Additionally, if we customize output selection, we immediately pick the inputs/outputs which means it's an early lock. These are however not added to the slate until we reach the 'sign' phase of the contract. Counterparties don't need to see our inputs/outputs before that. This means we always add inputs/outputs only when we have to and never before.
 
 Ideally we'd also separate side effects out of these functions e.g. computing the current_height
 or refreshing the outputs with updater::refresh_outputs(...). The current_height could be
@@ -30,65 +38,50 @@ communicated through a &ChainState parameter which would collect these values be
 Additionally, we could fetch the existing Context before the call to avoid doing db fetch.
 Separating side effects until the 'save_step' part would make these functions much easier to test.
 
+#### Not supported
+
+ - More than two parties. `num_participants` is validated to 1 (self-spend) or 2.
+ - Payment proof types other than invoice (type 1). A variant that doesn't depend on your
+   position in the contract is RFC work.
+ - `--send` and `--receive` take a human readable amount, as `send` and `invoice` do.
+   Everything reaching the API, including `--make-outputs`, is nanogrin.
+ - Writing the signed transaction in the same commit as the wallet state. `store_tx`
+   writes a file outside LMDB, as it does for every transaction in the wallet;
+   `contract::utils::save_step` describes the window and how to recover.
+
 #### TODOs
 
- - for payjoins, instead of doing Some("any"), find an actual input and put the actual commitment in --use-inputs (only do that if none is added). Think about how to do that in a way that would be nice also for the API. Maybe the API should just take "any" which gets transformed into one of the random inputs. Maybe the "any" is ok. It seems to work fine, might be better if we selected the input though.
- - sometimes the slatepack outputs with a \n for some reason which makes pasting it register \n as the end of paste and crashes?
- - remove casting decimals, we should accept value in nanogrins (including --make-outputs option), never as 0.1 Grin through the interface. Casting should be left to the gui wallet logic
- - separate side-effects out from the main computations
- - is keys::next_available_key(..) safe from race-conditions? (do we lock?)
- - ensure counterparty can't make you overpay fees through num_participants param
- - make sure the stored transaction is saved correctly at each step (TxLogEntry has stored_tx field, check other fields as well)
- - make sure the transaction log contains all the necessary data (check TODO comments on tx log entry functions)
- - graceful error handling
- - replace mutable objects with immutable when possible
- - make sure we lock the wallet when needed (check wallet_lock!() macro that is used in api/owner.rs)
- - add support for more than 2 parties (includes a new 'setup' API endpoint and command)
- - do we avoid using "too recent" outputs? e.g. though with depth < 10
- - remove unneeded imports
- - think if Context.setup_args.net_change type should be u64. If you make it i64, you divide it's size by 2. Perhaps it would be
-   better to have a u64 field and another field called 'positive' of type bool.
- - add --no-setup to 'new' command
- - add early-payment proofs. Make sure we have a symmetric variant of a payment proof to avoid having different proofs based on which position you are in the contract signing. Ideally, the position would be irrelevant.
- - what happens if you call contract sign on some slate that was not initiated as a contract slate and has different context values?
- - make sure you handle all the flows with coinbase outputs as well
- - check if they can trick you by providing a slate with different inputs/outputs that are yours
- - we have --add-outputs, but we should also lock if we use the --use-inputs param
- - check if contract_accounts_switch.rs is a legit scenario. It might need to return an error if the wallet is trying to sign with a different account
+ - `next_child_for` reads the derivation index and writes it back in two batches, so
+   concurrent steps can be handed the same child key. `next_child` has the same shape.
+ - Contract input selection requires a single confirmation.
+ - `--use-inputs` names the inputs to spend but doesn't lock them, unlike `--add-outputs`.
+ - Check the commitments we contributed are the ones that come back, so a counterparty
+   can't return a slate carrying inputs or outputs of ours that we didn't put there.
+ - Handle the coinbase flows. Coinbase inputs are built into the slate, but nothing else
+   is exercised.
+ - For a payjoin, `--use-inputs any` picks an input for us; naming the commitment would be
+   clearer, for the API as much as the CLI.
+ - `new` has no `--no-setup`.
+ - Separate side effects out from the computation, as described above.
+ - Decide whether `contract_accounts_switch.rs` is a legitimate scenario, or whether
+   signing under a different account than setup should error.
 
 #### Tests
- - test contract_fee (various test around fee contribution with 1 or 2 parties)
- - test save_step functionality (stored tx, context, logs,..)
- - test different output selection in step1 and step3
- - test foreign API for contract new and sign
- - test a case where the receiver doesn't have an input available (either not enough confirmations or no inputs)
- - contract_rsr.rs asserts that you get amount_credited=5, should it subtract the fees?
- - test using more than a single input
- - test that sending then again the same slatepack doesn't produce a new signature (to avoid leaking key)
- - test locking:
-	* test that outputs are locked after you sign
-	* test early locking when using --make-outputs or --use-inputs etc.
- - test --no-payjoin
- - test accounts
- - test 0-value outputs
- - test that if --no-payjoin is used, this doesn't mean that we early lock (we shouldn't). Same for --make-outputs
- - test slate content through steps
- - test negative cases (not enough funds, using input that doesn't exist, make outputs that go over the value, sign twice,...)
-
-#### DONE
- - Always "late-add" inputs/outputs to the slate
- - _Always_ add a change output, even if the change output ends up being a 0-value output
 
+ - signing the same slate twice, which `verify_not_signed` guards
+ - zero-value outputs, and `--make-outputs` beyond what is available
+ - a receiver with no spendable input, through lack of funds or of confirmations
+ - contracts spending more than one input
+ - slate contents at each step, not only the end state
+ - the foreign API entry points for `new` and `sign`
+ - fee contribution across one and two parties
 
 #### save_step
 
     // TODO:
     //  - is_signed should be derived from the slate
-    //  - Check what happens if the batch fails. Also think about possible race conditions because
-    // 	   of the time delay between the id was picked and saved.
     //  - Consider taking ownership of Context here. It should not be used after this is called.
 
-
 ### Side-effects
 
 #### Setup