chore: review feedback improvements

e2e/tests/stake_app_transfer.feature

@@ -17,3 +17,5 @@ Feature: App Stake Transfer Namespace
         And the user should wait for the "application" module "TransferApplicationStake" message to be submitted
         And the "application" for account "app3" is staked with "1000070" uPOKT
         And the account balance of "app3" should be "0" uPOKT "less" than before
+        And the user verifies the "application" for account "app2" is not staked
+        And the account balance of "app2" should be "0" uPOKT "more" than before

proto/poktroll/application/tx.proto

@@ -80,9 +80,9 @@ message MsgUndelegateFromGateway {
 message MsgUndelegateFromGatewayResponse {}
 
 message MsgTransferApplicationStake {
-  option (cosmos.msg.v1.signer) = "address";
-  string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
-  string beneficiary = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  option (cosmos.msg.v1.signer) = "source_address";
+  string source_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  string destination_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 }
 
 message MsgTransferApplicationStakeResponse {

x/application/keeper/msg_server_transfer_application_stake.go

@@ -8,6 +8,8 @@ import (
 	"github.com/pokt-network/poktroll/x/application/types"
 )
 
+// TransferApplicationStake transfers the stake (held in escrow in the application
+// module account) from a source to a (new) destination application account .
 func (k msgServer) TransferApplicationStake(ctx context.Context, msg *types.MsgTransferApplicationStake) (*types.MsgTransferApplicationStakeResponse, error) {
 	isSuccessful := false
 	defer telemetry.EventSuccessCounter(
@@ -22,38 +24,34 @@ func (k msgServer) TransferApplicationStake(ctx context.Context, msg *types.MsgT
 		return nil, err
 	}
 
-	_, isBeneficiaryFound := k.GetApplication(ctx, msg.Beneficiary)
-	if isBeneficiaryFound {
-		return nil, types.ErrAppDuplicateAddress.Wrapf("beneficiary (%q) exists", msg.Beneficiary)
+	_, isDstFound := k.GetApplication(ctx, msg.GetDestinationAddress())
+	if isDstFound {
+		return nil, types.ErrAppDuplicateAddress.Wrapf("destination application (%q) exists", msg.GetDestinationAddress())
 	}
 
-	foundApp, isAppFound := k.GetApplication(ctx, msg.Address)
+	srcApp, isAppFound := k.GetApplication(ctx, msg.GetSourceAddress())
 	if !isAppFound {
-		return nil, types.ErrAppNotFound.Wrapf("application %q not found", msg.Address)
+		return nil, types.ErrAppNotFound.Wrapf("source application %q not found", msg.GetSourceAddress())
 	}
 
-	beneficiary := k.createApplication(ctx, &types.MsgStakeApplication{
-		Address:  msg.Beneficiary,
-		Stake:    foundApp.Stake,
-		Services: foundApp.ServiceConfigs,
-	})
+	// Create a new application derived from the source application.
+	dstApp := srcApp
+	dstApp.Address = msg.GetDestinationAddress()
 
 	// TODO_TEST: add E2E coverage to assert #DelegateeGatewayAddresses and #PendingUndelegations
-	// are present on the beneficiary application.
-	beneficiary.DelegateeGatewayAddresses = foundApp.DelegateeGatewayAddresses
-	beneficiary.PendingUndelegations = foundApp.PendingUndelegations
+	// are present on the dstApp application.
 
-	// Update the beneficiary in the store
-	k.SetApplication(ctx, beneficiary)
-	logger.Info(fmt.Sprintf("Successfully transferred application stake from app (%s) to beneficiary (%s)", foundApp.Address, beneficiary.Address))
+	// Update the dstApp in the store
+	k.SetApplication(ctx, dstApp)
+	logger.Info(fmt.Sprintf("Successfully transferred application stake from (%s) to (%s)", srcApp.Address, dstApp.Address))
 
 	// Remove the transferred app from the store
-	k.RemoveApplication(ctx, foundApp.Address)
-	logger.Info(fmt.Sprintf("Successfully removed the application: %+v", foundApp))
+	k.RemoveApplication(ctx, srcApp.GetAddress())
+	logger.Info(fmt.Sprintf("Successfully removed the application: %+v", srcApp))
 
 	isSuccessful = true
 
 	return &types.MsgTransferApplicationStakeResponse{
-		Application: &beneficiary,
+		Application: &dstApp,
 	}, nil
 }

x/application/keeper/msg_server_transfer_application_stake_test.go

@@ -18,19 +18,19 @@ func TestMsgServer_TransferApplicationStake_Success(t *testing.T) {
 	k, ctx := keepertest.ApplicationKeeper(t)
 	srv := appkeeper.NewMsgServerImpl(k)
 
-	// Generate an address for the application and beneficiary.
-	appAddr := sample.AccAddress()
-	beneficiaryAddr := sample.AccAddress()
+	// Generate an address for the source and destination applications.
+	srcAddr := sample.AccAddress()
+	dstAddr := sample.AccAddress()
 
 	// Verify that the app does not exist yet.
-	_, isAppFound := k.GetApplication(ctx, appAddr)
-	require.False(t, isAppFound)
+	_, isSrcFound := k.GetApplication(ctx, srcAddr)
+	require.False(t, isSrcFound)
 
 	expectedAppStake := &cosmostypes.Coin{Denom: "upokt", Amount: math.NewInt(100)}
 
 	// Prepare the application.
 	stakeMsg := &apptypes.MsgStakeApplication{
-		Address: appAddr,
+		Address: srcAddr,
 		Stake:   expectedAppStake,
 		Services: []*sharedtypes.ApplicationServiceConfig{
 			{
@@ -44,56 +44,56 @@ func TestMsgServer_TransferApplicationStake_Success(t *testing.T) {
 	require.NoError(t, err)
 
 	// Verify that the application exists.
-	foundApp, isAppFound := k.GetApplication(ctx, appAddr)
-	require.True(t, isAppFound)
-	require.Equal(t, appAddr, foundApp.Address)
-	require.Equal(t, expectedAppStake, foundApp.Stake)
-	require.Len(t, foundApp.ServiceConfigs, 1)
-	require.Equal(t, "svc1", foundApp.ServiceConfigs[0].Service.Id)
+	srcApp, isSrcFound := k.GetApplication(ctx, srcAddr)
+	require.True(t, isSrcFound)
+	require.Equal(t, srcAddr, srcApp.Address)
+	require.Equal(t, expectedAppStake, srcApp.Stake)
+	require.Len(t, srcApp.ServiceConfigs, 1)
+	require.Equal(t, "svc1", srcApp.ServiceConfigs[0].Service.Id)
 
-	// Transfer the application stake to the beneficiary.
-	transferStakeMsg := apptypes.NewMsgTransferApplicationStake(appAddr, beneficiaryAddr)
+	// Transfer the application stake from the source to the destination application address.
+	transferStakeMsg := apptypes.NewMsgTransferApplicationStake(srcAddr, dstAddr)
 
 	transferAppStakeRes, stakeTransferErr := srv.TransferApplicationStake(ctx, transferStakeMsg)
 	require.NoError(t, stakeTransferErr)
 
-	// Verify that the beneficiary was created with the same stake and service configs.
-	foundApp, isAppFound = k.GetApplication(ctx, beneficiaryAddr)
-	require.True(t, isAppFound)
-
-	foundBeneficiary, isBeneficiaryFound := k.GetApplication(ctx, beneficiaryAddr)
-	require.True(t, isBeneficiaryFound)
-	require.Equal(t, beneficiaryAddr, foundBeneficiary.Address)
-	require.Equal(t, expectedAppStake, foundBeneficiary.Stake)
-	require.Len(t, foundBeneficiary.ServiceConfigs, 1)
-	require.EqualValues(t, foundApp, foundBeneficiary)
-	require.EqualValues(t, &foundBeneficiary, transferAppStakeRes.Application)
-
-	// Verify that the original app was unstaked.
-	foundApp, isAppFound = k.GetApplication(ctx, appAddr)
-	require.False(t, isAppFound)
+	// Verify that the destination app was created with the correct state.
+	srcApp, isSrcFound = k.GetApplication(ctx, dstAddr)
+	require.True(t, isSrcFound)
+
+	dstApp, isDstFound := k.GetApplication(ctx, dstAddr)
+	require.True(t, isDstFound)
+	require.Equal(t, dstAddr, dstApp.Address)
+	require.Equal(t, expectedAppStake, dstApp.Stake)
+	require.Len(t, dstApp.ServiceConfigs, 1)
+	require.EqualValues(t, srcApp, dstApp)
+	require.EqualValues(t, &dstApp, transferAppStakeRes.Application)
+
+	// Verify that the source app was unstaked.
+	srcApp, isSrcFound = k.GetApplication(ctx, srcAddr)
+	require.False(t, isSrcFound)
 }
 
-func TestMsgServer_TransferApplicationStake_Error_BeneficiaryExists(t *testing.T) {
+func TestMsgServer_TransferApplicationStake_Error_DestinationExists(t *testing.T) {
 	k, ctx := keepertest.ApplicationKeeper(t)
 	srv := appkeeper.NewMsgServerImpl(k)
 
-	// Generate an address for the application and beneficiary.
-	appAddr := sample.AccAddress()
-	beneficiaryAddr := sample.AccAddress()
+	// Generate an address for the source and destination applications.
+	srcAddr := sample.AccAddress()
+	dstAddr := sample.AccAddress()
 
-	// Verify that neither the app nor the beneficiary exists yet.
-	_, isAppFound := k.GetApplication(ctx, appAddr)
-	require.False(t, isAppFound)
+	// Verify that neither the source nor the destination application exists yet.
+	_, isSrcFound := k.GetApplication(ctx, srcAddr)
+	require.False(t, isSrcFound)
 
-	_, isBeneficiaryFound := k.GetApplication(ctx, beneficiaryAddr)
-	require.False(t, isBeneficiaryFound)
+	_, isDstFound := k.GetApplication(ctx, dstAddr)
+	require.False(t, isDstFound)
 
 	expectedAppStake := &cosmostypes.Coin{Denom: "upokt", Amount: math.NewInt(100)}
 
 	// Prepare and stake the application.
 	appStakeMsg := &apptypes.MsgStakeApplication{
-		Address: appAddr,
+		Address: srcAddr,
 		Stake:   expectedAppStake,
 		Services: []*sharedtypes.ApplicationServiceConfig{
 			{
@@ -105,9 +105,9 @@ func TestMsgServer_TransferApplicationStake_Error_BeneficiaryExists(t *testing.T
 	_, err := srv.StakeApplication(ctx, appStakeMsg)
 	require.NoError(t, err)
 
-	// Prepare and stake the beneficiary.
-	beneficiaryStakeMsg := &apptypes.MsgStakeApplication{
-		Address: beneficiaryAddr,
+	// Prepare and stake the destination application.
+	dstAppStakeMsg := &apptypes.MsgStakeApplication{
+		Address: dstAddr,
 		Stake:   expectedAppStake,
 		Services: []*sharedtypes.ApplicationServiceConfig{
 			{
@@ -116,20 +116,20 @@ func TestMsgServer_TransferApplicationStake_Error_BeneficiaryExists(t *testing.T
 		},
 	}
 
-	_, err = srv.StakeApplication(ctx, beneficiaryStakeMsg)
+	_, err = srv.StakeApplication(ctx, dstAppStakeMsg)
 	require.NoError(t, err)
 
-	// Attempt to transfer the application stake to the beneficiary.
-	transferStakeMsg := apptypes.NewMsgTransferApplicationStake(appAddr, beneficiaryAddr)
+	// Attempt to transfer the source application stake to the destination.
+	transferStakeMsg := apptypes.NewMsgTransferApplicationStake(srcAddr, dstAddr)
 
 	_, err = srv.TransferApplicationStake(ctx, transferStakeMsg)
-	require.ErrorContains(t, err, apptypes.ErrAppDuplicateAddress.Wrapf("beneficiary (%q) exists", beneficiaryAddr).Error())
+	require.ErrorContains(t, err, apptypes.ErrAppDuplicateAddress.Wrapf("destination (%q) exists", dstAddr).Error())
 
 	// Verify that the original application still exists.
 	var foundApp apptypes.Application
-	foundApp, isAppFound = k.GetApplication(ctx, appAddr)
-	require.True(t, isAppFound)
-	require.Equal(t, appAddr, foundApp.Address)
+	foundApp, isSrcFound = k.GetApplication(ctx, srcAddr)
+	require.True(t, isSrcFound)
+	require.Equal(t, srcAddr, foundApp.Address)
 	require.Equal(t, int64(100), foundApp.Stake.Amount.Int64())
 	require.Len(t, foundApp.ServiceConfigs, 1)
 	require.Equal(t, "svc1", foundApp.ServiceConfigs[0].Service.Id)

x/application/module/autocli.go

@@ -99,9 +99,9 @@ func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions {
 				// 				},
 				{
 					RpcMethod:      "TransferApplicationStake",
-					Use:            "transfer [address] [beneficiary]",
-					Short:          "Transfer the application stake from [address] to [beneciciary]",
-					PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "address"}, {ProtoField: "beneficiary"}},
+					Use:            "transfer [source app address] [destination app address]",
+					Short:          "Transfer the application stake from [source app address] to [destination app address] and unstake the source application",
+					PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "source_address"}, {ProtoField: "destination_address"}},
 				},
 				// this line is used by ignite scaffolding # autocli/tx
 			},

x/application/simulation/transfer_application_stake.go

@@ -6,6 +6,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
+
 	"github.com/pokt-network/poktroll/x/application/keeper"
 	"github.com/pokt-network/poktroll/x/application/types"
 )
@@ -17,9 +18,11 @@ func SimulateMsgTransferApplicationStake(
 ) simtypes.Operation {
 	return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string,
 	) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
-		simAccount, _ := simtypes.RandomAcc(r, accs)
+		simSrcAppAccount, _ := simtypes.RandomAcc(r, accs)
+		simDstAppAccount, _ := simtypes.RandomAcc(r, accs)
 		msg := &types.MsgTransferApplicationStake{
-			Address: simAccount.Address.String(),
+			SourceAddress:      simSrcAppAccount.Address.String(),
+			DestinationAddress: simDstAppAccount.Address.String(),
 		}
 
 		// TODO: Handling the TransferApplicationStake simulation

x/application/types/message_transfer_application_stake.go

@@ -6,30 +6,30 @@ import (
 
 var _ cosmostypes.Msg = (*MsgTransferApplicationStake)(nil)
 
-func NewMsgTransferApplicationStake(address string, beneficiary string) *MsgTransferApplicationStake {
+func NewMsgTransferApplicationStake(srcAddr string, dstAddr string) *MsgTransferApplicationStake {
 	return &MsgTransferApplicationStake{
-		Address:     address,
-		Beneficiary: beneficiary,
+		SourceAddress:      srcAddr,
+		DestinationAddress: dstAddr,
 	}
 }
 
 func (msg *MsgTransferApplicationStake) ValidateBasic() error {
-	if msg.Address == "" {
-		return ErrAppInvalidAddress.Wrap("empty application address")
+	if msg.GetSourceAddress() == "" {
+		return ErrAppInvalidAddress.Wrap("empty source application address")
 	}
 
-	if msg.Beneficiary == "" {
-		return ErrAppInvalidAddress.Wrap("empty beneficiary address")
+	if msg.GetDestinationAddress() == "" {
+		return ErrAppInvalidAddress.Wrap("empty destination application address")
 	}
 
-	_, addrErr := cosmostypes.AccAddressFromBech32(msg.Address)
-	if addrErr != nil {
-		return ErrAppInvalidAddress.Wrapf("invalid application address (%s): %v", msg.Address, addrErr)
+	_, srcBech32Err := cosmostypes.AccAddressFromBech32(msg.GetSourceAddress())
+	if srcBech32Err != nil {
+		return ErrAppInvalidAddress.Wrapf("invalid source application address (%s): %v", msg.GetSourceAddress(), srcBech32Err)
 	}
 
-	_, beneficiaryErr := cosmostypes.AccAddressFromBech32(msg.Beneficiary)
-	if beneficiaryErr != nil {
-		return ErrAppInvalidAddress.Wrapf("invalid beneficiary address (%s): %v", msg.Address, beneficiaryErr)
+	_, dstBech32Err := cosmostypes.AccAddressFromBech32(msg.GetDestinationAddress())
+	if dstBech32Err != nil {
+		return ErrAppInvalidAddress.Wrapf("invalid destination application address (%s): %v", msg.GetDestinationAddress(), dstBech32Err)
 	}
 	return nil
 }

x/application/types/message_transfer_application_stake_test.go

@@ -3,37 +3,48 @@ package types
 import (
 	"testing"
 
-	"github.com/pokt-network/poktroll/testutil/sample"
 	"github.com/stretchr/testify/require"
+
+	"github.com/pokt-network/poktroll/testutil/sample"
 )
 
 func TestMsgTransferApplicationStake_ValidateBasic(t *testing.T) {
+	dupAddr := sample.AccAddress()
+
 	tests := []struct {
 		name string
 		msg  MsgTransferApplicationStake
 		err  error
 	}{
 		{
-			name: "invalid application address",
+			name: "invalid duplicate source address",
+			msg: MsgTransferApplicationStake{
+				SourceAddress:      dupAddr,
+				DestinationAddress: dupAddr,
+			},
+			err: ErrAppInvalidAddress,
+		},
+		{
+			name: "invalid bech32 source address",
 			msg: MsgTransferApplicationStake{
-				Address:     "invalid_address",
-				Beneficiary: sample.AccAddress(),
+				SourceAddress:      "invalid_address",
+				DestinationAddress: sample.AccAddress(),
 			},
 			err: ErrAppInvalidAddress,
 		},
 		{
-			name: "invalid beneficiary address",
+			name: "invalid bech32 destination address",
 			msg: MsgTransferApplicationStake{
-				Address:     sample.AccAddress(),
-				Beneficiary: "invalid_address",
+				SourceAddress:      sample.AccAddress(),
+				DestinationAddress: "invalid_address",
 			},
 			err: ErrAppInvalidAddress,
 		},
 		{
-			name: "valid application and beneficiary address",
+			name: "valid source and destination addresses",
 			msg: MsgTransferApplicationStake{
-				Address:     sample.AccAddress(),
-				Beneficiary: sample.AccAddress(),
+				SourceAddress:      sample.AccAddress(),
+				DestinationAddress: sample.AccAddress(),
 			},
 		},
 	}
@@ -42,7 +53,7 @@ func TestMsgTransferApplicationStake_ValidateBasic(t *testing.T) {
 			err := tt.msg.ValidateBasic()
 			if tt.err != nil {
 				require.ErrorIs(t, err, tt.err)
-				require.Contains(t, err.Error(), tt.msg.Address)
+				require.Contains(t, err.Error(), tt.msg.SourceAddress)
 				return
 			}
 			require.NoError(t, err)