soroban-cli
soroban-cli copied to clipboard
Add `ledger entry fetch` implementation
What
Adds ledger entry get implementation that allows to fetch ledger entries. Implements all of the supported ledger entry types
stellar ledger entry fetch account <ACCOUNT>
stellar ledger entry fetch contract <CONTRACT>
stellar ledger entry fetch config <ID>
stellar ledger entry fetch claimable-balance <ID>
stellar ledger entry fetch liquidity-pool <ID>
stellar ledger entry fetch wasm <HASH>
Why
#1916
Known limitations
- Depends on changes in rpc-client
- ~Most of the command variations have not been tested !~
- A command to fetch a key's ttl is not included, based on this commend in the rpc codebase, it looks like directly fetching a TTL entry is not available.
- The cargo ban failure should be resolved once https://github.com/stellar/rs-stellar-rpc-client/pull/26 is merged in and released and this PR is updated to use the newest release.
@elizabethengelman What's the status of this PR? Is this ticket the last one that needs to go in before we could close: https://github.com/stellar/stellar-cli/issues/1838
@janewang this PR is ready for review, along with https://github.com/stellar/rs-stellar-rpc-client/pull/26 which is a dependency. Once it gets merged, i think that https://github.com/stellar/stellar-cli/issues/1838 can be closed
@elizabethengelman https://github.com/stellar/rs-stellar-rpc-client/pull/26 is merged!
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
| Diff | Package | Supply Chain Security |
Vulnerability | Quality | Maintenance | License |
|---|---|---|---|---|---|---|
| cargo/soroban-sdk@23.0.1 ⏵ 23.0.2 | ||||||
| cargo/soroban-ledger-snapshot@23.0.1 ⏵ 23.0.2 | ||||||
| cargo/soroban-spec@23.0.1 ⏵ 23.0.2 | ||||||
| cargo/soroban-spec-rust@23.0.1 ⏵ 23.0.2 | ||||||
| cargo/chrono@0.4.41 ⏵ 0.4.42 | ||||||
| cargo/soroban-token-sdk@23.0.1 ⏵ 23.0.2 | ||||||
| cargo/stellar-asset-spec@23.0.1 ⏵ 23.0.2 |
@leighmcculloch that is super helpful feedback, thank you!
Honestly, I don't quite remember why I nested the trustline and data entries under the account subcommand, it may have been to make the cmd footprint smaller since the trustilne and data entries both need an account addreses in order to fetch them. 🤔 I do also remember talking about how these rpc commands in the CLI could do things to make the UX a bit easier to understand in one of the Lab/CLI huddles - but that may have been in reference to return values, and allowing users to see more human readable outputs.
But I agree that having this be more similar to Lab, and the RPC endpoint is structured makes sense! I did a quick once over and it looks like these are the changes that I'd need to make to have the CLI be more closely aligned:
-
stellar ledger entry fetch account <ACCOUNT>- stays the same except for no longer returning trustline & account data -
stellar ledger entry fetch account <ACCOUNT> --asset <ASSET>changes tostellar ledger entry fetch trustline <ASSET> --account <ACCOUNT> -
stellar ledger entry fetch account <ACCOUNT> --data-name <ASSET>changes tostellar ledger entry fetch data <DATA_NAME> --account <ACCOUNT> -
stellar ledger entry fetch account <ACCOUNT> --offer <OFFER>changes tostellar ledger entry fetch offer <OFFER> --account <ACCOUNT> -
stellar ledger entry fetch contract <CONTRACT>changes tostellar ledger entry fetch contract-data <CONTRACT> -
stellar ledger entry fetch config <ID>- stays the same -
stellar ledger entry fetch claimable-balance <ID>- stays the same -
stellar ledger entry fetch liquidity-pool <ID>- stays the same -
stellar ledger entry fetch wasm <HASH>changes tostellar ledger entry fetch contract-code <HASH>
stellar ledger entry fetch trustline <ASSET> --account <ACCOUNT>
For these commands I would favour options for all inputs, rather than a mix of positional and named options. Positional options work really well when there is a clear subject, but here neither the asset or account are the subject of the command. Both are foreign keys that together make a primary key.
For consistency I would follow suit throughout the other commands too, even in cases where there is only a single input. Otherwise the experience will be inconsistent when navigating between commands, for example:
stellar ledger entry fetch account <ACCOUNT>
stellar ledger entry fetch trustline --account <ACCOUNT> --asset <ASSET>
stellar ledger entry fetch contract-data <CONTRACT>
There should be more inputs to contract-data. It would need the durability, and key too I would expect.
Good call in keeping the inputs consistent, makes sense to me
@leighmcculloch this should be ready for re-review when you get a chance! 🙏
Note that if the commands stay with positional arguments and continue to support multiple it may be harder to do the batch form suggested above in the future.
the currently failing bindings typescript test should be fixed once https://github.com/stellar/stellar-cli/pull/2268 is merged in