Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Conversion utility methods for Neo-cli #535

Merged
merged 23 commits into from
May 15, 2020

Conversation

meevee98
Copy link
Contributor

@meevee98 meevee98 commented Jan 16, 2020

It's an update from #455.

This pull request has only the conversion utility methods because I'm still unable to connect to Neo 3 testnet to test some of show methods.

parse <value>
image

@shargon
Copy link
Member

shargon commented Jan 16, 2020

What do you think about tool parse, and accept all: address, pubkey or scripthash, and show all that it can resolve at same time?

@erikzhang
Copy link
Member

I prefer to add these functions to GUI.

@superboyiii
Copy link
Member

@meevee98 @shargon @erikzhang I've already made a simple tool to support most of conversion method. https://github.com/neo-ngd/Neo3-Tool. If you'd like, could adjust and merge it into your code.

@meevee98
Copy link
Contributor Author

What do you think about tool parse, and accept all: address, pubkey or scripthash, and show all that it can resolve at same time?

I agree that it's better than having a different command for each conversion. I will modify the code in this way.

@meevee98 @shargon @erikzhang I've already made a simple tool to support most of conversion method. https://github.com/neo-ngd/Neo3-Tool. If you'd like, could adjust and merge it into your code.

Nice. If I may, I will include the other conversions to this implementation.

I prefer to add these functions to GUI.

I think that these commands are also useful in neo-cli.
When we deploy a invoke a smart contract using the neo-cli, the return result is showed encoded in Base64, so a converting tool is needed anyway. If it's in the same application, the work gets faster, doesn't it?

@melanke
Copy link
Contributor

melanke commented Feb 5, 2020

I would love to have this commands on Neo-Cli, thank you Meevee

@meevee98
Copy link
Contributor Author

I've updated this pull request with @shargon's suggestion of tool parse command.
I've also included the other conversion methods from @superboyiii's tool.

It's a unique command now that accepts any value and returns the possible conversions

var parseFunctions = new Dictionary<string, Func<string, string>>()
{
{ "Address to BigEnd ScriptHash", AddressToBigEndScripthash },
{ "Address to LittleEnd ScriptHash", AddressToLittleEndScripthash },
Copy link
Member

Choose a reason for hiding this comment

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

We should have only one, the official one.

Copy link
Member

Choose a reason for hiding this comment

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

Agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed to convert only to big end script hash because that's the script hash that is shown with transactions and blocks.
What do you think? Is it okay or should I change it to little end?

neo-cli/CLI/MainService.cs Outdated Show resolved Hide resolved
neo-cli/CLI/MainService.cs Outdated Show resolved Hide resolved
neo-cli/CLI/MainService.cs Outdated Show resolved Hide resolved
neo-cli/CLI/MainService.cs Outdated Show resolved Hide resolved
/// <summary>
/// Converts an Base64 hex string to big integer value
/// </summary>
/// <param name="bytearray">
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to use single line summary when it's not a multiline comment, but it's something personal.

@nicolegys
Copy link
Contributor

Tested.

Do we still need to support converting a little end script hash to address?
There is only the big end script hash in your comment, but, in your code, the little one is also included.
If we support both of them, users must pay close attention to the format. 0x61ce772f7a725f93d8311dba9994dd8c50e3e64b and 61ce772f7a725f93d8311dba9994dd8c50e3e64b are quite different.

image

@meevee98
Copy link
Contributor Author

It's no problem for me @shargon

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

Maybe the file it's better to be more generic like .Tools

@shargon
Copy link
Member

shargon commented Mar 30, 2020

image

Maybe we should filter in base64ToString to only ASCII chars. And HexString to String to "non-empty"

image

Also it's not possible to be Hex if there odd letters

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Nice job, this will be useful in some cases.
It is good to have these utility methods on neo-cli as well.

Perhaps @shargon last comment shall be applied.

@shargon
Copy link
Member

shargon commented Mar 30, 2020

@meevee98 I am not sure if it's my computer, but it spend a lot of time in something trivial, could you check it?

image

@meevee98
Copy link
Contributor Author

Maybe we should filter in base64ToString to only ASCII chars. And HexString to String to "non-empty"
Also it's not possible to be Hex if there odd letters

@shargon I agree with the empty strings, but I think that the conversions to a string with non-ascii characters should be kept.
I've seen in many smart contracts the concatenation between a string prefix and a bytearray been used and the result usually can't be converted to a string with only ascii characters, so it is possible to be a valid hex string even if there are some odd letters.

For example, this hex string is converted to a string with many non-ascii characters
image
But that is because part of the hex string is the scripthash of an address
image

@meevee98 I am not sure if it's my computer, but it spend a lot of time in something trivial, could you check it?
image

I'm not having this delay 🤔
image

@shargon
Copy link
Member

shargon commented Apr 1, 2020

but I think that the conversions to a string with non-ascii characters should be kept.

Ok :)

@shargon
Copy link
Member

shargon commented Apr 1, 2020

I think that 0.2 seconds in your computer it's a lot of time.... we need to find the issue

@meevee98
Copy link
Contributor Author

meevee98 commented Apr 1, 2020

@shargon I was able to reduce the time to 0.1 seconds by removing the exception throwing in the conversion methods and replacing them with return null.
image
Could you check if the time is reduced on your computer as well?

@superboyiii
Copy link
Member

@meevee98 It's out of date, could you update it to the latest?

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

I think it is good to have these features.
On NeoCompiler many people replies to us that they use the website to use such features online.

@shargon shargon merged commit d103bc5 into neo-project:master May 15, 2020
@shargon shargon deleted the conversion-methods branch May 15, 2020 16:18
joeqian10 added a commit to joeqian10/neo-node that referenced this pull request May 18, 2020
Conversion utility methods for Neo-cli (neo-project#535)
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

7 participants