Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aljo242 merged 10 commits into from
Feb 6, 2025
Merged

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

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4010ecc
fix account lockup
tac0turtle Feb 6, 2025
89072df
cleanup
tac0turtle Feb 6, 2025
fc30bfc
test cases
tac0turtle Feb 6, 2025
7bbfc12
add system test
tac0turtle Feb 6, 2025
5c9b493
remove print statement
tac0turtle Feb 6, 2025
78d0911
replace unit test with systemtest
tac0turtle Feb 6, 2025
bbe53d0
fix integration tests
tac0turtle Feb 6, 2025
efd7bde
remove system test
tac0turtle Feb 6, 2025
99707ac
Merge branch 'main' into marko/lockup_account_fix
Feb 6, 2025
419ecf4
Merge branch 'main' into marko/lockup_account_fix
Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
526 changes: 110 additions & 416 deletions api/cosmos/accounts/defaults/lockup/v1/tx.pulsar.go
View file
Open in desktop

Large diffs are not rendered by default.

7 changes: 0 additions & 7 deletions tests/integration/v2/accounts/lockup/continous_lockup_test_suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))},
}
Expand All @@ -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))},
}
Expand All @@ -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))},
}
Expand All @@ -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)),
}
Expand All @@ -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)
Expand All @@ -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)),
}
Expand Down Expand Up @@ -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)),
}
Expand Down
6 changes: 0 additions & 6 deletions tests/integration/v2/accounts/lockup/delayed_lockup_test_suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))},
}
Expand All @@ -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))},
}
Expand All @@ -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)),
}
Expand All @@ -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)
Expand All @@ -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)),
}
Expand Down Expand Up @@ -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))},
}
Expand Down
7 changes: 0 additions & 7 deletions tests/integration/v2/accounts/lockup/periodic_lockup_test_suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))},
}
Expand All @@ -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))},
}
Expand All @@ -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))},
}
Expand All @@ -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)),
}
Expand All @@ -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)
Expand All @@ -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)),
}
Expand Down Expand Up @@ -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)),
}
Expand Down
7 changes: 0 additions & 7 deletions tests/integration/v2/accounts/lockup/permanent_lockup_test_suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))},
}
Expand All @@ -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))},
}
Expand All @@ -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)),
}
Expand All @@ -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)
Expand All @@ -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)),
}
Expand All @@ -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))},
}
Expand All @@ -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)),
}
Expand Down
6 changes: 0 additions & 6 deletions x/accounts/defaults/lockup/continuous_locking_account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
})
Expand All @@ -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)),
})
Expand All @@ -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)),
})
Expand All @@ -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)),
})
Expand Down Expand Up @@ -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))),
})
Expand All @@ -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))),
})
Expand Down
6 changes: 0 additions & 6 deletions x/accounts/defaults/lockup/delayed_locking_account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
})
Expand All @@ -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)),
})
Expand All @@ -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)),
})
Expand All @@ -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)),
})
Expand Down Expand Up @@ -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))),
})
Expand All @@ -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))),
})
Expand Down
20 changes: 10 additions & 10 deletions x/accounts/defaults/lockup/lockup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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")
}

Expand Down
Loading
Loading