cli

cli copied to clipboard

Learn more about the bounty on Ignite's Discord.

Introduce a new command set called ignite node.

Implement these commands under:

• ignite node query bank balances - we could set q as an alias to query
• ignite node tx bank send

Please note that we need to define their respective top level commands for the above commands.

In the ignite/cmd package we should create a file for each command:

cmd/
    node.go 
    node_query.go
    node_query_bank.go
    node_query_bank_balances.go
    node_tx.go
    node_tx_bank.go
    node_tx_bank_send.go

Flags:

• ignite node command set should have an --rpc flag. Value of this should be localhost:26657 by default (Tendermint RPC endpoint of the chain). -used by cosmosclient

• --keyring-backend for ignite node tx. -used by cosmosclient/cosmosaccount

• --home for ignite node tx -used by cosmosclient/cosmosaccount

• --from for ignite node tx: set "default" as the default value. -used by cosmosclient/cosmosaccount

• for the ignite node q bank balances take a look at the gaiad q bank balances to see if we need to add more flags. For ex. we need to add the ones related to pagination and filtering.

You need to initialize ignite/pkg/cosmosclient inside ignite/cmd and have all the business logic implement there for these new commands.

So, we possibly need to implement these new funcs in cosmosclient and cosmosaccount packages (open to discussions):

// in `pkg/cosmosaccount`
// implement following to create a vanila `cosmosaccount.Account` from an account address
// we will need this function for `cosmosclient.BankBalances` and `cosmosclient.BankSend` when an account address
// directly provided. If the account name is provided, we use `cosmosclient.Client.Account()` func to get the account.
cosmosaccount.ToAccount(address string) (cosmosaccount.Account, error)

// in `pkg/cosmosclient`
cosmosclient.BankBalances(account cosmosaccount.Account) (cosmosclient.Balances, error)

// in `pkg/cosmosclient`
// `fromAccountName` to be passed to `cosmosclient.BroadcastTx`
cosmosclient.BankSend(fromAccountName string, toAccount cosmosaccount.Account, amount sdktypes.Coins) error

Call these new functions in the cmd/ package and print their outputs by using the cliui package. For this please do not forget initializing cliui. Sample here. We may use the clui.PrintTable for printing the balances. And just an OK message for the send with the info about how many tokens sent. Please check cliui.Printf for that. Sample here.

The ignite network command also has a setup similar this to use cosmosclient, you can get inspiration from there:

These commands should be able to be used for any chain. To be able to sign and broadcast transactions properly, we may need to determine and use the correct account address (account specified through the --from flag). Determining means, we need to find out the prefix of the address. Maybe this can be found by using the RPC APIs. If not, let's also introduce a --address-prefix flag to ignite node tx command set. After we know the prefix, we need to set it to cosmosclient.

Sample usages:

• ignite chain q bank balances [cosmos... / or / account-name ] --node [https://rpc.cosmos.network:443](https://rpc.cosmos.network/)
• ignite chain tx bank send alice [cosmos... / or / account-name ] 1uatom --from alice --prefix cosmos --node https://rpc.cosmos.network:443

account-name above and also the value of --from taken from the account names when you use ignite account command set.

These balances and send commands pretty similar to the ones in gaiad but with an exception that we need to make them compatible with all Cosmos-SDK based chains and make it use accounts from ignite account.

Make sure to add an integration test that will:

• Scaffold a chain with a custom address prefix
• ignite node tx bank send: Send tokens to another account from ignite account
• ignite node q bank balances: Assert the balances of the sender and received to see they match after send.

There is a lot of similarities in this to what the lens project is doing (minus the multi-chain aspect). I wonder if there are some tricks or potentially re-usability we can leverage from that project? https://github.com/strangelove-ventures/lens

I believe in reusability it could be great if can be used but meanwhile I have some concerns:

• SDK does not fully comply semver. This could potentially create problems in the long run if lens' SDK version do not match with what we use and if there are breaking changes in between
• We need to be able to attach ignite account to it. (CLI's cosmosclient is totally compatible with that)
• We need to be able to enforce the arguments/flags syntax that's being proposed by this issue
• They might be panicing or handling errors differently in their inner packages or manipulating the globals in the SDK that may conflicts with the ones in the CLI
• Whenever we need to add a new functionality and if this wasn't exists in the lens, we need to create a PR to them possibly get blocked during the process
• Do they depend on a config file of their own/ can we bypass that or it's hard coded?

Yeah, might be that it won't work directly to reuse. And/or that it might add dependencies that can cause problems down the line. Worst case scenario, maybe there are some things they've done we can borrow/get inspiration for to do the same things they are :)

@bjaanes Hey hey! You have been assigned, thank you for showing interest in this bounty!

Pretty exciting that we'll have a new command set! :star_struck:

cc @okwme

@ilgooz I was not able to find a reliable way to get the bech32 prefix through rpc. The closest I got was querying auth accounts which prints those addresses. I am not sure if this command also list validator accounts as well and would potentially be a bit clumsy.

I'm guessing maybe just having a flag is the easiest (and safest) option. If you prefer fetching accounts and parsing them I don't mind that either.

In the case that you prefer the account-prefix flag: should it be mandatory or default to "cosmos"?

@bjaanes yep I think a prefix flag is totally fine and by default it would be cosmos.

@ilgooz I've got this working fairly well, but I wanted to know about the behavior of the cli vs a typical chain cli command. Normally, when sending txs, the cli asks for confirmation before sending broadcasting the actual transaction. Do we want to do the same? It looks like there is not implemented anything like that for the network commands?

There is also a lot of flags for most tx commands, do we know how deep we want support to be here? I can imagine perhaps flags for gas and fees? But what about for instance ledger, offline (i.e., not send the tx, only output)?

Edit: Addition to this question. Would it make sense to try to reuse the cosmos-sdk flags for queries and transactions, or would you prefer to re-implement them? https://github.com/cosmos/cosmos-sdk/blob/main/client/flags/flags.go

I think we can keep it simple for now, and directly broadcast the TX without a confirmation. But as you say, we can add a flag to set the mode to only print the TX I think.

We can have the gas and fees flags but no need for broadcast type.

I think we can just define the flags in the CLI, they may not necessarily follow the same signature with SDK's.

Ledger support will be added later to the ignite accounts cmd.

@ilgooz That makes sense!

I was wondering if it would be OK to add a flag to the ignite account commands to be able to specify the home folder of the keyring? For instance the typical keys flag of keyring-dir? The reason I want this is to be able to create accounts during testing and cleaning them up. If you have a better option to do this, please let me know.

Hey, we already have a home dir set for it here. It's just not customizable through flags because I guess there is no reason for it to be since it meant to be used for any chain. How do you feel about it?

@bjaanes if you mean unit testing (rather than manual testing), cosmosaccount also supports in memory backend.

@bjaanes if you mean unit testing (rather than manual testing), cosmosaccount also supports in memory backend.

Sorry, I should have been more clear. I meant for integration testing. In memory doesn't really work for that.

Just for now I added a --keyring-dir flag similar to the ones in keys. It doesn't add too much complexity I would say, but if you'd rather not have it there another option is to make it an "invisible" flag. I don't have any clear use cases for using a different dir for this, but perhaps someone would want do use a different dir if they use the cli for CI or some other automation. But not sure.

Makes sense! Then I think we can make it a public flag even but better if we set its default to cosmosaccount. KeyringHome. 🙏

Btw, seems like this new flag should be added to all commands that use ignite account

• ignite relayer
• relevant commands after ignite network
• tx commands under ignite node
• ignite account

That makes perfect sense. Will do that!

@ilgooz PR ready to go: https://github.com/ignite-hq/cli/pull/2539