From 103bc53fc1ddd9d4f4418c90e91a539e550b19db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Thu, 6 Jan 2022 19:51:26 +0100 Subject: [PATCH] paych: Don't return settling/collected chennals from OutboundActiveByFromTo --- documentation/en/default-lotus-config.toml | 8 ++ paychmgr/paych.go | 2 +- paychmgr/paych_test.go | 12 +-- paychmgr/paychget_test.go | 108 ++++++++++++++++++++- paychmgr/simple.go | 2 +- paychmgr/store.go | 20 +++- 6 files changed, 140 insertions(+), 12 deletions(-) diff --git a/documentation/en/default-lotus-config.toml b/documentation/en/default-lotus-config.toml index 3cb8977b290..016e1290cb9 100644 --- a/documentation/en/default-lotus-config.toml +++ b/documentation/en/default-lotus-config.toml @@ -121,6 +121,14 @@ # env var: LOTUS_CLIENT_SIMULTANEOUSTRANSFERSFORRETRIEVAL #SimultaneousTransfersForRetrieval = 20 + # Require that retrievals perform no on-chain retrievals. Paid retrievals + # without existing payment channels with available funds will fail instead + # of automatically performing on-chain operations. + # + # type: bool + # env var: LOTUS_CLIENT_OFFCHAINRETRIEVAL + #OffChainRetrieval = false + [Wallet] # type: string diff --git a/paychmgr/paych.go b/paychmgr/paych.go index 16c6604c6ca..5fdb4d8842e 100644 --- a/paychmgr/paych.go +++ b/paychmgr/paych.go @@ -106,7 +106,7 @@ func (ca *channelAccessor) outboundActiveByFromTo(ctx context.Context, from, to ca.lk.Lock() defer ca.lk.Unlock() - return ca.store.OutboundActiveByFromTo(ctx, from, to) + return ca.store.OutboundActiveByFromTo(ctx, ca.api, from, to) } // createVoucher creates a voucher with the given specification, setting its diff --git a/paychmgr/paych_test.go b/paychmgr/paych_test.go index 165c945b8cb..24214d1b585 100644 --- a/paychmgr/paych_test.go +++ b/paychmgr/paych_test.go @@ -465,18 +465,18 @@ func TestAddVoucherInboundWalletKey(t *testing.T) { toAcct := tutils.NewActorAddr(t, "toAct") // Create an actor for the channel in state - act := &types.Actor{ - Code: builtin.AccountActorCodeID, - Head: cid.Cid{}, - Nonce: 0, - Balance: types.NewInt(20), - } mock := newMockManagerAPI() mock.setAccountAddress(fromAcct, from) mock.setAccountAddress(toAcct, to) + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } mock.setPaychState(ch, act, paychmock.NewMockPayChState(fromAcct, toAcct, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) // Create a manager diff --git a/paychmgr/paychget_test.go b/paychmgr/paychget_test.go index f16b146f627..c53a85d86a0 100644 --- a/paychmgr/paychget_test.go +++ b/paychmgr/paychget_test.go @@ -132,6 +132,14 @@ func TestPaychGetCreateChannelThenAddFunds(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -219,6 +227,14 @@ func TestPaychGetCreatePrefundedChannelThenAddFunds(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -406,6 +422,14 @@ func TestPaychGetRecoverAfterAddFundsError(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -502,6 +526,14 @@ func TestPaychGetRestartAfterCreateChannelMsg(t *testing.T) { mock2 := newMockManagerAPI() defer mock2.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock2.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr2, err := newManager(store, mock2) require.NoError(t, err) @@ -566,6 +598,14 @@ func TestPaychGetRestartAfterAddFundsMsg(t *testing.T) { mock := newMockManagerAPI() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -590,6 +630,8 @@ func TestPaychGetRestartAfterAddFundsMsg(t *testing.T) { mock2 := newMockManagerAPI() defer mock2.close() + mock2.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr2, err := newManager(store, mock2) require.NoError(t, err) @@ -625,10 +667,19 @@ func TestPaychGetWait(t *testing.T) { from := tutils.NewIDAddr(t, 101) to := tutils.NewIDAddr(t, 102) + expch := tutils.NewIDAddr(t, 100) mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(expch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -637,7 +688,6 @@ func TestPaychGetWait(t *testing.T) { _, createMsgCid, err := mgr.GetPaych(ctx, from, to, amt, onChainReserve) require.NoError(t, err) - expch := tutils.NewIDAddr(t, 100) go func() { // 3. Send response response := testChannelResponse(t, expch) @@ -763,6 +813,14 @@ func TestPaychGetMergeAddFunds(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -859,6 +917,14 @@ func TestPaychGetMergePrefundAndReserve(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -955,6 +1021,14 @@ func TestPaychGetMergePrefundAndReservePrefunded(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -1052,6 +1126,14 @@ func TestPaychGetMergePrefundAndReservePrefundedOneOffchain(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -1130,6 +1212,14 @@ func TestPaychGetMergePrefundAndReservePrefundedBothOffchainOneFail(t *testing.T mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -1208,6 +1298,14 @@ func TestPaychGetMergePrefundAndReserveOneOffchainOneFail(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) @@ -1295,6 +1393,14 @@ func TestPaychGetMergeAddFundsCtxCancelOne(t *testing.T) { mock := newMockManagerAPI() defer mock.close() + act := &types.Actor{ + Code: builtin.AccountActorCodeID, + Head: cid.Cid{}, + Nonce: 0, + Balance: types.NewInt(20), + } + mock.setPaychState(ch, act, paychmock.NewMockPayChState(from, to, abi.ChainEpoch(0), make(map[uint64]paych.LaneState))) + mgr, err := newManager(store, mock) require.NoError(t, err) diff --git a/paychmgr/simple.go b/paychmgr/simple.go index ef0f81b8748..56bea0d27f8 100644 --- a/paychmgr/simple.go +++ b/paychmgr/simple.go @@ -436,7 +436,7 @@ func (ca *channelAccessor) processTask(merged *mergedFundsReq, amt, avail types. // Get the payment channel for the from/to addresses. // Note: It's ok if we get ErrChannelNotTracked. It just means we need to // create a channel. - channelInfo, err := ca.store.OutboundActiveByFromTo(ctx, ca.from, ca.to) + channelInfo, err := ca.store.OutboundActiveByFromTo(ctx, ca.api, ca.from, ca.to) if err != nil && err != ErrChannelNotTracked { return &paychFundsRes{err: err} } diff --git a/paychmgr/store.go b/paychmgr/store.go index d5c8e198049..7d6ee32be67 100644 --- a/paychmgr/store.go +++ b/paychmgr/store.go @@ -6,9 +6,8 @@ import ( "errors" "fmt" - "golang.org/x/xerrors" - "github.com/google/uuid" + "golang.org/x/xerrors" "github.com/filecoin-project/lotus/chain/types" @@ -380,7 +379,7 @@ func (ps *Store) GetMessage(ctx context.Context, mcid cid.Cid) (*MsgInfo, error) // OutboundActiveByFromTo looks for outbound channels that have not been // settled, with the given from / to addresses -func (ps *Store) OutboundActiveByFromTo(ctx context.Context, from address.Address, to address.Address) (*ChannelInfo, error) { +func (ps *Store) OutboundActiveByFromTo(ctx context.Context, sma stateManagerAPI, from address.Address, to address.Address) (*ChannelInfo, error) { return ps.findChan(ctx, func(ci *ChannelInfo) bool { if ci.Direction != DirOutbound { return false @@ -388,6 +387,21 @@ func (ps *Store) OutboundActiveByFromTo(ctx context.Context, from address.Addres if ci.Settling { return false } + + if ci.Channel != nil { + _, st, err := sma.GetPaychState(ctx, *ci.Channel, nil) + if err != nil { + return false + } + sat, err := st.SettlingAt() + if err != nil { + return false + } + if sat != 0 { + return false + } + } + return ci.Control == from && ci.Target == to }) }