address review feedback

This commit is contained in:
whyrusleeping 2019-08-13 11:47:40 -07:00
parent 12acee5242
commit 47efaece60
2 changed files with 14 additions and 8 deletions

View File

@ -545,6 +545,11 @@ func (a *FullNodeAPI) PaychVoucherAdd(ctx context.Context, ch address.Address, s
return a.PaychMgr.AddVoucher(ctx, ch, sv) return a.PaychMgr.AddVoucher(ctx, ch, sv)
} }
// PaychVoucherCreate creates a new signed voucher on the given payment channel
// with the given lane and amount. The value passed in is exactly the value
// that will be used to create the voucher, so if previous vouchers exist, the
// actual additional value of this voucher will only be the difference between
// the two.
func (a *FullNodeAPI) PaychVoucherCreate(ctx context.Context, pch address.Address, amt types.BigInt, lane uint64) (*types.SignedVoucher, error) { func (a *FullNodeAPI) PaychVoucherCreate(ctx context.Context, pch address.Address, amt types.BigInt, lane uint64) (*types.SignedVoucher, error) {
ci, err := a.PaychMgr.GetChannelInfo(pch) ci, err := a.PaychMgr.GetChannelInfo(pch)
if err != nil { if err != nil {
@ -556,11 +561,6 @@ func (a *FullNodeAPI) PaychVoucherCreate(ctx context.Context, pch address.Addres
return nil, err return nil, err
} }
// REVIEW ME: The amount passed into this method is the absolute value on
// 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.
sv := &types.SignedVoucher{ sv := &types.SignedVoucher{
Lane: lane, Lane: lane,
Nonce: nonce, Nonce: nonce,
@ -579,7 +579,9 @@ func (a *FullNodeAPI) PaychVoucherCreate(ctx context.Context, pch address.Addres
sv.Signature = sig sv.Signature = sig
// REVIEWME: Should we immediately add this to the voucher store? or leave that to the caller? if err := a.PaychMgr.AddVoucher(ctx, pch, sv); err != nil {
return nil, xerrors.Errorf("failed to persist voucher: %w", err)
}
return sv, nil return sv, nil
} }

View File

@ -72,6 +72,10 @@ func (pm *Manager) CheckVoucherValid(ctx context.Context, ch address.Address, sv
if err != nil { if err != nil {
return err return err
} }
// TODO: technically, either party may create and sign a voucher.
// However, for now, we only accept them from the channel creator.
// More complex handling logic can be added later
if err := sv.Signature.Verify(pca.From, vb); err != nil { if err := sv.Signature.Verify(pca.From, vb); err != nil {
return err return err
} }
@ -79,10 +83,10 @@ func (pm *Manager) CheckVoucherValid(ctx context.Context, ch address.Address, sv
sendAmount := sv.Amount sendAmount := sv.Amount
// now check the lane state // now check the lane state
// TODO: should check against vouchers in our local store too
// there might be something conflicting
ls, ok := pca.LaneStates[fmt.Sprint(sv.Lane)] ls, ok := pca.LaneStates[fmt.Sprint(sv.Lane)]
if !ok { if !ok {
// TODO: should check against vouchers in our local store too
// there might be something conflicting
} else { } else {
if ls.Closed { if ls.Closed {
return fmt.Errorf("voucher is on a closed lane") return fmt.Errorf("voucher is on a closed lane")