-
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(hubl): integrate client/v2 signing #17960
Conversation
@julienrbrt your pull request is missing a changelog! |
Haven't tested this against v0.50 nodes. I'm going to finish this first to be able to: julienrbrt/simapp#29 Unless someone knows other remote nodes. |
WalkthroughThis change introduces a new function for keyring management and updates the existing one. It also adds keyring support to the client context and the Builder struct in the remote file. The keyring package from Cosmos SDK is now being used. Changes
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- tools/hubl/go.mod
- tools/hubl/go.sum
Files selected for processing (2)
- tools/hubl/internal/keyring.go (2} hunks)
- tools/hubl/internal/remote.go (2} hunks)
Additional comments: 7
tools/hubl/internal/keyring.go (2)
25-51: The new function
getKeyring
is introduced to create and return asdkkeyring.Keyring
instance. It takes achainName
as an argument and uses it to determine the keyring backend and directory. IfchainName
is empty, it defaults to the global keyring directory. The function also registers interfaces and initializes the codec. This function is a good example of modularity and separation of concerns, as it encapsulates the logic for creating a keyring instance. However, it's important to ensure that error handling is done properly in the upstream code where this function is called.94-97: The
KeyringCmd
function is updated to usesdkkeyring.New
instead ofkeyring.New
to create a keyring instance. This change is consistent with the introduction of thegetKeyring
function and the shift towards usingsdkkeyring.Keyring
. Ensure that this change doesn't break any existing functionality that relies on theKeyringCmd
function.tools/hubl/internal/remote.go (5)
21-21: The import of the "github.com/cosmos/cosmos-sdk/crypto/keyring" package is a new addition. Ensure that this package is used in the code and that it is compatible with the existing codebase.
70-78: The
getKeyring
function is being called to get a keyring for the chain. Ensure that error handling for this function is done correctly and that the keyring is used appropriately in the rest of the code.75-78: The
keyring.NewAutoCLIKeyring
function is being called to create an auto CLI keyring. Ensure that error handling for this function is done correctly and that the auto CLI keyring is used appropriately in the rest of the code.80-84: The
clientCtx
is being updated with theWithKeyring
method call. This is a new addition and should be verified to ensure that it is compatible with the existing codebase and that it does not introduce any breaking changes.86-98: The
Keyring
field is being added to theBuilder
struct. This is a new addition and should be verified to ensure that it is compatible with the existing codebase and that it does not introduce any breaking changes.
Leaving dnm. Still haven't had time to test against a v0.50 node and it should use latest client/v2 as there is fixes there since since PR. I will do that soonish. |
Description
Blocked on #17913Author 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
make lint
andmake test
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 changeSummary by CodeRabbit