Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

Hide contract script in search results for compactness/easier reading #1028

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

hal0x2328
Copy link
Member

What current issue(s) does this address, or what feature is it adding?

Searching the chain for contracts is problematic because the hex script for every matching contract is shown in the search output. Generally when searching, the goal is trying to locate a specific scripthash rather than dump every contract's lengthy script in hex.

How did you solve this problem?

By hiding the contract script output in the search results. show contract can still be used to acquire the script hex of a contract, so no functionality is lost.

How did you make sure your solution works?

Manual testing.

Are there any special changes in the code that we should be aware of?

No.

Please check the following, if applicable:

  • Did you add any tests?
  • Did you run make lint?
  • Did you run make test?
  • Are you making a PR to a feature branch or development rather than master?
  • Did you add an entry to CHANGELOG.rst? (if not, please do)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 85.313% when pulling d8dbff5 on hal0x2328:development into a7f8392 on CityOfZion:development.

Copy link
Member

@ixje ixje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. One request; please update the command help description to indicate there is an optional "verbose" flag that can be called. I don't want to go back to the old CLI interface where there were hidden flags that you had to know.

@hal0x2328
Copy link
Member Author

Good idea. One request; please update the command help description to indicate there is an optional "verbose" flag that can be called. I don't want to go back to the old CLI interface where there were hidden flags that you had to know.

There's actually no command-line flag to force the showing of the scripts in my patch. It really didn't seem useful to me at all to add that - for the reason you mentioned (yet another option to know about) and because I can't think of a single use-case for showing the script in a search result - it's not like you're going to look at it and say "yeah, there's that contract I was looking for with the byte 0xfe at offset 0x12a" (after manually counting that many bytes in the string).

And usually by the time you've listed around 5 contracts you've already exceeded your scroll buffer max line size so even if there were a use-case for displaying every contract script in the search results it would be hobbled by that limitation as well.

@ixje
Copy link
Member

ixje commented Nov 7, 2019

Fair, I agree it's unlikely that you'll ever want to see the script in a search or anywhere on the prompt. Please do change the PR include the parameters and returntype in the normal JSON output. The sc deploy command is one of the places that prints this information and there this information is useful. I've personally also used contract search a couple of times to lookup smart contract input parameters. Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants