Skip to content

Commit

Permalink
paych: Don't return settling/collected chennals from OutboundActiveBy…
Browse files Browse the repository at this point in the history
…FromTo
  • Loading branch information
magik6k committed Jan 6, 2022
1 parent f40338c commit 28de5d1
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 12 deletions.
8 changes: 8 additions & 0 deletions documentation/en/default-lotus-config.toml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion paychmgr/paych.go
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions paychmgr/paych_test.go
Expand Up @@ -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
Expand Down
108 changes: 107 additions & 1 deletion paychmgr/paychget_test.go
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion paychmgr/simple.go
Expand Up @@ -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}
}
Expand Down
20 changes: 17 additions & 3 deletions paychmgr/store.go
Expand Up @@ -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"

Expand Down Expand Up @@ -380,14 +379,29 @@ 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
}
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
})
}
Expand Down

0 comments on commit 28de5d1

Please sign in to comment.