Add nav unit conversion to upgrade handler. (#1816)

* Add CHANGELOG entry. * Add function to get marker addr from key. * Add conversion func to upgrade handler, and an iterate all function to marker keeper. * Update changelog entry. * Add tests for upgrade. * Add test for upgrade. * Add test for GetMarkerFromNetAssetValue and fix bug. * Add additional test case for ConvertNAVUnits. * Add test for IterateAllNetAssetMarkers. * Add error checks.
provenance-io · Jan 24, 2024 · eb95a08 · eb95a08
1 parent fb5d84d
commit eb95a08
Show file tree

Hide file tree

Showing 7 changed files with 267 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Features
 
 * Add the ibcratelimit module [#1498](https://github.com/provenance-io/provenance/issues/1498).
+* Add fix for NAV units to tourmaline upgrade handler [#1815](https://github.com/provenance-io/provenance/issues/1815).
 
 ### Improvements
 

diff --git a/app/upgrades.go b/app/upgrades.go
@@ -127,6 +127,7 @@ var upgrades = map[string]appUpgrade{
 			}
 
 			removeInactiveValidatorDelegations(ctx, app)
+			convertNavUnits(ctx, app)
 
 			return vm, nil
 		},
@@ -141,6 +142,7 @@ var upgrades = map[string]appUpgrade{
 			}
 
 			removeInactiveValidatorDelegations(ctx, app)
+			convertNavUnits(ctx, app)
 
 			return vm, nil
 		},
@@ -349,3 +351,27 @@ func updateIbcMarkerDenomMetadata(ctx sdk.Context, app *App) {
 	})
 	ctx.Logger().Info("Done updating ibc marker denom metadata")
 }
+
+// convertNavUnits iterates all the net asset values and updates their units if they are using usd.
+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) {
+		if nav.Price.Denom == markertypes.UsdDenom {
+			nav.Price.Amount = nav.Price.Amount.Mul(math.NewInt(10))
+			marker, err := app.MarkerKeeper.GetMarker(ctx, markerAddr)
+			if err != nil {
+				ctx.Logger().Error(fmt.Sprintf("Unable to get marker for address: %s, error: %s", markerAddr, err))
+				return false
+			}
+			err = app.MarkerKeeper.SetNetAssetValue(ctx, marker, nav, "upgrade")
+			if err != nil {
+				ctx.Logger().Error(fmt.Sprintf("Unable to set net asset value for marker: %s, error: %s", markerAddr, err))
+				return false
+			}
+		}
+		return false
+	})
+	if err != nil {
+		ctx.Logger().Error(fmt.Sprintf("Unable to iterate all net asset values error: %s", err))
+	}
+}
diff --git a/app/upgrades_test.go b/app/upgrades_test.go
@@ -425,13 +425,17 @@ func (s *UpgradeTestSuite) TestSaffron() {
 }
 
 func (s *UpgradeTestSuite) TestTourmalineRC1() {
-	expInLog := []string{}
+	expInLog := []string{
+		"INF Converting NAV units",
+	}
 
 	s.AssertUpgradeHandlerLogs("tourmaline-rc1", expInLog, nil)
 }
 
 func (s *UpgradeTestSuite) TestTourmaline() {
-	expInLog := []string{}
+	expInLog := []string{
+		"INF Converting NAV units",
+	}
 
 	s.AssertUpgradeHandlerLogs("tourmaline", expInLog, nil)
 }
@@ -780,3 +784,110 @@ func (s *UpgradeTestSuite) TestSetExchangeParams() {
 		})
 	}
 }
+
+func (s *UpgradeTestSuite) TestConvertNAVUnits() {
+	tests := []struct {
+		name       string
+		markerNavs []sdk.Coins
+		expected   []sdk.Coins
+	}{
+		{
+			name:       "should work with no markers",
+			markerNavs: []sdk.Coins{},
+			expected:   []sdk.Coins{},
+		},
+		{
+			name: "should work with one marker no usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1)),
+			},
+			expected: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1)),
+			},
+		},
+		{
+			name: "should work with multiple markers no usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1)),
+				sdk.NewCoins(sdk.NewInt64Coin("georgethedog", 2)),
+			},
+			expected: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1)),
+				sdk.NewCoins(sdk.NewInt64Coin("georgethedog", 2)),
+			},
+		},
+		{
+			name: "should work with one marker with usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1), sdk.NewInt64Coin(markertypes.UsdDenom, 2)),
+			},
+			expected: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1), sdk.NewInt64Coin(markertypes.UsdDenom, 20)),
+			},
+		},
+		{
+			name: "should work with multiple markers with usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1), sdk.NewInt64Coin(markertypes.UsdDenom, 3)),
+				sdk.NewCoins(sdk.NewInt64Coin("georgethedog", 2), sdk.NewInt64Coin(markertypes.UsdDenom, 4)),
+			},
+			expected: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1), sdk.NewInt64Coin(markertypes.UsdDenom, 30)),
+				sdk.NewCoins(sdk.NewInt64Coin("georgethedog", 2), sdk.NewInt64Coin(markertypes.UsdDenom, 40)),
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		s.Run(tc.name, func() {
+			// Create the marker
+			for i, prices := range tc.markerNavs {
+				address := sdk.AccAddress(fmt.Sprintf("marker%d", i))
+				marker := markertypes.NewEmptyMarkerAccount(fmt.Sprintf("coin%d", i), address.String(), []markertypes.AccessGrant{})
+				marker.Supply = sdk.OneInt()
+				s.Require().NoError(s.app.MarkerKeeper.AddMarkerAccount(s.ctx, marker), "AddMarkerAccount() error")
+
+				var navs []markertypes.NetAssetValue
+				for _, price := range prices {
+					navs = append(navs, markertypes.NewNetAssetValue(price, uint64(1)))
+					navAddr := sdk.AccAddress(price.Denom)
+					if acc, _ := s.app.MarkerKeeper.GetMarkerByDenom(s.ctx, price.Denom); acc == nil {
+						navMarker := markertypes.NewEmptyMarkerAccount(price.Denom, navAddr.String(), []markertypes.AccessGrant{})
+						navMarker.Supply = sdk.OneInt()
+						s.Require().NoError(s.app.MarkerKeeper.AddMarkerAccount(s.ctx, navMarker), "AddMarkerAccount() error")
+					}
+				}
+				s.Require().NoError(s.app.MarkerKeeper.AddSetNetAssetValues(s.ctx, marker, navs, "AddSetNetAssetValues() error"))
+			}
+
+			// Test Logic
+			convertNavUnits(s.ctx, s.app)
+			for i := range tc.markerNavs {
+				marker, err := s.app.MarkerKeeper.GetMarkerByDenom(s.ctx, fmt.Sprintf("coin%d", i))
+				s.Require().NoError(err, "GetMarkerByDenom() error")
+				var prices sdk.Coins
+
+				s.app.MarkerKeeper.IterateNetAssetValues(s.ctx, marker.GetAddress(), func(state markertypes.NetAssetValue) (stop bool) {
+					prices = append(prices, state.Price)
+					return false
+				})
+				s.Require().EqualValues(tc.expected[i], prices, "should update prices correctly for nav")
+			}
+
+			// Destroy the marker
+			for i, prices := range tc.markerNavs {
+				coin := fmt.Sprintf("coin%d", i)
+				marker, err := s.app.MarkerKeeper.GetMarkerByDenom(s.ctx, coin)
+				s.Require().NoError(err, "GetMarkerByDenom() error")
+				s.app.MarkerKeeper.RemoveMarker(s.ctx, marker)
+
+				// We need to remove the nav markers
+				for _, price := range prices {
+					if navMarker, _ := s.app.MarkerKeeper.GetMarkerByDenom(s.ctx, price.Denom); navMarker != nil {
+						s.app.MarkerKeeper.RemoveMarker(s.ctx, navMarker)
+					}
+				}
+			}
+		})
+	}
+}
diff --git a/x/marker/keeper/keeper.go b/x/marker/keeper/keeper.go
@@ -324,6 +324,24 @@ func (k Keeper) IterateNetAssetValues(ctx sdk.Context, markerAddr sdk.AccAddress
 	return nil
 }
 
+// IterateAllNetAssetValues iterates all net asset values
+func (k Keeper) IterateAllNetAssetValues(ctx sdk.Context, handler func(sdk.AccAddress, types.NetAssetValue) (stop bool)) error {
+	store := ctx.KVStore(k.storeKey)
+	it := sdk.KVStorePrefixIterator(store, types.NetAssetValuePrefix)
+	defer it.Close()
+	for ; it.Valid(); it.Next() {
+		markerAddr := types.GetMarkerFromNetAssetValueKey(it.Key())
+		var markerNav types.NetAssetValue
+		err := k.cdc.Unmarshal(it.Value(), &markerNav)
+		if err != nil {
+			return err
+		} else if handler(markerAddr, markerNav) {
+			break
+		}
+	}
+	return nil
+}
+
 // RemoveNetAssetValues removes all net asset values for a marker
 func (k Keeper) RemoveNetAssetValues(ctx sdk.Context, markerAddr sdk.AccAddress) {
 	store := ctx.KVStore(k.storeKey)

diff --git a/x/marker/keeper/keeper_test.go b/x/marker/keeper/keeper_test.go
@@ -1588,6 +1588,100 @@ func TestAddRemoveSendDeny(t *testing.T) {
 	}
 }
 
+func TestIterateAllNetAssetValues(t *testing.T) {
+	app := simapp.Setup(t)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+
+	tests := []struct {
+		name       string
+		markerNavs []sdk.Coins
+		expected   int
+	}{
+		{
+			name:       "should work with no markers",
+			markerNavs: []sdk.Coins{},
+			expected:   0,
+		},
+		{
+			name: "should work with one marker no usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1)),
+			},
+			expected: 1,
+		},
+		{
+			name: "should work with multiple markers no usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1)),
+				sdk.NewCoins(sdk.NewInt64Coin("georgethedog", 2)),
+			},
+			expected: 2,
+		},
+		{
+			name: "should work with one marker with usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1), sdk.NewInt64Coin(types.UsdDenom, 2)),
+			},
+			expected: 2,
+		},
+		{
+			name: "should work with multiple markers with usd denom",
+			markerNavs: []sdk.Coins{
+				sdk.NewCoins(sdk.NewInt64Coin("jackthecat", 1), sdk.NewInt64Coin(types.UsdDenom, 3)),
+				sdk.NewCoins(sdk.NewInt64Coin("georgethedog", 2), sdk.NewInt64Coin(types.UsdDenom, 4)),
+			},
+			expected: 4,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Create the marker
+			for i, prices := range tc.markerNavs {
+				address := sdk.AccAddress(fmt.Sprintf("marker%d", i))
+				marker := types.NewEmptyMarkerAccount(fmt.Sprintf("coin%d", i), address.String(), []types.AccessGrant{})
+				marker.Supply = sdk.OneInt()
+				require.NoError(t, app.MarkerKeeper.AddMarkerAccount(ctx, marker), "AddMarkerAccount() error")
+
+				var navs []types.NetAssetValue
+				for _, price := range prices {
+					navs = append(navs, types.NewNetAssetValue(price, uint64(1)))
+					navAddr := sdk.AccAddress(price.Denom)
+					if acc, _ := app.MarkerKeeper.GetMarkerByDenom(ctx, price.Denom); acc == nil {
+						navMarker := types.NewEmptyMarkerAccount(price.Denom, navAddr.String(), []types.AccessGrant{})
+						navMarker.Supply = sdk.OneInt()
+						require.NoError(t, app.MarkerKeeper.AddMarkerAccount(ctx, navMarker), "AddMarkerAccount() error")
+					}
+				}
+				require.NoError(t, app.MarkerKeeper.AddSetNetAssetValues(ctx, marker, navs, "AddSetNetAssetValues() error"))
+			}
+
+			// Test Logic
+			count := 0
+			app.MarkerKeeper.IterateAllNetAssetValues(ctx, func(aa sdk.AccAddress, nav types.NetAssetValue) (stop bool) {
+				count += 1
+				return false
+			})
+			assert.Equal(t, tc.expected, count, "should iterate the correct number of times")
+
+			// Destroy the marker
+			for i, prices := range tc.markerNavs {
+				coin := fmt.Sprintf("coin%d", i)
+				marker, err := app.MarkerKeeper.GetMarkerByDenom(ctx, coin)
+				require.NoError(t, err, "GetMarkerByDenom() error")
+				app.MarkerKeeper.RemoveMarker(ctx, marker)
+
+				// We need to remove the nav markers
+				for _, price := range prices {
+					if navMarker, _ := app.MarkerKeeper.GetMarkerByDenom(ctx, price.Denom); navMarker != nil {
+						app.MarkerKeeper.RemoveMarker(ctx, navMarker)
+					}
+				}
+			}
+		})
+	}
+}
+
 func TestReqAttrBypassAddrs(t *testing.T) {
 	// Tests both GetReqAttrBypassAddrs and IsReqAttrBypassAddr.
 	expectedNames := []string{

diff --git a/x/marker/types/key.go b/x/marker/types/key.go
@@ -99,3 +99,10 @@ func NetAssetValueKeyPrefix(markerAddr sdk.AccAddress) []byte {
 func NetAssetValueKey(markerAddr sdk.AccAddress, denom string) []byte {
 	return append(NetAssetValueKeyPrefix(markerAddr), denom...)
 }
+
+// GetMarkerFromNetAssetValueKey returns the marker address in the NetAssetValue key.
+func GetMarkerFromNetAssetValueKey(key []byte) sdk.AccAddress {
+	markerKeyLen := key[1]
+	markerAddr := sdk.AccAddress(key[2 : markerKeyLen+2])
+	return markerAddr
+}
diff --git a/x/marker/types/key_test.go b/x/marker/types/key_test.go
@@ -55,6 +55,14 @@ func TestNetAssetValueKey(t *testing.T) {
 	assert.Equal(t, "nhash", string(navKey[denomArrLen+2:]))
 }
 
+func TestGetMarkerFromNetAssetValueKey(t *testing.T) {
+	addr, err := MarkerAddress("nhash")
+	require.NoError(t, err)
+	navKey := NetAssetValueKey(addr, "nhash")
+	addr2 := GetMarkerFromNetAssetValueKey(navKey)
+	assert.Equal(t, addr, addr2, "should match original marker address")
+}
+
 func TestDenySendMarkerPrefix(t *testing.T) {
 	addr, err := MarkerAddress("nhash")
 	require.NoError(t, err, "MarkerAddress(nhash)")