feat: command changes from tutorial feedback

0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup

MIT License

25 stars 22 forks source link

feat: command changes from tutorial feedback #322

Closed mFragaBA closed 1 month ago

mFragaBA commented 2 months ago

addresses some points from #314, particularly the ones under the Command changes section

Feedback changes:

renames the CLI binary to miden
for miden account show we now show all account info and we remove the flags to show the vault/script/etc.
created a root import command (so now you do miden import (account|note) <args>) and removed the ones under input-notes and account
added a way to display default account
added top-level export command
added defaults for some root commands (for account, input-note, tags and tx we always default to the List subcommand)

Follow-up work (opening new issues before merge):

Make it so that account --list works and account list does not, and similarly for other commands
Find a proper way to export account state. The main problem is that we don't store the RNG seed to build an AccountData which is what we use to import account data.

bobbinth commented 1 month ago

created a root import command (so now you do miden import (account|note) <args>) and removed the ones under input-notes and account

Not for this PR: I'm wondering if we can have just miden import <filepaths> --no-verify and the client will figure out based on the file whether it is importing accounts or notes. One way to do this is to add something like a 4-byte "magic" value in front of each file during serialization. This could then be used to identify the type of object we are dealing with.

bobbinth commented 1 month ago

for miden account show we now show all account info and we remove the flags to show the vault/script/etc.

Similar thought to the ones discussed elsewhere (and probably not for this PR): for me personally, miden account --show <account_id> would have been a more natural command.

mFragaBA commented 1 month ago

created a root import command (so now you do miden import (account|note) <args>) and removed the ones under input-notes and account

Not for this PR: I'm wondering if we can have just miden import <filepaths> --no-verify and the client will figure out based on the file whether it is importing accounts or notes. One way to do this is to add something like a 4-byte "magic" value in front of each file during serialization. This could then be used to identify the type of object we are dealing with.

If we want to handle both, couldn't we just try deserializing into one of the two structs and if that fails we try with the other one? That should be doable in the current state

bobbinth commented 1 month ago

If we want to handle both, couldn't we just try deserializing into one of the two structs and if that fails we try with the other one? That should be doable in the current state

Yep - that would work too. With a "magic" value upfront, error messaging could be a bit cleaner - but that's a pretty minor thing.

igamigo commented 1 month ago

You also don't risk having a file deserializing as an incorrect struct, which is probably not a likely scenario (especially considering the difference between eg accounts and notes) but that could definitely happen eventually.