Skip to content
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.

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

feat(accounts): re-introduce bundler #21562

Merged
merged 22 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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

Large diffs are not rendered by default.

660 changes: 523 additions & 137 deletions api/cosmos/accounts/v1/tx.pulsar.go

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import (
"cosmossdk.io/x/staking"
stakingkeeper "cosmossdk.io/x/staking/keeper"
stakingtypes "cosmossdk.io/x/staking/types"
txdecode "cosmossdk.io/x/tx/decode"
"cosmossdk.io/x/tx/signing"
"cosmossdk.io/x/upgrade"
upgradekeeper "cosmossdk.io/x/upgrade/keeper"
Expand Down Expand Up @@ -213,6 +214,13 @@ func NewSimApp(
appCodec := codec.NewProtoCodec(interfaceRegistry)
legacyAmino := codec.NewLegacyAmino()
signingCtx := interfaceRegistry.SigningContext()
txDecoder, err := txdecode.NewDecoder(txdecode.Options{
testinginprod marked this conversation as resolved.
Show resolved Hide resolved
SigningContext: signingCtx,
ProtoCodec: appCodec,
})
if err != nil {
panic(err)
}
txConfig := authtx.NewTxConfig(appCodec, signingCtx.AddressCodec(), signingCtx.ValidatorAddressCodec(), authtx.DefaultSignModes)

govModuleAddr, err := signingCtx.AddressCodec().BytesToString(authtypes.NewModuleAddress(govtypes.ModuleName))
Expand Down Expand Up @@ -305,6 +313,7 @@ func NewSimApp(
runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), logger.With(log.ModuleKey, "x/accounts"), runtime.EnvWithMsgRouterService(app.MsgServiceRouter()), runtime.EnvWithQueryRouterService(app.GRPCQueryRouter())),
signingCtx.AddressCodec(),
appCodec.InterfaceRegistry(),
txDecoder,
// TESTING: do not add
accountstd.AddAccount("counter", counter.NewAccount),
accountstd.AddAccount("aa_minimal", account_abstraction.NewMinimalAbstractedAccount),
Expand Down
103 changes: 0 additions & 103 deletions tests/e2e/accounts/account_abstraction_test.go

This file was deleted.

261 changes: 261 additions & 0 deletions tests/integration/accounts/bundler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
package accounts

import (
"context"
"fmt"
"testing"

gogoproto "github.com/cosmos/gogoproto/proto"
"github.com/stretchr/testify/require"

account_abstractionv1 "cosmossdk.io/x/accounts/interfaces/account_abstraction/v1"
banktypes "cosmossdk.io/x/bank/types"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
)

func TestMsgServer_ExecuteBundle(t *testing.T) {
t.Run("bundle success", func(t *testing.T) {
f := initFixture(t, func(ctx context.Context, msg *account_abstractionv1.MsgAuthenticate) (*account_abstractionv1.MsgAuthenticateResponse, error) {
return &account_abstractionv1.MsgAuthenticateResponse{}, nil
})

recipient := f.mustAddr([]byte("recipient"))
feeAmt := sdk.NewInt64Coin("atom", 100)
sendAmt := sdk.NewInt64Coin("atom", 200)

f.mint(f.mockAccountAddress, feeAmt, sendAmt)

tx := makeTx(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: recipient,
Amount: sdk.NewCoins(sendAmt),
}, []byte("pass"), &account_abstractionv1.TxExtension{
AuthenticationGasLimit: 2400,
BundlerPaymentMessages: []*codectypes.Any{wrapAny(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: f.bundler,
Amount: sdk.NewCoins(feeAmt),
})},
BundlerPaymentGasLimit: 30000,
ExecutionGasLimit: 30000,
})

bundleResp := f.runBundle(tx)
require.Len(t, bundleResp.Responses, 1)

txResp := bundleResp.Responses[0]

require.Empty(t, txResp.Error)
require.NotZero(t, txResp.AuthenticationGasUsed)
require.NotZero(t, txResp.BundlerPaymentGasUsed)
require.NotZero(t, txResp.ExecutionGasUsed)

// asses responses
require.Len(t, txResp.BundlerPaymentResponses, 1)
require.Equal(t, txResp.BundlerPaymentResponses[0].TypeUrl, "/cosmos.bank.v1beta1.MsgSendResponse")

require.Len(t, txResp.ExecutionResponses, 1)
require.Equal(t, txResp.ExecutionResponses[0].TypeUrl, "/cosmos.bank.v1beta1.MsgSendResponse")

// ensure sends have happened
require.Equal(t, f.balance(f.bundler, feeAmt.Denom), feeAmt)
require.Equal(t, f.balance(recipient, sendAmt.Denom), sendAmt)
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the order of arguments in require.Equal

In the require.Equal function, the expected value should be the first argument after t, followed by the actual value. Currently, the arguments are reversed.

Apply this diff to fix the argument order:

-require.Equal(t, f.balance(f.bundler, feeAmt.Denom), feeAmt)
+require.Equal(t, feeAmt, f.balance(f.bundler, feeAmt.Denom))

-require.Equal(t, f.balance(recipient, sendAmt.Denom), sendAmt)
+require.Equal(t, sendAmt, f.balance(recipient, sendAmt.Denom))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
require.Equal(t, f.balance(f.bundler, feeAmt.Denom), feeAmt)
require.Equal(t, f.balance(recipient, sendAmt.Denom), sendAmt)
require.Equal(t, feeAmt, f.balance(f.bundler, feeAmt.Denom))
require.Equal(t, sendAmt, f.balance(recipient, sendAmt.Denom))

})

t.Run("tx fails at auth step", func(t *testing.T) {
f := initFixture(t, func(ctx context.Context, msg *account_abstractionv1.MsgAuthenticate) (*account_abstractionv1.MsgAuthenticateResponse, error) {
return &account_abstractionv1.MsgAuthenticateResponse{}, fmt.Errorf("sentinel")
})
recipient := f.mustAddr([]byte("recipient"))
feeAmt := sdk.NewInt64Coin("atom", 100)
sendAmt := sdk.NewInt64Coin("atom", 200)
f.mint(f.mockAccountAddress, feeAmt, sendAmt)

tx := makeTx(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: recipient,
Amount: sdk.NewCoins(sendAmt),
}, []byte("pass"), &account_abstractionv1.TxExtension{
AuthenticationGasLimit: 2400,
BundlerPaymentMessages: []*codectypes.Any{wrapAny(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: f.bundler,
Amount: sdk.NewCoins(feeAmt),
})},
BundlerPaymentGasLimit: 30000,
ExecutionGasLimit: 30000,
})

bundleResp := f.runBundle(tx)

require.Len(t, bundleResp.Responses, 1)

txResp := bundleResp.Responses[0]
require.NotEmpty(t, txResp.Error)
require.Contains(t, txResp.Error, "sentinel")
require.NotZero(t, txResp.AuthenticationGasUsed)
require.Zero(t, txResp.BundlerPaymentGasUsed)
require.Zero(t, txResp.ExecutionGasUsed)
require.Empty(t, txResp.BundlerPaymentResponses)
require.Empty(t, txResp.ExecutionResponses)

// ensure auth side effects are not persisted in case of failures
})

t.Run("tx fails at pay bundler step", func(t *testing.T) {
f := initFixture(t, func(ctx context.Context, msg *account_abstractionv1.MsgAuthenticate) (*account_abstractionv1.MsgAuthenticateResponse, error) {
return &account_abstractionv1.MsgAuthenticateResponse{}, nil
})

recipient := f.mustAddr([]byte("recipient"))
feeAmt := sdk.NewInt64Coin("atom", 100)
sendAmt := sdk.NewInt64Coin("atom", 200)

f.mint(f.mockAccountAddress, feeAmt, sendAmt)

tx := makeTx(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: recipient,
Amount: sdk.NewCoins(sendAmt),
}, []byte("pass"), &account_abstractionv1.TxExtension{
AuthenticationGasLimit: 2400,
BundlerPaymentMessages: []*codectypes.Any{
wrapAny(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: f.bundler,
Amount: sdk.NewCoins(feeAmt.AddAmount(feeAmt.Amount.AddRaw(100))),
}),
wrapAny(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: f.bundler,
Amount: sdk.NewCoins(feeAmt.AddAmount(feeAmt.Amount.AddRaw(30000))),
}),
},
BundlerPaymentGasLimit: 30000,
ExecutionGasLimit: 30000,
})

bundleResp := f.runBundle(tx)
require.Len(t, bundleResp.Responses, 1)

txResp := bundleResp.Responses[0]

require.NotEmpty(t, txResp.Error)
require.Contains(t, txResp.Error, "bundler payment failed")
require.NotZero(t, txResp.AuthenticationGasUsed)
require.NotZero(t, txResp.BundlerPaymentGasUsed)

require.Empty(t, txResp.BundlerPaymentResponses)
require.Zero(t, txResp.ExecutionGasUsed)
require.Empty(t, txResp.ExecutionResponses)

// ensure bundler payment side effects are not persisted
require.True(t, f.balance(f.bundler, feeAmt.Denom).IsZero())
})

t.Run("tx fails at execution step", func(t *testing.T) {
f := initFixture(t, func(ctx context.Context, msg *account_abstractionv1.MsgAuthenticate) (*account_abstractionv1.MsgAuthenticateResponse, error) {
return &account_abstractionv1.MsgAuthenticateResponse{}, nil
})

recipient := f.mustAddr([]byte("recipient"))
feeAmt := sdk.NewInt64Coin("atom", 100)
sendAmt := sdk.NewInt64Coin("atom", 40000) // this fails

f.mint(f.mockAccountAddress, feeAmt)

tx := makeTx(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: recipient,
Amount: sdk.NewCoins(sendAmt),
}, []byte("pass"), &account_abstractionv1.TxExtension{
AuthenticationGasLimit: 2400,
BundlerPaymentMessages: []*codectypes.Any{
wrapAny(t, &banktypes.MsgSend{
FromAddress: f.mustAddr(f.mockAccountAddress),
ToAddress: f.bundler,
Amount: sdk.NewCoins(feeAmt),
}),
},
BundlerPaymentGasLimit: 30000,
ExecutionGasLimit: 30000,
})

bundleResp := f.runBundle(tx)
require.Len(t, bundleResp.Responses, 1)

txResp := bundleResp.Responses[0]

require.NotEmpty(t, txResp.Error)
require.Contains(t, txResp.Error, "execution failed")

require.NotZero(t, txResp.AuthenticationGasUsed)

require.NotZero(t, txResp.BundlerPaymentGasUsed)
require.NotEmpty(t, txResp.BundlerPaymentResponses)
require.Equal(t, f.balance(f.bundler, feeAmt.Denom), feeAmt) // ensure bundler payment side effects are persisted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the order of arguments in require.Equal

Similarly, in line 200, the arguments in require.Equal are reversed. The expected value should come first.

Apply this diff to fix the argument order:

-require.Equal(t, f.balance(f.bundler, feeAmt.Denom), feeAmt) // ensure bundler payment side effects are persisted
+require.Equal(t, feeAmt, f.balance(f.bundler, feeAmt.Denom)) // ensure bundler payment side effects are persisted
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
require.Equal(t, f.balance(f.bundler, feeAmt.Denom), feeAmt) // ensure bundler payment side effects are persisted
require.Equal(t, feeAmt, f.balance(f.bundler, feeAmt.Denom)) // ensure bundler payment side effects are persisted


require.NotZero(t, txResp.ExecutionGasUsed)
require.Empty(t, txResp.ExecutionResponses)

// ensure execution side effects are not persisted
// aka recipient must not have money
require.True(t, f.balance(recipient, feeAmt.Denom).IsZero())
})
Comment on lines +20 to +208
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor tests using a table-driven approach to reduce duplication

The sub-tests within TestMsgServer_ExecuteBundle contain duplicated code for initializing fixtures, variables, and transactions. Refactoring these tests into a table-driven format can improve maintainability and make it easier to add new test cases.

Here's an example of how you might restructure the tests:

func TestMsgServer_ExecuteBundle(t *testing.T) {
	tests := []struct {
		name             string
		authResponse     func(ctx context.Context, msg *account_abstractionv1.MsgAuthenticate) (*account_abstractionv1.MsgAuthenticateResponse, error)
		feeAmt           sdk.Coin
		sendAmt          sdk.Coin
		mintCoins        []sdk.Coin
		modifySendAmt    bool
		expectedError    string
		verifyBalances   func(t *testing.T, f *fixture)
		verifyTxResponse func(t *testing.T, txResp *account_abstractionv1.TxResponse)
	}{
		{
			name: "bundle success",
			authResponse: func(ctx context.Context, msg *account_abstractionv1.MsgAuthenticate) (*account_abstractionv1.MsgAuthenticateResponse, error) {
				return &account_abstractionv1.MsgAuthenticateResponse{}, nil
			},
			feeAmt:    sdk.NewInt64Coin("atom", 100),
			sendAmt:   sdk.NewInt64Coin("atom", 200),
			mintCoins: []sdk.Coin{sdk.NewInt64Coin("atom", 100), sdk.NewInt64Coin("atom", 200)},
			expectedError: "",
			verifyBalances: func(t *testing.T, f *fixture) {
				feeAmt := sdk.NewInt64Coin("atom", 100)
				sendAmt := sdk.NewInt64Coin("atom", 200)
				require.Equal(t, feeAmt, f.balance(f.bundler, feeAmt.Denom))
				require.Equal(t, sendAmt, f.balance(f.mustAddr([]byte("recipient")), sendAmt.Denom))
			},
		},
		{
			name: "tx fails at auth step",
			authResponse: func(ctx context.Context, msg *account_abstractionv1.MsgAuthenticate) (*account_abstractionv1.MsgAuthenticateResponse, error) {
				return &account_abstractionv1.MsgAuthenticateResponse{}, fmt.Errorf("sentinel")
			},
			feeAmt:        sdk.NewInt64Coin("atom", 100),
			sendAmt:       sdk.NewInt64Coin("atom", 200),
			mintCoins:     []sdk.Coin{sdk.NewInt64Coin("atom", 100), sdk.NewInt64Coin("atom", 200)},
			expectedError: "sentinel",
			verifyBalances: func(t *testing.T, f *fixture) {
				require.True(t, f.balance(f.bundler, "atom").IsZero())
			},
		},
		// Additional test cases...
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			f := initFixture(t, tt.authResponse)
			recipient := f.mustAddr([]byte("recipient"))
			f.mint(f.mockAccountAddress, tt.mintCoins...)

			tx := makeTx(t, &banktypes.MsgSend{
				FromAddress: f.mustAddr(f.mockAccountAddress),
				ToAddress:   recipient,
				Amount:      sdk.NewCoins(tt.sendAmt),
			}, []byte("pass"), &account_abstractionv1.TxExtension{
				AuthenticationGasLimit: 2400,
				BundlerPaymentMessages: []*codectypes.Any{wrapAny(t, &banktypes.MsgSend{
					FromAddress: f.mustAddr(f.mockAccountAddress),
					ToAddress:   f.bundler,
					Amount:      sdk.NewCoins(tt.feeAmt),
				})},
				BundlerPaymentGasLimit: 30000,
				ExecutionGasLimit:      30000,
			})

			bundleResp := f.runBundle(tx)
			require.Len(t, bundleResp.Responses, 1)
			txResp := bundleResp.Responses[0]

			if tt.expectedError != "" {
				require.NotEmpty(t, txResp.Error)
				require.Contains(t, txResp.Error, tt.expectedError)
			} else {
				require.Empty(t, txResp.Error)
			}

			if tt.verifyBalances != nil {
				tt.verifyBalances(t, f)
			}

			if tt.verifyTxResponse != nil {
				tt.verifyTxResponse(t, txResp)
			}
		})
	}
}

}

func makeTx(t *testing.T, msg gogoproto.Message, sig []byte, xt *account_abstractionv1.TxExtension) []byte {
anyMsg, err := codectypes.NewAnyWithValue(msg)
require.NoError(t, err)

anyXt, err := codectypes.NewAnyWithValue(xt)
require.NoError(t, err)

tx := &txtypes.Tx{
Body: &txtypes.TxBody{
Messages: []*codectypes.Any{anyMsg},
Memo: "",
TimeoutHeight: 0,
Unordered: false,
TimeoutTimestamp: nil,
ExtensionOptions: []*codectypes.Any{anyXt},
NonCriticalExtensionOptions: nil,
},
AuthInfo: &txtypes.AuthInfo{
SignerInfos: []*txtypes.SignerInfo{
{
PublicKey: nil,
ModeInfo: &txtypes.ModeInfo{Sum: &txtypes.ModeInfo_Single_{Single: &txtypes.ModeInfo_Single{Mode: signingtypes.SignMode_SIGN_MODE_UNSPECIFIED}}},
Sequence: 0,
},
},
Fee: nil,
},
Signatures: [][]byte{sig},
}

bodyBytes, err := tx.Body.Marshal()
require.NoError(t, err)

authInfoBytes, err := tx.AuthInfo.Marshal()
require.NoError(t, err)

txRaw, err := (&txtypes.TxRaw{
BodyBytes: bodyBytes,
AuthInfoBytes: authInfoBytes,
Signatures: tx.Signatures,
}).Marshal()
require.NoError(t, err)
return txRaw
}

func wrapAny(t *testing.T, msg gogoproto.Message) *codectypes.Any {
t.Helper()
any, err := codectypes.NewAnyWithValue(msg)
require.NoError(t, err)
return any
}
Loading
Loading