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

Co-authored-by: Alex | Interchain Labs <[email protected]>
cosmos · Feb 6, 2025 · 5cedd50 · 5cedd50
1 parent f7f301d
commit 5cedd50
Show file tree

Hide file tree

Showing 14 changed files with 230 additions and 710 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/v2/accounts/lockup/continous_lockup_test_suite.go b/tests/integration/v2/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))},
 		}
@@ -70,7 +68,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))},
 		}
@@ -82,7 +79,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)),
 		}
@@ -105,7 +101,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, s.accountsKeeper, accountAddr, accOwner)
@@ -116,7 +111,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)),
 		}
@@ -148,7 +142,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/v2/accounts/lockup/delayed_lockup_test_suite.go b/tests/integration/v2/accounts/lockup/delayed_lockup_test_suite.go
@@ -46,7 +46,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))},
 		}
@@ -55,7 +54,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))},
 		}
@@ -64,7 +62,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)),
 		}
@@ -87,7 +84,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, s.accountsKeeper, accountAddr, accOwner)
@@ -98,7 +94,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)),
 		}
@@ -131,7 +126,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/v2/accounts/lockup/periodic_lockup_test_suite.go b/tests/integration/v2/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))},
 		}
@@ -84,7 +82,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))},
 		}
@@ -101,7 +98,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)),
 		}
@@ -124,7 +120,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, s.accountsKeeper, accountAddr, accOwner)
@@ -135,7 +130,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)),
 		}
@@ -167,7 +161,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/v2/accounts/lockup/permanent_lockup_test_suite.go b/tests/integration/v2/accounts/lockup/permanent_lockup_test_suite.go
@@ -44,7 +44,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))},
 		}
@@ -53,7 +52,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))},
 		}
@@ -62,7 +60,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)),
 		}
@@ -85,7 +82,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, s.accountsKeeper, accountAddr, accOwner)
@@ -96,7 +92,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)),
 		}
@@ -122,7 +117,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))},
 		}
@@ -142,7 +136,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
@@ -43,7 +43,6 @@ func TestContinuousAccountDelegate(t *testing.T) {
 
 	acc := setupContinuousAccount(t, sdkCtx, ss)
 	_, err := acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
@@ -62,7 +61,6 @@ func TestContinuousAccountDelegate(t *testing.T) {
 	})
 
 	_, err = acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(5)),
 	})
@@ -86,7 +84,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 +95,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 +125,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 +139,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
@@ -40,7 +40,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)),
 	})
@@ -59,7 +58,6 @@ func TestDelayedAccountDelegate(t *testing.T) {
 	})
 
 	_, err = acc.Delegate(sdkCtx, &lockuptypes.MsgDelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(5)),
 	})
@@ -83,7 +81,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)),
 	})
@@ -95,7 +92,6 @@ func TestDelayedAccountUndelegate(t *testing.T) {
 
 	// Undelegate
 	_, err = acc.Undelegate(sdkCtx, &lockuptypes.MsgUndelegate{
-		Sender:           "owner",
 		ValidatorAddress: valAddress,
 		Amount:           sdk.NewCoin("test", math.NewInt(1)),
 	})
@@ -126,7 +122,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))),
 	})
@@ -141,7 +136,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")
 	}