Payment Channel Manager makeover

[hannahhoward]

What

The Payment Channel Manager has some known issues.

Most prominently, it doesn't look at the lane limit, which set by spec-actors: filecoin-project/venus#4067

We may need to track funds on a per-lane basis, so that the overall voucher amounts don't exceed funds available filecoin-project/venus#4046

We should track payment channel creation state before it finishes on the chain possibly:
filecoin-project/venus#4045

There's even a design doc for solving the payment channel lane allocation limit: https://docs.google.com/document/d/1JT11U-1OELRHBNqMh4YJlzDtWChVQdkrvFnMZffYIc0/edit?ts=5eb3276f#

How

We should start with solving the lane allocation issue, which will neccesitate some significant redesign.

This can provide an opportunity to cleanup code and increase test coverage

We may want to look at extracting the payment channel to its own repo -- other language implementation teams looking at using markets software may want to use this code.

A solution to tracking payment channel creation state before it's on chain will likely fall out of solving the lane limit issue

regarding tracking funds on a per lane basis, we can solve this seperately when the module has been written

[hannahhoward]

Linking #2187

[magik6k]

It should also track Close/Settle calls and send unclaimed vouchers when needed

[ghost]

I like the basic framework we have now with payment channels, vouchers, etc. And all of the calls are very logical. However, creating a payment channel in practice is so unreliable for me (fails more often than not) that I've spent a lot of time in the debugger figuring out how all this stuff works. Having been over the code so many times, I would propose these ideas.

1. Get rid of GetOrCreatePaymentChannel in favor of a simpler model where the user decides whether they want to create a new payment channel or add funds to an existing one.

2. In store.go we acquire and retain the store lock until the end of the WaitFor...() method (e.g., go pm.waitForPaychCreateMsg(ctx, mcid) and the lines immediately following). But that wait can be several minutes! Retaining exclusive access for so long causes Lotus to lock up on other cli operations that need the store lock, like viewing wallet balances. My first thought was: does the store actually need to be locked at all when we're just waiting for messages to come in? (I don't know.) If yes, perhaps a reader/writer lock, if Go has that pattern, would reduce contention for simple read-only operations on the store.

One way out of the lock problem entirely is just to decide that the API and CLI will not block on any on-chain operation. Instead, we'd provide an asynchronous pattern. The user would a start API method that fires off a message to the message pool and returns immediately. Then, there is a second API call available to the user to poll for the completion status of his operation. User can exponentially back off the polling.

3. There's something wrong with the CBOR ser/de methods for Message and SignedMessage. The main symptom is a WARN-level message in the daemon log saying "cbor input had wrong number of fields," but that warning correlates perfectly for me with failure of the payment channel to get created. (I can repro it by just doing a CLI lotus paych get <any to addr> <any from addr> <any amt> and then opening up the log and seeing that warning.)

4. This is more of a question than a suggestion. Other than wasted gas, is there any downside to creating multiple payment channels for a given {to,from} pairing? It feels like we're adding a relatively high complexity burden (lanes, the whole GetOrCreate thing, merging vouchers) to avoid an edge case where a poorly written client program creates redundant payment channels, which I do not see as something terribly worrisome.

[hannahhoward]

#2297

[hannahhoward]

#2128
#2194
#2295
#2580
#2587
#2585
#2582

[dirkmc]

@hannahhoward I think all the referenced issues have been closed, or require further input from OP. I would suggest closing this issue and tracking any remaining issues individually.