neo-project / neo-devpack-dotnet

NEO Development Pack
MIT License
78 stars 100 forks source link

`BigInteger` comparer for `ByteString` #863

Open cschuchardt88 opened 6 months ago

cschuchardt88 commented 6 months ago

I don't know if this ApplicationEngine problem or compiler. But we need to do this. It compilers fine. But doesn't work. Because one is an Integer and one is a ByteString.

        public static bool Main(ByteString i)
        {
            ByteString stored = Storage.Get(Storage.CurrentContext, "i");
            return i == stored;
        }
cschuchardt88 commented 6 months ago

@Jim8y

Jim8y commented 6 months ago

Wait, what? Aren't them all string?

cschuchardt88 commented 6 months ago

No, BigInteger is an Integer and the ByteString im guess is either a String or ByteArray

biginteger::main[1] ...
False
biginteger::main['\x01'] ...
True
biginteger::debugfunction main
VMState.BREAK SourceCodeBreakpoint BigIntegerTest.cs line 21 instructionPointer 47: return i == stored;
stored: {'type': 'ByteString', 'value': 'AQ=='}
i: {'type': 'Integer', 'value': '1'}
Jim8y commented 6 months ago

I mean in your code:

    public static bool Main(ByteString i)
        {
            ByteString stored = Storage.Get(Storage.CurrentContext, "i");
            return i == stored;
        }
ByteString i == ByteString stored

I dont see any problem here

cschuchardt88 commented 6 months ago

Passing an Integer in as a parameter will make it equal False. For example in the test if you pass in 1 it will be False and passing in \x01 will return True

Hecate2 commented 6 months ago

I think the compiler is OK, because it is always just an INITSLOT for the start of a method.

cschuchardt88 commented 6 months ago

How are you calling the method? In `RPC?

Hecate2 commented 6 months ago
using System.ComponentModel;
using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Attributes;
using Neo.SmartContract.Framework.Services;

namespace BigIntegerTest
{
    [DisplayName("BigIntegerTest")]
    [ManifestExtra("Author", "Hecate2")]
    [ManifestExtra("Email", "developer@neo.org")]
    [ManifestExtra("Description", "ByteString comparison")]
    public class BigIntegerTest : SmartContract
    {
        public static void _deploy(object data, bool update)
        {
            Storage.Put(Storage.CurrentContext, "i", 1);
        }
        public static bool Main(ByteString i)
        {
            ByteString stored = Storage.Get(Storage.CurrentContext, "i");
            return i == stored;
        }
    }
}
from neo_fairy_client import FairyClient, Hash160Str
user = Hash160Str('0xb1983fa2479a0c8e2beae032d2df564b5451b7a5')
c = FairyClient(fairy_session='biginteger', wallet_address_or_scripthash=user)
c.virutal_deploy_from_path('./bin/sc/BigIntegerTest.nef')
print(c.invokefunction('main', [1]))
print(c.invokefunction('main', ['\x01']))
c.delete_source_code_breakpoints()
c.set_source_code_breakpoint('BigIntegerTest.cs', 21)
print(c.debug_function_with_session('main', [1]))
print('stored:', c.get_variable_value_by_name('stored'))
print('i:', c.get_variable_value_by_name('i'))
biginteger::main[1] relay=True [{'account': '0xb1983fa2479a0c8e2beae032d2df564b5451b7a5', 'scopes': 'CalledByEntry', 'allowedcontracts': [], 'allowedgroups': [], 'rules': []}]
False
biginteger::main['\x01'] relay=True [{'account': '0xb1983fa2479a0c8e2beae032d2df564b5451b7a5', 'scopes': 'CalledByEntry', 'allowedcontracts': [], 'allowedgroups': [], 'rules': []}]
True
biginteger::debugfunction main[1]
VMState.BREAK SourceCodeBreakpoint BigIntegerTest.cs line 21 instructionPointer 47: return i == stored;
stored: {'type': 'ByteString', 'value': 'AQ=='}
i: {'type': 'Integer', 'value': '1'}
Hecate2 commented 6 months ago

How are you calling the method? In `RPC?

Yes, in neo-fairy-client.

cschuchardt88 commented 6 months ago

You using neon-js or RpcClient?

Jim8y commented 6 months ago

neo-fairy-client development by them is a very useful tool, @cschuchardt88 you can check it out.

cschuchardt88 commented 6 months ago

@Jim8y either way. There is a problem with the ByteString and BigInteger comparer in NCCS. Because ByteString == BigInteger is not equal when BigInteger. We need to add type checking validation or fix by CONVERT parameter to type that is specified in the parameter of the method. NCCS should fail it someone passes in a ByteArray into a parameter that is BigInteger unless you auto CONVERT to Biginteger. The types are to loose. You need to strict them to the types that they are. Because ApplicationEngine does care.

Jim8y commented 6 months ago

you want extra parameter type check,,,, make sense,,,, and we can make it, ABI actually contains the parameter type.

cschuchardt88 commented 6 months ago

@Jim8y looks like when you pass in type Integer into ApplicationEngine. It stores as Integer in VM. Which is ok. But compiler should have type checking and strict parameters to their type for the reason in our case. so NCCS should throw error for stored == i forcing you to do stored == (ByteString)i or stored == i.ToByteArray() in the contract. So this way NCCS generates an CONVERT opcode for in VM

Hecate2 commented 6 months ago

In our contract example, we never defined any BigInteger, and it should be quite OK to compare two ByteStrings. Maybe we can add a method in the RPC and neo-cli to invoke contracts with strict types.

cschuchardt88 commented 6 months ago

@Hecate2 we wouldn't have the contract manifest to compare too.

roman-khimov commented 6 months ago

Hello, https://github.com/neo-project/neo/issues/1891.