Skip to content

Add UInt160 and UInt256#347

Closed
ShawnYun wants to merge 4 commits intoneo-project:masterfrom
ShawnYun:add-uint160
Closed

Add UInt160 and UInt256#347
ShawnYun wants to merge 4 commits intoneo-project:masterfrom
ShawnYun:add-uint160

Conversation

@ShawnYun
Copy link
Copy Markdown
Contributor

@ShawnYun ShawnYun commented Sep 2, 2020

Close #208

Comment on lines +163 to +181
public static UInt160 HexStringToUInt160(this string value)
{
var hexString = value;
if (value.Length == 42 && value.Substring(0, 2) == "0x")
hexString = value.Substring(2, 40);
if (hexString.Length != 40)
throw new Exception();
byte[] result = new byte[20];
for (int i = 0; i < 20; i++)
{
var str = hexString.Substring(i * 2, 2);
var byte1 = str.Substring(0, 1).StringToByte();
var byte2 = str.Substring(1, 1).StringToByte();
var byte3 = byte1 << 4;
var res = (byte)(byte3 + byte2);
result[i] = res;
}
return new UInt160(result.Reverse());
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is too complicated for a contract and may cost a huge amount of GAS.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Or we can add a InteropService?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this be done in compiler?

Copy link
Copy Markdown
Contributor Author

@ShawnYun ShawnYun Sep 2, 2020

Choose a reason for hiding this comment

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

For static variables, we can do the conversion in the compiler.
But for function parameters, we can't get the parameter values at compile time.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you give a scenario?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On the other hand, the conversion is not used too often, so the GAS consumption is acceptable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't use a UInt256 as a contract parameter.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can't use a UInt256 as a contract parameter.

Why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You mean use public static UInt256 TestUInt256(UInt256 str)
and we transfer a String as a parameter?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can't use a UInt256 as a contract parameter.

Doesn't Neo already support using UInt160/256 as a contract parameter type? https://github.com/neo-project/neo/blob/master/src/neo/SmartContract/ContractParameterType.cs#L12

@devhawk
Copy link
Copy Markdown
Contributor

devhawk commented Sep 17, 2020

What's the progress on this? Can we get this in for Preview 4 neo-project/neo#1936?

{
public class Contract_UInt : SmartContract.Framework.SmartContract
{
public static UInt160 TestUInt160(String str)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should have tests that take a UInt160/256 as an input parameter too

@devhawk
Copy link
Copy Markdown
Contributor

devhawk commented Sep 17, 2020

We should also add ECPoint support to smart contract framework. It behaves similarly to UInt160/256. However, we might want to do this as a separate PR

case "System.Numerics.BigInteger": return "Integer";
case "System.Byte[]": return "ByteArray";
case "System.Byte[]":
case "Neo.SmartContract.Framework.UInt160":
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we returning UInt160/256 as a byte array?

HexToBytes,
ToScriptHash,
ToBigInteger,
ToUInt160,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ToUInt256?

Copy link
Copy Markdown
Contributor

@devhawk devhawk left a comment

Choose a reason for hiding this comment

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

Obsoleted by #362

@ShawnYun
Copy link
Copy Markdown
Contributor Author

As #362, Performing conversion through VM is expensive, so we can close it.

@ShawnYun ShawnYun closed this Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add UInt160 and UInt256 data types

4 participants