-
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
feat(x/tx): implement SIGN_MODE_DIRECT_AUX handler #15380
Conversation
} | ||
|
||
if options.SignersContext == nil { | ||
h.signersContext = signing.NewGetSignersContext(signing.GetSignersOptions{}) |
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 should be able to pass the file resolver into these options. I think it just requires us changing the options struct to use the correct interface
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.
Can you explain a bit more what you mean?
Here a user could set GetSignersContext.protoFiles to whatever they want to by constructing it themselves and passing it in.
Edit: Are you saying that GetSignersContext
ought to type protoFiles as the interface protodesc.Resolver
instead of protoregistry.Files
?
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.
Yeah, but if they pass in FileResolver
but not GetSignersContext
, the GetSignersContext
should be constructed with that FileResolver
. Generally FileResolver
and TypeResolver
should be propagated to any component that needs them because the probuf types in a scope should all be the same.
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.
Ok this will be a refactor to GetSignersContext then, right?
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.
Yes a bit. We probably need a new interface that extends FileResolver
…-sdk into kocubinski/sign-direct-aux
Description
Closes: #15169
This patch ports logic from x/auth/tx/direct_aux.go and x/auth/tx/direct_aux_test.go to this new API.
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...
!
to 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.
I have...
!
in the type prefix if API or client breaking change