Add Payments to the Exchange module (#1868)

* [1703]: Update proto stuff to have all the paymen stuff and stuff.

* [1703]: Stub out all the msg server, query server, and msg methods.

* [1703]: Create the Payment.Validate() method and tweak ValidateExternalID to not contain the entire external id when it's too long.

* [1703]: Create Payment.String() so that things can compile

* [1703]: Fix unit tests that broke because I change the external id too-long error message.

* [1703]: Set some default payment fee param values and add validation checks on the new fields. Write the state store stuff for the new entries.

* [1703]: Add some payment fee consumer stuff.

* [1703]: Put the params key strings in some constants and fix a couple params-related unit tests that broke with recent changes.

* [1703]: Payment validation and hopefully an improvement on the Payment.String() method. Unit tests on those too.

* [1703]: New msg validation funcs and their unit tests.

* [1703]: Add some params test cases to cover new keeper stuff.

* [1703]: Create key stuff for payments.

* [1703]: Unit tests on the target to payment index key creators.

* [1703]: Rename the target to payments index key prefix makers to include the word 'Key'.

* [1703]: Write most of the payment stuff in the keeper and fill out the msg server endpoint methods.

* [1703]: Fill in the query endpoint methods.

* [1703]: Add payments to the geneis doc.

* [1703]: Add validation to genesis payments. Add payments to genesis import and export. Unit test on that stuff too.

* [1703]: Add some payment-related events to the protos.

* [1703]: Constructors for the new events and emit them when doing keeper stuff.

* [1703]: Unit tests on the new event constructors.

* [1703]: Unit tests on the GetPayment and CreatePayment keeper methods.

* [1703]: Fix AcceptPayment to correctly release the hold on the payment before trying to move funds.

* [1703]: Update RejectPayments to check for at least one source.

* [1703]: AcceptPayment, tweak the errors a tiny bit and use the existing record for the sends and stuff. In RejectPayment, require a target and source, and make sure the existing target isn't empty.

* [1703]: Unit tests on AcceptPayment and RejectPayment.

* [1703]: In RejectPayment: return an error if the target is empty and skip duplicated source entries. In CancelPayments: return an error if the source is empty and skip duplicated external ids.

* [1703]: Unit tests on RejectPayments and CancelPayments.

* [1703]: Update CancelPayments to use requirePaymentFromStore and improve the same-target error when it's empty.

* [1703]: Unit tests on UpdatePaymentTarget. In all the payment keeper tests, check that the payments and index entries match.

* [1703]: Maintain the ordering of the fee entries in params when they're being read out of state.

* [1703]: Unit test on the GetPaymentsForTargetAndSource, IteratePayments, and CalculatePaymentFees keeper funcs.

* [1703]: Consume the commitment settlment fee only if the settlement succeeds.

* [1703]: In RejectPayments, include the sources index in the error message. In ChangePaymentTarget, allow the new target to be empty.

* [1703]: Unit tests on the msg server endpoints.

* [1703]: In the GetPayment query endpoint, ignore any error that comes back from GetPayment and treat it like that payment doesn't exist.

* [1703]: Unit tests on the new query endpoints.

* [1703]: Add TODOs for the CLI stuff and spec stuff.

* [1703]: For the genaccounts tests, fix empty stuff related to payments.

* [1703]: Fix a couple CLI unit tests that were failing because of the added fields in Params.

* [1703]: Add a bunch of TODOs for the CLI stuff.

* [1703]: Create the payment tx CLI Stuff.

* [1703]: Add payment queries to CLI.

* [1703]: Unit tests on the new flags stuff.

* [1703]: Make it possible to include an empty string in the external ids of the CancelPayments command. Require a source in RejectPayment. Require either from or source with ChangePaymentTarget. Unit tests on all the tx setup funcs.

* [1703]: In the setup tests, ensure that each of the expected flags also appears in the command's Use.

* [1703]: Make a reusalbe use string for indicating there are pagination flags. Make the external id optional in MakeQueryGetPayment. Unit tests on the new query CLI setup stuff.

* [1703]: Update the cli test suite setup to include a bunch of payments.

* [1703]: Make the GetPayment query return an error if the payment isn't found.

* [1703]: Unit tests on the CreatePayment command.

* [1703]: Unit tests on the rest of the tx commands.

* [1703]: Add a 'no results' test case to the GetPaymentsWithSource and GetPaymentsWithTarget keeper unit tests.

* [1703]: Unit tests on the query commands.

* [1703]: Fix the names of the payment keeper unit tests.

* [1703]: Add payments to state, messages, events, queries, and params spec docs.

* [1703]: Update the line numbers in all the proto links in the spec docs.

* [1703]: Standardize the empty lines before headers in the concepts spec doc.

* [1703]: Add payments to the concepts spec doc.

* [1703]: Create the tourmaline-rc3 upgrade to set the new exchange params relating to payments.

* [1703]: Add changelog entries.

* [1703]: Add some missing function comments in the cli test helpers.

* [1703]: Fix typo in the CmdQueryGetPayment Short.

* [1703]: Tweak the new payments concepts spec docs to clarify some stuff.

* [1703]: Remove line that was accidentally added to runMsgServerTestCase, but didn't do anything.

* [1703]: Add an expected log entry to TestTourmaline indicating that the payment params are being set.

* [1703]: Add comment to parsePaymentStoreValue indicating that nil, nil is returned when the value is empty (or nil).

* [1703]: Remove the AddressString decoration from a couple external id proto fields.

* [1703]: Change default exchange payment params to 10hash to create and 8hash to accept.

* [1703]: Add some logging to the queries to help identify issues when there are any.

---------

Co-authored-by: Ira Miller <[email protected]>

CHANGELOG.md

@@ -37,7 +37,18 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
-* nothing
+### Features
+
+* Add [Payments](x/exchange/spec/01_concepts.md#payments) to the exchange module [#1703](https://github.com/provenance-io/provenance/issues/1703).
+  Payments allow two parties to trade assets securely and asynchronously.
+
+### Improvements
+
+* Add a `tourmaline-rc3` upgrade handler to set some new exchange module params related to payments [#1703](https://github.com/provenance-io/provenance/issues/1703).
+
+### Bug Fixes
+
+* In `MarketCommitmentSettle`, only consume the settlement fee if the settlement succeeds [#1703](https://github.com/provenance-io/provenance/issues/1703).
 
 ---
 

app/test_helpers.go

@@ -394,6 +394,23 @@ func createIncrementalAccounts(accNum int) []sdk.AccAddress {
 	return addresses
 }
 
+// createIncrementalAccountsLong is a strategy used by addTestAddrs() in order to generate 32-byte addresses in ascending order.
+func createIncrementalAccountsLong(accNum int) []sdk.AccAddress {
+	var addresses []sdk.AccAddress
+
+	// There's nothing special about this base other than it's 30 bytes long (60 hex chars => 30 bytes).
+	// That leaves 2 bytes for the incrementing number = 65536 addrs max.
+	// It's the result of two calls to uuidgen with the last 4 chars removed.
+	base := "9B4006D1F9794F07BEC52279C3C31480CCC9A1EB3FD64F628CC405E4E2E2"
+	for i := 0; i < accNum; i++ {
+		addrHex := fmt.Sprintf("%s%04X", base, i)
+		addr, _ := sdk.AccAddressFromHexUnsafe(addrHex)
+		addresses = append(addresses, addr)
+	}
+
+	return addresses
+}
+
 // AddTestAddrsFromPubKeys adds the addresses into the App providing only the public keys.
 func AddTestAddrsFromPubKeys(app *App, ctx sdk.Context, pubKeys []cryptotypes.PubKey, accAmt sdkmath.Int) {
 	initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))
@@ -403,18 +420,21 @@ func AddTestAddrsFromPubKeys(app *App, ctx sdk.Context, pubKeys []cryptotypes.Pu
 	}
 }
 
-// AddTestAddrs constructs and returns accNum amount of accounts with an
-// initial balance of accAmt in random order
+// AddTestAddrs constructs and returns accNum amount of accounts with an initial balance of accAmt in random order.
 func AddTestAddrs(app *App, ctx sdk.Context, accNum int, accAmt sdkmath.Int) []sdk.AccAddress {
 	return addTestAddrs(app, ctx, accNum, accAmt, createRandomAccounts)
 }
 
-// AddTestAddrsIncremental constructs and returns accNum amount of accounts with an
-// initial balance of accAmt in random order
+// AddTestAddrsIncremental creates accNum accounts with 20-byte incrementing addresses and initialBondAmount of bond denom.
 func AddTestAddrsIncremental(app *App, ctx sdk.Context, accNum int, accAmt sdkmath.Int) []sdk.AccAddress {
 	return addTestAddrs(app, ctx, accNum, accAmt, createIncrementalAccounts)
 }
 
+// AddTestAddrsIncrementalLong creates accNum accounts with 32-byte incrementing addresses and initialBondAmount of bond denom.
+func AddTestAddrsIncrementalLong(app *App, ctx sdk.Context, accNum int, initialBondAmount sdkmath.Int) []sdk.AccAddress {
+	return addTestAddrs(app, ctx, accNum, initialBondAmount, createIncrementalAccountsLong)
+}
+
 func addTestAddrs(app *App, ctx sdk.Context, accNum int, accAmt sdkmath.Int, strategy GenerateAccountStrategy) []sdk.AccAddress {
 	testAddrs := strategy(accNum)
 

app/upgrades.go

@@ -133,6 +133,12 @@ var upgrades = map[string]appUpgrade{
 		},
 	},
 	"tourmaline-rc2": {}, // upgrade for v1.18.0-rc2
+	"tourmaline-rc3": { // upgrade for v1.18.0-rc3
+		Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) {
+			setExchangePaymentParamsToDefaults(ctx, app)
+			return vm, nil
+		},
+	},
 	"tourmaline": { // upgrade for v1.18.0
 		Added: []string{ibcratelimit.ModuleName},
 		Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) {
@@ -148,6 +154,8 @@ var upgrades = map[string]appUpgrade{
 			// This isn't in an rc because it was handled via gov prop for testnet.
 			updateMsgFeesNhashPerMil(ctx, app)
 
+			setExchangePaymentParamsToDefaults(ctx, app)
+
 			return vm, nil
 		},
 	},
@@ -357,6 +365,7 @@ func updateIbcMarkerDenomMetadata(ctx sdk.Context, app *App) {
 }
 
 // convertNavUnits iterates all the net asset values and updates their units if they are using usd.
+// TODO: Remove with the tourmaline handlers.
 func convertNavUnits(ctx sdk.Context, app *App) {
 	ctx.Logger().Info("Converting NAV units.")
 	err := app.MarkerKeeper.IterateAllNetAssetValues(ctx, func(markerAddr sdk.AccAddress, nav markertypes.NetAssetValue) (stop bool) {
@@ -382,6 +391,7 @@ func convertNavUnits(ctx sdk.Context, app *App) {
 }
 
 // updateMsgFeesNhashPerMil updates the MsgFees Params to set the NhashPerUsdMil to 40,000,000.
+// TODO: Remove with the tourmaline handlers.
 func updateMsgFeesNhashPerMil(ctx sdk.Context, app *App) {
 	var newVal uint64 = 40_000_000
 	ctx.Logger().Info(fmt.Sprintf("Setting MsgFees Params NhashPerUsdMil to %d.", newVal))
@@ -390,3 +400,19 @@ func updateMsgFeesNhashPerMil(ctx sdk.Context, app *App) {
 	app.MsgFeesKeeper.SetParams(ctx, params)
 	ctx.Logger().Info("Done setting MsgFees Params NhashPerUsdMil.")
 }
+
+// setExchangePaymentParamsToDefaults updates the exchange module params to have the default create payment and accept payment values.
+// TODO: Remove with the tourmaline handlers.
+func setExchangePaymentParamsToDefaults(ctx sdk.Context, app *App) {
+	ctx.Logger().Info("Setting exchange module payment params to defaults.")
+	defaultParams := exchange.DefaultParams()
+	curParams := app.ExchangeKeeper.GetParams(ctx)
+	if curParams == nil {
+		curParams = defaultParams
+	} else {
+		curParams.FeeCreatePaymentFlat = defaultParams.FeeCreatePaymentFlat
+		curParams.FeeAcceptPaymentFlat = defaultParams.FeeAcceptPaymentFlat
+	}
+	app.ExchangeKeeper.SetParams(ctx, curParams)
+	ctx.Logger().Info("Done setting exchange module payment params to defaults.")
+}

app/upgrades_test.go

@@ -442,12 +442,22 @@ func (s *UpgradeTestSuite) TestTourmalineRC2() {
 	s.Require().Empty(upgrades[key], "upgrades[%q]", key)
 }
 
+func (s *UpgradeTestSuite) TestTourmalineRC3() {
+	expInLog := []string{
+		"INF Setting exchange module payment params to defaults.",
+		"INF Done setting exchange module payment params to defaults.",
+	}
+
+	s.AssertUpgradeHandlerLogs("tourmaline-rc3", expInLog, nil)
+}
+
 func (s *UpgradeTestSuite) TestTourmaline() {
 	expInLog := []string{
 		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
 		"INF Removing all delegations from validators that have been inactive (unbonded) for 21 days.",
 		"INF Converting NAV units.",
 		"INF Setting MsgFees Params NhashPerUsdMil to 40000000.",
+		"INF Setting exchange module payment params to defaults.",
 	}
 
 	s.AssertUpgradeHandlerLogs("tourmaline", expInLog, nil)
@@ -974,3 +984,62 @@ func (s *UpgradeTestSuite) TestUpdateMsgFeesNhashPerMil() {
 		})
 	}
 }
+
+func (s *UpgradeTestSuite) TestSetExchangePaymentParamsToDefaults() {
+	origParams := s.app.ExchangeKeeper.GetParams(s.ctx)
+	defer s.app.ExchangeKeeper.SetParams(s.ctx, origParams)
+
+	defaultParams := exchange.DefaultParams()
+
+	tests := []struct {
+		name           string
+		existingParams *exchange.Params
+		expectedParams *exchange.Params
+	}{
+		{
+			name:           "no params set yet",
+			existingParams: nil,
+			expectedParams: defaultParams,
+		},
+		{
+			name: "params set previously",
+			existingParams: &exchange.Params{
+				DefaultSplit:         333,
+				DenomSplits:          []exchange.DenomSplit{{Denom: "date", Split: 99}},
+				FeeCreatePaymentFlat: nil,
+				FeeAcceptPaymentFlat: nil,
+			},
+			expectedParams: &exchange.Params{
+				DefaultSplit:         333,
+				DenomSplits:          []exchange.DenomSplit{{Denom: "date", Split: 99}},
+				FeeCreatePaymentFlat: defaultParams.FeeCreatePaymentFlat,
+				FeeAcceptPaymentFlat: defaultParams.FeeAcceptPaymentFlat,
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		s.Run(tc.name, func() {
+			s.app.ExchangeKeeper.SetParams(s.ctx, tc.existingParams)
+			expInLog := []string{
+				"INF Setting exchange module payment params to defaults.",
+				"INF Done setting exchange module payment params to defaults.",
+			}
+
+			// Reset the log buffer and call the func. Relog the output if it panics.
+			s.logBuffer.Reset()
+			testFunc := func() {
+				setExchangePaymentParamsToDefaults(s.ctx, s.app)
+			}
+			didNotPanic := s.Assert().NotPanics(testFunc, "setExchangePaymentParamsToDefaults")
+			logOutput := s.GetLogOutput("setExchangePaymentParamsToDefaults")
+			if !didNotPanic {
+				return
+			}
+			s.AssertLogContents(logOutput, expInLog, nil, true, "setExchangePaymentParamsToDefaults")
+
+			actParams := s.app.ExchangeKeeper.GetParams(s.ctx)
+			s.Assert().Equal(tc.expectedParams, actParams, "Exchange Params after setExchangePaymentParamsToDefaults")
+		})
+	}
+}