From 83de950381939516b49c4b85e7b252a790e1e414 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 7 Dec 2022 11:21:52 -0700 Subject: [PATCH 01/27] [14124]: Define a SendRestriction function type. --- x/bank/keeper/restrictions.go | 41 +++ x/bank/keeper/restrictions_test.go | 499 +++++++++++++++++++++++++++++ 2 files changed, 540 insertions(+) create mode 100644 x/bank/keeper/restrictions.go create mode 100644 x/bank/keeper/restrictions_test.go diff --git a/x/bank/keeper/restrictions.go b/x/bank/keeper/restrictions.go new file mode 100644 index 000000000000..82d2185a06c8 --- /dev/null +++ b/x/bank/keeper/restrictions.go @@ -0,0 +1,41 @@ +package keeper + +import sdk "github.com/cosmos/cosmos-sdk/types" + +// A SendRestrictionFn is a function that can restrict sends and/or provide a new receiver address. +type SendRestrictionFn func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (newToAddr sdk.AccAddress, err error) + +// Then creates a composite restriction that runs this one then the provided second one. +func (r SendRestrictionFn) Then(second SendRestrictionFn) SendRestrictionFn { + return ComposeSendRestrictions(r, second) +} + +// ComposeSendRestrictions combines multiple SendRestrictions into one. +// nil entries are ignored. +// If all entries are nil, nil is returned. +// If exactly one entry is not nil, it is returned. +// Otherwise, a new SendRestrictionFn is returned that runs the non-nil restrictions in the order they are given. +func ComposeSendRestrictions(restrictions ...SendRestrictionFn) SendRestrictionFn { + toRun := make([]SendRestrictionFn, 0, len(restrictions)) + for _, r := range restrictions { + if r != nil { + toRun = append(toRun, r) + } + } + switch len(toRun) { + case 0: + return nil + case 1: + return toRun[0] + } + return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + var err error + for _, r := range toRun { + toAddr, err = r(ctx, fromAddr, toAddr, amt) + if err != nil { + return toAddr, err + } + } + return toAddr, err + } +} diff --git a/x/bank/keeper/restrictions_test.go b/x/bank/keeper/restrictions_test.go new file mode 100644 index 000000000000..4824c96c9645 --- /dev/null +++ b/x/bank/keeper/restrictions_test.go @@ -0,0 +1,499 @@ +package keeper_test + +import ( + "errors" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/bank/keeper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "testing" +) + +// SendRestrictionArgs are the args provided to a SendRestrictionFn function. +type SendRestrictionArgs struct { + FromAddr sdk.AccAddress + ToAddr sdk.AccAddress + Coins sdk.Coins +} + +// SendRestrictionTestHelper is a struct with stuff helpful for testing the SendRestrictionFn stuff. +type SendRestrictionTestHelper struct { + Calls []*SendRestrictionArgs +} + +func NewSendRestrictionTestHelper() *SendRestrictionTestHelper { + return &SendRestrictionTestHelper{Calls: make([]*SendRestrictionArgs, 0, 2)} +} + +// RecordCall makes note that the provided args were used as a funcion call. +func (s *SendRestrictionTestHelper) RecordCall(fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) { + s.Calls = append(s.Calls, s.NewArgs(fromAddr, toAddr, coins)) +} + +// NewCalls is just a shorter way to create a []*SendRestrictionArgs. +func (s *SendRestrictionTestHelper) NewCalls(args ...*SendRestrictionArgs) []*SendRestrictionArgs { + return args +} + +// NewArgs creates a new SendRestrictionArgs. +func (s *SendRestrictionTestHelper) NewArgs(fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) *SendRestrictionArgs { + return &SendRestrictionArgs{ + FromAddr: fromAddr, + ToAddr: toAddr, + Coins: coins, + } +} + +// NoOpRestriction creates a new SendRestrictionFn function that records the arguments it's called with and returns the provided toAddr. +func (s *SendRestrictionTestHelper) NoOpRestriction() keeper.SendRestrictionFn { + return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { + s.RecordCall(fromAddr, toAddr, coins) + return toAddr, nil + } +} + +// NewToRestriction creates a new SendRestrictionFn function that returns a different toAddr than provided. +func (s *SendRestrictionTestHelper) NewToRestriction(addr sdk.AccAddress) keeper.SendRestrictionFn { + return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { + s.RecordCall(fromAddr, toAddr, coins) + return addr, nil + } +} + +// ErrorRestriction creates a new SendRestrictionFn function that returns a nil toAddr and an error. +func (s *SendRestrictionTestHelper) ErrorRestriction(message string) keeper.SendRestrictionFn { + return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { + s.RecordCall(fromAddr, toAddr, coins) + return nil, errors.New(message) + } +} + +// SendRestrictionTestParams are parameters to test regarding calling a SendRestrictionFn. +type SendRestrictionTestParams struct { + // ExpNil is whether to expect the provided SendRestrictionFn to be nil. + // If it is true, the rest of these test params are ignored. + ExpNil bool + // FromAddr is the SendRestrictionFn fromAddr input. + FromAddr sdk.AccAddress + // ToAddr is the SendRestrictionFn toAddr input. + ToAddr sdk.AccAddress + // Coins is the SendRestrictionFn coins input. + Coins sdk.Coins + // ExpAddr is the expected return address. + ExpAddr sdk.AccAddress + // ExpErr is the expected return error string. + ExpErr string + // ExpCalls is the args of all the SendRestrictionFn calls that end up being made. + ExpCalls []*SendRestrictionArgs +} + +// TestActual tests the provided SendRestrictionFn using the provided test parameters. +func (s *SendRestrictionTestHelper) TestActual(t *testing.T, tp *SendRestrictionTestParams, actual keeper.SendRestrictionFn) { + t.Helper() + if tp.ExpNil { + require.Nil(t, actual, "ComposeSendRestrictions result") + } else { + require.NotNil(t, actual, "ComposeSendRestrictions result") + s.Calls = s.Calls[:0] + addr, err := actual(sdk.Context{}, tp.FromAddr, tp.ToAddr, tp.Coins) + if len(tp.ExpErr) != 0 { + assert.EqualError(t, err, tp.ExpErr, "composite SendRestrictionFn output error") + } else { + assert.NoError(t, err, "composite SendRestrictionFn output error") + } + assert.Equal(t, tp.ExpAddr, addr, "composite SendRestrictionFn output address") + assert.Equal(t, tp.ExpCalls, s.Calls, "args given to funcs in composite SendRestrictionFn") + } +} + +func TestSendRestriction_Then(t *testing.T) { + frAddr := sdk.AccAddress("fromaddr____________") + addr0 := sdk.AccAddress("0addr_______________") + addr1 := sdk.AccAddress("1addr_______________") + addr2 := sdk.AccAddress("2addr_______________") + addr3 := sdk.AccAddress("3addr_______________") + addr4 := sdk.AccAddress("4addr_______________") + coins := sdk.NewCoins(sdk.NewInt64Coin("acoin", 2), sdk.NewInt64Coin("bcoin", 4)) + + h := NewSendRestrictionTestHelper() + + tests := []struct { + name string + base keeper.SendRestrictionFn + second keeper.SendRestrictionFn + exp *SendRestrictionTestParams + }{ + { + name: "nil nil", + base: nil, + second: nil, + exp: &SendRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "nil noop", + base: nil, + second: h.NoOpRestriction(), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: addr1, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + }, + }, + { + name: "noop nil", + base: h.NoOpRestriction(), + second: nil, + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: addr1, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + }, + }, + { + name: "noop noop", + base: h.NoOpRestriction(), + second: h.NoOpRestriction(), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: addr1, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr1, coins)), + }, + }, + { + name: "setter setter", + base: h.NewToRestriction(addr2), + second: h.NewToRestriction(addr3), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: addr3, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr2, coins)), + }, + }, + { + name: "setter error", + base: h.NewToRestriction(addr2), + second: h.ErrorRestriction("this is a test error"), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: nil, + ExpErr: "this is a test error", + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr2, coins)), + }, + }, + { + name: "error setter", + base: h.ErrorRestriction("another test error"), + second: h.NewToRestriction(addr3), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: nil, + ExpErr: "another test error", + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + }, + }, + { + name: "error error", + base: h.ErrorRestriction("first test error"), + second: h.ErrorRestriction("second test error"), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: nil, + ExpErr: "first test error", + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + }, + }, + { + name: "double chain", + base: keeper.ComposeSendRestrictions(h.NewToRestriction(addr1), h.NewToRestriction(addr2)), + second: keeper.ComposeSendRestrictions(h.NewToRestriction(addr3), h.NewToRestriction(addr4)), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr0, + Coins: coins, + ExpAddr: addr4, + ExpCalls: h.NewCalls( + h.NewArgs(frAddr, addr0, coins), + h.NewArgs(frAddr, addr1, coins), + h.NewArgs(frAddr, addr2, coins), + h.NewArgs(frAddr, addr3, coins), + ), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var actual keeper.SendRestrictionFn + testFunc := func() { + actual = tc.base.Then(tc.second) + } + require.NotPanics(t, testFunc, "SendRestrictionFn.Then") + h.TestActual(t, tc.exp, actual) + }) + } +} + +func TestComposeSendRestrictions(t *testing.T) { + rz := func(rs ...keeper.SendRestrictionFn) []keeper.SendRestrictionFn { + return rs + } + frAddr := sdk.AccAddress("fromaddr____________") + addr0 := sdk.AccAddress("0addr_______________") + addr1 := sdk.AccAddress("1addr_______________") + addr2 := sdk.AccAddress("2addr_______________") + addr3 := sdk.AccAddress("3addr_______________") + addr4 := sdk.AccAddress("4addr_______________") + coins := sdk.NewCoins(sdk.NewInt64Coin("acoin", 2), sdk.NewInt64Coin("bcoin", 4)) + + h := NewSendRestrictionTestHelper() + + tests := []struct { + name string + input []keeper.SendRestrictionFn + exp *SendRestrictionTestParams + }{ + { + name: "nil list", + input: nil, + exp: &SendRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "empty list", + input: rz(), + exp: &SendRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "only nil entry", + input: rz(nil), + exp: &SendRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "five nil entries", + input: rz(nil, nil, nil, nil, nil), + exp: &SendRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "only noop entry", + input: rz(h.NoOpRestriction()), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr0, + Coins: coins, + ExpAddr: addr0, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins)), + }, + }, + { + name: "only error entry", + input: rz(h.ErrorRestriction("test error")), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr0, + Coins: coins, + ExpAddr: nil, + ExpErr: "test error", + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins)), + }, + }, + { + name: "noop nil nil", + input: rz(h.NoOpRestriction(), nil, nil), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr0, + Coins: coins, + ExpAddr: addr0, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins)), + }, + }, + { + name: "nil noop nil", + input: rz(nil, h.NoOpRestriction(), nil), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: addr1, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + }, + }, + { + name: "nil nil noop", + input: rz(nil, nil, h.NoOpRestriction()), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr2, + Coins: coins, + ExpAddr: addr2, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr2, coins)), + }, + }, + { + name: "noop noop nil", + input: rz(h.NoOpRestriction(), h.NoOpRestriction(), nil), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr0, + Coins: coins, + ExpAddr: addr0, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins), h.NewArgs(frAddr, addr0, coins)), + }, + }, + { + name: "noop nil noop", + input: rz(h.NoOpRestriction(), nil, h.NoOpRestriction()), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr1, + Coins: coins, + ExpAddr: addr1, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr1, coins)), + }, + }, + { + name: "nil noop noop", + input: rz(nil, h.NoOpRestriction(), h.NoOpRestriction()), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr2, + Coins: coins, + ExpAddr: addr2, + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr2, coins), h.NewArgs(frAddr, addr2, coins)), + }, + }, + { + name: "noop noop noop", + input: rz(h.NoOpRestriction(), h.NoOpRestriction(), h.NoOpRestriction()), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr3, + Coins: coins, + ExpAddr: addr3, + ExpCalls: h.NewCalls( + h.NewArgs(frAddr, addr3, coins), + h.NewArgs(frAddr, addr3, coins), + h.NewArgs(frAddr, addr3, coins), + ), + }, + }, + { + name: "err noop noop", + input: rz(h.ErrorRestriction("first error"), h.NoOpRestriction(), h.NoOpRestriction()), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr4, + Coins: coins, + ExpAddr: nil, + ExpErr: "first error", + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr4, coins)), + }, + }, + { + name: "noop err noop", + input: rz(h.NoOpRestriction(), h.ErrorRestriction("second error"), h.NoOpRestriction()), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr4, + Coins: coins, + ExpAddr: nil, + ExpErr: "second error", + ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr4, coins), h.NewArgs(frAddr, addr4, coins)), + }, + }, + { + name: "noop noop err", + input: rz(h.NoOpRestriction(), h.NoOpRestriction(), h.ErrorRestriction("third error")), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr4, + Coins: coins, + ExpAddr: nil, + ExpErr: "third error", + ExpCalls: h.NewCalls( + h.NewArgs(frAddr, addr4, coins), + h.NewArgs(frAddr, addr4, coins), + h.NewArgs(frAddr, addr4, coins), + ), + }, + }, + { + name: "new-to err err", + input: rz(h.NewToRestriction(addr0), h.ErrorRestriction("second error"), h.ErrorRestriction("third error")), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr4, + Coins: coins, + ExpAddr: nil, + ExpErr: "second error", + ExpCalls: h.NewCalls( + h.NewArgs(frAddr, addr4, coins), + h.NewArgs(frAddr, addr0, coins), + ), + }, + }, + { + name: "big bang", + input: rz( + h.NoOpRestriction(), nil, h.NewToRestriction(addr1), // Called with orig toAddr. + nil, h.NoOpRestriction(), h.NewToRestriction(addr2), // Called with addr1 toAddr. + h.NewToRestriction(addr3), // Called with addr2 toAddr. + nil, h.NoOpRestriction(), h.NewToRestriction(addr4), // Called with addr3 toAddr. + nil, h.NoOpRestriction(), nil, nil, h.ErrorRestriction("oops, an error"), // Called with addr4 toAddr. + h.NewToRestriction(addr0), nil, h.NoOpRestriction(), // Not called. + ), + exp: &SendRestrictionTestParams{ + FromAddr: frAddr, + ToAddr: addr0, + Coins: coins, + ExpAddr: nil, + ExpErr: "oops, an error", + ExpCalls: h.NewCalls( + h.NewArgs(frAddr, addr0, coins), + h.NewArgs(frAddr, addr0, coins), + h.NewArgs(frAddr, addr1, coins), + h.NewArgs(frAddr, addr1, coins), + h.NewArgs(frAddr, addr2, coins), + h.NewArgs(frAddr, addr3, coins), + h.NewArgs(frAddr, addr3, coins), + h.NewArgs(frAddr, addr4, coins), + h.NewArgs(frAddr, addr4, coins), + ), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var actual keeper.SendRestrictionFn + testFunc := func() { + actual = keeper.ComposeSendRestrictions(tc.input...) + } + require.NotPanics(t, testFunc, "ComposeSendRestrictions") + h.TestActual(t, tc.exp, actual) + }) + } +} From 1648159504f227c1b3fa0ef60591f56adb40f433 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 7 Dec 2022 16:29:36 -0700 Subject: [PATCH 02/27] [14124]: Move the MintingRestrictionFn in with the SendRestrictionFn and give it similar stuff. Add unit tests for it. --- x/bank/keeper/keeper.go | 17 +- x/bank/keeper/restrictions.go | 55 ++- x/bank/keeper/restrictions_test.go | 623 ++++++++++++++++++++++++----- 3 files changed, 575 insertions(+), 120 deletions(-) diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 4fbb622ad9e6..4c2b7fd93ddd 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -59,8 +59,6 @@ type BaseKeeper struct { mintCoinsRestrictionFn MintingRestrictionFn } -type MintingRestrictionFn func(ctx sdk.Context, coins sdk.Coins) error - // GetPaginatedTotalSupply queries for the supply, ignoring 0 coins, with a given pagination func (k BaseKeeper) GetPaginatedTotalSupply(ctx sdk.Context, pagination *query.PageRequest) (sdk.Coins, *query.PageResponse, error) { store := ctx.KVStore(k.storeKey) @@ -108,7 +106,7 @@ func NewBaseKeeper( ak: ak, cdc: cdc, storeKey: storeKey, - mintCoinsRestrictionFn: func(ctx sdk.Context, coins sdk.Coins) error { return nil }, + mintCoinsRestrictionFn: NoOpMintingRestrictionFn, } } @@ -118,18 +116,7 @@ func NewBaseKeeper( // // bankKeeper.WithMintCoinsRestriction(restriction1).WithMintCoinsRestriction(restriction2) func (k BaseKeeper) WithMintCoinsRestriction(check MintingRestrictionFn) BaseKeeper { - oldRestrictionFn := k.mintCoinsRestrictionFn - k.mintCoinsRestrictionFn = func(ctx sdk.Context, coins sdk.Coins) error { - err := check(ctx, coins) - if err != nil { - return err - } - err = oldRestrictionFn(ctx, coins) - if err != nil { - return err - } - return nil - } + k.mintCoinsRestrictionFn = check.Then(k.mintCoinsRestrictionFn) return k } diff --git a/x/bank/keeper/restrictions.go b/x/bank/keeper/restrictions.go index 82d2185a06c8..298d89953eb5 100644 --- a/x/bank/keeper/restrictions.go +++ b/x/bank/keeper/restrictions.go @@ -2,15 +2,66 @@ package keeper import sdk "github.com/cosmos/cosmos-sdk/types" -// A SendRestrictionFn is a function that can restrict sends and/or provide a new receiver address. +// A MintingRestrictionFn can restrict minting of coins. +type MintingRestrictionFn func(ctx sdk.Context, coins sdk.Coins) error + +var _ MintingRestrictionFn = NoOpMintingRestrictionFn + +// NoOpMintingRestrictionFn is a no-op MintingRestrictionFn. +func NoOpMintingRestrictionFn(_ sdk.Context, _ sdk.Coins) error { + return nil +} + +// Then creates a composite restriction that runs this one then the provided second one. +func (r MintingRestrictionFn) Then(second MintingRestrictionFn) MintingRestrictionFn { + return ComposeMintingRestrictions(r, second) +} + +// ComposeMintingRestrictions combines multiple MintingRestrictionFn into one. +// nil entries are ignored. +// If all entries are nil, nil is returned. +// If exactly one entry is not nil, it is returned. +// Otherwise, a new MintingRestrictionFn is returned that runs the non-nil restrictions in the order they are given. +func ComposeMintingRestrictions(restrictions ...MintingRestrictionFn) MintingRestrictionFn { + toRun := make([]MintingRestrictionFn, 0, len(restrictions)) + for _, r := range restrictions { + if r != nil { + toRun = append(toRun, r) + } + } + switch len(toRun) { + case 0: + return nil + case 1: + return toRun[0] + } + return func(ctx sdk.Context, coins sdk.Coins) error { + for _, r := range toRun { + err := r(ctx, coins) + if err != nil { + return err + } + } + return nil + } +} + +// A SendRestrictionFn can restrict sends and/or provide a new receiver address. type SendRestrictionFn func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (newToAddr sdk.AccAddress, err error) +var _ SendRestrictionFn = NoOpSendRestrictionFn + +// NoOpSendRestrictionFn is a no-op SendRestrictionFn. +func NoOpSendRestrictionFn(_ sdk.Context, _, toAddr sdk.AccAddress, _ sdk.Coins) (sdk.AccAddress, error) { + return toAddr, nil +} + // Then creates a composite restriction that runs this one then the provided second one. func (r SendRestrictionFn) Then(second SendRestrictionFn) SendRestrictionFn { return ComposeSendRestrictions(r, second) } -// ComposeSendRestrictions combines multiple SendRestrictions into one. +// ComposeSendRestrictions combines multiple SendRestrictionFn into one. // nil entries are ignored. // If all entries are nil, nil is returned. // If exactly one entry is not nil, it is returned. diff --git a/x/bank/keeper/restrictions_test.go b/x/bank/keeper/restrictions_test.go index 4824c96c9645..f357632177bc 100644 --- a/x/bank/keeper/restrictions_test.go +++ b/x/bank/keeper/restrictions_test.go @@ -9,8 +9,391 @@ import ( "testing" ) +// MintingRestrictionArgs are the args provided to a MintingRestrictionFn function. +type MintingRestrictionArgs struct { + Name string + Coins sdk.Coins +} + +// MintingRestrictionTestHelper is a struct with stuff helpful for testing the MintingRestrictionFn stuff. +type MintingRestrictionTestHelper struct { + Calls []*MintingRestrictionArgs +} + +func NewMintingRestrictionTestHelper() *MintingRestrictionTestHelper { + return &MintingRestrictionTestHelper{Calls: make([]*MintingRestrictionArgs, 0, 2)} +} + +// RecordCall makes note that the provided args were used as a funcion call. +func (s *MintingRestrictionTestHelper) RecordCall(name string, coins sdk.Coins) { + s.Calls = append(s.Calls, s.NewArgs(name, coins)) +} + +// NewCalls is just a shorter way to create a []*MintingRestrictionArgs. +func (s *MintingRestrictionTestHelper) NewCalls(args ...*MintingRestrictionArgs) []*MintingRestrictionArgs { + return args +} + +// NewArgs creates a new MintingRestrictionArgs. +func (s *MintingRestrictionTestHelper) NewArgs(name string, coins sdk.Coins) *MintingRestrictionArgs { + return &MintingRestrictionArgs{ + Name: name, + Coins: coins, + } +} + +// NamedRestriction creates a new MintingRestrictionFn function that records the arguments it's called with and returns nil. +func (s *MintingRestrictionTestHelper) NamedRestriction(name string) keeper.MintingRestrictionFn { + return func(_ sdk.Context, coins sdk.Coins) error { + s.RecordCall(name, coins) + return nil + } +} + +// ErrorRestriction creates a new MintingRestrictionFn function that returns an error. +func (s *MintingRestrictionTestHelper) ErrorRestriction(message string) keeper.MintingRestrictionFn { + return func(_ sdk.Context, coins sdk.Coins) error { + s.RecordCall(message, coins) + return errors.New(message) + } +} + +// MintingRestrictionTestParams are parameters to test regarding calling a MintingRestrictionFn. +type MintingRestrictionTestParams struct { + // ExpNil is whether to expect the provided MintingRestrictionFn to be nil. + // If it is true, the rest of these test params are ignored. + ExpNil bool + // Coins is the MintingRestrictionFn coins input. + Coins sdk.Coins + // ExpErr is the expected return error string. + ExpErr string + // ExpCalls is the args of all the MintingRestrictionFn calls that end up being made. + ExpCalls []*MintingRestrictionArgs +} + +// TestActual tests the provided MintingRestrictionFn using the provided test parameters. +func (s *MintingRestrictionTestHelper) TestActual(t *testing.T, tp *MintingRestrictionTestParams, actual keeper.MintingRestrictionFn) { + t.Helper() + if tp.ExpNil { + require.Nil(t, actual, "resulting MintingRestrictionFn") + } else { + require.NotNil(t, actual, "resulting MintingRestrictionFn") + s.Calls = s.Calls[:0] + err := actual(sdk.Context{}, tp.Coins) + if len(tp.ExpErr) != 0 { + assert.EqualError(t, err, tp.ExpErr, "composite MintingRestrictionFn output error") + } else { + assert.NoError(t, err, "composite MintingRestrictionFn output error") + } + assert.Equal(t, tp.ExpCalls, s.Calls, "args given to funcs in composite MintingRestrictionFn") + } +} + +func TestMintingRestriction_Then(t *testing.T) { + coins := sdk.NewCoins(sdk.NewInt64Coin("acoin", 2), sdk.NewInt64Coin("bcoin", 4)) + + h := NewMintingRestrictionTestHelper() + + tests := []struct { + name string + base keeper.MintingRestrictionFn + second keeper.MintingRestrictionFn + exp *MintingRestrictionTestParams + }{ + { + name: "nil nil", + base: nil, + second: nil, + exp: &MintingRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "nil noop", + base: nil, + second: h.NamedRestriction("noop"), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("noop", coins)), + }, + }, + { + name: "noop nil", + base: h.NamedRestriction("noop"), + second: nil, + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("noop", coins)), + }, + }, + { + name: "noop noop", + base: h.NamedRestriction("noop1"), + second: h.NamedRestriction("noop2"), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("noop1", coins), h.NewArgs("noop2", coins)), + }, + }, + { + name: "noop error", + base: h.NamedRestriction("noop"), + second: h.ErrorRestriction("this is a test error"), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "this is a test error", + ExpCalls: h.NewCalls(h.NewArgs("noop", coins), h.NewArgs("this is a test error", coins)), + }, + }, + { + name: "error noop", + base: h.ErrorRestriction("another test error"), + second: h.NamedRestriction("noop"), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "another test error", + ExpCalls: h.NewCalls(h.NewArgs("another test error", coins)), + }, + }, + { + name: "error error", + base: h.ErrorRestriction("first test error"), + second: h.ErrorRestriction("second test error"), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "first test error", + ExpCalls: h.NewCalls(h.NewArgs("first test error", coins)), + }, + }, + { + name: "double chain", + base: keeper.ComposeMintingRestrictions(h.NamedRestriction("r1"), h.NamedRestriction("r2")), + second: keeper.ComposeMintingRestrictions(h.NamedRestriction("r3"), h.NamedRestriction("r4")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls( + h.NewArgs("r1", coins), + h.NewArgs("r2", coins), + h.NewArgs("r3", coins), + h.NewArgs("r4", coins), + ), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var actual keeper.MintingRestrictionFn + testFunc := func() { + actual = tc.base.Then(tc.second) + } + require.NotPanics(t, testFunc, "MintingRestrictionFn.Then") + h.TestActual(t, tc.exp, actual) + }) + } +} + +func TestComposeMintingRestrictions(t *testing.T) { + rz := func(rs ...keeper.MintingRestrictionFn) []keeper.MintingRestrictionFn { + return rs + } + coins := sdk.NewCoins(sdk.NewInt64Coin("ccoin", 8), sdk.NewInt64Coin("dcoin", 16)) + + h := NewMintingRestrictionTestHelper() + + tests := []struct { + name string + input []keeper.MintingRestrictionFn + exp *MintingRestrictionTestParams + }{ + { + name: "nil list", + input: nil, + exp: &MintingRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "empty list", + input: rz(), + exp: &MintingRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "only nil entry", + input: rz(nil), + exp: &MintingRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "five nil entries", + input: rz(nil, nil, nil, nil, nil), + exp: &MintingRestrictionTestParams{ + ExpNil: true, + }, + }, + { + name: "only noop entry", + input: rz(h.NamedRestriction("noop")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("noop", coins)), + }, + }, + { + name: "only error entry", + input: rz(h.ErrorRestriction("test error")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "test error", + ExpCalls: h.NewCalls(h.NewArgs("test error", coins)), + }, + }, + { + name: "noop nil nil", + input: rz(h.NamedRestriction("noop"), nil, nil), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("noop", coins)), + }, + }, + { + name: "nil noop nil", + input: rz(nil, h.NamedRestriction("noop"), nil), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("noop", coins)), + }, + }, + { + name: "nil nil noop", + input: rz(nil, nil, h.NamedRestriction("noop")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("noop", coins)), + }, + }, + { + name: "noop noop nil", + input: rz(h.NamedRestriction("r1"), h.NamedRestriction("r2"), nil), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("r1", coins), h.NewArgs("r2", coins)), + }, + }, + { + name: "noop nil noop", + input: rz(h.NamedRestriction("r1"), nil, h.NamedRestriction("r2")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("r1", coins), h.NewArgs("r2", coins)), + }, + }, + { + name: "nil noop noop", + input: rz(nil, h.NamedRestriction("r1"), h.NamedRestriction("r2")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("r1", coins), h.NewArgs("r2", coins)), + }, + }, + { + name: "noop noop noop", + input: rz(h.NamedRestriction("r1"), h.NamedRestriction("r2"), h.NamedRestriction("r3")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpCalls: h.NewCalls(h.NewArgs("r1", coins), h.NewArgs("r2", coins), h.NewArgs("r3", coins)), + }, + }, + { + name: "err noop noop", + input: rz(h.ErrorRestriction("first error"), h.NamedRestriction("r2"), h.NamedRestriction("r3")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "first error", + ExpCalls: h.NewCalls(h.NewArgs("first error", coins)), + }, + }, + { + name: "noop err noop", + input: rz(h.NamedRestriction("r1"), h.ErrorRestriction("second error"), h.NamedRestriction("r3")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "second error", + ExpCalls: h.NewCalls(h.NewArgs("r1", coins), h.NewArgs("second error", coins)), + }, + }, + { + name: "noop noop err", + input: rz(h.NamedRestriction("r1"), h.NamedRestriction("r2"), h.ErrorRestriction("third error")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "third error", + ExpCalls: h.NewCalls(h.NewArgs("r1", coins), h.NewArgs("r2", coins), h.NewArgs("third error", coins)), + }, + }, + { + name: "noop err err", + input: rz(h.NamedRestriction("r1"), h.ErrorRestriction("second error"), h.ErrorRestriction("third error")), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "second error", + ExpCalls: h.NewCalls(h.NewArgs("r1", coins), h.NewArgs("second error", coins)), + }, + }, + { + name: "big bang", + input: rz( + h.NamedRestriction("r1"), nil, h.NamedRestriction("r2"), nil, + h.NamedRestriction("r3"), h.NamedRestriction("r4"), h.NamedRestriction("r5"), + nil, h.NamedRestriction("r6"), h.NamedRestriction("r7"), nil, + h.NamedRestriction("r8"), nil, nil, h.ErrorRestriction("oops, an error"), + h.NamedRestriction("r9"), nil, h.NamedRestriction("ra"), // Not called. + ), + exp: &MintingRestrictionTestParams{ + Coins: coins, + ExpErr: "oops, an error", + ExpCalls: h.NewCalls( + h.NewArgs("r1", coins), + h.NewArgs("r2", coins), + h.NewArgs("r3", coins), + h.NewArgs("r4", coins), + h.NewArgs("r5", coins), + h.NewArgs("r6", coins), + h.NewArgs("r7", coins), + h.NewArgs("r8", coins), + h.NewArgs("oops, an error", coins), + ), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var actual keeper.MintingRestrictionFn + testFunc := func() { + actual = keeper.ComposeMintingRestrictions(tc.input...) + } + require.NotPanics(t, testFunc, "ComposeMintingRestrictions") + h.TestActual(t, tc.exp, actual) + }) + } +} + +func TestNoOpMintingRestrictionFn(t *testing.T) { + var err error + testFunc := func() { + err = keeper.NoOpMintingRestrictionFn(sdk.Context{}, sdk.Coins{}) + } + require.NotPanics(t, testFunc, "NoOpMintingRestrictionFn") + assert.NoError(t, err, "NoOpSendRestrictionFn error") +} + // SendRestrictionArgs are the args provided to a SendRestrictionFn function. type SendRestrictionArgs struct { + Name string FromAddr sdk.AccAddress ToAddr sdk.AccAddress Coins sdk.Coins @@ -26,8 +409,8 @@ func NewSendRestrictionTestHelper() *SendRestrictionTestHelper { } // RecordCall makes note that the provided args were used as a funcion call. -func (s *SendRestrictionTestHelper) RecordCall(fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) { - s.Calls = append(s.Calls, s.NewArgs(fromAddr, toAddr, coins)) +func (s *SendRestrictionTestHelper) RecordCall(name string, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) { + s.Calls = append(s.Calls, s.NewArgs(name, fromAddr, toAddr, coins)) } // NewCalls is just a shorter way to create a []*SendRestrictionArgs. @@ -36,26 +419,27 @@ func (s *SendRestrictionTestHelper) NewCalls(args ...*SendRestrictionArgs) []*Se } // NewArgs creates a new SendRestrictionArgs. -func (s *SendRestrictionTestHelper) NewArgs(fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) *SendRestrictionArgs { +func (s *SendRestrictionTestHelper) NewArgs(name string, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) *SendRestrictionArgs { return &SendRestrictionArgs{ + Name: name, FromAddr: fromAddr, ToAddr: toAddr, Coins: coins, } } -// NoOpRestriction creates a new SendRestrictionFn function that records the arguments it's called with and returns the provided toAddr. -func (s *SendRestrictionTestHelper) NoOpRestriction() keeper.SendRestrictionFn { +// NamedRestriction creates a new SendRestrictionFn function that records the arguments it's called with and returns the provided toAddr. +func (s *SendRestrictionTestHelper) NamedRestriction(name string) keeper.SendRestrictionFn { return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { - s.RecordCall(fromAddr, toAddr, coins) + s.RecordCall(name, fromAddr, toAddr, coins) return toAddr, nil } } // NewToRestriction creates a new SendRestrictionFn function that returns a different toAddr than provided. -func (s *SendRestrictionTestHelper) NewToRestriction(addr sdk.AccAddress) keeper.SendRestrictionFn { +func (s *SendRestrictionTestHelper) NewToRestriction(name string, addr sdk.AccAddress) keeper.SendRestrictionFn { return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { - s.RecordCall(fromAddr, toAddr, coins) + s.RecordCall(name, fromAddr, toAddr, coins) return addr, nil } } @@ -63,7 +447,7 @@ func (s *SendRestrictionTestHelper) NewToRestriction(addr sdk.AccAddress) keeper // ErrorRestriction creates a new SendRestrictionFn function that returns a nil toAddr and an error. func (s *SendRestrictionTestHelper) ErrorRestriction(message string) keeper.SendRestrictionFn { return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { - s.RecordCall(fromAddr, toAddr, coins) + s.RecordCall(message, fromAddr, toAddr, coins) return nil, errors.New(message) } } @@ -91,9 +475,9 @@ type SendRestrictionTestParams struct { func (s *SendRestrictionTestHelper) TestActual(t *testing.T, tp *SendRestrictionTestParams, actual keeper.SendRestrictionFn) { t.Helper() if tp.ExpNil { - require.Nil(t, actual, "ComposeSendRestrictions result") + require.Nil(t, actual, "resulting SendRestrictionFn") } else { - require.NotNil(t, actual, "ComposeSendRestrictions result") + require.NotNil(t, actual, "resulting SendRestrictionFn") s.Calls = s.Calls[:0] addr, err := actual(sdk.Context{}, tp.FromAddr, tp.ToAddr, tp.Coins) if len(tp.ExpErr) != 0 { @@ -107,13 +491,13 @@ func (s *SendRestrictionTestHelper) TestActual(t *testing.T, tp *SendRestriction } func TestSendRestriction_Then(t *testing.T) { - frAddr := sdk.AccAddress("fromaddr____________") + fromAddr := sdk.AccAddress("fromaddr____________") addr0 := sdk.AccAddress("0addr_______________") addr1 := sdk.AccAddress("1addr_______________") addr2 := sdk.AccAddress("2addr_______________") addr3 := sdk.AccAddress("3addr_______________") addr4 := sdk.AccAddress("4addr_______________") - coins := sdk.NewCoins(sdk.NewInt64Coin("acoin", 2), sdk.NewInt64Coin("bcoin", 4)) + coins := sdk.NewCoins(sdk.NewInt64Coin("ecoin", 32), sdk.NewInt64Coin("fcoin", 64)) h := NewSendRestrictionTestHelper() @@ -134,75 +518,84 @@ func TestSendRestriction_Then(t *testing.T) { { name: "nil noop", base: nil, - second: h.NoOpRestriction(), + second: h.NamedRestriction("noop"), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: addr1, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + ExpCalls: h.NewCalls(h.NewArgs("noop", fromAddr, addr1, coins)), }, }, { name: "noop nil", - base: h.NoOpRestriction(), + base: h.NamedRestriction("noop"), second: nil, exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: addr1, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + ExpCalls: h.NewCalls(h.NewArgs("noop", fromAddr, addr1, coins)), }, }, { name: "noop noop", - base: h.NoOpRestriction(), - second: h.NoOpRestriction(), + base: h.NamedRestriction("noop1"), + second: h.NamedRestriction("noop2"), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: addr1, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr1, coins)), + ExpCalls: h.NewCalls( + h.NewArgs("noop1", fromAddr, addr1, coins), + h.NewArgs("noop2", fromAddr, addr1, coins), + ), }, }, { name: "setter setter", - base: h.NewToRestriction(addr2), - second: h.NewToRestriction(addr3), + base: h.NewToRestriction("r1", addr2), + second: h.NewToRestriction("r2", addr3), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: addr3, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr2, coins)), + ExpCalls: h.NewCalls( + h.NewArgs("r1", fromAddr, addr1, coins), + h.NewArgs("r2", fromAddr, addr2, coins), + ), }, }, { name: "setter error", - base: h.NewToRestriction(addr2), + base: h.NewToRestriction("r1", addr2), second: h.ErrorRestriction("this is a test error"), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: nil, ExpErr: "this is a test error", - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr2, coins)), + ExpCalls: h.NewCalls(h.NewArgs( + "r1", fromAddr, addr1, coins), + h.NewArgs("this is a test error", fromAddr, addr2, coins), + ), }, }, { name: "error setter", base: h.ErrorRestriction("another test error"), - second: h.NewToRestriction(addr3), + second: h.NewToRestriction("r2", addr3), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: nil, ExpErr: "another test error", - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + ExpCalls: h.NewCalls(h.NewArgs("another test error", fromAddr, addr1, coins)), }, }, { @@ -210,28 +603,28 @@ func TestSendRestriction_Then(t *testing.T) { base: h.ErrorRestriction("first test error"), second: h.ErrorRestriction("second test error"), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: nil, ExpErr: "first test error", - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + ExpCalls: h.NewCalls(h.NewArgs("first test error", fromAddr, addr1, coins)), }, }, { name: "double chain", - base: keeper.ComposeSendRestrictions(h.NewToRestriction(addr1), h.NewToRestriction(addr2)), - second: keeper.ComposeSendRestrictions(h.NewToRestriction(addr3), h.NewToRestriction(addr4)), + base: keeper.ComposeSendRestrictions(h.NewToRestriction("r1", addr1), h.NewToRestriction("r2", addr2)), + second: keeper.ComposeSendRestrictions(h.NewToRestriction("r3", addr3), h.NewToRestriction("r4", addr4)), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr0, Coins: coins, ExpAddr: addr4, ExpCalls: h.NewCalls( - h.NewArgs(frAddr, addr0, coins), - h.NewArgs(frAddr, addr1, coins), - h.NewArgs(frAddr, addr2, coins), - h.NewArgs(frAddr, addr3, coins), + h.NewArgs("r1", fromAddr, addr0, coins), + h.NewArgs("r2", fromAddr, addr1, coins), + h.NewArgs("r3", fromAddr, addr2, coins), + h.NewArgs("r4", fromAddr, addr3, coins), ), }, }, @@ -253,13 +646,13 @@ func TestComposeSendRestrictions(t *testing.T) { rz := func(rs ...keeper.SendRestrictionFn) []keeper.SendRestrictionFn { return rs } - frAddr := sdk.AccAddress("fromaddr____________") + fromAddr := sdk.AccAddress("fromaddr____________") addr0 := sdk.AccAddress("0addr_______________") addr1 := sdk.AccAddress("1addr_______________") addr2 := sdk.AccAddress("2addr_______________") addr3 := sdk.AccAddress("3addr_______________") addr4 := sdk.AccAddress("4addr_______________") - coins := sdk.NewCoins(sdk.NewInt64Coin("acoin", 2), sdk.NewInt64Coin("bcoin", 4)) + coins := sdk.NewCoins(sdk.NewInt64Coin("gcoin", 128), sdk.NewInt64Coin("hcoin", 256)) h := NewSendRestrictionTestHelper() @@ -298,189 +691,201 @@ func TestComposeSendRestrictions(t *testing.T) { }, { name: "only noop entry", - input: rz(h.NoOpRestriction()), + input: rz(h.NamedRestriction("noop")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr0, Coins: coins, ExpAddr: addr0, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins)), + ExpCalls: h.NewCalls(h.NewArgs("noop", fromAddr, addr0, coins)), }, }, { name: "only error entry", input: rz(h.ErrorRestriction("test error")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr0, Coins: coins, ExpAddr: nil, ExpErr: "test error", - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins)), + ExpCalls: h.NewCalls(h.NewArgs("test error", fromAddr, addr0, coins)), }, }, { name: "noop nil nil", - input: rz(h.NoOpRestriction(), nil, nil), + input: rz(h.NamedRestriction("noop"), nil, nil), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr0, Coins: coins, ExpAddr: addr0, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins)), + ExpCalls: h.NewCalls(h.NewArgs("noop", fromAddr, addr0, coins)), }, }, { name: "nil noop nil", - input: rz(nil, h.NoOpRestriction(), nil), + input: rz(nil, h.NamedRestriction("noop"), nil), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: addr1, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins)), + ExpCalls: h.NewCalls(h.NewArgs("noop", fromAddr, addr1, coins)), }, }, { name: "nil nil noop", - input: rz(nil, nil, h.NoOpRestriction()), + input: rz(nil, nil, h.NamedRestriction("noop")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr2, Coins: coins, ExpAddr: addr2, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr2, coins)), + ExpCalls: h.NewCalls(h.NewArgs("noop", fromAddr, addr2, coins)), }, }, { name: "noop noop nil", - input: rz(h.NoOpRestriction(), h.NoOpRestriction(), nil), + input: rz(h.NamedRestriction("r1"), h.NamedRestriction("r2"), nil), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr0, Coins: coins, ExpAddr: addr0, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr0, coins), h.NewArgs(frAddr, addr0, coins)), + ExpCalls: h.NewCalls( + h.NewArgs("r1", fromAddr, addr0, coins), + h.NewArgs("r2", fromAddr, addr0, coins), + ), }, }, { name: "noop nil noop", - input: rz(h.NoOpRestriction(), nil, h.NoOpRestriction()), + input: rz(h.NamedRestriction("r1"), nil, h.NamedRestriction("r2")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr1, Coins: coins, ExpAddr: addr1, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr1, coins), h.NewArgs(frAddr, addr1, coins)), + ExpCalls: h.NewCalls( + h.NewArgs("r1", fromAddr, addr1, coins), + h.NewArgs("r2", fromAddr, addr1, coins), + ), }, }, { name: "nil noop noop", - input: rz(nil, h.NoOpRestriction(), h.NoOpRestriction()), + input: rz(nil, h.NamedRestriction("r1"), h.NamedRestriction("r2")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr2, Coins: coins, ExpAddr: addr2, - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr2, coins), h.NewArgs(frAddr, addr2, coins)), + ExpCalls: h.NewCalls( + h.NewArgs("r1", fromAddr, addr2, coins), + h.NewArgs("r2", fromAddr, addr2, coins), + ), }, }, { name: "noop noop noop", - input: rz(h.NoOpRestriction(), h.NoOpRestriction(), h.NoOpRestriction()), + input: rz(h.NamedRestriction("r1"), h.NamedRestriction("r2"), h.NamedRestriction("r3")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr3, Coins: coins, ExpAddr: addr3, ExpCalls: h.NewCalls( - h.NewArgs(frAddr, addr3, coins), - h.NewArgs(frAddr, addr3, coins), - h.NewArgs(frAddr, addr3, coins), + h.NewArgs("r1", fromAddr, addr3, coins), + h.NewArgs("r2", fromAddr, addr3, coins), + h.NewArgs("r3", fromAddr, addr3, coins), ), }, }, { name: "err noop noop", - input: rz(h.ErrorRestriction("first error"), h.NoOpRestriction(), h.NoOpRestriction()), + input: rz(h.ErrorRestriction("first error"), h.NamedRestriction("r2"), h.NamedRestriction("r3")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr4, Coins: coins, ExpAddr: nil, ExpErr: "first error", - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr4, coins)), + ExpCalls: h.NewCalls(h.NewArgs("first error", fromAddr, addr4, coins)), }, }, { name: "noop err noop", - input: rz(h.NoOpRestriction(), h.ErrorRestriction("second error"), h.NoOpRestriction()), + input: rz(h.NamedRestriction("r1"), h.ErrorRestriction("second error"), h.NamedRestriction("r3")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr4, Coins: coins, ExpAddr: nil, ExpErr: "second error", - ExpCalls: h.NewCalls(h.NewArgs(frAddr, addr4, coins), h.NewArgs(frAddr, addr4, coins)), + ExpCalls: h.NewCalls( + h.NewArgs("r1", fromAddr, addr4, coins), + h.NewArgs("second error", fromAddr, addr4, coins), + ), }, }, { name: "noop noop err", - input: rz(h.NoOpRestriction(), h.NoOpRestriction(), h.ErrorRestriction("third error")), + input: rz(h.NamedRestriction("r1"), h.NamedRestriction("r2"), h.ErrorRestriction("third error")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr4, Coins: coins, ExpAddr: nil, ExpErr: "third error", ExpCalls: h.NewCalls( - h.NewArgs(frAddr, addr4, coins), - h.NewArgs(frAddr, addr4, coins), - h.NewArgs(frAddr, addr4, coins), + h.NewArgs("r1", fromAddr, addr4, coins), + h.NewArgs("r2", fromAddr, addr4, coins), + h.NewArgs("third error", fromAddr, addr4, coins), ), }, }, { name: "new-to err err", - input: rz(h.NewToRestriction(addr0), h.ErrorRestriction("second error"), h.ErrorRestriction("third error")), + input: rz(h.NewToRestriction("r1", addr0), h.ErrorRestriction("second error"), h.ErrorRestriction("third error")), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr4, Coins: coins, ExpAddr: nil, ExpErr: "second error", ExpCalls: h.NewCalls( - h.NewArgs(frAddr, addr4, coins), - h.NewArgs(frAddr, addr0, coins), + h.NewArgs("r1", fromAddr, addr4, coins), + h.NewArgs("second error", fromAddr, addr0, coins), ), }, }, { name: "big bang", input: rz( - h.NoOpRestriction(), nil, h.NewToRestriction(addr1), // Called with orig toAddr. - nil, h.NoOpRestriction(), h.NewToRestriction(addr2), // Called with addr1 toAddr. - h.NewToRestriction(addr3), // Called with addr2 toAddr. - nil, h.NoOpRestriction(), h.NewToRestriction(addr4), // Called with addr3 toAddr. - nil, h.NoOpRestriction(), nil, nil, h.ErrorRestriction("oops, an error"), // Called with addr4 toAddr. - h.NewToRestriction(addr0), nil, h.NoOpRestriction(), // Not called. + h.NamedRestriction("r1"), nil, h.NewToRestriction("r2", addr1), // Called with orig toAddr. + nil, h.NamedRestriction("r3"), h.NewToRestriction("r4", addr2), // Called with addr1 toAddr. + h.NewToRestriction("r5", addr3), // Called with addr2 toAddr. + nil, h.NamedRestriction("r6"), h.NewToRestriction("r7", addr4), // Called with addr3 toAddr. + nil, h.NamedRestriction("r8"), nil, nil, h.ErrorRestriction("oops, an error"), // Called with addr4 toAddr. + h.NewToRestriction("r9", addr0), nil, h.NamedRestriction("ra"), // Not called. ), exp: &SendRestrictionTestParams{ - FromAddr: frAddr, + FromAddr: fromAddr, ToAddr: addr0, Coins: coins, ExpAddr: nil, ExpErr: "oops, an error", ExpCalls: h.NewCalls( - h.NewArgs(frAddr, addr0, coins), - h.NewArgs(frAddr, addr0, coins), - h.NewArgs(frAddr, addr1, coins), - h.NewArgs(frAddr, addr1, coins), - h.NewArgs(frAddr, addr2, coins), - h.NewArgs(frAddr, addr3, coins), - h.NewArgs(frAddr, addr3, coins), - h.NewArgs(frAddr, addr4, coins), - h.NewArgs(frAddr, addr4, coins), + h.NewArgs("r1", fromAddr, addr0, coins), + h.NewArgs("r2", fromAddr, addr0, coins), + h.NewArgs("r3", fromAddr, addr1, coins), + h.NewArgs("r4", fromAddr, addr1, coins), + h.NewArgs("r5", fromAddr, addr2, coins), + h.NewArgs("r6", fromAddr, addr3, coins), + h.NewArgs("r7", fromAddr, addr3, coins), + h.NewArgs("r8", fromAddr, addr4, coins), + h.NewArgs("oops, an error", fromAddr, addr4, coins), ), }, }, @@ -497,3 +902,15 @@ func TestComposeSendRestrictions(t *testing.T) { }) } } + +func TestNoOpSendRestrictionFn(t *testing.T) { + expAddr := sdk.AccAddress("__expectedaddr__") + var addr sdk.AccAddress + var err error + testFunc := func() { + addr, err = keeper.NoOpSendRestrictionFn(sdk.Context{}, sdk.AccAddress("first_addr"), expAddr, sdk.Coins{}) + } + require.NotPanics(t, testFunc, "NoOpSendRestrictionFn") + assert.NoError(t, err, "NoOpSendRestrictionFn error") + assert.Equal(t, expAddr, addr, "NoOpSendRestrictionFn addr") +} From 66b2b93149be51ad9fc1b28d2d8f85b256666690 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 7 Dec 2022 17:48:33 -0700 Subject: [PATCH 03/27] [14124]: Change InputOutputCoins to accept only a single input since the MsgMultiSend can now only contain a single input (#12610 and #12670). --- tests/integration/bank/keeper/keeper_test.go | 26 +++++-------- x/bank/keeper/keeper_test.go | 40 ++++++++------------ x/bank/keeper/msg_server.go | 2 +- x/bank/keeper/send.go | 40 ++++++++++---------- x/bank/types/msgs.go | 14 +++---- x/gov/testutil/expected_keepers_mocks.go | 8 ++-- 6 files changed, 55 insertions(+), 75 deletions(-) diff --git a/tests/integration/bank/keeper/keeper_test.go b/tests/integration/bank/keeper/keeper_test.go index 6f2b391fef3d..010e89704241 100644 --- a/tests/integration/bank/keeper/keeper_test.go +++ b/tests/integration/bank/keeper/keeper_test.go @@ -392,14 +392,12 @@ func (suite *IntegrationTestSuite) TestInputOutputNewAccount() { suite.Require().Nil(suite.accountKeeper.GetAccount(ctx, addr2)) suite.Require().Empty(suite.bankKeeper.GetAllBalances(ctx, addr2)) - inputs := []types.Input{ - {Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, - } + input := types.Input{Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))} outputs := []types.Output{ {Address: addr2.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, } - suite.Require().NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs)) + suite.Require().NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) expected := sdk.NewCoins(newFooCoin(30), newBarCoin(10)) acc2Balances := suite.bankKeeper.GetAllBalances(ctx, addr2) @@ -423,9 +421,7 @@ func (suite *IntegrationTestSuite) TestInputOutputCoins() { acc3 := suite.accountKeeper.NewAccountWithAddress(ctx, addr3) suite.accountKeeper.SetAccount(ctx, acc3) - input := []types.Input{ - {Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20))}, - } + input := types.Input{Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20))} outputs := []types.Output{ {Address: addr2.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, {Address: addr3.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, @@ -436,11 +432,9 @@ func (suite *IntegrationTestSuite) TestInputOutputCoins() { suite.Require().NoError(testutil.FundAccount(suite.bankKeeper, ctx, addr1, balances)) - insufficientInput := []types.Input{ - { - Address: addr1.String(), - Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)), - }, + insufficientInput := types.Input{ + Address: addr1.String(), + Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)), } insufficientOutputs := []types.Output{ {Address: addr2.String(), Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100))}, @@ -664,11 +658,9 @@ func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() { coins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50), sdk.NewInt64Coin(barDenom, 100)) newCoins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50)) newCoins2 := sdk.NewCoins(sdk.NewInt64Coin(barDenom, 100)) - input := []types.Input{ - { - Address: addr.String(), - Coins: coins, - }, + input := types.Input{ + Address: addr.String(), + Coins: coins, } outputs := []types.Output{ {Address: addr3.String(), Coins: newCoins}, diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 45746bd39ecb..0b320fb6e3a2 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -467,14 +467,12 @@ func (suite *KeeperTestSuite) TestInputOutputNewAccount() { require.Empty(suite.bankKeeper.GetAllBalances(ctx, accAddrs[1])) suite.mockInputOutputCoins([]authtypes.AccountI{authtypes.NewBaseAccountWithAddress(accAddrs[0])}, []sdk.AccAddress{accAddrs[1]}) - inputs := []banktypes.Input{ - {Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, - } + input := banktypes.Input{Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))} outputs := []banktypes.Output{ {Address: accAddrs[1].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, } - require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs)) + require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) expected := sdk.NewCoins(newFooCoin(30), newBarCoin(10)) acc2Balances := suite.bankKeeper.GetAllBalances(ctx, accAddrs[1]) @@ -487,37 +485,33 @@ func (suite *KeeperTestSuite) TestInputOutputCoins() { balances := sdk.NewCoins(newFooCoin(90), newBarCoin(30)) acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) - inputs := []banktypes.Input{ - {Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20))}, - } + input := banktypes.Input{Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20))} outputs := []banktypes.Output{ {Address: accAddrs[1].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, {Address: accAddrs[2].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))}, } - require.Error(suite.bankKeeper.InputOutputCoins(ctx, inputs, []banktypes.Output{})) + require.Error(suite.bankKeeper.InputOutputCoins(ctx, input, []banktypes.Output{})) suite.authKeeper.EXPECT().GetAccount(suite.ctx, accAddrs[0]).Return(acc0) - require.Error(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs)) + require.Error(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) suite.mockFundAccount(accAddrs[0]) require.NoError(banktestutil.FundAccount(suite.bankKeeper, ctx, accAddrs[0], balances)) - insufficientInputs := []banktypes.Input{ - { - Address: accAddrs[0].String(), - Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)), - }, + insufficientInput := banktypes.Input{ + Address: accAddrs[0].String(), + Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)), } insufficientOutputs := []banktypes.Output{ {Address: accAddrs[1].String(), Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100))}, {Address: accAddrs[2].String(), Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100))}, } - require.Error(suite.bankKeeper.InputOutputCoins(ctx, insufficientInputs, insufficientOutputs)) + require.Error(suite.bankKeeper.InputOutputCoins(ctx, insufficientInput, insufficientOutputs)) suite.mockInputOutputCoins([]authtypes.AccountI{acc0}, accAddrs[1:3]) - require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs)) + require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) acc1Balances := suite.bankKeeper.GetAllBalances(ctx, accAddrs[0]) expected := sdk.NewCoins(newFooCoin(30), newBarCoin(10)) @@ -725,11 +719,9 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { coins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50), sdk.NewInt64Coin(barDenom, 100)) newCoins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50)) newCoins2 := sdk.NewCoins(sdk.NewInt64Coin(barDenom, 100)) - inputs := []banktypes.Input{ - { - Address: accAddrs[0].String(), - Coins: coins, - }, + input := banktypes.Input{ + Address: accAddrs[0].String(), + Coins: coins, } outputs := []banktypes.Output{ {Address: accAddrs[2].String(), Coins: newCoins}, @@ -737,7 +729,7 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { } suite.authKeeper.EXPECT().GetAccount(suite.ctx, accAddrs[0]).Return(acc0) - require.Error(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs)) + require.Error(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) events := ctx.EventManager().ABCIEvents() require.Equal(0, len(events)) @@ -747,7 +739,7 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { require.NoError(banktestutil.FundAccount(suite.bankKeeper, ctx, accAddrs[0], sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50), sdk.NewInt64Coin(barDenom, 100)))) suite.mockInputOutputCoins([]authtypes.AccountI{acc0}, accAddrs[2:4]) - require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs)) + require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) events = ctx.EventManager().ABCIEvents() require.Equal(12, len(events)) // 12 events because account funding causes extra minting + coin_spent + coin_recv events @@ -772,7 +764,7 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { newCoins2 = sdk.NewCoins(sdk.NewInt64Coin(barDenom, 100)) suite.mockInputOutputCoins([]authtypes.AccountI{acc0}, accAddrs[2:4]) - require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs)) + require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) events = ctx.EventManager().ABCIEvents() require.Equal(30, len(events)) // 27 due to account funding + coin_spent + coin_recv events diff --git a/x/bank/keeper/msg_server.go b/x/bank/keeper/msg_server.go index 3f2d5bac1197..ad554c9f4347 100644 --- a/x/bank/keeper/msg_server.go +++ b/x/bank/keeper/msg_server.go @@ -82,7 +82,7 @@ func (k msgServer) MultiSend(goCtx context.Context, msg *types.MsgMultiSend) (*t } } - err := k.InputOutputCoins(ctx, msg.Inputs, msg.Outputs) + err := k.InputOutputCoins(ctx, msg.Inputs[0], msg.Outputs) if err != nil { return nil, err } diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index dad56a3f8827..f88f2a19e009 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -18,7 +18,7 @@ import ( type SendKeeper interface { ViewKeeper - InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error + InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error GetParams(ctx sdk.Context) types.Params @@ -119,33 +119,31 @@ func (k BaseSendKeeper) SetParams(ctx sdk.Context, params types.Params) error { return nil } -// InputOutputCoins performs multi-send functionality. It accepts a series of -// inputs that correspond to a series of outputs. It returns an error if the -// inputs and outputs don't line up or if any single transfer of tokens fails. -func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error { +// InputOutputCoins performs multi-send functionality. It accepts an +// input that corresponds to a series of outputs. It returns an error if the +// input and outputs don't line up or if any single transfer of tokens fails. +func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error { // Safety check ensuring that when sending coins the keeper must maintain the // Check supply invariant and validity of Coins. - if err := types.ValidateInputsOutputs(inputs, outputs); err != nil { + if err := types.ValidateInputOutputs(input, outputs); err != nil { return err } - for _, in := range inputs { - inAddress, err := sdk.AccAddressFromBech32(in.Address) - if err != nil { - return err - } + inAddress, err := sdk.AccAddressFromBech32(input.Address) + if err != nil { + return err + } - err = k.subUnlockedCoins(ctx, inAddress, in.Coins) - if err != nil { - return err - } - ctx.EventManager().EmitEvent( - sdk.NewEvent( - sdk.EventTypeMessage, - sdk.NewAttribute(types.AttributeKeySender, in.Address), - ), - ) + err = k.subUnlockedCoins(ctx, inAddress, input.Coins) + if err != nil { + return err } + ctx.EventManager().EmitEvent( + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(types.AttributeKeySender, input.Address), + ), + ) for _, out := range outputs { outAddress, err := sdk.AccAddressFromBech32(out.Address) diff --git a/x/bank/types/msgs.go b/x/bank/types/msgs.go index 988d0964861b..beb442696200 100644 --- a/x/bank/types/msgs.go +++ b/x/bank/types/msgs.go @@ -92,7 +92,7 @@ func (msg MsgMultiSend) ValidateBasic() error { return ErrNoOutputs } - return ValidateInputsOutputs(msg.Inputs, msg.Outputs) + return ValidateInputOutputs(msg.Inputs[0], msg.Outputs) } // GetSignBytes Implements Msg. @@ -165,17 +165,15 @@ func NewOutput(addr sdk.AccAddress, coins sdk.Coins) Output { } } -// ValidateInputsOutputs validates that each respective input and output is +// ValidateInputOutputs validates that each respective input and output is // valid and that the sum of inputs is equal to the sum of outputs. -func ValidateInputsOutputs(inputs []Input, outputs []Output) error { +func ValidateInputOutputs(input Input, outputs []Output) error { var totalIn, totalOut sdk.Coins - for _, in := range inputs { - if err := in.ValidateBasic(); err != nil { - return err - } - totalIn = totalIn.Add(in.Coins...) + if err := input.ValidateBasic(); err != nil { + return err } + totalIn = input.Coins for _, out := range outputs { if err := out.ValidateBasic(); err != nil { diff --git a/x/gov/testutil/expected_keepers_mocks.go b/x/gov/testutil/expected_keepers_mocks.go index e119ee2bf6e9..5519c7fc776d 100644 --- a/x/gov/testutil/expected_keepers_mocks.go +++ b/x/gov/testutil/expected_keepers_mocks.go @@ -505,17 +505,17 @@ func (mr *MockBankKeeperMockRecorder) InitGenesis(arg0, arg1 interface{}) *gomoc } // InputOutputCoins mocks base method. -func (m *MockBankKeeper) InputOutputCoins(ctx types.Context, inputs []types1.Input, outputs []types1.Output) error { +func (m *MockBankKeeper) InputOutputCoins(ctx types.Context, input types1.Input, outputs []types1.Output) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "InputOutputCoins", ctx, inputs, outputs) + ret := m.ctrl.Call(m, "InputOutputCoins", ctx, input, outputs) ret0, _ := ret[0].(error) return ret0 } // InputOutputCoins indicates an expected call of InputOutputCoins. -func (mr *MockBankKeeperMockRecorder) InputOutputCoins(ctx, inputs, outputs interface{}) *gomock.Call { +func (mr *MockBankKeeperMockRecorder) InputOutputCoins(ctx, input, outputs interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InputOutputCoins", reflect.TypeOf((*MockBankKeeper)(nil).InputOutputCoins), ctx, inputs, outputs) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InputOutputCoins", reflect.TypeOf((*MockBankKeeper)(nil).InputOutputCoins), ctx, input, outputs) } // IsSendEnabledCoin mocks base method. From 845480dc629478dc22334ac993228575bc49f23e Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 7 Dec 2022 18:52:11 -0700 Subject: [PATCH 04/27] [14124]: Add a SendRestrictionFn to the send keeper. Rename the existing SendCoins to SendCoinsWithoutRestriction and create a new SendCoins that just applies the restriction then calls SendCoinsWithoutRestriction. Make restriction calls from InputOutputCoins too. --- x/bank/keeper/keeper_test.go | 114 +++++++++++++++++++++++ x/bank/keeper/send.go | 37 ++++++-- x/gov/testutil/expected_keepers_mocks.go | 14 +++ 3 files changed, 158 insertions(+), 7 deletions(-) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 0b320fb6e3a2..6075573be81d 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "errors" "fmt" "strings" "testing" @@ -561,6 +562,119 @@ func (suite *KeeperTestSuite) TestSendCoins() { require.Equal(newBarCoin(25), coins[0], "expected only bar coins in the account balance, got: %v", coins) } +func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { + type restrictionArgs struct { + ctx sdk.Context + fromAddr sdk.AccAddress + toAddr sdk.AccAddress + amt sdk.Coins + } + var actualRestrictionArgs *restrictionArgs + restrictionError := func(message string) keeper.SendRestrictionFn { + return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + actualRestrictionArgs = &restrictionArgs{ + ctx: ctx, + fromAddr: fromAddr, + toAddr: toAddr, + amt: amt, + } + return nil, errors.New(message) + } + } + restrictionPassthrough := func() keeper.SendRestrictionFn { + return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + actualRestrictionArgs = &restrictionArgs{ + ctx: ctx, + fromAddr: fromAddr, + toAddr: toAddr, + amt: amt, + } + return toAddr, nil + } + } + restrictionNewTo := func(newToAddr sdk.AccAddress) keeper.SendRestrictionFn { + return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + actualRestrictionArgs = &restrictionArgs{ + ctx: ctx, + fromAddr: fromAddr, + toAddr: toAddr, + amt: amt, + } + return newToAddr, nil + } + } + type expBal struct { + addr sdk.AccAddress + amt sdk.Coins + } + + setupCtx := suite.ctx + balances := sdk.NewCoins(newFooCoin(1000), newBarCoin(500)) + fromAddr := accAddrs[0] + fromAcc := authtypes.NewBaseAccountWithAddress(fromAddr) + toAddr1 := accAddrs[1] + toAddr2 := accAddrs[2] + + suite.mockFundAccount(accAddrs[0]) + suite.Require().NoError(banktestutil.FundAccount(suite.bankKeeper, setupCtx, accAddrs[0], balances)) + + tests := []struct { + name string + fn keeper.SendRestrictionFn + toAddr sdk.AccAddress + amt sdk.Coins + expArgs *restrictionArgs + expErr string + expBals []*expBal + }{ + { + name: "nil restriction", + fn: nil, + toAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(5)), + expArgs: nil, + expBals: []*expBal{ + {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(995), newBarCoin(500))}, + {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(5))}, + {addr: toAddr2, amt: sdk.Coins{}}, + }, + }, + // TODO[14124]: Add more test cases. + } + + _ = restrictionError + _ = restrictionPassthrough + _ = restrictionNewTo + + for _, tc := range tests { + suite.Run(tc.name, func() { + // TODO[14124]: provide tc.fn to the bank keeper. + ctx := suite.ctx + suite.mockSendCoins(ctx, fromAcc, tc.toAddr) + var err error + testFunc := func() { + err = suite.bankKeeper.SendCoins(ctx, fromAddr, tc.toAddr, tc.amt) + } + suite.Require().NotPanics(testFunc, "SendCoins") + if len(tc.expErr) > 0 { + suite.Assert().EqualError(err, tc.expErr, "SendCoins error") + } else { + suite.Assert().NoError(err, "SendCoins error") + } + if tc.expArgs != nil { + suite.Assert().Equal(tc.expArgs.ctx, actualRestrictionArgs.ctx, "ctx provided to restriction") + suite.Assert().Equal(tc.expArgs.fromAddr, actualRestrictionArgs.fromAddr, "fromAddr provided to restriction") + suite.Assert().Equal(tc.expArgs.toAddr, actualRestrictionArgs.toAddr, "toAddr provided to restriction") + suite.Assert().Equal(tc.expArgs.amt, actualRestrictionArgs.amt, "amt provided to restriction") + } + for i, bal := range tc.expBals { + actual := suite.bankKeeper.GetAllBalances(ctx, bal.addr) + suite.Assert().Equal(bal.amt, actual, "expected balance[%d]", i) + } + }) + } +} + func (suite *KeeperTestSuite) TestSendCoins_Invalid_SendLockedCoins() { balances := sdk.NewCoins(newFooCoin(50)) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index f88f2a19e009..ab0649d4452b 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -20,6 +20,7 @@ type SendKeeper interface { InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error + SendCoinsWithoutRestriction(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error GetParams(ctx sdk.Context) types.Params SetParams(ctx sdk.Context, params types.Params) error @@ -58,6 +59,8 @@ type BaseSendKeeper struct { // the address capable of executing a MsgUpdateParams message. Typically, this // should be the x/gov module account. authority string + + sendRestrictionFn SendRestrictionFn } func NewBaseSendKeeper( @@ -72,12 +75,13 @@ func NewBaseSendKeeper( } return BaseSendKeeper{ - BaseViewKeeper: NewBaseViewKeeper(cdc, storeKey, ak), - cdc: cdc, - ak: ak, - storeKey: storeKey, - blockedAddrs: blockedAddrs, - authority: authority, + BaseViewKeeper: NewBaseViewKeeper(cdc, storeKey, ak), + cdc: cdc, + ak: ak, + storeKey: storeKey, + blockedAddrs: blockedAddrs, + authority: authority, + sendRestrictionFn: nil, } } @@ -150,6 +154,12 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, out if err != nil { return err } + if k.sendRestrictionFn != nil { + outAddress, err = k.sendRestrictionFn(ctx, inAddress, outAddress, out.Coins) + if err != nil { + return err + } + } err = k.addCoins(ctx, outAddress, out.Coins) if err != nil { return err @@ -158,7 +168,7 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, out ctx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeTransfer, - sdk.NewAttribute(types.AttributeKeyRecipient, out.Address), + sdk.NewAttribute(types.AttributeKeyRecipient, outAddress.String()), sdk.NewAttribute(sdk.AttributeKeyAmount, out.Coins.String()), ), ) @@ -180,6 +190,19 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, out // SendCoins transfers amt coins from a sending account to a receiving account. // An error is returned upon failure. func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error { + if k.sendRestrictionFn != nil { + var err error + toAddr, err = k.sendRestrictionFn(ctx, fromAddr, toAddr, amt) + if err != nil { + return err + } + } + return k.SendCoinsWithoutRestriction(ctx, fromAddr, toAddr, amt) +} + +// SendCoinsWithoutRestriction transfers amt coins from a sending account to a receiving account without checking the injectable restrictions. +// An error is returned upon failure. +func (k BaseSendKeeper) SendCoinsWithoutRestriction(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error { err := k.subUnlockedCoins(ctx, fromAddr, amt) if err != nil { return err diff --git a/x/gov/testutil/expected_keepers_mocks.go b/x/gov/testutil/expected_keepers_mocks.go index 5519c7fc776d..366b8e051394 100644 --- a/x/gov/testutil/expected_keepers_mocks.go +++ b/x/gov/testutil/expected_keepers_mocks.go @@ -724,6 +724,20 @@ func (mr *MockBankKeeperMockRecorder) SendCoinsFromModuleToModule(ctx, senderMod return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromModuleToModule", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromModuleToModule), ctx, senderModule, recipientModule, amt) } +// SendCoinsWithoutRestriction mocks base method. +func (m *MockBankKeeper) SendCoinsWithoutRestriction(ctx types.Context, fromAddr, toAddr types.AccAddress, amt types.Coins) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SendCoinsWithoutRestriction", ctx, fromAddr, toAddr, amt) + ret0, _ := ret[0].(error) + return ret0 +} + +// SendCoinsWithoutRestriction indicates an expected call of SendCoinsWithoutRestriction. +func (mr *MockBankKeeperMockRecorder) SendCoinsWithoutRestriction(ctx, fromAddr, toAddr, amt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsWithoutRestriction", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsWithoutRestriction), ctx, fromAddr, toAddr, amt) +} + // SendEnabled mocks base method. func (m *MockBankKeeper) SendEnabled(arg0 context.Context, arg1 *types1.QuerySendEnabledRequest) (*types1.QuerySendEnabledResponse, error) { m.ctrl.T.Helper() From df544628a9806137dd5be8e16c328d51edd3711d Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 7 Dec 2022 19:11:37 -0700 Subject: [PATCH 05/27] [14124]: Remove the SendCoinsWithoutRestriction and always run restrictions. It can be up to each restriction to decide if it should be bypassed. --- x/bank/keeper/send.go | 6 ------ x/gov/testutil/expected_keepers_mocks.go | 14 -------------- 2 files changed, 20 deletions(-) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index ab0649d4452b..53afceee0c49 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -20,7 +20,6 @@ type SendKeeper interface { InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error - SendCoinsWithoutRestriction(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error GetParams(ctx sdk.Context) types.Params SetParams(ctx sdk.Context, params types.Params) error @@ -197,12 +196,7 @@ func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAd return err } } - return k.SendCoinsWithoutRestriction(ctx, fromAddr, toAddr, amt) -} -// SendCoinsWithoutRestriction transfers amt coins from a sending account to a receiving account without checking the injectable restrictions. -// An error is returned upon failure. -func (k BaseSendKeeper) SendCoinsWithoutRestriction(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error { err := k.subUnlockedCoins(ctx, fromAddr, amt) if err != nil { return err diff --git a/x/gov/testutil/expected_keepers_mocks.go b/x/gov/testutil/expected_keepers_mocks.go index 366b8e051394..5519c7fc776d 100644 --- a/x/gov/testutil/expected_keepers_mocks.go +++ b/x/gov/testutil/expected_keepers_mocks.go @@ -724,20 +724,6 @@ func (mr *MockBankKeeperMockRecorder) SendCoinsFromModuleToModule(ctx, senderMod return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromModuleToModule", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromModuleToModule), ctx, senderModule, recipientModule, amt) } -// SendCoinsWithoutRestriction mocks base method. -func (m *MockBankKeeper) SendCoinsWithoutRestriction(ctx types.Context, fromAddr, toAddr types.AccAddress, amt types.Coins) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SendCoinsWithoutRestriction", ctx, fromAddr, toAddr, amt) - ret0, _ := ret[0].(error) - return ret0 -} - -// SendCoinsWithoutRestriction indicates an expected call of SendCoinsWithoutRestriction. -func (mr *MockBankKeeperMockRecorder) SendCoinsWithoutRestriction(ctx, fromAddr, toAddr, amt interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsWithoutRestriction", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsWithoutRestriction), ctx, fromAddr, toAddr, amt) -} - // SendEnabled mocks base method. func (m *MockBankKeeper) SendEnabled(arg0 context.Context, arg1 *types1.QuerySendEnabledRequest) (*types1.QuerySendEnabledResponse, error) { m.ctrl.T.Helper() From ba605c321f54d4c2593b0a0b1f8dadf9d100f25e Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 00:11:02 -0700 Subject: [PATCH 06/27] [14124]: Create a struct to hold the send restriction function that can be updated from inside the keeper without needing a pointer receiver. --- x/bank/keeper/restrictions.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/x/bank/keeper/restrictions.go b/x/bank/keeper/restrictions.go index 298d89953eb5..82913bc86b43 100644 --- a/x/bank/keeper/restrictions.go +++ b/x/bank/keeper/restrictions.go @@ -90,3 +90,26 @@ func ComposeSendRestrictions(restrictions ...SendRestrictionFn) SendRestrictionF return toAddr, err } } + +// SendRestriction is a struct that houses a SendRestrictionFn. +// It exists so that the SendRestrictionFn can be updated in the SendKeeper without needing to have a pointer receiver. +type SendRestriction struct { + Fn SendRestrictionFn +} + +// NewSendRestriction creates a new SendRestriction with nil send restriction. +func NewSendRestriction() *SendRestriction { + return &SendRestriction{ + Fn: nil, + } +} + +// Append adds the provided restriction to this, to be run after the existing function. +func (r *SendRestriction) Append(restriction SendRestrictionFn) { + r.Fn = r.Fn.Then(restriction) +} + +// Prepend adds the provided restriction to this, to be run before the existing function. +func (r *SendRestriction) Prepend(restriction SendRestrictionFn) { + r.Fn = restriction.Then(r.Fn) +} From 66e847486793213a1bff4341039b30caa47b03aa Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 00:13:44 -0700 Subject: [PATCH 07/27] [14124]: Switch the keeper to use this SendRestriction holder and give the keeper AppendSendRestriction and PrependSendRestriction. --- x/bank/keeper/send.go | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 53afceee0c49..7646a2531ade 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -18,6 +18,9 @@ import ( type SendKeeper interface { ViewKeeper + AppendSendRestriction(restriction SendRestrictionFn) + PrependSendRestriction(restriction SendRestrictionFn) + InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error @@ -59,7 +62,7 @@ type BaseSendKeeper struct { // should be the x/gov module account. authority string - sendRestrictionFn SendRestrictionFn + sendRestriction *SendRestriction } func NewBaseSendKeeper( @@ -74,16 +77,26 @@ func NewBaseSendKeeper( } return BaseSendKeeper{ - BaseViewKeeper: NewBaseViewKeeper(cdc, storeKey, ak), - cdc: cdc, - ak: ak, - storeKey: storeKey, - blockedAddrs: blockedAddrs, - authority: authority, - sendRestrictionFn: nil, + BaseViewKeeper: NewBaseViewKeeper(cdc, storeKey, ak), + cdc: cdc, + ak: ak, + storeKey: storeKey, + blockedAddrs: blockedAddrs, + authority: authority, + sendRestriction: NewSendRestriction(), } } +// AppendSendRestriction adds the provided SendRestrictionFn to run after previously provided restrictions. +func (k BaseSendKeeper) AppendSendRestriction(restriction SendRestrictionFn) { + k.sendRestriction.Append(restriction) +} + +// PrependSendRestriction adds the provided SendRestrictionFn to run before previously provided restrictions. +func (k BaseSendKeeper) PrependSendRestriction(restriction SendRestrictionFn) { + k.sendRestriction.Prepend(restriction) +} + // GetAuthority returns the x/bank module's authority. func (k BaseSendKeeper) GetAuthority() string { return k.authority @@ -153,8 +166,8 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, out if err != nil { return err } - if k.sendRestrictionFn != nil { - outAddress, err = k.sendRestrictionFn(ctx, inAddress, outAddress, out.Coins) + if k.sendRestriction.Fn != nil { + outAddress, err = k.sendRestriction.Fn(ctx, inAddress, outAddress, out.Coins) if err != nil { return err } @@ -189,9 +202,9 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, out // SendCoins transfers amt coins from a sending account to a receiving account. // An error is returned upon failure. func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error { - if k.sendRestrictionFn != nil { + if k.sendRestriction.Fn != nil { var err error - toAddr, err = k.sendRestrictionFn(ctx, fromAddr, toAddr, amt) + toAddr, err = k.sendRestriction.Fn(ctx, fromAddr, toAddr, amt) if err != nil { return err } From bc988d9bf3393016cd75b929d6db91f73f9ead80 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 00:14:34 -0700 Subject: [PATCH 08/27] [14124]: Add some more unit test cases on the SendCoins w/restrictions. --- x/bank/keeper/export_test.go | 12 +++++ x/bank/keeper/keeper_test.go | 98 +++++++++++++++++++++++++++++------- 2 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 x/bank/keeper/export_test.go diff --git a/x/bank/keeper/export_test.go b/x/bank/keeper/export_test.go new file mode 100644 index 000000000000..6ed22da65456 --- /dev/null +++ b/x/bank/keeper/export_test.go @@ -0,0 +1,12 @@ +package keeper + +// This file exists in the keeper package to expose some private things +// for the purpose of testing in the keeper_test package. + +func (k BaseSendKeeper) SetSendRestriction(restriction SendRestrictionFn) { + k.sendRestriction.Fn = restriction +} + +func (k BaseSendKeeper) GetSendRestrictionFn() SendRestrictionFn { + return k.sendRestriction.Fn +} diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 6075573be81d..e8cbaf80ee5f 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -619,38 +619,96 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { suite.Require().NoError(banktestutil.FundAccount(suite.bankKeeper, setupCtx, accAddrs[0], balances)) tests := []struct { - name string - fn keeper.SendRestrictionFn - toAddr sdk.AccAddress - amt sdk.Coins - expArgs *restrictionArgs - expErr string - expBals []*expBal + name string + fn keeper.SendRestrictionFn + toAddr sdk.AccAddress + finalAddr sdk.AccAddress + amt sdk.Coins + expArgs *restrictionArgs + expErr string + expBals []*expBal }{ { - name: "nil restriction", - fn: nil, - toAddr: toAddr1, - amt: sdk.NewCoins(newFooCoin(5)), - expArgs: nil, + name: "nil restriction", + fn: nil, + toAddr: toAddr1, + finalAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(5)), + expArgs: nil, expBals: []*expBal{ {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(995), newBarCoin(500))}, {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(5))}, {addr: toAddr2, amt: sdk.Coins{}}, }, }, - // TODO[14124]: Add more test cases. + { + name: "passthrough restriction", + fn: restrictionPassthrough(), + toAddr: toAddr1, + finalAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(10)), + expArgs: &restrictionArgs{ + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(10)), + }, + expBals: []*expBal{ + {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(985), newBarCoin(500))}, + {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(15))}, + {addr: toAddr2, amt: sdk.Coins{}}, + }, + }, + { + name: "new to addr restriction", + fn: restrictionNewTo(toAddr2), + toAddr: toAddr1, + finalAddr: toAddr2, + amt: sdk.NewCoins(newBarCoin(27)), + expArgs: &restrictionArgs{ + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newBarCoin(27)), + }, + expBals: []*expBal{ + {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(985), newBarCoin(473))}, + {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(15))}, + {addr: toAddr2, amt: sdk.NewCoins(newBarCoin(27))}, + }, + }, + { + name: "restriction returns error", + fn: restrictionError("test restriction error"), + toAddr: toAddr1, + finalAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(100), newBarCoin(200)), + expArgs: &restrictionArgs{ + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(100), newBarCoin(200)), + }, + expErr: "test restriction error", + expBals: []*expBal{ + {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(985), newBarCoin(473))}, + {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(15))}, + {addr: toAddr2, amt: sdk.NewCoins(newBarCoin(27))}, + }, + }, } - _ = restrictionError - _ = restrictionPassthrough - _ = restrictionNewTo - for _, tc := range tests { suite.Run(tc.name, func() { - // TODO[14124]: provide tc.fn to the bank keeper. + existingSendRestrictionFn := suite.bankKeeper.GetSendRestrictionFn() + defer suite.bankKeeper.SetSendRestriction(existingSendRestrictionFn) + actualRestrictionArgs = nil + suite.bankKeeper.SetSendRestriction(tc.fn) ctx := suite.ctx - suite.mockSendCoins(ctx, fromAcc, tc.toAddr) + if len(tc.expErr) == 0 { + suite.mockSendCoins(ctx, fromAcc, tc.finalAddr) + } + var err error testFunc := func() { err = suite.bankKeeper.SendCoins(ctx, fromAddr, tc.toAddr, tc.amt) @@ -666,6 +724,8 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { suite.Assert().Equal(tc.expArgs.fromAddr, actualRestrictionArgs.fromAddr, "fromAddr provided to restriction") suite.Assert().Equal(tc.expArgs.toAddr, actualRestrictionArgs.toAddr, "toAddr provided to restriction") suite.Assert().Equal(tc.expArgs.amt, actualRestrictionArgs.amt, "amt provided to restriction") + } else { + suite.Assert().Nil(actualRestrictionArgs, "args provided to a restriction") } for i, bal := range tc.expBals { actual := suite.bankKeeper.GetAllBalances(ctx, bal.addr) From 3f6ea3a15852efbef14cd7d702670836953f2455 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 01:14:32 -0700 Subject: [PATCH 09/27] [14124]: Add unit tests on InputOutputCoins with restrictions. --- x/bank/keeper/keeper_test.go | 341 ++++++++++++++++++++++++++++++++--- 1 file changed, 316 insertions(+), 25 deletions(-) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index e8cbaf80ee5f..66a1d1b4599d 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -525,6 +525,294 @@ func (suite *KeeperTestSuite) TestInputOutputCoins() { require.Equal(expected, acc3Balances) } +func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { + type restrictionArgs struct { + ctx sdk.Context + fromAddr sdk.AccAddress + toAddr sdk.AccAddress + amt sdk.Coins + } + var actualRestrictionArgs []*restrictionArgs + restrictionError := func(messages ...string) keeper.SendRestrictionFn { + i := -1 + return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + actualRestrictionArgs = append(actualRestrictionArgs, &restrictionArgs{ + ctx: ctx, + fromAddr: fromAddr, + toAddr: toAddr, + amt: amt, + }) + i += 1 + if i < len(messages) { + if len(messages[i]) > 0 { + return nil, errors.New(messages[i]) + } + } + return toAddr, nil + } + } + restrictionPassthrough := func() keeper.SendRestrictionFn { + return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + actualRestrictionArgs = append(actualRestrictionArgs, &restrictionArgs{ + ctx: ctx, + fromAddr: fromAddr, + toAddr: toAddr, + amt: amt, + }) + return toAddr, nil + } + } + restrictionNewTo := func(newToAddrs ...sdk.AccAddress) keeper.SendRestrictionFn { + i := -1 + return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + actualRestrictionArgs = append(actualRestrictionArgs, &restrictionArgs{ + ctx: ctx, + fromAddr: fromAddr, + toAddr: toAddr, + amt: amt, + }) + i += 1 + if i < len(newToAddrs) { + if len(newToAddrs[i]) > 0 { + return newToAddrs[i], nil + } + } + return toAddr, nil + } + } + type expBals struct { + from sdk.Coins + to1 sdk.Coins + to2 sdk.Coins + } + + setupCtx := suite.ctx + balances := sdk.NewCoins(newFooCoin(1000), newBarCoin(500)) + fromAddr := accAddrs[0] + fromAcc := authtypes.NewBaseAccountWithAddress(fromAddr) + inputAccs := []authtypes.AccountI{fromAcc} + toAddr1 := accAddrs[1] + toAddr2 := accAddrs[2] + + suite.mockFundAccount(accAddrs[0]) + suite.Require().NoError(banktestutil.FundAccount(suite.bankKeeper, setupCtx, accAddrs[0], balances)) + + tests := []struct { + name string + fn keeper.SendRestrictionFn + inputCoins sdk.Coins + outputs []banktypes.Output + outputAddrs []sdk.AccAddress + expArgs []*restrictionArgs + expErr string + expBals expBals + }{ + { + name: "nil restriction", + fn: nil, + inputCoins: sdk.NewCoins(newFooCoin(5)), + outputs: []banktypes.Output{{Address: toAddr1.String(), Coins: sdk.NewCoins(newFooCoin(5))}}, + outputAddrs: []sdk.AccAddress{toAddr1}, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(995), newBarCoin(500)), + to1: sdk.NewCoins(newFooCoin(5)), + to2: sdk.Coins{}, + }, + }, + { + name: "passthrough restriction single output", + fn: restrictionPassthrough(), + inputCoins: sdk.NewCoins(newFooCoin(10)), + outputs: []banktypes.Output{{Address: toAddr1.String(), Coins: sdk.NewCoins(newFooCoin(10))}}, + outputAddrs: []sdk.AccAddress{toAddr1}, + expArgs: []*restrictionArgs{ + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(10)), + }, + }, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(985), newBarCoin(500)), + to1: sdk.NewCoins(newFooCoin(15)), + to2: sdk.Coins{}, + }, + }, + { + name: "new to restriction single output", + fn: restrictionNewTo(toAddr2), + inputCoins: sdk.NewCoins(newFooCoin(26)), + outputs: []banktypes.Output{{Address: toAddr1.String(), Coins: sdk.NewCoins(newFooCoin(26))}}, + outputAddrs: []sdk.AccAddress{toAddr2}, + expArgs: []*restrictionArgs{ + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(26)), + }, + }, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(959), newBarCoin(500)), + to1: sdk.NewCoins(newFooCoin(15)), + to2: sdk.NewCoins(newFooCoin(26)), + }, + }, + { + name: "error restriction single output", + fn: restrictionError("restriction test error"), + inputCoins: sdk.NewCoins(newBarCoin(88)), + outputs: []banktypes.Output{{Address: toAddr1.String(), Coins: sdk.NewCoins(newBarCoin(88))}}, + outputAddrs: []sdk.AccAddress{}, + expArgs: []*restrictionArgs{ + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newBarCoin(88)), + }, + }, + expErr: "restriction test error", + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(959), newBarCoin(412)), + to1: sdk.NewCoins(newFooCoin(15)), + to2: sdk.NewCoins(newFooCoin(26)), + }, + }, + { + name: "passthrough restriction two outputs", + fn: restrictionPassthrough(), + inputCoins: sdk.NewCoins(newFooCoin(11), newBarCoin(12)), + outputs: []banktypes.Output{ + {Address: toAddr1.String(), Coins: sdk.NewCoins(newFooCoin(11))}, + {Address: toAddr2.String(), Coins: sdk.NewCoins(newBarCoin(12))}, + }, + outputAddrs: []sdk.AccAddress{toAddr1, toAddr2}, + expArgs: []*restrictionArgs{ + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(11)), + }, + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr2, + amt: sdk.NewCoins(newBarCoin(12)), + }, + }, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(948), newBarCoin(400)), + to1: sdk.NewCoins(newFooCoin(26)), + to2: sdk.NewCoins(newFooCoin(26), newBarCoin(12)), + }, + }, + { + name: "error restriction two outputs error on second", + fn: restrictionError("", "second restriction error"), + inputCoins: sdk.NewCoins(newFooCoin(44)), + outputs: []banktypes.Output{ + {Address: toAddr1.String(), Coins: sdk.NewCoins(newFooCoin(12))}, + {Address: toAddr2.String(), Coins: sdk.NewCoins(newFooCoin(32))}, + }, + outputAddrs: []sdk.AccAddress{toAddr1}, + expArgs: []*restrictionArgs{ + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newFooCoin(12)), + }, + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr2, + amt: sdk.NewCoins(newFooCoin(32)), + }, + }, + expErr: "second restriction error", + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(904), newBarCoin(400)), + to1: sdk.NewCoins(newFooCoin(38)), + to2: sdk.NewCoins(newFooCoin(26), newBarCoin(12)), + }, + }, + { + name: "new to restriction two outputs", + fn: restrictionNewTo(toAddr2, toAddr1), + inputCoins: sdk.NewCoins(newBarCoin(35)), + outputs: []banktypes.Output{ + {Address: toAddr1.String(), Coins: sdk.NewCoins(newBarCoin(10))}, + {Address: toAddr2.String(), Coins: sdk.NewCoins(newBarCoin(25))}, + }, + outputAddrs: []sdk.AccAddress{toAddr1, toAddr2}, + expArgs: []*restrictionArgs{ + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr1, + amt: sdk.NewCoins(newBarCoin(10)), + }, + { + ctx: suite.ctx, + fromAddr: fromAddr, + toAddr: toAddr2, + amt: sdk.NewCoins(newBarCoin(25)), + }, + }, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(904), newBarCoin(365)), + to1: sdk.NewCoins(newFooCoin(38), newBarCoin(25)), + to2: sdk.NewCoins(newFooCoin(26), newBarCoin(22)), + }, + }, + } + + for _, tc := range tests { + suite.Run(tc.name, func() { + existingSendRestrictionFn := suite.bankKeeper.GetSendRestrictionFn() + defer suite.bankKeeper.SetSendRestriction(existingSendRestrictionFn) + actualRestrictionArgs = nil + suite.bankKeeper.SetSendRestriction(tc.fn) + ctx := suite.ctx + suite.mockInputOutputCoins(inputAccs, tc.outputAddrs) + input := banktypes.Input{ + Address: fromAddr.String(), + Coins: tc.inputCoins, + } + + var err error + testFunc := func() { + err = suite.bankKeeper.InputOutputCoins(ctx, input, tc.outputs) + } + suite.Require().NotPanics(testFunc, "InputOutputCoins") + if len(tc.expErr) > 0 { + suite.Assert().EqualError(err, tc.expErr, "InputOutputCoins error") + } else { + suite.Assert().NoError(err, "InputOutputCoins error") + } + if len(tc.expArgs) > 0 { + for i, expArgs := range tc.expArgs { + suite.Assert().Equal(expArgs.ctx, actualRestrictionArgs[i].ctx, "[%d] ctx provided to restriction", i) + suite.Assert().Equal(expArgs.fromAddr, actualRestrictionArgs[i].fromAddr, "[%d] fromAddr provided to restriction", i) + suite.Assert().Equal(expArgs.toAddr, actualRestrictionArgs[i].toAddr, "[%d] toAddr provided to restriction", i) + suite.Assert().Equal(expArgs.amt.String(), actualRestrictionArgs[i].amt.String(), "[%d] amt provided to restriction", i) + } + } else { + suite.Assert().Nil(actualRestrictionArgs, "args provided to a restriction") + } + fromBal := suite.bankKeeper.GetAllBalances(ctx, fromAddr) + suite.Assert().Equal(tc.expBals.from.String(), fromBal.String(), "fromAddr balance") + to1Bal := suite.bankKeeper.GetAllBalances(ctx, toAddr1) + suite.Assert().Equal(tc.expBals.to1.String(), to1Bal.String(), "toAddr1 balance") + to2Bal := suite.bankKeeper.GetAllBalances(ctx, toAddr2) + suite.Assert().Equal(tc.expBals.to2.String(), to2Bal.String(), "toAddr2 balance") + }) + } +} + func (suite *KeeperTestSuite) TestSendCoins() { ctx := suite.ctx require := suite.Require() @@ -603,9 +891,10 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { return newToAddr, nil } } - type expBal struct { - addr sdk.AccAddress - amt sdk.Coins + type expBals struct { + from sdk.Coins + to1 sdk.Coins + to2 sdk.Coins } setupCtx := suite.ctx @@ -626,7 +915,7 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { amt sdk.Coins expArgs *restrictionArgs expErr string - expBals []*expBal + expBals expBals }{ { name: "nil restriction", @@ -635,10 +924,10 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { finalAddr: toAddr1, amt: sdk.NewCoins(newFooCoin(5)), expArgs: nil, - expBals: []*expBal{ - {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(995), newBarCoin(500))}, - {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(5))}, - {addr: toAddr2, amt: sdk.Coins{}}, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(995), newBarCoin(500)), + to1: sdk.NewCoins(newFooCoin(5)), + to2: sdk.Coins{}, }, }, { @@ -653,10 +942,10 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { toAddr: toAddr1, amt: sdk.NewCoins(newFooCoin(10)), }, - expBals: []*expBal{ - {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(985), newBarCoin(500))}, - {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(15))}, - {addr: toAddr2, amt: sdk.Coins{}}, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(985), newBarCoin(500)), + to1: sdk.NewCoins(newFooCoin(15)), + to2: sdk.Coins{}, }, }, { @@ -671,10 +960,10 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { toAddr: toAddr1, amt: sdk.NewCoins(newBarCoin(27)), }, - expBals: []*expBal{ - {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(985), newBarCoin(473))}, - {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(15))}, - {addr: toAddr2, amt: sdk.NewCoins(newBarCoin(27))}, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(985), newBarCoin(473)), + to1: sdk.NewCoins(newFooCoin(15)), + to2: sdk.NewCoins(newBarCoin(27)), }, }, { @@ -690,10 +979,10 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { amt: sdk.NewCoins(newFooCoin(100), newBarCoin(200)), }, expErr: "test restriction error", - expBals: []*expBal{ - {addr: fromAddr, amt: sdk.NewCoins(newFooCoin(985), newBarCoin(473))}, - {addr: toAddr1, amt: sdk.NewCoins(newFooCoin(15))}, - {addr: toAddr2, amt: sdk.NewCoins(newBarCoin(27))}, + expBals: expBals{ + from: sdk.NewCoins(newFooCoin(985), newBarCoin(473)), + to1: sdk.NewCoins(newFooCoin(15)), + to2: sdk.NewCoins(newBarCoin(27)), }, }, } @@ -723,14 +1012,16 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { suite.Assert().Equal(tc.expArgs.ctx, actualRestrictionArgs.ctx, "ctx provided to restriction") suite.Assert().Equal(tc.expArgs.fromAddr, actualRestrictionArgs.fromAddr, "fromAddr provided to restriction") suite.Assert().Equal(tc.expArgs.toAddr, actualRestrictionArgs.toAddr, "toAddr provided to restriction") - suite.Assert().Equal(tc.expArgs.amt, actualRestrictionArgs.amt, "amt provided to restriction") + suite.Assert().Equal(tc.expArgs.amt.String(), actualRestrictionArgs.amt.String(), "amt provided to restriction") } else { suite.Assert().Nil(actualRestrictionArgs, "args provided to a restriction") } - for i, bal := range tc.expBals { - actual := suite.bankKeeper.GetAllBalances(ctx, bal.addr) - suite.Assert().Equal(bal.amt, actual, "expected balance[%d]", i) - } + fromBal := suite.bankKeeper.GetAllBalances(ctx, fromAddr) + suite.Assert().Equal(tc.expBals.from.String(), fromBal.String(), "fromAddr balance") + to1Bal := suite.bankKeeper.GetAllBalances(ctx, toAddr1) + suite.Assert().Equal(tc.expBals.to1.String(), to1Bal.String(), "toAddr1 balance") + to2Bal := suite.bankKeeper.GetAllBalances(ctx, toAddr2) + suite.Assert().Equal(tc.expBals.to2.String(), to2Bal.String(), "toAddr2 balance") }) } } From a5b1afecc457298a6b7ee4c4815676a369b602bb Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 01:29:43 -0700 Subject: [PATCH 10/27] [14124]: Add unit tests on append and prepend send restrictions. --- x/bank/keeper/keeper_test.go | 58 ++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 66a1d1b4599d..fce40828d8df 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -196,6 +196,64 @@ func (suite *KeeperTestSuite) mockUnDelegateCoins(ctx sdk.Context, acc authtypes suite.authKeeper.EXPECT().GetAccount(ctx, mAcc.GetAddress()).Return(mAcc) } +func (suite *KeeperTestSuite) TestAppendSendRestriction() { + var calls []int + testRestriction := func(index int) keeper.SendRestrictionFn { + return func(_ sdk.Context, _, _ sdk.AccAddress, _ sdk.Coins) (sdk.AccAddress, error) { + calls = append(calls, index) + return nil, nil + } + } + + bk := suite.bankKeeper + + // Initial append of the test restriction. + bk.SetSendRestriction(nil) + bk.AppendSendRestriction(testRestriction(1)) + _, _ = bk.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) + suite.Require().Equal([]int{1}, calls, "restriction calls after first append") + + // Append the test restriction again. + calls = nil + bk.AppendSendRestriction(testRestriction(2)) + _, _ = bk.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) + suite.Require().Equal([]int{1, 2}, calls, "restriction counter after second append") + + // make sure the original bank keeper has the restrictions too. + calls = nil + _, _ = suite.bankKeeper.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) + suite.Require().Equal([]int{1, 2}, calls, "restriction counter from original bank keeper") +} + +func (suite *KeeperTestSuite) TestPrependSendRestriction() { + var calls []int + testRestriction := func(index int) keeper.SendRestrictionFn { + return func(_ sdk.Context, _, _ sdk.AccAddress, _ sdk.Coins) (sdk.AccAddress, error) { + calls = append(calls, index) + return nil, nil + } + } + + bk := suite.bankKeeper + + // Initial append of the test restriction. + bk.SetSendRestriction(nil) + bk.PrependSendRestriction(testRestriction(1)) + _, _ = bk.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) + suite.Require().Equal([]int{1}, calls, "restriction calls after first append") + + // Append the test restriction again. + calls = nil + bk.PrependSendRestriction(testRestriction(2)) + _, _ = bk.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) + suite.Require().Equal([]int{2, 1}, calls, "restriction counter after second append") + + // make sure the original bank keeper has the restrictions too. + calls = nil + _, _ = suite.bankKeeper.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) + suite.Require().Equal([]int{2, 1}, calls, "restriction counter from original bank keeper") +} + func (suite *KeeperTestSuite) TestGetAuthority() { NewKeeperWithAuthority := func(authority string) keeper.BaseKeeper { return keeper.NewBaseKeeper( From 64bb3c1d492cb94ceab59e34fe9b1ea2f2bea62c Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 01:34:31 -0700 Subject: [PATCH 11/27] [14124]: Update the gov mocks to include the new Append and Prepend restriction functions. --- x/gov/testutil/expected_keepers_mocks.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/x/gov/testutil/expected_keepers_mocks.go b/x/gov/testutil/expected_keepers_mocks.go index 5519c7fc776d..5428f9a84a1c 100644 --- a/x/gov/testutil/expected_keepers_mocks.go +++ b/x/gov/testutil/expected_keepers_mocks.go @@ -145,6 +145,18 @@ func (mr *MockBankKeeperMockRecorder) AllBalances(arg0, arg1 interface{}) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AllBalances", reflect.TypeOf((*MockBankKeeper)(nil).AllBalances), arg0, arg1) } +// AppendSendRestriction mocks base method. +func (m *MockBankKeeper) AppendSendRestriction(restriction keeper.SendRestrictionFn) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "AppendSendRestriction", restriction) +} + +// AppendSendRestriction indicates an expected call of AppendSendRestriction. +func (mr *MockBankKeeperMockRecorder) AppendSendRestriction(restriction interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AppendSendRestriction", reflect.TypeOf((*MockBankKeeper)(nil).AppendSendRestriction), restriction) +} + // Balance mocks base method. func (m *MockBankKeeper) Balance(arg0 context.Context, arg1 *types1.QueryBalanceRequest) (*types1.QueryBalanceResponse, error) { m.ctrl.T.Helper() @@ -668,6 +680,18 @@ func (mr *MockBankKeeperMockRecorder) Params(arg0, arg1 interface{}) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Params", reflect.TypeOf((*MockBankKeeper)(nil).Params), arg0, arg1) } +// PrependSendRestriction mocks base method. +func (m *MockBankKeeper) PrependSendRestriction(restriction keeper.SendRestrictionFn) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "PrependSendRestriction", restriction) +} + +// PrependSendRestriction indicates an expected call of PrependSendRestriction. +func (mr *MockBankKeeperMockRecorder) PrependSendRestriction(restriction interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrependSendRestriction", reflect.TypeOf((*MockBankKeeper)(nil).PrependSendRestriction), restriction) +} + // SendCoins mocks base method. func (m *MockBankKeeper) SendCoins(ctx types.Context, fromAddr, toAddr types.AccAddress, amt types.Coins) error { m.ctrl.T.Helper() From e38880181d2a86b0444d731c5bcbb63b3d295c82 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 10:03:31 -0700 Subject: [PATCH 12/27] [14124]: Update changelog. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51229f5d15c1..cb38b2e35014 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/auth) [#13210](https://github.com/cosmos/cosmos-sdk/pull/13210) Add `Query/AccountInfo` endpoint for simplified access to basic account info. * (x/consensus) [#12905](https://github.com/cosmos/cosmos-sdk/pull/12905) Create a new `x/consensus` module that is now responsible for maintaining Tendermint consensus parameters instead of `x/param`. Legacy types remain in order to facilitate parameter migration from the deprecated `x/params`. App developers should ensure that they execute `baseapp.MigrateParams` during their chain upgrade. These legacy types will be removed in a future release. * (client/tx) [#13670](https://github.com/cosmos/cosmos-sdk/pull/13670) Add validation in `BuildUnsignedTx` to prevent simple inclusion of valid mnemonics +* (x/bank) [#14224](https://github.com/cosmos/cosmos-sdk/pull/14224) Allow injection of restrictions on transfers using `AppendSendRestriction` or `PrependSendRestriction`. ### Improvements @@ -196,6 +197,7 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf * Remove listening APIs from the caching layer (it should only listen to the `rootmulti.Store`) * Add three new options to file streaming service constructor. * Modify `ABCIListener` such that any error from any method will always halt the app via `panic` +* (x/bank) [#14224](https://github.com/cosmos/cosmos-sdk/pull/14224) `InputOutputCoins` now only allows for a single `Input`. ### CLI Breaking Changes From 1ef3412a0517dd264569d76e607ba3a3979b8c66 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 10:27:44 -0700 Subject: [PATCH 13/27] [14124]: Fix a couple test failure messages. --- x/bank/keeper/keeper_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index fce40828d8df..12e875139593 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -217,12 +217,12 @@ func (suite *KeeperTestSuite) TestAppendSendRestriction() { calls = nil bk.AppendSendRestriction(testRestriction(2)) _, _ = bk.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) - suite.Require().Equal([]int{1, 2}, calls, "restriction counter after second append") + suite.Require().Equal([]int{1, 2}, calls, "restriction calls after second append") // make sure the original bank keeper has the restrictions too. calls = nil _, _ = suite.bankKeeper.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) - suite.Require().Equal([]int{1, 2}, calls, "restriction counter from original bank keeper") + suite.Require().Equal([]int{1, 2}, calls, "restriction calls from original bank keeper") } func (suite *KeeperTestSuite) TestPrependSendRestriction() { @@ -246,12 +246,12 @@ func (suite *KeeperTestSuite) TestPrependSendRestriction() { calls = nil bk.PrependSendRestriction(testRestriction(2)) _, _ = bk.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) - suite.Require().Equal([]int{2, 1}, calls, "restriction counter after second append") + suite.Require().Equal([]int{2, 1}, calls, "restriction calls after second append") // make sure the original bank keeper has the restrictions too. calls = nil _, _ = suite.bankKeeper.GetSendRestrictionFn()(suite.ctx, nil, nil, nil) - suite.Require().Equal([]int{2, 1}, calls, "restriction counter from original bank keeper") + suite.Require().Equal([]int{2, 1}, calls, "restriction calls from original bank keeper") } func (suite *KeeperTestSuite) TestGetAuthority() { From 60fbf14d4a59565c17902c1beb62a75b6d08c422 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 8 Dec 2022 10:28:08 -0700 Subject: [PATCH 14/27] [14124]: Fix some import ordering. --- x/bank/keeper/restrictions_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x/bank/keeper/restrictions_test.go b/x/bank/keeper/restrictions_test.go index f357632177bc..0b598002858c 100644 --- a/x/bank/keeper/restrictions_test.go +++ b/x/bank/keeper/restrictions_test.go @@ -2,11 +2,13 @@ package keeper_test import ( "errors" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/bank/keeper" + "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/bank/keeper" ) // MintingRestrictionArgs are the args provided to a MintingRestrictionFn function. From 66ec36fc0e263c53ca7e7367047bcb7c399a9c8e Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Fri, 9 Dec 2022 10:38:15 -0700 Subject: [PATCH 15/27] [14124]: Update spec doc with SendKeeper Append and Prepend SendRestriction functions. --- x/bank/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/bank/README.md b/x/bank/README.md index 0ef532d0ac52..0b2b603e586e 100644 --- a/x/bank/README.md +++ b/x/bank/README.md @@ -236,6 +236,9 @@ accounts. The send keeper does not alter the total supply (mint or burn coins). type SendKeeper interface { ViewKeeper + AppendSendRestriction(restriction SendRestrictionFn) + PrependSendRestriction(restriction SendRestrictionFn) + InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error From 28f37ede162d2a8c26683c99459b045a9b0cf434 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 11 Jan 2023 17:40:02 -0700 Subject: [PATCH 16/27] [14124]: Fix test compilation (due to AccountI change). --- x/bank/keeper/keeper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index f0e3fefe28cf..c9193f414772 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -649,7 +649,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { balances := sdk.NewCoins(newFooCoin(1000), newBarCoin(500)) fromAddr := accAddrs[0] fromAcc := authtypes.NewBaseAccountWithAddress(fromAddr) - inputAccs := []authtypes.AccountI{fromAcc} + inputAccs := []sdk.AccountI{fromAcc} toAddr1 := accAddrs[1] toAddr2 := accAddrs[2] From a0befb38b564ee64383573c2ddd427c486e4cf78 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 11 Jan 2023 17:55:42 -0700 Subject: [PATCH 17/27] [14124]: Lint fixes. --- x/bank/keeper/keeper_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index c9193f414772..108446f97653 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -601,7 +601,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { toAddr: toAddr, amt: amt, }) - i += 1 + i++ if i < len(messages) { if len(messages[i]) > 0 { return nil, errors.New(messages[i]) @@ -630,7 +630,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { toAddr: toAddr, amt: amt, }) - i += 1 + i++ if i < len(newToAddrs) { if len(newToAddrs[i]) > 0 { return newToAddrs[i], nil From 327f218ce2830494246389cf9916982ac51fbbda Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 12 Jan 2023 10:30:49 -0700 Subject: [PATCH 18/27] [14124]: Add some extra info to the restriction composition function comments. --- x/bank/keeper/restrictions.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/bank/keeper/restrictions.go b/x/bank/keeper/restrictions.go index 82913bc86b43..c47b69dff56d 100644 --- a/x/bank/keeper/restrictions.go +++ b/x/bank/keeper/restrictions.go @@ -22,6 +22,7 @@ func (r MintingRestrictionFn) Then(second MintingRestrictionFn) MintingRestricti // If all entries are nil, nil is returned. // If exactly one entry is not nil, it is returned. // Otherwise, a new MintingRestrictionFn is returned that runs the non-nil restrictions in the order they are given. +// The composition runs each minting restriction until an error is encountered and returns that error. func ComposeMintingRestrictions(restrictions ...MintingRestrictionFn) MintingRestrictionFn { toRun := make([]MintingRestrictionFn, 0, len(restrictions)) for _, r := range restrictions { @@ -66,6 +67,8 @@ func (r SendRestrictionFn) Then(second SendRestrictionFn) SendRestrictionFn { // If all entries are nil, nil is returned. // If exactly one entry is not nil, it is returned. // Otherwise, a new SendRestrictionFn is returned that runs the non-nil restrictions in the order they are given. +// The composition runs each send restriction until an error is encountered and returns that error, +// otherwise it returns the toAddr of the last send restriction. func ComposeSendRestrictions(restrictions ...SendRestrictionFn) SendRestrictionFn { toRun := make([]SendRestrictionFn, 0, len(restrictions)) for _, r := range restrictions { From c9b5bab221e30da72981d8e52553f4cb4dab9cf9 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Mon, 17 Apr 2023 16:32:33 -0600 Subject: [PATCH 19/27] [14124]: Remove the API Breaking change listing since another PR did that. --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca6fa0078a9a..93d3d25a207a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -161,7 +161,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * `InterfaceRegistry` is now a private interface and implements `protodesc.Resolver` plus the `RangeFiles` method All implementations of `InterfaceRegistry` by other users must now embed the official implementation. * `AminoCodec` is marked as deprecated. -* (x/bank) [#14224](https://github.com/cosmos/cosmos-sdk/pull/14224) `InputOutputCoins` now only allows for a single `Input`. ### Client Breaking Changes From f54f8fae8af8f6900a06d286769c62465bd999cc Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 18 Apr 2023 17:26:56 -0600 Subject: [PATCH 20/27] [14124]: Move the definition of the restriction functions into the types package so it's easier for other modules to create expected keepers for them. --- x/bank/keeper/export_test.go | 6 +- x/bank/keeper/keeper.go | 8 +-- x/bank/keeper/keeper_test.go | 25 ++++---- x/bank/keeper/send.go | 12 ++-- x/bank/{keeper => types}/restrictions.go | 2 +- x/bank/{keeper => types}/restrictions_test.go | 58 +++++++++---------- x/gov/testutil/expected_keepers_mocks.go | 6 +- 7 files changed, 60 insertions(+), 57 deletions(-) rename x/bank/{keeper => types}/restrictions.go (99%) rename x/bank/{keeper => types}/restrictions_test.go (94%) diff --git a/x/bank/keeper/export_test.go b/x/bank/keeper/export_test.go index 6ed22da65456..bf358135df1c 100644 --- a/x/bank/keeper/export_test.go +++ b/x/bank/keeper/export_test.go @@ -1,12 +1,14 @@ package keeper +import "github.com/cosmos/cosmos-sdk/x/bank/types" + // This file exists in the keeper package to expose some private things // for the purpose of testing in the keeper_test package. -func (k BaseSendKeeper) SetSendRestriction(restriction SendRestrictionFn) { +func (k BaseSendKeeper) SetSendRestriction(restriction types.SendRestrictionFn) { k.sendRestriction.Fn = restriction } -func (k BaseSendKeeper) GetSendRestrictionFn() SendRestrictionFn { +func (k BaseSendKeeper) GetSendRestrictionFn() types.SendRestrictionFn { return k.sendRestriction.Fn } diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 780da3cafee5..c0574c0722f8 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -22,7 +22,7 @@ var _ Keeper = (*BaseKeeper)(nil) // between accounts. type Keeper interface { SendKeeper - WithMintCoinsRestriction(MintingRestrictionFn) BaseKeeper + WithMintCoinsRestriction(types.MintingRestrictionFn) BaseKeeper InitGenesis(sdk.Context, *types.GenesisState) ExportGenesis(sdk.Context) *types.GenesisState @@ -58,7 +58,7 @@ type BaseKeeper struct { ak types.AccountKeeper cdc codec.BinaryCodec storeKey storetypes.StoreKey - mintCoinsRestrictionFn MintingRestrictionFn + mintCoinsRestrictionFn types.MintingRestrictionFn } // GetPaginatedTotalSupply queries for the supply, ignoring 0 coins, with a given pagination @@ -97,7 +97,7 @@ func NewBaseKeeper( ak: ak, cdc: cdc, storeKey: storeKey, - mintCoinsRestrictionFn: NoOpMintingRestrictionFn, + mintCoinsRestrictionFn: types.NoOpMintingRestrictionFn, } } @@ -106,7 +106,7 @@ func NewBaseKeeper( // Previous restriction functions can be nested as such: // // bankKeeper.WithMintCoinsRestriction(restriction1).WithMintCoinsRestriction(restriction2) -func (k BaseKeeper) WithMintCoinsRestriction(check MintingRestrictionFn) BaseKeeper { +func (k BaseKeeper) WithMintCoinsRestriction(check types.MintingRestrictionFn) BaseKeeper { k.mintCoinsRestrictionFn = check.Then(k.mintCoinsRestrictionFn) return k } diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 6b7cbbeef444..8037fe541efe 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -9,13 +9,14 @@ import ( "testing" "time" - "cosmossdk.io/math" abci "github.com/cometbft/cometbft/abci/types" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" cmttime "github.com/cometbft/cometbft/types/time" "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" + "cosmossdk.io/math" + storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/baseapp" @@ -235,7 +236,7 @@ func (suite *KeeperTestSuite) mockUnDelegateCoins(ctx sdk.Context, acc, mAcc sdk func (suite *KeeperTestSuite) TestAppendSendRestriction() { var calls []int - testRestriction := func(index int) keeper.SendRestrictionFn { + testRestriction := func(index int) banktypes.SendRestrictionFn { return func(_ sdk.Context, _, _ sdk.AccAddress, _ sdk.Coins) (sdk.AccAddress, error) { calls = append(calls, index) return nil, nil @@ -264,7 +265,7 @@ func (suite *KeeperTestSuite) TestAppendSendRestriction() { func (suite *KeeperTestSuite) TestPrependSendRestriction() { var calls []int - testRestriction := func(index int) keeper.SendRestrictionFn { + testRestriction := func(index int) banktypes.SendRestrictionFn { return func(_ sdk.Context, _, _ sdk.AccAddress, _ sdk.Coins) (sdk.AccAddress, error) { calls = append(calls, index) return nil, nil @@ -632,7 +633,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { amt sdk.Coins } var actualRestrictionArgs []*restrictionArgs - restrictionError := func(messages ...string) keeper.SendRestrictionFn { + restrictionError := func(messages ...string) banktypes.SendRestrictionFn { i := -1 return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { actualRestrictionArgs = append(actualRestrictionArgs, &restrictionArgs{ @@ -650,7 +651,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { return toAddr, nil } } - restrictionPassthrough := func() keeper.SendRestrictionFn { + restrictionPassthrough := func() banktypes.SendRestrictionFn { return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { actualRestrictionArgs = append(actualRestrictionArgs, &restrictionArgs{ ctx: ctx, @@ -661,7 +662,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { return toAddr, nil } } - restrictionNewTo := func(newToAddrs ...sdk.AccAddress) keeper.SendRestrictionFn { + restrictionNewTo := func(newToAddrs ...sdk.AccAddress) banktypes.SendRestrictionFn { i := -1 return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { actualRestrictionArgs = append(actualRestrictionArgs, &restrictionArgs{ @@ -698,7 +699,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() { tests := []struct { name string - fn keeper.SendRestrictionFn + fn banktypes.SendRestrictionFn inputCoins sdk.Coins outputs []banktypes.Output outputAddrs []sdk.AccAddress @@ -957,7 +958,7 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { amt sdk.Coins } var actualRestrictionArgs *restrictionArgs - restrictionError := func(message string) keeper.SendRestrictionFn { + restrictionError := func(message string) banktypes.SendRestrictionFn { return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { actualRestrictionArgs = &restrictionArgs{ ctx: ctx, @@ -968,7 +969,7 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { return nil, errors.New(message) } } - restrictionPassthrough := func() keeper.SendRestrictionFn { + restrictionPassthrough := func() banktypes.SendRestrictionFn { return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { actualRestrictionArgs = &restrictionArgs{ ctx: ctx, @@ -979,7 +980,7 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { return toAddr, nil } } - restrictionNewTo := func(newToAddr sdk.AccAddress) keeper.SendRestrictionFn { + restrictionNewTo := func(newToAddr sdk.AccAddress) banktypes.SendRestrictionFn { return func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { actualRestrictionArgs = &restrictionArgs{ ctx: ctx, @@ -1008,7 +1009,7 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { tests := []struct { name string - fn keeper.SendRestrictionFn + fn banktypes.SendRestrictionFn toAddr sdk.AccAddress finalAddr sdk.AccAddress amt sdk.Coins @@ -1866,7 +1867,7 @@ func (suite *KeeperTestSuite) TestMintCoinRestrictions() { } for _, test := range tests { - keeper := suite.bankKeeper.WithMintCoinsRestriction(keeper.MintingRestrictionFn(test.restrictionFn)) + keeper := suite.bankKeeper.WithMintCoinsRestriction(banktypes.MintingRestrictionFn(test.restrictionFn)) for _, testCase := range test.testCases { if testCase.expectPass { suite.mockMintCoins(multiPermAcc) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 0b877fe45e70..2adf18303671 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -20,8 +20,8 @@ import ( type SendKeeper interface { ViewKeeper - AppendSendRestriction(restriction SendRestrictionFn) - PrependSendRestriction(restriction SendRestrictionFn) + AppendSendRestriction(restriction types.SendRestrictionFn) + PrependSendRestriction(restriction types.SendRestrictionFn) InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error @@ -64,7 +64,7 @@ type BaseSendKeeper struct { // should be the x/gov module account. authority string - sendRestriction *SendRestriction + sendRestriction *types.SendRestriction } func NewBaseSendKeeper( @@ -85,17 +85,17 @@ func NewBaseSendKeeper( storeKey: storeKey, blockedAddrs: blockedAddrs, authority: authority, - sendRestriction: NewSendRestriction(), + sendRestriction: types.NewSendRestriction(), } } // AppendSendRestriction adds the provided SendRestrictionFn to run after previously provided restrictions. -func (k BaseSendKeeper) AppendSendRestriction(restriction SendRestrictionFn) { +func (k BaseSendKeeper) AppendSendRestriction(restriction types.SendRestrictionFn) { k.sendRestriction.Append(restriction) } // PrependSendRestriction adds the provided SendRestrictionFn to run before previously provided restrictions. -func (k BaseSendKeeper) PrependSendRestriction(restriction SendRestrictionFn) { +func (k BaseSendKeeper) PrependSendRestriction(restriction types.SendRestrictionFn) { k.sendRestriction.Prepend(restriction) } diff --git a/x/bank/keeper/restrictions.go b/x/bank/types/restrictions.go similarity index 99% rename from x/bank/keeper/restrictions.go rename to x/bank/types/restrictions.go index c47b69dff56d..3690eb9d67f0 100644 --- a/x/bank/keeper/restrictions.go +++ b/x/bank/types/restrictions.go @@ -1,4 +1,4 @@ -package keeper +package types import sdk "github.com/cosmos/cosmos-sdk/types" diff --git a/x/bank/keeper/restrictions_test.go b/x/bank/types/restrictions_test.go similarity index 94% rename from x/bank/keeper/restrictions_test.go rename to x/bank/types/restrictions_test.go index 0b598002858c..c346617f47b9 100644 --- a/x/bank/keeper/restrictions_test.go +++ b/x/bank/types/restrictions_test.go @@ -1,4 +1,4 @@ -package keeper_test +package types_test import ( "errors" @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/bank/keeper" + "github.com/cosmos/cosmos-sdk/x/bank/types" ) // MintingRestrictionArgs are the args provided to a MintingRestrictionFn function. @@ -45,7 +45,7 @@ func (s *MintingRestrictionTestHelper) NewArgs(name string, coins sdk.Coins) *Mi } // NamedRestriction creates a new MintingRestrictionFn function that records the arguments it's called with and returns nil. -func (s *MintingRestrictionTestHelper) NamedRestriction(name string) keeper.MintingRestrictionFn { +func (s *MintingRestrictionTestHelper) NamedRestriction(name string) types.MintingRestrictionFn { return func(_ sdk.Context, coins sdk.Coins) error { s.RecordCall(name, coins) return nil @@ -53,7 +53,7 @@ func (s *MintingRestrictionTestHelper) NamedRestriction(name string) keeper.Mint } // ErrorRestriction creates a new MintingRestrictionFn function that returns an error. -func (s *MintingRestrictionTestHelper) ErrorRestriction(message string) keeper.MintingRestrictionFn { +func (s *MintingRestrictionTestHelper) ErrorRestriction(message string) types.MintingRestrictionFn { return func(_ sdk.Context, coins sdk.Coins) error { s.RecordCall(message, coins) return errors.New(message) @@ -74,7 +74,7 @@ type MintingRestrictionTestParams struct { } // TestActual tests the provided MintingRestrictionFn using the provided test parameters. -func (s *MintingRestrictionTestHelper) TestActual(t *testing.T, tp *MintingRestrictionTestParams, actual keeper.MintingRestrictionFn) { +func (s *MintingRestrictionTestHelper) TestActual(t *testing.T, tp *MintingRestrictionTestParams, actual types.MintingRestrictionFn) { t.Helper() if tp.ExpNil { require.Nil(t, actual, "resulting MintingRestrictionFn") @@ -98,8 +98,8 @@ func TestMintingRestriction_Then(t *testing.T) { tests := []struct { name string - base keeper.MintingRestrictionFn - second keeper.MintingRestrictionFn + base types.MintingRestrictionFn + second types.MintingRestrictionFn exp *MintingRestrictionTestParams }{ { @@ -169,8 +169,8 @@ func TestMintingRestriction_Then(t *testing.T) { }, { name: "double chain", - base: keeper.ComposeMintingRestrictions(h.NamedRestriction("r1"), h.NamedRestriction("r2")), - second: keeper.ComposeMintingRestrictions(h.NamedRestriction("r3"), h.NamedRestriction("r4")), + base: types.ComposeMintingRestrictions(h.NamedRestriction("r1"), h.NamedRestriction("r2")), + second: types.ComposeMintingRestrictions(h.NamedRestriction("r3"), h.NamedRestriction("r4")), exp: &MintingRestrictionTestParams{ Coins: coins, ExpCalls: h.NewCalls( @@ -185,7 +185,7 @@ func TestMintingRestriction_Then(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - var actual keeper.MintingRestrictionFn + var actual types.MintingRestrictionFn testFunc := func() { actual = tc.base.Then(tc.second) } @@ -196,7 +196,7 @@ func TestMintingRestriction_Then(t *testing.T) { } func TestComposeMintingRestrictions(t *testing.T) { - rz := func(rs ...keeper.MintingRestrictionFn) []keeper.MintingRestrictionFn { + rz := func(rs ...types.MintingRestrictionFn) []types.MintingRestrictionFn { return rs } coins := sdk.NewCoins(sdk.NewInt64Coin("ccoin", 8), sdk.NewInt64Coin("dcoin", 16)) @@ -205,7 +205,7 @@ func TestComposeMintingRestrictions(t *testing.T) { tests := []struct { name string - input []keeper.MintingRestrictionFn + input []types.MintingRestrictionFn exp *MintingRestrictionTestParams }{ { @@ -374,9 +374,9 @@ func TestComposeMintingRestrictions(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - var actual keeper.MintingRestrictionFn + var actual types.MintingRestrictionFn testFunc := func() { - actual = keeper.ComposeMintingRestrictions(tc.input...) + actual = types.ComposeMintingRestrictions(tc.input...) } require.NotPanics(t, testFunc, "ComposeMintingRestrictions") h.TestActual(t, tc.exp, actual) @@ -387,7 +387,7 @@ func TestComposeMintingRestrictions(t *testing.T) { func TestNoOpMintingRestrictionFn(t *testing.T) { var err error testFunc := func() { - err = keeper.NoOpMintingRestrictionFn(sdk.Context{}, sdk.Coins{}) + err = types.NoOpMintingRestrictionFn(sdk.Context{}, sdk.Coins{}) } require.NotPanics(t, testFunc, "NoOpMintingRestrictionFn") assert.NoError(t, err, "NoOpSendRestrictionFn error") @@ -431,7 +431,7 @@ func (s *SendRestrictionTestHelper) NewArgs(name string, fromAddr, toAddr sdk.Ac } // NamedRestriction creates a new SendRestrictionFn function that records the arguments it's called with and returns the provided toAddr. -func (s *SendRestrictionTestHelper) NamedRestriction(name string) keeper.SendRestrictionFn { +func (s *SendRestrictionTestHelper) NamedRestriction(name string) types.SendRestrictionFn { return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { s.RecordCall(name, fromAddr, toAddr, coins) return toAddr, nil @@ -439,7 +439,7 @@ func (s *SendRestrictionTestHelper) NamedRestriction(name string) keeper.SendRes } // NewToRestriction creates a new SendRestrictionFn function that returns a different toAddr than provided. -func (s *SendRestrictionTestHelper) NewToRestriction(name string, addr sdk.AccAddress) keeper.SendRestrictionFn { +func (s *SendRestrictionTestHelper) NewToRestriction(name string, addr sdk.AccAddress) types.SendRestrictionFn { return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { s.RecordCall(name, fromAddr, toAddr, coins) return addr, nil @@ -447,7 +447,7 @@ func (s *SendRestrictionTestHelper) NewToRestriction(name string, addr sdk.AccAd } // ErrorRestriction creates a new SendRestrictionFn function that returns a nil toAddr and an error. -func (s *SendRestrictionTestHelper) ErrorRestriction(message string) keeper.SendRestrictionFn { +func (s *SendRestrictionTestHelper) ErrorRestriction(message string) types.SendRestrictionFn { return func(_ sdk.Context, fromAddr, toAddr sdk.AccAddress, coins sdk.Coins) (sdk.AccAddress, error) { s.RecordCall(message, fromAddr, toAddr, coins) return nil, errors.New(message) @@ -474,7 +474,7 @@ type SendRestrictionTestParams struct { } // TestActual tests the provided SendRestrictionFn using the provided test parameters. -func (s *SendRestrictionTestHelper) TestActual(t *testing.T, tp *SendRestrictionTestParams, actual keeper.SendRestrictionFn) { +func (s *SendRestrictionTestHelper) TestActual(t *testing.T, tp *SendRestrictionTestParams, actual types.SendRestrictionFn) { t.Helper() if tp.ExpNil { require.Nil(t, actual, "resulting SendRestrictionFn") @@ -505,8 +505,8 @@ func TestSendRestriction_Then(t *testing.T) { tests := []struct { name string - base keeper.SendRestrictionFn - second keeper.SendRestrictionFn + base types.SendRestrictionFn + second types.SendRestrictionFn exp *SendRestrictionTestParams }{ { @@ -615,8 +615,8 @@ func TestSendRestriction_Then(t *testing.T) { }, { name: "double chain", - base: keeper.ComposeSendRestrictions(h.NewToRestriction("r1", addr1), h.NewToRestriction("r2", addr2)), - second: keeper.ComposeSendRestrictions(h.NewToRestriction("r3", addr3), h.NewToRestriction("r4", addr4)), + base: types.ComposeSendRestrictions(h.NewToRestriction("r1", addr1), h.NewToRestriction("r2", addr2)), + second: types.ComposeSendRestrictions(h.NewToRestriction("r3", addr3), h.NewToRestriction("r4", addr4)), exp: &SendRestrictionTestParams{ FromAddr: fromAddr, ToAddr: addr0, @@ -634,7 +634,7 @@ func TestSendRestriction_Then(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - var actual keeper.SendRestrictionFn + var actual types.SendRestrictionFn testFunc := func() { actual = tc.base.Then(tc.second) } @@ -645,7 +645,7 @@ func TestSendRestriction_Then(t *testing.T) { } func TestComposeSendRestrictions(t *testing.T) { - rz := func(rs ...keeper.SendRestrictionFn) []keeper.SendRestrictionFn { + rz := func(rs ...types.SendRestrictionFn) []types.SendRestrictionFn { return rs } fromAddr := sdk.AccAddress("fromaddr____________") @@ -660,7 +660,7 @@ func TestComposeSendRestrictions(t *testing.T) { tests := []struct { name string - input []keeper.SendRestrictionFn + input []types.SendRestrictionFn exp *SendRestrictionTestParams }{ { @@ -895,9 +895,9 @@ func TestComposeSendRestrictions(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - var actual keeper.SendRestrictionFn + var actual types.SendRestrictionFn testFunc := func() { - actual = keeper.ComposeSendRestrictions(tc.input...) + actual = types.ComposeSendRestrictions(tc.input...) } require.NotPanics(t, testFunc, "ComposeSendRestrictions") h.TestActual(t, tc.exp, actual) @@ -910,7 +910,7 @@ func TestNoOpSendRestrictionFn(t *testing.T) { var addr sdk.AccAddress var err error testFunc := func() { - addr, err = keeper.NoOpSendRestrictionFn(sdk.Context{}, sdk.AccAddress("first_addr"), expAddr, sdk.Coins{}) + addr, err = types.NoOpSendRestrictionFn(sdk.Context{}, sdk.AccAddress("first_addr"), expAddr, sdk.Coins{}) } require.NotPanics(t, testFunc, "NoOpSendRestrictionFn") assert.NoError(t, err, "NoOpSendRestrictionFn error") diff --git a/x/gov/testutil/expected_keepers_mocks.go b/x/gov/testutil/expected_keepers_mocks.go index 9d8c1d7d5ae4..3bb59003cfb7 100644 --- a/x/gov/testutil/expected_keepers_mocks.go +++ b/x/gov/testutil/expected_keepers_mocks.go @@ -175,7 +175,7 @@ func (mr *MockBankKeeperMockRecorder) AllBalances(arg0, arg1 interface{}) *gomoc } // AppendSendRestriction mocks base method. -func (m *MockBankKeeper) AppendSendRestriction(restriction keeper.SendRestrictionFn) { +func (m *MockBankKeeper) AppendSendRestriction(restriction types0.SendRestrictionFn) { m.ctrl.T.Helper() m.ctrl.Call(m, "AppendSendRestriction", restriction) } @@ -724,7 +724,7 @@ func (mr *MockBankKeeperMockRecorder) Params(arg0, arg1 interface{}) *gomock.Cal } // PrependSendRestriction mocks base method. -func (m *MockBankKeeper) PrependSendRestriction(restriction keeper.SendRestrictionFn) { +func (m *MockBankKeeper) PrependSendRestriction(restriction types0.SendRestrictionFn) { m.ctrl.T.Helper() m.ctrl.Call(m, "PrependSendRestriction", restriction) } @@ -987,7 +987,7 @@ func (mr *MockBankKeeperMockRecorder) ValidateBalance(ctx, addr interface{}) *go } // WithMintCoinsRestriction mocks base method. -func (m *MockBankKeeper) WithMintCoinsRestriction(arg0 keeper.MintingRestrictionFn) keeper.BaseKeeper { +func (m *MockBankKeeper) WithMintCoinsRestriction(arg0 types0.MintingRestrictionFn) keeper.BaseKeeper { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "WithMintCoinsRestriction", arg0) ret0, _ := ret[0].(keeper.BaseKeeper) From c6e1dcac8f102ee7b197dd1b5de4b61d644a1386 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 19 Apr 2023 16:00:08 -0600 Subject: [PATCH 21/27] [14124]: Move the wrapper for the SendRestrictionFn back into the keeper since it's specific to the keeper and a bit confusing to have right next to the SendRestrictionFn. --- x/bank/README.md | 1 + x/bank/keeper/export_test.go | 4 +- x/bank/keeper/send.go | 72 ++++++++++++++++++++++++++++-------- x/bank/types/restrictions.go | 23 ------------ 4 files changed, 59 insertions(+), 41 deletions(-) diff --git a/x/bank/README.md b/x/bank/README.md index 9c884b5d32bd..42a194bdc942 100644 --- a/x/bank/README.md +++ b/x/bank/README.md @@ -238,6 +238,7 @@ type SendKeeper interface { AppendSendRestriction(restriction SendRestrictionFn) PrependSendRestriction(restriction SendRestrictionFn) + ClearSendRestriction() InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error diff --git a/x/bank/keeper/export_test.go b/x/bank/keeper/export_test.go index bf358135df1c..91960e9a8a82 100644 --- a/x/bank/keeper/export_test.go +++ b/x/bank/keeper/export_test.go @@ -6,9 +6,9 @@ import "github.com/cosmos/cosmos-sdk/x/bank/types" // for the purpose of testing in the keeper_test package. func (k BaseSendKeeper) SetSendRestriction(restriction types.SendRestrictionFn) { - k.sendRestriction.Fn = restriction + k.sendRestriction.fn = restriction } func (k BaseSendKeeper) GetSendRestrictionFn() types.SendRestrictionFn { - return k.sendRestriction.Fn + return k.sendRestriction.fn } diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 2adf18303671..2bb4f674b28d 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -22,6 +22,7 @@ type SendKeeper interface { AppendSendRestriction(restriction types.SendRestrictionFn) PrependSendRestriction(restriction types.SendRestrictionFn) + ClearSendRestriction() InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error SendCoins(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error @@ -64,7 +65,7 @@ type BaseSendKeeper struct { // should be the x/gov module account. authority string - sendRestriction *types.SendRestriction + sendRestriction *sendRestriction } func NewBaseSendKeeper( @@ -85,18 +86,23 @@ func NewBaseSendKeeper( storeKey: storeKey, blockedAddrs: blockedAddrs, authority: authority, - sendRestriction: types.NewSendRestriction(), + sendRestriction: newSendRestriction(), } } // AppendSendRestriction adds the provided SendRestrictionFn to run after previously provided restrictions. func (k BaseSendKeeper) AppendSendRestriction(restriction types.SendRestrictionFn) { - k.sendRestriction.Append(restriction) + k.sendRestriction.append(restriction) } // PrependSendRestriction adds the provided SendRestrictionFn to run before previously provided restrictions. func (k BaseSendKeeper) PrependSendRestriction(restriction types.SendRestrictionFn) { - k.sendRestriction.Prepend(restriction) + k.sendRestriction.prepend(restriction) +} + +// ClearSendRestriction removes the send restriction (if there is one). +func (k BaseSendKeeper) ClearSendRestriction() { + k.sendRestriction.clear() } // GetAuthority returns the x/bank module's authority. @@ -159,11 +165,9 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, out return err } - if k.sendRestriction.Fn != nil { - outAddress, err = k.sendRestriction.Fn(ctx, inAddress, outAddress, out.Coins) - if err != nil { - return err - } + outAddress, err = k.sendRestriction.apply(ctx, inAddress, outAddress, out.Coins) + if err != nil { + return err } if err := k.addCoins(ctx, outAddress, out.Coins); err != nil { @@ -195,15 +199,13 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, out // SendCoins transfers amt coins from a sending account to a receiving account. // An error is returned upon failure. func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error { - if k.sendRestriction.Fn != nil { - var err error - toAddr, err = k.sendRestriction.Fn(ctx, fromAddr, toAddr, amt) - if err != nil { - return err - } + var err error + toAddr, err = k.sendRestriction.apply(ctx, fromAddr, toAddr, amt) + if err != nil { + return err } - err := k.subUnlockedCoins(ctx, fromAddr, amt) + err = k.subUnlockedCoins(ctx, fromAddr, amt) if err != nil { return err } @@ -447,3 +449,41 @@ func (k BaseSendKeeper) getSendEnabledOrDefault(ctx sdk.Context, denom string, d return defaultVal } + +// sendRestriction is a struct that houses a SendRestrictionFn. +// It exists so that the SendRestrictionFn can be updated in the SendKeeper without needing to have a pointer receiver. +type sendRestriction struct { + fn types.SendRestrictionFn +} + +// newSendRestriction creates a new sendRestriction with nil send restriction. +func newSendRestriction() *sendRestriction { + return &sendRestriction{ + fn: nil, + } +} + +// append adds the provided restriction to this, to be run after the existing function. +func (r *sendRestriction) append(restriction types.SendRestrictionFn) { + r.fn = r.fn.Then(restriction) +} + +// prepend adds the provided restriction to this, to be run before the existing function. +func (r *sendRestriction) prepend(restriction types.SendRestrictionFn) { + r.fn = restriction.Then(r.fn) +} + +// clear removes the send restriction (sets it to nil). +func (r *sendRestriction) clear() { + r.fn = nil +} + +var _ types.SendRestrictionFn = sendRestriction{}.apply + +// apply applies the send restriction if there is one. If not, it's a no-op. +func (r sendRestriction) apply(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + if r.fn == nil { + return toAddr, nil + } + return r.fn(ctx, fromAddr, toAddr, amt) +} diff --git a/x/bank/types/restrictions.go b/x/bank/types/restrictions.go index 3690eb9d67f0..7a224a334ead 100644 --- a/x/bank/types/restrictions.go +++ b/x/bank/types/restrictions.go @@ -93,26 +93,3 @@ func ComposeSendRestrictions(restrictions ...SendRestrictionFn) SendRestrictionF return toAddr, err } } - -// SendRestriction is a struct that houses a SendRestrictionFn. -// It exists so that the SendRestrictionFn can be updated in the SendKeeper without needing to have a pointer receiver. -type SendRestriction struct { - Fn SendRestrictionFn -} - -// NewSendRestriction creates a new SendRestriction with nil send restriction. -func NewSendRestriction() *SendRestriction { - return &SendRestriction{ - Fn: nil, - } -} - -// Append adds the provided restriction to this, to be run after the existing function. -func (r *SendRestriction) Append(restriction SendRestrictionFn) { - r.Fn = r.Fn.Then(restriction) -} - -// Prepend adds the provided restriction to this, to be run before the existing function. -func (r *SendRestriction) Prepend(restriction SendRestrictionFn) { - r.Fn = restriction.Then(r.Fn) -} From 0000a6e983a1c32d660db72243aebed79bf1626c Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 19 Apr 2023 17:09:05 -0600 Subject: [PATCH 22/27] [14124]: Regen mocks to get the ClearSendRestriction. --- x/gov/testutil/expected_keepers_mocks.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/x/gov/testutil/expected_keepers_mocks.go b/x/gov/testutil/expected_keepers_mocks.go index 3bb59003cfb7..6a18f680673a 100644 --- a/x/gov/testutil/expected_keepers_mocks.go +++ b/x/gov/testutil/expected_keepers_mocks.go @@ -229,6 +229,18 @@ func (mr *MockBankKeeperMockRecorder) BurnCoins(ctx, moduleName, amt interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BurnCoins", reflect.TypeOf((*MockBankKeeper)(nil).BurnCoins), ctx, moduleName, amt) } +// ClearSendRestriction mocks base method. +func (m *MockBankKeeper) ClearSendRestriction() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "ClearSendRestriction") +} + +// ClearSendRestriction indicates an expected call of ClearSendRestriction. +func (mr *MockBankKeeperMockRecorder) ClearSendRestriction() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClearSendRestriction", reflect.TypeOf((*MockBankKeeper)(nil).ClearSendRestriction)) +} + // DelegateCoins mocks base method. func (m *MockBankKeeper) DelegateCoins(ctx types.Context, delegatorAddr, moduleAccAddr types.AccAddress, amt types.Coins) error { m.ctrl.T.Helper() From 2f61c504b985e60131eaac6ab93be84419937fed Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Wed, 19 Apr 2023 17:27:48 -0600 Subject: [PATCH 23/27] Fix broken reference to moved MintingRestrictionFn definition. --- tests/integration/bank/keeper/keeper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/bank/keeper/keeper_test.go b/tests/integration/bank/keeper/keeper_test.go index c4edbf85a731..2f7917f25026 100644 --- a/tests/integration/bank/keeper/keeper_test.go +++ b/tests/integration/bank/keeper/keeper_test.go @@ -1372,7 +1372,7 @@ func TestMintCoinRestrictions(t *testing.T) { for _, test := range tests { f.bankKeeper = keeper.NewBaseKeeper(f.appCodec, f.fetchStoreKey(types.StoreKey), f.accountKeeper, nil, authtypes.NewModuleAddress(govtypes.ModuleName).String(), - ).WithMintCoinsRestriction(keeper.MintingRestrictionFn(test.restrictionFn)) + ).WithMintCoinsRestriction(types.MintingRestrictionFn(test.restrictionFn)) for _, testCase := range test.testCases { if testCase.expectPass { assert.NilError(t, From 1c009f5bbba40c329fee5ed66b9cc893b05d132c Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Mon, 5 Jun 2023 09:40:21 -0600 Subject: [PATCH 24/27] [14124]: Fix compilation issue after merge. --- x/bank/keeper/send.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 5c0d4f4f8d9f..43c0ea77cba1 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -165,8 +165,9 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input, ), ) + var outAddress sdk.AccAddress for _, out := range outputs { - outAddress, err := k.ak.AddressCodec().StringToBytes(out.Address) + outAddress, err = k.ak.AddressCodec().StringToBytes(out.Address) if err != nil { return err } @@ -492,11 +493,11 @@ func (r *sendRestriction) clear() { r.fn = nil } -var _ types.SendRestrictionFn = sendRestriction{}.apply +var _ types.SendRestrictionFn = (*sendRestriction)(nil).apply // apply applies the send restriction if there is one. If not, it's a no-op. -func (r sendRestriction) apply(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { - if r.fn == nil { +func (r *sendRestriction) apply(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + if r == nil || r.fn == nil { return toAddr, nil } return r.fn(ctx, fromAddr, toAddr, amt) From 0f36433c451625f019f7c51cf4fa5cdee0e9086f Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 29 Jun 2023 17:54:19 -0600 Subject: [PATCH 25/27] Move the send restriction application in SendCoins to after subUnlockedCoins so that it a) more closely refelects InputOutputCoins, and b) does the most common failure thing first. --- x/bank/keeper/send.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index a96ab9c378f9..074cb217b8a6 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -206,12 +206,12 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input, // An error is returned upon failure. func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error { var err error - toAddr, err = k.sendRestriction.apply(ctx, fromAddr, toAddr, amt) + err = k.subUnlockedCoins(ctx, fromAddr, amt) if err != nil { return err } - err = k.subUnlockedCoins(ctx, fromAddr, amt) + toAddr, err = k.sendRestriction.apply(ctx, fromAddr, toAddr, amt) if err != nil { return err } From acbc96c34c863c10ba84b4bd1b09c6f831077a0c Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 29 Jun 2023 17:54:46 -0600 Subject: [PATCH 26/27] Add some spec documentation about the send restrictions. --- x/bank/README.md | 80 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/x/bank/README.md b/x/bank/README.md index 62a93250062b..990634b8b3b3 100644 --- a/x/bank/README.md +++ b/x/bank/README.md @@ -260,6 +260,86 @@ type SendKeeper interface { } ``` +#### Send Restrictions + +The `SendKeeper` applies a `SendRestrictionFn` before each transfer of funds. + +```golang +// A SendRestrictionFn can restrict sends and/or provide a new receiver address. +type SendRestrictionFn func(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (newToAddr sdk.AccAddress, err error) +``` + +After the `SendKeeper` (or `BaseKeeper`) has been created, send restrictions can be added to it using the `AppendSendRestriction` or `PrependSendRestriction` functions. +Both functions compose the provided restriction with any previously provided restrictions. +`AppendSendRestriction` adds the provided restriction to be run after any previously provided send restrictions. +`PrependSendRestriction` adds the restriction to be run before any previously provided send restrictions. +The composition will short-circuit when an error is encountered. I.e. if the first one returns an error, the second is not run. + +During `SendCoins`, the send restriction is applied after coins are removed from the from address, but before adding them to the to address. +During `InputOutputCoins`, the send restriction is applied after the input coins are removed and once for each output before the funds are added. + +A send restriction function should make use of a custom value in the context to allow bypassing that specific restriction. + +For example, in your module's keeper package, you'd define the send restriction function: + +```golang +var _ banktypes.SendRestrictionFn = Keeper{}.SendRestrictionFn + +func (k Keeper) SendRestrictionFn(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.AccAddress, error) { + // Bypass if the context says to. + if mymodule.HasBypass(ctx) { + return toAddr, nil + } + + // Your custom send restriction logic goes here. + return nil, errors.New("not implemented") +} +``` + +The bank keeper should be provided to your keeper's constructor so the send restriction can be added to it: + +```golang +func NewKeeper(cdc codec.BinaryCodec, storeKey storetypes.StoreKey, bankKeeper mymodule.BankKeeper) Keeper { + rv := Keeper{/*...*/} + bankKeeper.AppendSendRestriction(rv.SendRestrictionFn) + return rv +} +``` + +Then, in the `mymodule` package, define the context helpers: + +```golang +const bypassKey = "bypass-mymodule-restriction" + +// WithBypass returns a new context that will cause the mymodule bank send restriction to be skipped. +func WithBypass(ctx context.Context) context.Context { + return sdk.UnwrapSDKContext(ctx).WithValue(bypassKey, true) +} + +// WithoutBypass returns a new context that will cause the mymodule bank send restriction to not be skipped. +func WithoutBypass(ctx context.Context) context.Context { + return sdk.UnwrapSDKContext(ctx).WithValue(bypassKey, false) +} + +// HasBypass checks the context to see if the mymodule bank send restriction should be skipped. +func HasBypass(ctx context.Context) bool { + bypassValue := ctx.Value(bypassKey) + if bypassValue == nil { + return false + } + bypass, isBool := bypassValue.(bool) + return isBool && bypass +} +``` + +Now, anywhere where you want to use `SendCoins` or `InputOutputCoins`, but you don't want your send restriction applied: + +```golang +func (k Keeper) DoThing(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error { + return k.bankKeeper.SendCoins(mymodule.WithBypass(ctx), fromAddr, toAddr, amt) +} +``` + ### ViewKeeper The view keeper provides read-only access to account balances. The view keeper does not have balance alteration functionality. All balance lookups are `O(1)`. From d4eca5db24dfb22444c2046175fe7ace26658153 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Fri, 30 Jun 2023 12:51:57 -0600 Subject: [PATCH 27/27] [14124]: Fix unit tests that broke when I changed the location of the restiction check in SendCoins. --- x/bank/keeper/keeper_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 853fcb8f8584..e2178ae4b2cc 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -1159,7 +1159,7 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { }, expErr: "test restriction error", expBals: expBals{ - from: sdk.NewCoins(newFooCoin(985), newBarCoin(473)), + from: sdk.NewCoins(newFooCoin(885), newBarCoin(273)), to1: sdk.NewCoins(newFooCoin(15)), to2: sdk.NewCoins(newBarCoin(27)), }, @@ -1173,7 +1173,9 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { actualRestrictionArgs = nil suite.bankKeeper.SetSendRestriction(tc.fn) ctx := suite.ctx - if len(tc.expErr) == 0 { + if len(tc.expErr) > 0 { + suite.authKeeper.EXPECT().GetAccount(ctx, fromAddr).Return(fromAcc) + } else { suite.mockSendCoins(ctx, fromAcc, tc.finalAddr) }