From cac3d07abb078b41f88fb655d12aae647292e613 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 12 Aug 2020 17:27:04 -0700 Subject: [PATCH] Fix payment channel for actor changes Also, allocate the next available lane, instead of max+1. That way, we fill holes. Finally, check some error cases. --- paychmgr/mock_test.go | 7 +-- paychmgr/paych.go | 8 ++-- paychmgr/paych_test.go | 99 +++++++++++++++++++++++++----------------- paychmgr/state.go | 19 +++++--- 4 files changed, 80 insertions(+), 53 deletions(-) diff --git a/paychmgr/mock_test.go b/paychmgr/mock_test.go index 6b46b710a..2ccb6e1d7 100644 --- a/paychmgr/mock_test.go +++ b/paychmgr/mock_test.go @@ -66,10 +66,11 @@ func (sm *mockStateManager) setPaychState(a address.Address, actor *types.Actor, sm.paychState[a] = mockPchState{actor, state} } -func (sm *mockStateManager) storeLaneStates(laneStates []*paych.LaneState) (cid.Cid, error) { +func (sm *mockStateManager) storeLaneStates(laneStates map[uint64]paych.LaneState) (cid.Cid, error) { arr := adt.MakeEmptyArray(sm.store) - for _, ls := range laneStates { - if err := arr.Set(ls.ID, ls); err != nil { + for i, ls := range laneStates { + ls := ls + if err := arr.Set(i, &ls); err != nil { return cid.Undef, err } } diff --git a/paychmgr/paych.go b/paychmgr/paych.go index 87576dec2..d4b02ec11 100644 --- a/paychmgr/paych.go +++ b/paychmgr/paych.go @@ -331,11 +331,14 @@ func (ca *channelAccessor) laneState(ctx context.Context, state *paych.State, ch // very large index. var ls paych.LaneState laneStates := make(map[uint64]*paych.LaneState, lsamt.Length()) - lsamt.ForEach(&ls, func(i int64) error { + err = lsamt.ForEach(&ls, func(i int64) error { current := ls - laneStates[ls.ID] = ¤t + laneStates[uint64(i)] = ¤t return nil }) + if err != nil { + return nil, err + } // Apply locally stored vouchers vouchers, err := ca.store.VouchersForPaych(ch) @@ -353,7 +356,6 @@ func (ca *channelAccessor) laneState(ctx context.Context, state *paych.State, ch ls, ok := laneStates[v.Voucher.Lane] if !ok { ls = &paych.LaneState{ - ID: v.Voucher.Lane, Redeemed: types.NewInt(0), Nonce: 0, } diff --git a/paychmgr/paych_test.go b/paychmgr/paych_test.go index 14746d7f7..56aa9ebb2 100644 --- a/paychmgr/paych_test.go +++ b/paychmgr/paych_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/filecoin-project/specs-actors/actors/builtin" + "github.com/filecoin-project/specs-actors/actors/util/adt" "github.com/ipfs/go-cid" "github.com/filecoin-project/lotus/lib/sigs" @@ -39,6 +40,8 @@ func TestPaychOutbound(t *testing.T) { toAcct := tutils.NewIDAddr(t, 202) mock := newMockManagerAPI() + arr, err := adt.MakeEmptyArray(mock.store).Root() + require.NoError(t, err) mock.setAccountState(fromAcct, account.State{Address: from}) mock.setAccountState(toAcct, account.State{Address: to}) mock.setPaychState(ch, nil, paych.State{ @@ -47,6 +50,7 @@ func TestPaychOutbound(t *testing.T) { ToSend: big.NewInt(0), SettlingAt: abi.ChainEpoch(0), MinSettleHeight: abi.ChainEpoch(0), + LaneStates: arr, }) mgr, err := newManager(store, mock) @@ -76,6 +80,8 @@ func TestPaychInbound(t *testing.T) { toAcct := tutils.NewIDAddr(t, 202) mock := newMockManagerAPI() + arr, err := adt.MakeEmptyArray(mock.store).Root() + require.NoError(t, err) mock.setAccountState(fromAcct, account.State{Address: from}) mock.setAccountState(toAcct, account.State{Address: to}) mock.setPaychState(ch, nil, paych.State{ @@ -84,6 +90,7 @@ func TestPaychInbound(t *testing.T) { ToSend: big.NewInt(0), SettlingAt: abi.ChainEpoch(0), MinSettleHeight: abi.ChainEpoch(0), + LaneStates: arr, }) mgr, err := newManager(store, mock) @@ -127,7 +134,7 @@ func TestCheckVoucherValid(t *testing.T) { voucherAmount big.Int voucherLane uint64 voucherNonce uint64 - laneStates []*paych.LaneState + laneStates map[uint64]paych.LaneState }{{ name: "passes when voucher amount < balance", key: fromKeyPrivate, @@ -157,11 +164,12 @@ func TestCheckVoucherValid(t *testing.T) { voucherAmount: big.NewInt(5), voucherLane: 1, voucherNonce: 2, - laneStates: []*paych.LaneState{{ - ID: 1, - Redeemed: big.NewInt(2), - Nonce: 3, - }}, + laneStates: map[uint64]paych.LaneState{ + 1: { + Redeemed: big.NewInt(2), + Nonce: 3, + }, + }, }, { name: "passes when nonce higher", key: fromKeyPrivate, @@ -170,11 +178,12 @@ func TestCheckVoucherValid(t *testing.T) { voucherAmount: big.NewInt(5), voucherLane: 1, voucherNonce: 3, - laneStates: []*paych.LaneState{{ - ID: 1, - Redeemed: big.NewInt(2), - Nonce: 2, - }}, + laneStates: map[uint64]paych.LaneState{ + 1: { + Redeemed: big.NewInt(2), + Nonce: 2, + }, + }, }, { name: "passes when nonce for different lane", key: fromKeyPrivate, @@ -183,11 +192,12 @@ func TestCheckVoucherValid(t *testing.T) { voucherAmount: big.NewInt(5), voucherLane: 2, voucherNonce: 2, - laneStates: []*paych.LaneState{{ - ID: 1, - Redeemed: big.NewInt(2), - Nonce: 3, - }}, + laneStates: map[uint64]paych.LaneState{ + 1: { + Redeemed: big.NewInt(2), + Nonce: 3, + }, + }, }, { name: "fails when voucher has higher nonce but lower value than lane state", expectError: true, @@ -197,11 +207,12 @@ func TestCheckVoucherValid(t *testing.T) { voucherAmount: big.NewInt(5), voucherLane: 1, voucherNonce: 3, - laneStates: []*paych.LaneState{{ - ID: 1, - Redeemed: big.NewInt(6), - Nonce: 2, - }}, + laneStates: map[uint64]paych.LaneState{ + 1: { + Redeemed: big.NewInt(6), + Nonce: 2, + }, + }, }, { name: "fails when voucher + ToSend > balance", expectError: true, @@ -224,11 +235,13 @@ func TestCheckVoucherValid(t *testing.T) { voucherAmount: big.NewInt(6), voucherLane: 1, voucherNonce: 2, - laneStates: []*paych.LaneState{{ - ID: 1, // Lane 1 (same as voucher lane 1) - Redeemed: big.NewInt(4), - Nonce: 1, - }}, + laneStates: map[uint64]paych.LaneState{ + // Lane 1 (same as voucher lane 1) + 1: { + Redeemed: big.NewInt(4), + Nonce: 1, + }, + }, }, { // required balance = toSend + total redeemed // = 1 + 4 (lane 2) + 6 (voucher lane 1) @@ -242,11 +255,13 @@ func TestCheckVoucherValid(t *testing.T) { voucherAmount: big.NewInt(6), voucherLane: 1, voucherNonce: 1, - laneStates: []*paych.LaneState{{ - ID: 2, // Lane 2 (different from voucher lane 1) - Redeemed: big.NewInt(4), - Nonce: 1, - }}, + laneStates: map[uint64]paych.LaneState{ + // Lane 2 (different from voucher lane 1) + 2: { + Redeemed: big.NewInt(4), + Nonce: 1, + }, + }, }} for _, tcase := range tcases { @@ -311,15 +326,16 @@ func TestCheckVoucherValidCountingAllLanes(t *testing.T) { actorBalance := big.NewInt(10) toSend := big.NewInt(1) - laneStates := []*paych.LaneState{{ - ID: 1, - Nonce: 1, - Redeemed: big.NewInt(3), - }, { - ID: 2, - Nonce: 1, - Redeemed: big.NewInt(4), - }} + laneStates := map[uint64]paych.LaneState{ + 1: { + Nonce: 1, + Redeemed: big.NewInt(3), + }, + 2: { + Nonce: 1, + Redeemed: big.NewInt(4), + }, + } act := &types.Actor{ Code: builtin.AccountActorCodeID, @@ -630,6 +646,8 @@ func testSetupMgrWithChannel(ctx context.Context, t *testing.T) (*Manager, addre toAcct := tutils.NewActorAddr(t, "toAct") mock := newMockManagerAPI() + arr, err := adt.MakeEmptyArray(mock.store).Root() + require.NoError(t, err) mock.setAccountState(fromAcct, account.State{Address: from}) mock.setAccountState(toAcct, account.State{Address: to}) @@ -645,6 +663,7 @@ func testSetupMgrWithChannel(ctx context.Context, t *testing.T) (*Manager, addre ToSend: big.NewInt(0), SettlingAt: abi.ChainEpoch(0), MinSettleHeight: abi.ChainEpoch(0), + LaneStates: arr, }) store := NewStore(ds_sync.MutexWrap(ds.NewMapDatastore())) diff --git a/paychmgr/state.go b/paychmgr/state.go index 03c44a8a7..015479913 100644 --- a/paychmgr/state.go +++ b/paychmgr/state.go @@ -2,6 +2,7 @@ package paychmgr import ( "context" + "errors" "github.com/filecoin-project/specs-actors/actors/util/adt" @@ -77,14 +78,18 @@ func (ca *stateAccessor) nextLaneFromState(ctx context.Context, st *paych.State) return 0, nil } - maxLane := uint64(0) - var ls paych.LaneState - laneStates.ForEach(&ls, func(i int64) error { - if ls.ID > maxLane { - maxLane = ls.ID + nextID := int64(0) + stopErr := errors.New("stop") + if err := laneStates.ForEach(nil, func(i int64) error { + if nextID < i { + // We've found a hole. Stop here. + return stopErr } + nextID++ return nil - }) + }); err != nil && err != stopErr { + return 0, err + } - return maxLane + 1, nil + return uint64(nextID), nil }