Implement payment channel manager #132
Conversation
|
Alright, This is ready for review. |
| return cid.Undef, err | ||
| } | ||
|
|
||
| // TODO: should we wait for it...? |
There was a problem hiding this comment.
I'd say no. We have ChainWait which is easy enough to use, and eventually it should allow clients to specify things like the number of confirmations, which is sort of important here.
| return fmt.Errorf("nonce too low") | ||
| } | ||
|
|
||
| sendAmount = types.BigSub(sv.Amount, ls.Redeemed) |
There was a problem hiding this comment.
Do we want to allow this to be negative in case sv.Amount is somehow less than ls.Redeemed?
There was a problem hiding this comment.
Yes actually, the way the system is architected allows for money to flow in both directions. So if the target of the payment channel wants to pay money back to the creator, they can create a voucher with a lower amount, making this negative, and effectively send money back.
There was a problem hiding this comment.
Shouldn't this affect how we validate the signature few lines above?
paych/paych.go
Outdated
| ls, ok := pca.LaneStates[fmt.Sprint(sv.Lane)] | ||
| if !ok { | ||
| // TODO: should check against vouchers in our local store too | ||
| // there might be something conflicting |
There was a problem hiding this comment.
We should probably do that in both cases (ok || !ok aka. outside the if).
node/impl/full.go
Outdated
| // the channel. I think that a common usecase here might be 'i want to pay | ||
| // person X an additional 4 FIL'. If they just pass '4' for the amount, | ||
| // and the channel already has sent some amount, they wont actually be | ||
| // sending four additional filecoin. |
There was a problem hiding this comment.
At the very least we need to make sure it's clearly documented.
We may want new api which:
A. Opens new line with this relative amount, making sure that nonce == 0, picking the lane id for us, or
B. Adds current lane amount to the specified amount
For some reason I feel kind of weird about B (not showing the user what the resulting amount is), but it's probably the right thing
There was a problem hiding this comment.
Yeah, B feels weird to me too.
I think we can keep it as is for now, and as people actually start using it, we can adjust
node/impl/full.go
Outdated
|
|
||
| sv.Signature = sig | ||
|
|
||
| // REVIEWME: Should we immediately add this to the voucher store? or leave that to the caller? |
There was a problem hiding this comment.
Users may want to first call CheckVoucherValid, or leave that voucher for later / not use at all for whatever reason.
On the other hand not doing this will probably confuse someone at some point.
There was a problem hiding this comment.
exactly...
I think that for the usecases where people dont want it added to the local store, its easy enough to create your own vouchers manually
…to dev Merge pull request filecoin-project#132 from filecoin-project/fix-cpu-flag-detection fix: update cpu feature flag detection & rust feature usage
* Revert test timeouts and update Mir with debug messages
No description provided.