-
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(client): add WithFromName
to tx factory
#18852
Conversation
WalkthroughThe Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
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.
Makes sense, one nit
@@ -189,6 +190,12 @@ func (f Factory) WithKeybase(keybase keyring.Keyring) Factory { | |||
return f | |||
} | |||
|
|||
// WithFromName returns a copy of the Factory with updated fromName |
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 get this more descriptive and explain that it should most likely be clientCtx.FromName?
WithFromName
to tx factory
Can we add a changelog under Improvements as well? |
// WithFromName returns a copy of the Factory with updated fromName | ||
// fromName will be use for building a simulation tx. |
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.
Consider enhancing the comment for the WithFromName
method to explain its use case more clearly, such as indicating that it is used for building simulation transactions, potentially with the clientCtx.FromName
as the source of the name.
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.
lgtm
(cherry picked from commit 64dbee6) # Conflicts: # CHANGELOG.md
(cherry picked from commit 64dbee6) # Conflicts: # CHANGELOG.md
) Co-authored-by: gsai967 <[email protected]> Co-authored-by: Julien Robert <[email protected]>
) Co-authored-by: gsai967 <[email protected]> Co-authored-by: Julien Robert <[email protected]>
cosmos#18854) Co-authored-by: gsai967 <[email protected]> Co-authored-by: Julien Robert <[email protected]>
cosmos#18854) Co-authored-by: gsai967 <[email protected]> Co-authored-by: Julien Robert <[email protected]>
Description
Closes: #XXXX
In recent 0.47.7 version tx.Factory added new feild
fromName
, but it is only only set byNewFactoryCLI
we need to set
fromName
usingwithFromName()
because we need use this to simulate the txn without usingNewFactoryCLI
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.
I have...