fix(x/precisebank): Ensure exact reserve balance on integer carry when minting (#1932)

Fix reserve minting an extra coin when the recipient module both carries fractional over to integer balance AND remainder is insufficient. Adjusts fractional carry to simply send from reserve, instead of doing an additional mint. Add invariant to ensure reserve matches exactly with fractional balances + remainder, failing on both insufficient and excess funds.

x/precisebank/keeper/invariants.go

@@ -14,6 +14,7 @@ func RegisterInvariants(
 	k Keeper,
 	bk types.BankKeeper,
 ) {
+	ir.RegisterRoute(types.ModuleName, "reserve-backs-fractions", ReserveBacksFractionsInvariant(k))
 	ir.RegisterRoute(types.ModuleName, "balance-remainder-total", BalancedFractionalTotalInvariant(k))
 	ir.RegisterRoute(types.ModuleName, "valid-fractional-balances", ValidFractionalAmountsInvariant(k))
 	ir.RegisterRoute(types.ModuleName, "valid-remainder-amount", ValidRemainderAmountInvariant(k))
@@ -23,7 +24,12 @@ func RegisterInvariants(
 // AllInvariants runs all invariants of the X/precisebank module.
 func AllInvariants(k Keeper) sdk.Invariant {
 	return func(ctx sdk.Context) (string, bool) {
-		res, stop := BalancedFractionalTotalInvariant(k)(ctx)
+		res, stop := ReserveBacksFractionsInvariant(k)(ctx)
+		if stop {
+			return res, stop
+		}
+
+		res, stop = BalancedFractionalTotalInvariant(k)(ctx)
 		if stop {
 			return res, stop
 		}
@@ -47,6 +53,47 @@ func AllInvariants(k Keeper) sdk.Invariant {
 	}
 }
 
+// ReserveBacksFractionsInvariant checks that the total amount of backing
+// coins in the reserve is equal to the total amount of fractional balances,
+// such that the backing is always available to redeem all fractional balances
+// and there are no extra coins in the reserve that are not backing any
+// fractional balances.
+func ReserveBacksFractionsInvariant(k Keeper) sdk.Invariant {
+	return func(ctx sdk.Context) (string, bool) {
+		var (
+			msg    string
+			broken bool
+		)
+
+		fractionalBalSum := k.GetTotalSumFractionalBalances(ctx)
+		remainderAmount := k.GetRemainderAmount(ctx)
+
+		// Get the total amount of backing coins in the reserve
+		moduleAddr := k.ak.GetModuleAddress(types.ModuleName)
+		reserveIntegerBalance := k.bk.GetBalance(ctx, moduleAddr, types.IntegerCoinDenom)
+		reserveExtendedBalance := reserveIntegerBalance.Amount.Mul(types.ConversionFactor())
+
+		// The total amount of backing coins in the reserve should be equal to
+		// fractional balances + remainder amount
+		totalRequiredBacking := fractionalBalSum.Add(remainderAmount)
+
+		broken = !reserveExtendedBalance.Equal(totalRequiredBacking)
+		msg = fmt.Sprintf(
+			"%s reserve balance %s mismatches %s (fractional balances %s + remainder %s)\n",
+			types.ExtendedCoinDenom,
+			reserveExtendedBalance,
+			totalRequiredBacking,
+			fractionalBalSum,
+			remainderAmount,
+		)
+
+		return sdk.FormatInvariant(
+			types.ModuleName, "module reserve backing total fractional balances",
+			msg,
+		), broken
+	}
+}
+
 // ValidFractionalAmountsInvariant checks that all individual fractional
 // balances are valid.
 func ValidFractionalAmountsInvariant(k Keeper) sdk.Invariant {

x/precisebank/keeper/invariants_integration_test.go

@@ -0,0 +1,129 @@
+package keeper_test
+
+import (
+	"testing"
+
+	sdkmath "cosmossdk.io/math"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/kava-labs/kava/x/precisebank/keeper"
+	"github.com/kava-labs/kava/x/precisebank/testutil"
+	"github.com/kava-labs/kava/x/precisebank/types"
+	"github.com/stretchr/testify/suite"
+)
+
+type invariantsIntegrationTestSuite struct {
+	testutil.Suite
+}
+
+func (suite *invariantsIntegrationTestSuite) SetupTest() {
+	suite.Suite.SetupTest()
+}
+
+func TestInvariantsIntegrationTest(t *testing.T) {
+	suite.Run(t, new(invariantsIntegrationTestSuite))
+}
+
+func (suite *invariantsIntegrationTestSuite) FundReserve(amt sdkmath.Int) {
+	coins := sdk.NewCoins(sdk.NewCoin(types.IntegerCoinDenom, amt))
+	err := suite.BankKeeper.MintCoins(suite.Ctx, types.ModuleName, coins)
+	suite.Require().NoError(err)
+}
+
+func (suite *invariantsIntegrationTestSuite) TestReserveBackingFractionalInvariant() {
+	tests := []struct {
+		name       string
+		setupFn    func(ctx sdk.Context, k keeper.Keeper)
+		wantBroken bool
+		wantMsg    string
+	}{
+		{
+			"valid - empty state",
+			func(_ sdk.Context, _ keeper.Keeper) {},
+			false,
+			"",
+		},
+		{
+			"valid - fractional balances, no remainder",
+			func(ctx sdk.Context, k keeper.Keeper) {
+				k.SetFractionalBalance(ctx, sdk.AccAddress{1}, types.ConversionFactor().QuoRaw(2))
+				k.SetFractionalBalance(ctx, sdk.AccAddress{2}, types.ConversionFactor().QuoRaw(2))
+				// 1 integer backs same amount fractional
+				suite.FundReserve(sdk.NewInt(1))
+			},
+			false,
+			"",
+		},
+		{
+			"valid - fractional balances, with remainder",
+			func(ctx sdk.Context, k keeper.Keeper) {
+				k.SetFractionalBalance(ctx, sdk.AccAddress{1}, types.ConversionFactor().QuoRaw(2))
+				k.SetRemainderAmount(ctx, types.ConversionFactor().QuoRaw(2))
+				// 1 integer backs same amount fractional including remainder
+				suite.FundReserve(sdk.NewInt(1))
+			},
+			false,
+			"",
+		},
+		{
+			"invalid - no fractional balances, non-zero remainder",
+			func(ctx sdk.Context, k keeper.Keeper) {
+				k.SetRemainderAmount(ctx, types.ConversionFactor().QuoRaw(2))
+			},
+			true,
+			"precisebank: module reserve backing total fractional balances invariant\nakava reserve balance 0 mismatches 500000000000 (fractional balances 0 + remainder 500000000000)\n\n",
+		},
+		{
+			"invalid - insufficient reserve backing",
+			func(ctx sdk.Context, k keeper.Keeper) {
+				amt := types.ConversionFactor().QuoRaw(2)
+
+				// 0.5 int coins x 4
+				k.SetFractionalBalance(ctx, sdk.AccAddress{1}, amt)
+				k.SetFractionalBalance(ctx, sdk.AccAddress{2}, amt)
+				k.SetFractionalBalance(ctx, sdk.AccAddress{3}, amt)
+				k.SetRemainderAmount(ctx, amt)
+
+				// Needs 2 to back 0.5 x 4
+				suite.FundReserve(sdk.NewInt(1))
+			},
+			true,
+			"precisebank: module reserve backing total fractional balances invariant\nakava reserve balance 1000000000000 mismatches 2000000000000 (fractional balances 1500000000000 + remainder 500000000000)\n\n",
+		},
+		{
+			"invalid - excess reserve backing",
+			func(ctx sdk.Context, k keeper.Keeper) {
+				amt := types.ConversionFactor().QuoRaw(2)
+
+				// 0.5 int coins x 4
+				k.SetFractionalBalance(ctx, sdk.AccAddress{1}, amt)
+				k.SetFractionalBalance(ctx, sdk.AccAddress{2}, amt)
+				k.SetFractionalBalance(ctx, sdk.AccAddress{3}, amt)
+				k.SetRemainderAmount(ctx, amt)
+
+				// Needs 2 to back 0.5 x 4
+				suite.FundReserve(sdk.NewInt(3))
+			},
+			true,
+			"precisebank: module reserve backing total fractional balances invariant\nakava reserve balance 3000000000000 mismatches 2000000000000 (fractional balances 1500000000000 + remainder 500000000000)\n\n",
+		},
+	}
+
+	for _, tt := range tests {
+		suite.Run(tt.name, func() {
+			// Reset each time
+			suite.SetupTest()
+
+			tt.setupFn(suite.Ctx, suite.Keeper)
+
+			invariantFn := keeper.ReserveBacksFractionsInvariant(suite.Keeper)
+			msg, broken := invariantFn(suite.Ctx)
+
+			if tt.wantBroken {
+				suite.Require().True(broken, "invariant should be broken but is not")
+				suite.Require().Equal(tt.wantMsg, msg)
+			} else {
+				suite.Require().Falsef(broken, "invariant should not be broken but is: %s", msg)
+			}
+		})
+	}
+}

x/precisebank/keeper/mint.go

@@ -63,13 +63,28 @@ func (k Keeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) err
 	return k.mintExtendedCoin(ctx, moduleName, extendedAmount)
 }
 
-// mintExtendedCoin manages the minting of extended coins, and no other coins.
+// mintExtendedCoin manages the minting of only extended coins. This also
+// handles integer carry over from fractional balance to integer balance if
+// necessary depending on the fractional balance and minting amount. Ensures
+// that the reserve fully backs the additional minted amount, minting any extra
+// reserve integer coins if necessary.
+// 4 Cases:
+// 1. NO integer carry over, >= 0 remainder - no reserve mint
+// 2. NO integer carry over, negative remainder - mint 1 to reserve
+// 3. Integer carry over, >= 0 remainder
+//   - Transfer 1 integer from reserve -> account
+//
+// 4. Integer carry over, negative remainder
+//   - Transfer 1 integer from reserve -> account
+//   - Mint 1 to reserve
+//     Optimization:
+//   - Increase direct account mint amount by 1, no extra reserve mint
 func (k Keeper) mintExtendedCoin(
 	ctx sdk.Context,
-	moduleName string,
+	recipientModuleName string,
 	amt sdkmath.Int,
 ) error {
-	moduleAddr := k.ak.GetModuleAddress(moduleName)
+	moduleAddr := k.ak.GetModuleAddress(recipientModuleName)
 
 	// Get current module account fractional balance - 0 if not found
 	fractionalAmount := k.GetFractionalBalance(ctx, moduleAddr)
@@ -78,19 +93,57 @@ func (k Keeper) mintExtendedCoin(
 	integerMintAmount := amt.Quo(types.ConversionFactor())
 	fractionalMintAmount := amt.Mod(types.ConversionFactor())
 
+	// Get previous remainder amount, as we need to it before carry calculation
+	// for the optimization path.
+	prevRemainder := k.GetRemainderAmount(ctx)
+
+	// Deduct new remainder with minted fractional amount. This will result in
+	// two cases:
+	// 1. Zero or positive remainder: remainder is sufficient to back the minted
+	//   fractional amount. Reserve is also sufficient to back the minted amount
+	//   so no additional reserve integer coin is needed.
+	// 2. Negative remainder: remainder is insufficient to back the minted
+	//   fractional amount. Reserve will need to be increased to back the mint
+	//   amount.
+	newRemainder := prevRemainder.Sub(fractionalMintAmount)
+
 	// Get new fractional balance after minting, this could be greater than
 	// the conversion factor and must be checked for carry over to integer mint
 	// amount as being set as-is may cause fractional balance exceeding max.
 	newFractionalBalance := fractionalAmount.Add(fractionalMintAmount)
 
-	// If it carries over, add 1 to integer mint amount. In this case, it will
+	// Case #3 - Integer carry, remainder is sufficient (0 or positive)
-	// always be 1:
+	if newFractionalBalance.GTE(types.ConversionFactor()) && newRemainder.GTE(sdkmath.ZeroInt()) {
+		// Carry should send from reserve -> account, instead of minting an
+		// extra integer coin. Otherwise doing an extra mint will require a burn
+		// from reserves to maintain exact backing.
+		carryCoin := sdk.NewCoin(types.IntegerCoinDenom, sdkmath.OneInt())
+
+		// SendCoinsFromModuleToModule allows for sending coins even if the
+		// recipient module account is blocked.
+		if err := k.bk.SendCoinsFromModuleToModule(
+			ctx,
+			types.ModuleName,
+			recipientModuleName,
+			sdk.NewCoins(carryCoin),
+		); err != nil {
+			return err
+		}
+	}
+
+	// Case #4 - Integer carry, remainder is insufficient
+	// This is the optimization path where the integer mint amount is increased
+	// by 1, instead of doing both a reserve -> account transfer and reserve mint.
+	if newFractionalBalance.GTE(types.ConversionFactor()) && newRemainder.IsNegative() {
+		integerMintAmount = integerMintAmount.AddRaw(1)
+	}
+
+	// If it carries over, adjust the fractional balance to account for the
+	// previously added 1 integer amount.
 	// fractional amounts x and y where both x and y < ConversionFactor
 	// x + y < (2 * ConversionFactor) - 2
 	// x + y < 1 integer amount + fractional amount
 	if newFractionalBalance.GTE(types.ConversionFactor()) {
-		// Carry over to integer mint amount
-		integerMintAmount = integerMintAmount.AddRaw(1)
 		// Subtract 1 integer equivalent amount of fractional balance. Same
 		// behavior as using .Mod() in this case.
 		newFractionalBalance = newFractionalBalance.Sub(types.ConversionFactor())
@@ -103,7 +156,7 @@ func (k Keeper) mintExtendedCoin(
 
 		if err := k.bk.MintCoins(
 			ctx,
-			moduleName,
+			recipientModuleName,
 			sdk.NewCoins(integerMintCoin),
 		); err != nil {
 			return err
@@ -115,20 +168,28 @@ func (k Keeper) mintExtendedCoin(
 
 	// ----------------------------------------
 	// Update remainder & reserves to back minted fractional coins
-	prevRemainder := k.GetRemainderAmount(ctx)
-	// Deduct new remainder with minted fractional amount
-	newRemainder := prevRemainder.Sub(fractionalMintAmount)
 
-	if prevRemainder.LT(fractionalMintAmount) {
+	// Mint an additional reserve integer coin if remainder is insufficient.
-		// Need additional 1 integer coin in reserve to back minted fractional
+	// The remainder is the amount of fractional coins that can be minted and
+	// still be fully backed by reserve. If the remainder is less than the
+	// minted fractional amount, then the reserve needs to be increased to
+	// back the additional fractional amount.
+	// Optimization: This is only done when the integer amount does NOT carry,
+	// as a direct account mint is done instead of integer carry transfer +
+	// insufficient remainder reserve mint.
+	wasCarried := fractionalAmount.Add(fractionalMintAmount).GTE(types.ConversionFactor())
+	if prevRemainder.LT(fractionalMintAmount) && !wasCarried {
+		// Always only 1 integer coin, as fractionalMintAmount < ConversionFactor
 		reserveMintCoins := sdk.NewCoins(sdk.NewCoin(types.IntegerCoinDenom, sdkmath.OneInt()))
 		if err := k.bk.MintCoins(ctx, types.ModuleName, reserveMintCoins); err != nil {
 			return fmt.Errorf("failed to mint %s for reserve: %w", reserveMintCoins, err)
 		}
+	}
 
-		// Update remainder with value of minted integer coin. newRemainder is
+	// newRemainder will be negative if prevRemainder < fractionalMintAmount.
-		// currently negative at this point. This also means that it will always
+	// This needs to be adjusted back to the corresponding positive value. The
-		// be < conversionFactor after this operation and not require a Mod().
+	// remainder will be always < conversionFactor after add if it is negative.
+	if newRemainder.IsNegative() {
 		newRemainder = newRemainder.Add(types.ConversionFactor())
 	}
 

x/precisebank/keeper/mint_integration_test.go

@@ -177,6 +177,39 @@ func (suite *mintIntegrationTestSuite) TestMintCoins() {
 				},
 			},
 		},
+		{
+			"fractional only with carry",
+			minttypes.ModuleName,
+			[]mintTest{
+				{
+					// Start with (1/4 * 3) = 0.75
+					mintAmount:  cs(ci(types.ExtendedCoinDenom, types.ConversionFactor().QuoRaw(4).MulRaw(3))),
+					wantBalance: cs(ci(types.ExtendedCoinDenom, types.ConversionFactor().QuoRaw(4).MulRaw(3))),
+				},
+				{
+					// Add another 0.50 to incur carry to test reserve on carry
+					mintAmount:  cs(ci(types.ExtendedCoinDenom, types.ConversionFactor().QuoRaw(2))),
+					wantBalance: cs(ci(types.ExtendedCoinDenom, types.ConversionFactor().QuoRaw(4).MulRaw(5))),
+				},
+			},
+		},
+		{
+			"fractional only, resulting in exact carry and 0 remainder",
+			minttypes.ModuleName,
+			[]mintTest{
+				// mint 0.5, acc = 0.5, reserve = 1
+				{
+					mintAmount:  cs(ci(types.ExtendedCoinDenom, types.ConversionFactor().QuoRaw(2))),
+					wantBalance: cs(ci(types.ExtendedCoinDenom, types.ConversionFactor().QuoRaw(2))),
+				},
+				// mint another 0.5, acc = 1, reserve = 0
+				// Reserve actually goes down by 1 for integer carry
+				{
+					mintAmount:  cs(ci(types.ExtendedCoinDenom, types.ConversionFactor().QuoRaw(2))),
+					wantBalance: cs(ci(types.ExtendedCoinDenom, types.ConversionFactor())),
+				},
+			},
+		},
 		{
 			"exact carry",
 			minttypes.ModuleName,
@@ -299,9 +332,8 @@ func (suite *mintIntegrationTestSuite) TestMintCoins() {
 
 				// Ensure reserve is backing all minted fractions
 				allInvariantsFn := keeper.AllInvariants(suite.Keeper)
-				res, stop := allInvariantsFn(suite.Ctx)
+				msg, stop := allInvariantsFn(suite.Ctx)
-				suite.Require().False(stop, "invariant should not be broken")
+				suite.Require().Falsef(stop, "invariant should not be broken: %s", msg)
-				suite.Require().Empty(res, "unexpected invariant message: %s", res)
 			}
 		})
 	}
@@ -311,6 +343,7 @@ func FuzzMintCoins(f *testing.F) {
 	f.Add(int64(0))
 	f.Add(int64(100))
 	f.Add(types.ConversionFactor().Int64())
+	f.Add(types.ConversionFactor().QuoRaw(2).Int64())
 	f.Add(types.ConversionFactor().MulRaw(5).Int64())
 	f.Add(types.ConversionFactor().MulRaw(2).AddRaw(123948723).Int64())
 
@@ -328,6 +361,8 @@ func FuzzMintCoins(f *testing.F) {
 
 		mintCount := int64(10)
 
+		suite.T().Logf("minting %d %d times", amount, mintCount)
+
 		// Mint 10 times to include mints from non-zero balances
 		for i := int64(0); i < mintCount; i++ {
 			err := suite.Keeper.MintCoins(
@@ -345,15 +380,15 @@ func FuzzMintCoins(f *testing.F) {
 		suite.Require().Equalf(
 			amount*mintCount,
 			bal.Amount.Int64(),
-			"unexpected balance after minting %d 5 times",
+			"unexpected balance after minting %d %d times",
 			amount,
+			mintCount,
 		)
 
 		// Run Invariants to ensure remainder is backing all minted fractions
 		// and in a valid state
-		allInvariantsFn := keeper.AllInvariants(suite.Keeper)
+		res, stop := keeper.AllInvariants(suite.Keeper)(suite.Ctx)
-		res, stop := allInvariantsFn(suite.Ctx)
+		suite.False(stop, "invariant should not be broken")
-		suite.Require().False(stop, "invariant should not be broken")
+		suite.Empty(res, "unexpected invariant message")
-		suite.Require().Empty(res, "unexpected invariant message: %s", res)
 	})
 }

x/precisebank/keeper/mint_test.go

@@ -289,6 +289,20 @@ func TestMintCoins_ExpectedCalls(t *testing.T) {
 					Once()
 			}
 
+			// ----------------------------------------
+			// Set expectations for reserve minting when fractional amounts
+			// are minted & remainder is insufficient
+			mintFractionalAmount := extCoins.Amount.Mod(types.ConversionFactor())
+			currentRemainder := td.keeper.GetRemainderAmount(td.ctx)
+
+			causesIntegerCarry := fBal.Add(mintFractionalAmount).GTE(types.ConversionFactor())
+			remainderEnough := currentRemainder.GTE(mintFractionalAmount)
+
+			// Optimization: Carry & insufficient remainder is directly minted
+			if causesIntegerCarry && !remainderEnough {
+				extCoins = extCoins.AddAmount(types.ConversionFactor())
+			}
+
 			// ----------------------------------------
 			// Set expectations for minting fractional coins
 			if !extCoins.IsNil() && extCoins.IsPositive() {
@@ -298,9 +312,9 @@ func TestMintCoins_ExpectedCalls(t *testing.T) {
 					Once()
 
 				// Initial integer balance is always 0 for this test
-				totalNewBalance := tt.startFractionalBalance.Add(extCoins.Amount)
+				mintIntegerAmount := extCoins.Amount.Quo(types.ConversionFactor())
-				mintIntegerAmount := totalNewBalance.Quo(types.ConversionFactor())
 
+				// Minted coins does NOT include roll-over, simply excludes
 				mintCoins := cs(ci(types.IntegerCoinDenom, mintIntegerAmount))
 
 				// Only expect MintCoins to be called with mint coins with
@@ -316,15 +330,20 @@ func TestMintCoins_ExpectedCalls(t *testing.T) {
 				}
 			}
 
-			// ----------------------------------------
+			if causesIntegerCarry && remainderEnough {
-			// Set expectations for reserve minting when fractional amounts
+				td.bk.EXPECT().
-			// are minted & remainder is insufficient
+					SendCoinsFromModuleToModule(
-			mintFractionalAmount := extCoins.Amount.Mod(types.ConversionFactor())
+						td.ctx,
-			currentRemainder := td.keeper.GetRemainderAmount(td.ctx)
+						types.ModuleName,
+						minttypes.ModuleName,
+						cs(c(types.IntegerCoinDenom, 1)),
+					).
+					Return(nil).
+					Once()
+			}
 
-			remainderEnough := currentRemainder.GTE(mintFractionalAmount)
+			if !remainderEnough && !causesIntegerCarry {
-			if !remainderEnough {
+				reserveMintCoins := cs(c(types.IntegerCoinDenom, 1))
-				reserveMintCoins := cs(ci(types.IntegerCoinDenom, sdkmath.OneInt()))
 				td.bk.EXPECT().
 					// Mints to x/precisebank
 					MintCoins(td.ctx, types.ModuleName, reserveMintCoins).