fix(accounts): lockup check sender in context not in message (#23621)

Co-authored-by: Alex | Interchain Labs <[email protected]> (cherry picked from commit 5cedd50) # Conflicts: # api/cosmos/accounts/defaults/lockup/v1/tx.pulsar.go # x/accounts/defaults/lockup/continuous_locking_account_test.go # x/accounts/defaults/lockup/periodic_locking_account_test.go # x/accounts/defaults/lockup/permanent_locking_account_test.go
cosmos · Feb 6, 2025 · 8a30aa9 · 8a30aa9
1 parent ee9959b
commit 8a30aa9
Show file tree

Hide file tree

Showing 14 changed files with 5,266 additions and 284 deletions.
diff --git a/api/cosmos/accounts/defaults/lockup/v1/tx.pulsar.go b/api/cosmos/accounts/defaults/lockup/v1/tx.pulsar.go
diff --git a/tests/integration/accounts/lockup/continous_lockup_test_suite.go b/tests/integration/accounts/lockup/continous_lockup_test_suite.go
@@ -46,7 +46,6 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 
 	t.Run("error - execute message, wrong sender", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    addr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -55,7 +54,6 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 	})
 	t.Run("error - execute send message, insufficient fund", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -72,7 +70,6 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 	// Check if token is sendable
 	t.Run("ok - execute send message", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -85,7 +82,6 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 
 	t.Run("ok - execute delegate message", func(t *testing.T) {
 		msg := &types.MsgDelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -108,7 +104,6 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 	})
 	t.Run("ok - execute withdraw reward message", func(t *testing.T) {
 		msg := &types.MsgWithdrawReward{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 		}
 		err = s.executeTx(ctx, msg, app, accountAddr, accOwner)
@@ -119,7 +114,6 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 		require.NoError(t, err)
 		val := vals[0]
 		msg := &types.MsgUndelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -153,7 +147,6 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 	// test if tracking delegate work perfectly
 	t.Run("ok - execute delegate message", func(t *testing.T) {
 		msg := &types.MsgDelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}

diff --git a/tests/integration/accounts/lockup/delayed_lockup_test_suite.go b/tests/integration/accounts/lockup/delayed_lockup_test_suite.go
@@ -45,7 +45,6 @@ func (s *IntegrationTestSuite) TestDelayedLockingAccount() {
 
 	t.Run("error - execute message, wrong sender", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    addr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -54,7 +53,6 @@ func (s *IntegrationTestSuite) TestDelayedLockingAccount() {
 	})
 	t.Run("error - execute send message, insufficient fund", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -63,7 +61,6 @@ func (s *IntegrationTestSuite) TestDelayedLockingAccount() {
 	})
 	t.Run("ok - execute delegate message", func(t *testing.T) {
 		msg := &types.MsgDelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -86,7 +83,6 @@ func (s *IntegrationTestSuite) TestDelayedLockingAccount() {
 	})
 	t.Run("ok - execute withdraw reward message", func(t *testing.T) {
 		msg := &types.MsgWithdrawReward{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 		}
 		err = s.executeTx(ctx, msg, app, accountAddr, accOwner)
@@ -97,7 +93,6 @@ func (s *IntegrationTestSuite) TestDelayedLockingAccount() {
 		require.NoError(t, err)
 		val := vals[0]
 		msg := &types.MsgUndelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -133,7 +128,6 @@ func (s *IntegrationTestSuite) TestDelayedLockingAccount() {
 	// Check if token is sendable after unlock
 	t.Run("ok - execute send message", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}

diff --git a/tests/integration/accounts/lockup/periodic_lockup_test_suite.go b/tests/integration/accounts/lockup/periodic_lockup_test_suite.go
@@ -59,7 +59,6 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 
 	t.Run("error - execute message, wrong sender", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    addr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -69,7 +68,6 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 	// No token being unlocked yet
 	t.Run("error - execute send message, insufficient fund", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -86,7 +84,6 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 	// Check if 500 stake is sendable now
 	t.Run("ok - execute send message", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(500))},
 		}
@@ -105,7 +102,6 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 
 	t.Run("ok - execute delegate message", func(t *testing.T) {
 		msg := &types.MsgDelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -129,7 +125,6 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 	})
 	t.Run("ok - execute withdraw reward message", func(t *testing.T) {
 		msg := &types.MsgWithdrawReward{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 		}
 		err = s.executeTx(ctx, msg, app, accountAddr, accOwner)
@@ -140,7 +135,6 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 		require.NoError(t, err)
 		val := vals[0]
 		msg := &types.MsgUndelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -174,7 +168,6 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 
 	t.Run("ok - execute delegate message", func(t *testing.T) {
 		msg := &types.MsgDelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}

diff --git a/tests/integration/accounts/lockup/permanent_lockup_test_suite.go b/tests/integration/accounts/lockup/permanent_lockup_test_suite.go
@@ -43,7 +43,6 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 
 	t.Run("error - execute message, wrong sender", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    addr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -52,7 +51,6 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 	})
 	t.Run("error - execute send message, insufficient fund", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -61,7 +59,6 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 	})
 	t.Run("ok - execute delegate message", func(t *testing.T) {
 		msg := &types.MsgDelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -84,7 +81,6 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 	})
 	t.Run("ok - execute withdraw reward message", func(t *testing.T) {
 		msg := &types.MsgWithdrawReward{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 		}
 		err = s.executeTx(ctx, msg, app, accountAddr, accOwner)
@@ -95,7 +91,6 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 		require.NoError(t, err)
 		val := vals[0]
 		msg := &types.MsgUndelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(100)),
 		}
@@ -121,7 +116,6 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 
 	t.Run("ok - execute send message", func(t *testing.T) {
 		msg := &types.MsgSend{
-			Sender:    ownerAddrStr,
 			ToAddress: addr,
 			Amount:    sdk.Coins{sdk.NewCoin("stake", math.NewInt(100))},
 		}
@@ -143,7 +137,6 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 
 	t.Run("ok - execute delegate message", func(t *testing.T) {
 		msg := &types.MsgDelegate{
-			Sender:           ownerAddrStr,
 			ValidatorAddress: val.OperatorAddress,
 			Amount:           sdk.NewCoin("stake", math.NewInt(10)),
 		}

diff --git a/x/accounts/defaults/lockup/continuous_locking_account_test.go b/x/accounts/defaults/lockup/continuous_locking_account_test.go
@@ -41,8 +41,12 @@ func TestContinuousAccountDelegate(t *testing.T) {
 
 	acc := setupContinuousAccount(t, sdkCtx, ss)
 	_, err := acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
+<<<<<<< HEAD
 		Sender:           "owner",
 		ValidatorAddress: "val_address",
+=======
+		ValidatorAddress: valAddress,
+>>>>>>> 5cedd5048 (fix(accounts): lockup check sender in context not in message (#23621))
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
 	require.NoError(t, err)
@@ -60,8 +64,12 @@ func TestContinuousAccountDelegate(t *testing.T) {
 	})
 
 	_, err = acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
+<<<<<<< HEAD
 		Sender:           "owner",
 		ValidatorAddress: "val_address",
+=======
+		ValidatorAddress: valAddress,
+>>>>>>> 5cedd5048 (fix(accounts): lockup check sender in context not in message (#23621))
 		Amount:           sdk.NewCoin("test", math.NewInt(5)),
 	})
 	require.NoError(t, err)
@@ -86,7 +94,6 @@ func TestContinuousAccountUndelegate(t *testing.T) {
 	acc := setupContinuousAccount(t, sdkCtx, ss)
 	// Delegate first
 	_, err := acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
@@ -98,7 +105,6 @@ func TestContinuousAccountUndelegate(t *testing.T) {
 
 	// Undelegate
 	_, err = acc.Undelegate(sdkCtx, &lockuptypes.MsgUndelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
@@ -129,7 +135,6 @@ func TestContinuousAccountSendCoins(t *testing.T) {
 
 	acc := setupContinuousAccount(t, sdkCtx, ss)
 	_, err := acc.SendCoins(sdkCtx, &lockuptypes.MsgSend{
-		Sender:    "owner",
 		ToAddress: "receiver",
 		Amount:    sdk.NewCoins(sdk.NewCoin("test", math.NewInt(5))),
 	})
@@ -144,7 +149,6 @@ func TestContinuousAccountSendCoins(t *testing.T) {
 	})
 
 	_, err = acc.SendCoins(sdkCtx, &lockuptypes.MsgSend{
-		Sender:    "owner",
 		ToAddress: "receiver",
 		Amount:    sdk.NewCoins(sdk.NewCoin("test", math.NewInt(5))),
 	})

diff --git a/x/accounts/defaults/lockup/delayed_locking_account_test.go b/x/accounts/defaults/lockup/delayed_locking_account_test.go
@@ -42,7 +42,6 @@ func TestDelayedAccountDelegate(t *testing.T) {
 
 	acc := setupDelayedAccount(t, sdkCtx, ss)
 	_, err := acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
@@ -61,7 +60,6 @@ func TestDelayedAccountDelegate(t *testing.T) {
 	})
 
 	_, err = acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(5)),
 	})
@@ -87,7 +85,6 @@ func TestDelayedAccountUndelegate(t *testing.T) {
 	acc := setupDelayedAccount(t, sdkCtx, ss)
 	// Delegate first
 	_, err := acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
@@ -99,7 +96,6 @@ func TestDelayedAccountUndelegate(t *testing.T) {
 
 	// Undelegate
 	_, err = acc.Undelegate(sdkCtx, &lockuptypes.MsgUndelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
@@ -130,7 +126,6 @@ func TestDelayedAccountSendCoins(t *testing.T) {
 
 	acc := setupDelayedAccount(t, sdkCtx, ss)
 	_, err := acc.SendCoins(sdkCtx, &lockuptypes.MsgSend{
-		Sender:    "owner",
 		ToAddress: "receiver",
 		Amount:    sdk.NewCoins(sdk.NewCoin("test", math.NewInt(5))),
 	})
@@ -145,7 +140,6 @@ func TestDelayedAccountSendCoins(t *testing.T) {
 	})
 
 	_, err = acc.SendCoins(sdkCtx, &lockuptypes.MsgSend{
-		Sender:    "owner",
 		ToAddress: "receiver",
 		Amount:    sdk.NewCoins(sdk.NewCoin("test", math.NewInt(5))),
 	})

diff --git a/x/accounts/defaults/lockup/lockup.go b/x/accounts/defaults/lockup/lockup.go
@@ -128,7 +128,8 @@ func (bva *BaseLockup) Delegate(
 ) (
 	*lockuptypes.MsgExecuteMessagesResponse, error,
 ) {
-	err := bva.checkSender(ctx, msg.Sender)
+	sender := accountstd.Sender(ctx)
+	err := bva.checkSender(ctx, sender)
 	if err != nil {
 		return nil, err
 	}
@@ -183,7 +184,8 @@ func (bva *BaseLockup) Undelegate(
 ) (
 	*lockuptypes.MsgExecuteMessagesResponse, error,
 ) {
-	err := bva.checkSender(ctx, msg.Sender)
+	sender := accountstd.Sender(ctx)
+	err := bva.checkSender(ctx, sender)
 	if err != nil {
 		return nil, err
 	}
@@ -255,7 +257,8 @@ func (bva *BaseLockup) WithdrawReward(
 ) (
 	*lockuptypes.MsgExecuteMessagesResponse, error,
 ) {
-	err := bva.checkSender(ctx, msg.Sender)
+	sender := accountstd.Sender(ctx)
+	err := bva.checkSender(ctx, sender)
 	if err != nil {
 		return nil, err
 	}
@@ -282,7 +285,8 @@ func (bva *BaseLockup) SendCoins(
 ) (
 	*lockuptypes.MsgExecuteMessagesResponse, error,
 ) {
-	err := bva.checkSender(ctx, msg.Sender)
+	sender := accountstd.Sender(ctx)
+	err := bva.checkSender(ctx, sender)
 	if err != nil {
 		return nil, err
 	}
@@ -321,16 +325,12 @@ func (bva *BaseLockup) SendCoins(
 	return &lockuptypes.MsgExecuteMessagesResponse{Responses: resp}, nil
 }
 
-func (bva *BaseLockup) checkSender(ctx context.Context, sender string) error {
+func (bva *BaseLockup) checkSender(ctx context.Context, sender []byte) error {
 	owner, err := bva.Owner.Get(ctx)
 	if err != nil {
 		return sdkerrors.ErrInvalidAddress.Wrapf("invalid owner address: %s", err.Error())
 	}
-	senderBytes, err := bva.addressCodec.StringToBytes(sender)
-	if err != nil {
-		return sdkerrors.ErrInvalidAddress.Wrapf("invalid sender address: %s", err.Error())
-	}
-	if !bytes.Equal(owner, senderBytes) {
+	if !bytes.Equal(owner, sender) {
 		return errors.New("sender is not the owner of this vesting account")
 	}