From 635cf9201a1efcb367571c3716da84fd79403ebf Mon Sep 17 00:00:00 2001
From: Ira Miller <72319+iramiller@users.noreply.github.com>
Date: Mon, 8 Nov 2021 17:13:46 -0700
Subject: [PATCH] allow marker creation over existing accounts (#552)

---
 CHANGELOG.md                   |  2 +-
 x/marker/keeper/keeper_test.go | 36 ++++++++++++++++++++++++++++++++++
 x/marker/keeper/marker.go      | 17 +++++++++-------
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 59b0be4a74..0b765ec702 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -43,7 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * Fix typeos in marker log statements [#502](https://github.com/provenance-io/provenance/issues/502)
 * Set default coin type to network default [#534](https://github.com/provenance-io/provenance/issues/534)
 * Add logger to upgrade handler [#507](https://github.com/provenance-io/provenance/issues/507)
-
+* Allow markers to be created over existing accounts if they are not a marker and have a zero sequence [#520](https://github.com/provenance-io/provenance/issues/520)
 ### Bug Fixes
 
 * When deleting a scope, require the same permissions as when updating it [#473](https://github.com/provenance-io/provenance/issues/473)
diff --git a/x/marker/keeper/keeper_test.go b/x/marker/keeper/keeper_test.go
index d6cff79430..ae270d862b 100644
--- a/x/marker/keeper/keeper_test.go
+++ b/x/marker/keeper/keeper_test.go
@@ -66,6 +66,42 @@ func TestAccountMapperGetSet(t *testing.T) {
 	require.Error(t, err, "marker does not exist, should error")
 }
 
+func TestExistingAccounts(t *testing.T) {
+	//app, ctx := createTestApp(true)
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+	server := markerkeeper.NewMsgServerImpl(app.MarkerKeeper)
+
+	addr := types.MustGetMarkerAddress("testcoin")
+	pubkey := secp256k1.GenPrivKey().PubKey()
+	user := testUserAddress("testcoin")
+	manager := testUserAddress("manager")
+	existingBalance := sdk.NewCoin("coin", sdk.NewInt(1000))
+
+	// prefund the marker address so an account gets created before the marker does.
+	app.AccountKeeper.SetAccount(ctx, authtypes.NewBaseAccount(user, pubkey, 0, 0))
+	simapp.FundAccount(app, ctx, addr, sdk.NewCoins(existingBalance))
+	require.Equal(t, existingBalance, app.BankKeeper.GetBalance(ctx, addr, "coin"), "account balance must be set")
+
+	// Creating a marker over an account with zero sequence is fine.
+	_, err := server.AddMarker(sdk.WrapSDKContext(ctx), types.NewMsgAddMarkerRequest("testcoin", sdk.NewInt(30), user, manager, types.MarkerType_Coin, true, true))
+	require.NoError(t, err, "should allow a marker over existing account that has not signed anything.")
+
+	// existing coin balance must still be present
+	require.Equal(t, existingBalance, app.BankKeeper.GetBalance(ctx, addr, "coin"), "account balances must be preserved")
+
+	// Creating a marker over an existing marker fails.
+	_, err = server.AddMarker(sdk.WrapSDKContext(ctx), types.NewMsgAddMarkerRequest("testcoin", sdk.NewInt(30), user, manager, types.MarkerType_Coin, true, true))
+	require.Error(t, err, "fails because marker already exists")
+
+	// replace existing test account with a new copy that has a positive sequence number
+	app.AccountKeeper.SetAccount(ctx, authtypes.NewBaseAccount(user, pubkey, 0, 10))
+
+	// Creating a marker over an existing account with a positive sequence number fails.
+	_, err = server.AddMarker(sdk.WrapSDKContext(ctx), types.NewMsgAddMarkerRequest("testcoin", sdk.NewInt(30), user, manager, types.MarkerType_Coin, true, true))
+	require.Error(t, err, "should not allow creation over and existing account with a positive sequence number.")
+}
+
 func TestAccountUnrestrictedDenoms(t *testing.T) {
 	//app, ctx := createTestApp(true)
 	app := simapp.Setup(false)
diff --git a/x/marker/keeper/marker.go b/x/marker/keeper/marker.go
index a9d92ce0ea..8c88cb2d01 100644
--- a/x/marker/keeper/marker.go
+++ b/x/marker/keeper/marker.go
@@ -60,13 +60,16 @@ func (k Keeper) AddMarkerAccount(ctx sdk.Context, marker types.MarkerAccountI) e
 		return fmt.Errorf("marker address does not match expected %s for denom %s", markerAddress, marker.GetDenom())
 	}
 
-	// Should not exist yet
-	existing, err := k.GetMarker(ctx, markerAddress)
-	if err != nil {
-		return err
-	}
-	if existing != nil {
-		return fmt.Errorf("marker address already exists for %s", markerAddress)
+	// Should not exist yet (or if exists must not be a marker and must have a zero sequence number)
+	mac := k.authKeeper.GetAccount(ctx, markerAddress)
+	if mac != nil {
+		_, ok := mac.(types.MarkerAccountI)
+		if ok {
+			return fmt.Errorf("marker address already exists for %s", markerAddress)
+		} else if mac.GetSequence() > 0 {
+			// account exists, is not a marker, and has been signed for
+			return fmt.Errorf("account at %s is not a marker account", markerAddress.String())
+		}
 	}
 
 	// set base account number