-
Notifications
You must be signed in to change notification settings - Fork 4.1k
feat(accounts): re-introduce bundler #21562
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
Changes from 17 commits
1de73f3
3c62183
5ea5e72
fdda554
2714080
0db0437
d6bd905
aaecd12
1f08384
5769d1f
71b8cd8
9d155a2
a9df0a4
646f29a
501fff7
ad28604
7252890
3af41ef
7603361
1d76d91
d9972c0
8d347c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
This file was deleted.
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct the order of arguments in In the 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
Suggested change
|
||||||||||
| }) | ||||||||||
|
|
||||||||||
| 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 | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct the order of arguments in Similarly, in line 200, the arguments in 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
Suggested change
|
||||||||||
|
|
||||||||||
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 | ||||||||||
| } | ||||||||||
Uh oh!
There was an error while loading. Please reload this page.