-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Spike] feegrants in xaccounts #22236
base: main
Are you sure you want to change the base?
Conversation
type Dependencies = implementation.Dependencies | ||
|
||
var ( | ||
// todo: prefix conflicts when embedded in other account type. bump +100 |
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.
We would need some rules to reserve a range for this and other cross-cutting features in the future
@@ -327,10 +331,12 @@ func (a Account) RegisterInitHandler(builder *accountstd.InitBuilder) { | |||
func (a Account) RegisterExecuteHandlers(builder *accountstd.ExecuteBuilder) { | |||
accountstd.RegisterExecuteHandler(builder, a.SwapPubKey) | |||
accountstd.RegisterExecuteHandler(builder, a.Authenticate) // account abstraction | |||
a.Feegrant.RegisterExecuteHandlers(builder) |
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 integration here and in RegisterQueryHandlers would be easy to miss. Not sure what is the best approach here. Maybe via constructor?
} | ||
|
||
granterStr, err := k.addrCdc.BytesToString(granter) | ||
granteeAddrStr, err := k.addrCdc.BytesToString(grantee) |
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 logic was moved to the xaddress type. For production, we would still need all the data and logic until all accounts are migrated
protoimpl "google.golang.org/protobuf/runtime/protoimpl" | ||
anypb "google.golang.org/protobuf/types/known/anypb" | ||
io "io" | ||
reflect "reflect" |
Check notice
Code scanning / CodeQL
Sensitive package import Note
x.Allowance = value.Message().Interface().(*anypb.Any) | ||
default: | ||
if fd.IsExtension() { | ||
panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.feegrant.v1.Grant")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
if fd.IsExtension() { | ||
panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.feegrant.v1.Grant")) | ||
} | ||
panic(fmt.Errorf("message cosmos.accounts.defaults.feegrant.v1.Grant does not contain field %s", fd.FullName())) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
Description
The x/feegrant module let an account store grants to pay fees for other addresses. This is a cross-cutting functionality that can work with all account types. Porting this to the new x/accounts module comes with some challenges.
In this spike, I have implemented
accounts/defaults/feegrant
. The grants are stored within this contract type to replace the global index in x/feegrant.accounts/defaults/base
account as an examplee2e/accounts/feegrant
to store and use a grant for both setup casesNot started:
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...