Allow transfer agents to do exchange settlements with restricted coin…

…s. (#1835)

* [1834]: Add tourmaline-rc2 upgrade handler since I'm about to make a state-breaking change that'll require an upgrade.

* [1834]: Add context functions for passing in the transfer agent.

* [1834]: Pay attention to the newly available transfer agent in the send restriction. Move the deposit check to SendRestrictionFn (from validateSendDenom) since don't need to repeat that check for every denom. Add a check for withdraw on the from addr since that might now be a marker account. Tweak a couple error messages to distinguish them from eachother. In the transfer endpoint, if the destination is also a marker, check for deposit access on that marker.

* [1834]: Switch the TestAccountKeeperMintBurnCoins fix to provide the transfer agent instead of just bypassing the send restrictions.

* [1834]: If no force transfer, and coming from a marker, make sure the admin has withdraw on that marker. Unit tests on the TransferCoin function.

* [1834]: Unit tests on the send restriction function.

* [1834]: Set the transfer agent in the exchange stuff that does bank stuff.

* [1834]: Fix the unit tests that broke and add a couple new ones.

* [1834]: tiny tweak to a couple unit tests.

* [1834]: In AddSetNetAssetValues, emit the nav event even when the price denom's marker isn't found.

* [1834]: Tweak TestKeeper_WithdrawMarketFunds again to make it a little easier on test setup.

* [1834]: lint (kind of since it's in a test file that isn't linted, but whatever).

* [1834]: Add access validator functions to the marker to standardize the errors when an address doesn't have a certain role.

* [1834]: Remove redundant 'access' from error about not having access (the enum name starts with ACCESS_).

* [1834]: Standardize the no-access errors. In WithdrawCoins, if going to a marker, make sure they've got deposit on that marker. Require a marker to be active in ordr to send its funds.

* [1834]: Update TransferCoin. Add comment above authzHandler with the reason we don't check for withdraw and remove the check for withdraw.

* [1834]: Fix the error in validateSendDenom for sends to a marker to reference the correct denom and address.

* [1834]: Fix the tests that broke because I changed the error messages.

* [1834]: Update AddSetNetAssetValues. Try all entries even if an earlier one has an error. If the price marker does not exist, only emit the event if the nav is valid. Put some unit tests on that thing.

* [1834]: Add unit test on send restriction when we can't get attributes.

* [1834]: Fix some comments in the mocks, add a unit test making sure force-transfer access is ignored in the send restriction, and add force transfer to a couple access unit tests.

* [1834]: Update marker spec docs with all the recent chagnes.

* [1834]: For the exchange stuff, pay attention to blocked addresses in the off-chance one gets used.

* [1834]: Check that a marker is active before allowing a transfer.

* [1834]: In WithdrawCoins, make sure the recipient is not a bank blocked address.

* [1834]: Some tweaks to the spec docs to fix some typos, grammer, and flowchart stuff.

* [1834]: Redo the flows so that denied is on the left for all of them. Add some links between flows.

* [1834]: Fix the names of a bunch of keeper unit tests that claim to be testing account stuff.

* [1834]: Unit tests on WithdrawCoins and on Transfer of non-active coins.

* [1834]: Fix unit tests on WithdrawMarketFunds that broke because a call to BlockedAddr is now being made that wasn't expected previously. Add test for when the address is blocked.

* [1834]: Fix TestKeeper_SettleCommitments that broke because of the added calls to BlockedAddr that weren't previously expected.

* [1834]: Fix TestKeeper_DoTransfer that broke because we added calls to BlockedAddr that weren't listed as expected. Add a test for when an address is blocked.

* [1834]: Fix TestKeeper_SettleOrders that broke when I added calls to BlockedAddr.

* [1834]: Fix the FillAsks and FillBids unit tests that broke when I started calling BlockedAddr.

* [1834]: Remove TODO that's TODONE (unit tests on blocked addresses).

* [1834]: Update the exchange spec docs to include stuff about the transfer agent as well as a commitment settlement fee calc example.

* [1834]: Add changelog entries.

CHANGELOG.md

@@ -37,12 +37,24 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### Features
+
+* In the marker module's `SendRestrictionFn`, allow a transfer agent to be identified through the context [#1834](https://github.com/provenance-io/provenance/issues/1834).
+* In the exchange module, provide the admin as the transfer agent when attepting to move funds [#1834](https://github.com/provenance-io/provenance/issues/1834).
+
 ### Improvements
 
+* Add an empty `tourmaline-rc2` upgrade handler [#1834](https://github.com/provenance-io/provenance/issues/1834).
 * Add new force_transfer access that is required for an account to do a forced transfer ([#1829](https://github.com/provenance-io/provenance/issues/1829)).
 * Add exchange commitment stuff to CLI [PR 1830](https://github.com/provenance-io/provenance/pull/1830).
 * Update the MsgFees Params to set the nhash per usd-mil to 40,000,000 ($0.025/hash) [#1833](https://github.com/provenance-io/provenance/pull/1833).
 * Bid order prices are no longer restricted to amounts that can be evenly applied to a buyer settlement fee ratio [1834](https://github.com/provenance-io/provenance/pull/1843).
+* In the marker and exchange modules, help ensure funds don't get sent to blocked addresses [#1834](https://github.com/provenance-io/provenance/issues/1834).
+* Update marker and exchange spec docs to include info about transfer agents [#1834](https://github.com/provenance-io/provenance/issues/1834).
+
+### Bug Fixes
+
+* Prevent funds from going to or from a marker without the transfer agent having deposit or withdraw access (respectively) [#1834](https://github.com/provenance-io/provenance/issues/1834).
 
 ### API Breaking
 

app/upgrades.go

@@ -132,6 +132,7 @@ var upgrades = map[string]appUpgrade{
 			return vm, nil
 		},
 	},
+	"tourmaline-rc2": {}, // upgrade for v1.18.0-rc2
 	"tourmaline": { // upgrade for v1.18.0
 		Added: []string{ibcratelimit.ModuleName},
 		Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) {

app/upgrades_test.go

@@ -77,7 +77,7 @@ func (s *UpgradeTestSuite) LogIfError(err error, format string, args ...interfac
 func (s *UpgradeTestSuite) AssertUpgradeHandlerLogs(key string, expInLog, expNotInLog []string) (string, bool) {
 	s.T().Helper()
 
-	if !s.Assert().Contains(upgrades, key, "defined upgrades map") {
+	if !s.Assert().Contains(upgrades, key, "%q defined upgrades map", key) {
 		return "", false // If the upgrades map doesn't have that key, there's nothing more to do in here.
 	}
 	handler := upgrades[key].Handler
@@ -436,6 +436,12 @@ func (s *UpgradeTestSuite) TestTourmalineRC1() {
 	s.AssertUpgradeHandlerLogs("tourmaline-rc1", expInLog, expNotInLog)
 }
 
+func (s *UpgradeTestSuite) TestTourmalineRC2() {
+	key := "tourmaline-rc2"
+	s.Require().Contains(upgrades, key, "%q defined upgrades map", key)
+	s.Require().Empty(upgrades[key], "upgrades[%q]", key)
+}
+
 func (s *UpgradeTestSuite) TestTourmaline() {
 	expInLog := []string{
 		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",

internal/handlers/bank_send_restriction_test.go

@@ -1,6 +1,7 @@
 package handlers_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -163,7 +164,9 @@ func TestBankSend(tt *testing.T) {
 
 	// On a restricted coin with required attributes using an admin that does not have TRANSFER permission, but the receiver DOES have the required attributes.
 	tranferRAMarker := markertypes.NewMsgTransferRequest(addr2, addr2, addr3, sdk.NewInt64Coin(restrictedAttrMarkerDenom, 25))
-	ConstructAndSendTx(tt, *app, ctx, acct2, priv2, tranferRAMarker, txFailureCode, addr2.String()+" is not allowed to broker transfers")
+	expErr := fmt.Sprintf("%s does not have ACCESS_TRANSFER on restrictedmarkerattr marker (%s)",
+		addr2.String(), raMarkerAcct.GetAddress().String())
+	ConstructAndSendTx(tt, *app, ctx, acct2, priv2, tranferRAMarker, txFailureCode, expErr)
 	addr2afterBalance = app.BankKeeper.GetAllBalances(ctx, addr2).String()
 	assert.Equal(tt, "50nonrestrictedmarker,125restrictedmarker,75restrictedmarkerattr,999400000stake", addr2afterBalance, "addr1afterBalance")
 	addr2afterBalance = app.BankKeeper.GetAllBalances(ctx, addr3).String()

x/exchange/expected_keepers.go

@@ -24,6 +24,7 @@ type BankKeeper interface {
 	SendCoins(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error
 	SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
 	InputOutputCoins(ctx sdk.Context, inputs []banktypes.Input, outputs []banktypes.Output) error
+	BlockedAddr(addr sdk.AccAddress) bool
 }
 
 type HoldKeeper interface {

x/exchange/keeper/commitments.go

@@ -11,6 +11,7 @@ import (
 	"github.com/provenance-io/provenance/internal/antewrapper"
 	"github.com/provenance-io/provenance/internal/pioconfig"
 	"github.com/provenance-io/provenance/x/exchange"
+	markertypes "github.com/provenance-io/provenance/x/marker/types"
 )
 
 // getCommitmentAmount gets the amount that the given address has committed to the provided market.
@@ -367,15 +368,19 @@ func (k Keeper) CalculateCommitmentSettlementFee(ctx sdk.Context, req *exchange.
 
 // SettleCommitments orchestrates the transfer of committed funds and collection of fees by the market.
 func (k Keeper) SettleCommitments(ctx sdk.Context, req *exchange.MsgMarketCommitmentSettleRequest) error {
-	marketID := req.MarketId
+	admin, adminErr := sdk.AccAddressFromBech32(req.Admin)
+	if adminErr != nil {
+		return fmt.Errorf("invalid admin %q: %w", req.Admin, adminErr)
+	}
+
 	// Record all the navs.
-	k.recordNAVs(ctx, marketID, req.Navs)
+	k.recordNAVs(ctx, req.MarketId, req.Navs)
 
 	// Build the transfers
 	inputs := exchange.SimplifyAccountAmounts(req.Inputs)
 	outputs := exchange.SimplifyAccountAmounts(req.Outputs)
 	fees := exchange.SimplifyAccountAmounts(req.Fees)
-	transfers, err := exchange.BuildCommitmentTransfers(marketID, inputs, outputs, fees)
+	transfers, err := exchange.BuildCommitmentTransfers(req.MarketId, inputs, outputs, fees)
 	if err != nil {
 		return fmt.Errorf("failed to build transfers: %w", err)
 	}
@@ -384,15 +389,16 @@ func (k Keeper) SettleCommitments(ctx sdk.Context, req *exchange.MsgMarketCommit
 	inputsAndFees := make([]exchange.AccountAmount, 0, len(inputs)+len(fees))
 	inputsAndFees = append(inputsAndFees, inputs...)
 	inputsAndFees = append(inputsAndFees, fees...)
-	err = k.ReleaseCommitments(ctx, marketID, exchange.SimplifyAccountAmounts(inputsAndFees), req.EventTag)
+	err = k.ReleaseCommitments(ctx, req.MarketId, exchange.SimplifyAccountAmounts(inputsAndFees), req.EventTag)
 	if err != nil {
 		return fmt.Errorf("failed to release commitments on inputs and fees: %w", err)
 	}
 
 	// Do the transfers
+	xFerCtx := markertypes.WithTransferAgent(ctx, admin)
 	var xferErrs []error
 	for _, transfer := range transfers {
-		err = k.DoTransfer(ctx, transfer.Inputs, transfer.Outputs)
+		err = k.DoTransfer(xFerCtx, transfer.Inputs, transfer.Outputs)
 		if err != nil {
 			xferErrs = append(xferErrs, err)
 		}
@@ -402,7 +408,7 @@ func (k Keeper) SettleCommitments(ctx sdk.Context, req *exchange.MsgMarketCommit
 	}
 
 	// Commit the funds in the outputs.
-	err = k.addCommitmentsUnsafe(ctx, marketID, outputs, req.EventTag)
+	err = k.addCommitmentsUnsafe(ctx, req.MarketId, outputs, req.EventTag)
 	if err != nil {
 		return fmt.Errorf("failed to re-commit funds after transfer: %w", err)
 	}

x/exchange/keeper/commitments_test.go

@@ -1900,6 +1900,7 @@ func (s *TestSuite) TestKeeper_SettleCommitments() {
 				AddHold:     []*AddHoldArgs{NewAddHoldArgs(s.addr2, s.coins("10apple"), holdReason(4))},
 			},
 			expBankCalls: BankCalls{
+				BlockedAddr: []sdk.AccAddress{s.addr2},
 				SendCoins: []*SendCoinsArgs{
 					{fromAddr: s.addr3, toAddr: s.addr2, amt: s.coins("10apple")},
 				},
@@ -1958,6 +1959,7 @@ func (s *TestSuite) TestKeeper_SettleCommitments() {
 				AddHold:     []*AddHoldArgs{NewAddHoldArgs(s.addr5, s.coins("10apple,10banana"), holdReason(4))},
 			},
 			expBankCalls: BankCalls{
+				BlockedAddr: []sdk.AccAddress{s.addr5},
 				SendCoins: []*SendCoinsArgs{
 					{fromAddr: s.addr3, toAddr: s.addr5, amt: s.coins("10apple,10banana")},
 				},
@@ -2006,6 +2008,7 @@ func (s *TestSuite) TestKeeper_SettleCommitments() {
 				AddHold: []*AddHoldArgs{NewAddHoldArgs(s.addr2, s.coins("10apple"), holdReason(2))},
 			},
 			expBankCalls: BankCalls{
+				BlockedAddr: []sdk.AccAddress{s.addr2, s.marketAddr2},
 				SendCoins: []*SendCoinsArgs{
 					{fromAddr: s.addr4, toAddr: s.addr2, amt: s.coins("10apple")},
 				},
@@ -2088,6 +2091,7 @@ func (s *TestSuite) TestKeeper_SettleCommitments() {
 				},
 			},
 			expBankCalls: BankCalls{
+				BlockedAddr: []sdk.AccAddress{s.addr1, s.addr2, s.addr4, s.addr5, s.marketAddr2},
 				InputOutputCoins: []*InputOutputCoinsArgs{
 					{
 						inputs: []banktypes.Input{
@@ -2153,11 +2157,15 @@ func (s *TestSuite) TestKeeper_SettleCommitments() {
 				tc.holdKeeper = NewMockHoldKeeper()
 			}
 
+			admin, aErr := sdk.AccAddressFromBech32(tc.req.Admin)
+			s.Require().NoError(aErr, "AccAddressFromBech32(tc.req.Admin)")
 			for _, exp := range tc.expBankCalls.SendCoins {
 				exp.ctxHasQuarantineBypass = true
+				exp.ctxTransferAgent = admin
 			}
 			for _, exp := range tc.expBankCalls.InputOutputCoins {
 				exp.ctxHasQuarantineBypass = true
+				exp.ctxTransferAgent = admin
 			}
 
 			kpr := s.k.

x/exchange/keeper/fulfillment.go

@@ -222,35 +222,40 @@ func (k Keeper) FillAsks(ctx sdk.Context, msg *exchange.MsgFillAsksRequest) erro
 }
 
 // SettleOrders attempts to settle all the provided orders.
-func (k Keeper) SettleOrders(ctx sdk.Context, marketID uint32, askOrderIDs, bidOrderIds []uint64, expectPartial bool) error {
+func (k Keeper) SettleOrders(ctx sdk.Context, req *exchange.MsgMarketSettleRequest) error {
+	admin, adminErr := sdk.AccAddressFromBech32(req.Admin)
+	if adminErr != nil {
+		return fmt.Errorf("invalid admin %q: %w", req.Admin, adminErr)
+	}
+
 	store := k.getStore(ctx)
-	if err := validateMarketExists(store, marketID); err != nil {
+	if err := validateMarketExists(store, req.MarketId); err != nil {
 		return err
 	}
 
-	askOrders, aoerr := k.getAskOrders(store, marketID, askOrderIDs, "")
-	bidOrders, boerr := k.getBidOrders(store, marketID, bidOrderIds, "")
+	askOrders, aoerr := k.getAskOrders(store, req.MarketId, req.AskOrderIds, "")
+	bidOrders, boerr := k.getBidOrders(store, req.MarketId, req.BidOrderIds, "")
 	if aoerr != nil || boerr != nil {
 		return errors.Join(aoerr, boerr)
 	}
 
 	ratioGetter := func(denom string) (*exchange.FeeRatio, error) {
-		return getSellerSettlementRatio(store, marketID, denom)
+		return getSellerSettlementRatio(store, req.MarketId, denom)
 	}
 
 	settlement, err := exchange.BuildSettlement(askOrders, bidOrders, ratioGetter)
 	if err != nil {
 		return err
 	}
 
-	if !expectPartial && settlement.PartialOrderFilled != nil {
+	if !req.ExpectPartial && settlement.PartialOrderFilled != nil {
 		return fmt.Errorf("settlement resulted in unexpected partial order %d", settlement.PartialOrderFilled.GetOrderID())
 	}
-	if expectPartial && settlement.PartialOrderFilled == nil {
+	if req.ExpectPartial && settlement.PartialOrderFilled == nil {
 		return errors.New("settlement unexpectedly resulted in all orders fully filled")
 	}
 
-	return k.closeSettlement(ctx, store, marketID, settlement)
+	return k.closeSettlement(markertypes.WithTransferAgent(ctx, admin), store, req.MarketId, settlement)
 }
 
 // closeSettlement does all the processing needed to complete a settlement.