-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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): Allow funds to be sent to accounts on init and execute #19360
Changes from all commits
214f388
bba96a8
c126e7a
aa4e234
6ead670
ad6fc02
b4d8a42
e3faf2d
929f73a
46e11c8
37f5d06
3636942
a63550f
a01e49e
c269445
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.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,12 +35,12 @@ func TestAccountAbstraction(t *testing.T) { | |
|
||
_, aaAddr, err := ak.Init(ctx, "aa_minimal", accCreator, &rotationv1.MsgInit{ | ||
PubKeyBytes: privKey.PubKey().Bytes(), | ||
}) | ||
}, nil) | ||
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. The addition of |
||
require.NoError(t, err) | ||
|
||
_, aaFullAddr, err := ak.Init(ctx, "aa_full", accCreator, &rotationv1.MsgInit{ | ||
PubKeyBytes: privKey.PubKey().Bytes(), | ||
}) | ||
}, nil) | ||
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. Similar to the previous comment, the |
||
require.NoError(t, err) | ||
|
||
aaAddrStr, err := app.AuthKeeper.AddressCodec().BytesToString(aaAddr) | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,52 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package accounts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"google.golang.org/protobuf/proto" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
v1beta1 "cosmossdk.io/api/cosmos/base/v1beta1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cosmossdk.io/core/address" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cosmossdk.io/x/accounts/internal/implementation" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sdk "github.com/cosmos/cosmos-sdk/types" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+6
to
+11
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. Consider grouping SDK-related imports together for better readability. import (
"google.golang.org/protobuf/proto"
bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
v1beta1 "cosmossdk.io/api/cosmos/base/v1beta1"
"cosmossdk.io/core/address"
"cosmossdk.io/x/accounts/internal/implementation"
+
sdk "github.com/cosmos/cosmos-sdk/types"
) Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// protoV2GogoWrapper is a wrapper of a protov2 message into a gogo message. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// this is exceptionally allowed to enable accounts to be decoupled from | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// the SDK, since x/accounts can support only protov1 in its APIs. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// But in order to keep it decoupled from the SDK we need to use the API module. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// This is a hack to make an API module type work in x/accounts. Once the SDK | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// has protov2 support, we can migrate internal/implementation/encoding.go to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// work with protov2. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+14
to
+20
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. The comment explains a workaround for protobuf version compatibility. Ensure this is tracked for future removal once the SDK supports protov2 natively. Would you like me to open a GitHub issue to track the removal of this workaround once protov2 support is available in the SDK? 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. yes, please do! 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. @testinginprod: I'm creating a GitHub issue to track the removal of the protobuf version compatibility workaround once protov2 support is available in the SDK. Here's the command I used to create the issue: gh issue create --title "Track Removal of Protobuf Version Compatibility Workaround" --body "This issue tracks the removal of the protobuf version compatibility workaround implemented in `x/accounts/coin_transfer.go`. The workaround is necessary until the Cosmos SDK natively supports protov2.
The relevant discussion can be found in PR https://github.com/cosmos/cosmos-sdk/pull/19360 and the specific comment at https://github.com/cosmos/cosmos-sdk/pull/19360#discussion_r1481734092.
The workaround involves wrapping a protov2 message into a gogo message to decouple `x/accounts` from the SDK while maintaining compatibility. This should be revisited and potentially removed once the SDK has protov2 support." --repo cosmos/cosmos-sdk This issue will ensure that the necessary follow-up is tracked and addressed in due course. Thank you for highlighting the importance of this task! 🐇 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type protoV2GogoWrapper struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gogoProtoPlusV2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (h protoV2GogoWrapper) XXX_MessageName() string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return string(proto.MessageName(h.gogoProtoPlusV2)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func defaultCoinsTransferMsgFunc(addrCdc address.Codec) coinsTransferMsgFunc { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return func(from, to []byte, coins sdk.Coins) (implementation.ProtoMsg, implementation.ProtoMsg, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fromAddr, err := addrCdc.BytesToString(from) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil, nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
toAddr, err := addrCdc.BytesToString(to) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil, nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
v2Coins := make([]*v1beta1.Coin, len(coins)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for i, coin := range coins { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
v2Coins[i] = &v1beta1.Coin{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Denom: coin.Denom, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Amount: coin.Amount.String(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return protoV2GogoWrapper{&bankv1beta1.MsgSend{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
FromAddress: fromAddr, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ToAddress: toAddr, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Amount: v2Coins, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}}, new(bankv1beta1.MsgSendResponse), nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+29
to
+51
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. The if err != nil {
- return nil, nil, err
+ return nil, nil, fmt.Errorf("error converting from address bytes to string: %w", err)
}
...
if err != nil {
- return nil, nil, err
+ return nil, nil, fmt.Errorf("error converting to address bytes to string: %w", err)
} Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of
MsgHandlerRouter
andHasMsgHandler
interfaces introduces a new pattern for registering message handlers. Ensure the documentation for these interfaces is clear and provides enough context for developers to understand their usage.Committable suggestion