Neo as dotnet standard

[shargon]

Close #2994

[shargon]

It's a copy from

neo/src/Neo.VM/Utility.cs

Line 65 in cfffe4f

static int GetBitLength(this BigInteger i)

[cschuchardt88]

Its wrong. Doesn't calculate right, like really big numbers or leading zeros. That's for calculating Int32.

This is how you do it,

public static long GetBitLength(BigInteger num)
{
    var bytes = num.ToByteArray();
    var size = bytes.Length;
    if (size == 0) return 0;
    int v = bytes[size - 1]; // 8-bit value to find the log2 of 
    if (v == 0) return (size - 1) * 8;
    int r; // result of log2(v) will go here
    int shift;
    r = (v > 0xF) ? 4 : 0; v >>= r;
    shift = (v > 0x3) ? 2 : 0; v >>= shift; r |= shift;
    r |= (v >> 1);
    return (size - 1) * 8 + r + 1;
}

[cschuchardt88]

I have everything already built, so what's up with that?

[cschuchardt88]

Its wrong. Doesn't calculate right, like really big numbers or leading zeros. That's for calculating Int32.

This is how you do it,

public static long GetBitLength(BigInteger num)
{
    var bytes = num.ToByteArray();
    var size = bytes.Length;
    if (size == 0) return 0;
    int v = bytes[size - 1]; // 8-bit value to find the log2 of 
    if (v == 0) return (size - 1) * 8;
    int r; // result of log2(v) will go here
    int shift;
    r = (v > 0xF) ? 4 : 0; v >>= r;
    shift = (v > 0x3) ? 2 : 0; v >>= shift; r |= shift;
    r |= (v >> 1);
    return (size - 1) * 8 + r + 1;
}

[cschuchardt88]

Please make async.

using HttpResponseMessage response = await http.SendAsync(request);
using Stream stream = await response.EnsureSuccessStatusCode().Content.ReadAsStreamAsync();

[shargon]

Why we should change it?

[cschuchardt88]

instead of changing everywhere. You add sync in the P2P protocol function. But doesn't really matter, just looks better.

[shargon]

The problem is that then we need to set the method as async, and when we call it, do the same. I think that we have async code that it should be sync, like the smart contract execution

[shargon]

#3082 (comment)

We can copy the BigInteger code but it use _bits that is private, do you have any number to test that it's wrong? Because if it's wrong is already wrong in neo.vm

[cschuchardt88]

I got different Murmur128 hashes (It was with random). It's not true bit length the _bits thing your talking about; that function returns a long that's one reason. But it will work because Murmur128 only uses int32 anything in the future with anything bigger than int32 bit length will overflow. Just like in json with the decimal problem.

[shargon]

I got different Murmur128 hashes (It was with random). It's not true bit length the _bits thing your talking about; that function returns a long that's one reason. But it will work because Murmur128 only uses int32 anything in the future with anything bigger than int32 bit length will overflow. Just like in json with the decimal problem.

We can copy this one, but we don't have access to _bits without reflection, https://github.com/dotnet/runtime/blob/904c439de0fc5dd1a8d8e8913bd75c30f8288e55/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs#L3050

[cschuchardt88]

Well either way it needs to be long and human readable, easy to edit or update. The function im talking about is BitLen

[cschuchardt88]

Then why not do Math.Ceiling(num.log2(num< 0 ? -num: num+1)))?

[shargon]

human readable

You convince me xD

[Jim8y]

@shargon @cschuchardt88 will the update of GetBitLength change the behavior of existing contract?

[cschuchardt88]

@shargon @cschuchardt88 will the update of GetBitLength change the behavior of existing contract?

How? bit length is bit length. This is used in Murmur for calculating a hash. We have UT for that.

[shargon]

Done! can we merge?

[Jim8y]

bro, you can not remove

   if (b.Length == 1 || (b.Length == 2 && b[1] == 0))
            {
                return BitLen(value.Sign > 0 ? b[0] : (byte)(255 - b[0]));
            }

[Jim8y]

try to run the test under standard mode, you'll see that it returns 8 for values 0-255

[Jim8y]

the test you run will always work cause they run in net 7.0 and calling BigInteger. bitLength, not yours.

[shargon]

bro, you can not remove

   if (b.Length == 1 || (b.Length == 2 && b[1] == 0))
            {
                return BitLen(value.Sign > 0 ? b[0] : (byte)(255 - b[0]));
            }

Sorry, my fault, I just reverted the changes xD

[cschuchardt88]

BitLen should be called BitCount not length. Seeing how it deals with the bit count and not the length of bits.

[Jim8y]

BitLen should be called BitCount not length. Seeing how it deals with the bit count and not the length of bits.

Lets stop worring about none functional issues here. Too many issues in the core that are much more serious and need our effort. These can be updated when you open some pr to fix some problems. No need to discussion.

[cschuchardt88]

BitLen should be called BitCount not length. Seeing how it deals with the bit count and not the length of bits.

Lets stop worring about none functional issues here. Too many issues in the core that are much more serious and need our effort. These can be updated when you open some pr to fix some problems. No need to discussion.

Yes, but why should any developer waste the time of another developer? I been spending two weeks not building new CLI and dealing with all these little problems. Just to get told You fix it. Why should i review anything if no one listens? Reviewing every line takes time.

[Jim8y]

BitLen should be called BitCount not length. Seeing how it deals with the bit count and not the length of bits.

Lets stop worring about none functional issues here. Too many issues in the core that are much more serious and need our effort. These can be updated when you open some pr to fix some problems. No need to discussion.

Yes, but why should any developer waste the time of another developer? I been spending two weeks not building new CLI and dealing with all these little problems. Just to get told You fix it. Why should i review anything if no one listens? Reviewing every line takes time.

Then focus on real problem while reviewing, if the code is not your style, comment, but not as a reason to block pr if the code runs its functionality. Three of us have very different code styles, will be in deadlock if we just can not convince each other on things like name style. This is not google project, we dont have that much time for those elegent things. @cschuchardt88 you know how many issues are there in the code base, so you also know how many work are waiting us to fix.

[Jim8y]

Ok, let me repeat it here: We are not looking for the best solution here, we are looking for a working solution. We are not optimizing the neo core, we are still implementing it. Have it first, optimize it later. We just dont have that much time to paly around with name games, implementing the core making it work is just the most basic task we have, we have Abstract Account, Service Layer, any many many real tasks to implement in the future and communities need them already.

[Jim8y]

Neo core is a tool, and people need it urgently, they leave if we dont provide them service, they will not care about the structure of the core, not the code style of the core, not how many reusebale code that is not reused.

[Jim8y]

So the most important principle of working in the core after THE security is make it easy to access, easy to set up, easy to use, easy to update, easy to write contract. That is it, for me, code is not important, Ok, code is nothing if no one use it.

[cschuchardt88]

Neo core is a tool, and people need it urgently, they leave if we dont provide them service, they will not care about the structure of the core, not the code style of the core, not how many reusebale code that is not reused.

We should then stop all development and ONLY fix bugs and optimize the repo/code. No point in adding new features. Still takes forever for hotfixes.

If it quacks like a duck and walks like a duck. It's a duck, right? The point is the duck will always be a duck. But now it old and missing feathers, can't fly and has broken legs. What you going to do with the duck? You going to cut its legs off? Pluck all its feathers? Of course not. You would take care of it and help it, right? Not neglect it and watch it die or suffer?

[Jim8y]

Neo core is a tool, and people need it urgently, they leave if we dont provide them service, they will not care about the structure of the core, not the code style of the core, not how many reusebale code that is not reused.

We should then stop all development and ONLY fix bugs and optimize the repo/code. No point in adding new features. Still takes forever for hotfixes.

Only if its your personal project, you can make your own project perfect. Chasing a perfect code is OK if its just your work, but not for a project that has tasks. We get new core-devs to make things happen, things that can help the communities, not having people to do coding art.

You have to understand that we are not coding artiest, i am not trying to stop you from working to polish the project, but you should not block the general progress because of that.

I am not saying we should not have a solid coding style, but that should not be the first priority. And should not be a reason of rejecting merge. You know how hard it is to communicate via github on different time zones across the world. And you know that in the past a pr takes months to get merged, please dont let that heppen again.

[Jim8y]

I will support you and your work of making it good, but that should not be a reason of review decision. at least not now,

[Jim8y]

I got prs being blocked because of namespace style for weeks, during that process, tons of conflicts, and endless updating and fixing,,,my energy just got wasted cause i have to take weeks and even months to keep maintain that pr even though the logic and functionalities have never changed. Most of the time just waiting for someone to answer a sinmple question

While there is no rule at all saying i can not or should not do that. If we dont have that rule at all, dont use that rule to block anything. It is not just reviewing wasting time, maintaining a pr for months is also frustrating.

[Jim8y]

Security, Efficiency, Format, editconifg style, or other rule that you want to add before you use to block a pr please.