Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigInteger comparer for ByteString #863

Open
cschuchardt88 opened this issue Jan 16, 2024 · 17 comments
Open

BigInteger comparer for ByteString #863

cschuchardt88 opened this issue Jan 16, 2024 · 17 comments

Comments

@cschuchardt88
Copy link
Member

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
Copy link
Member Author

@Jim8y

@Jim8y
Copy link
Contributor

Jim8y commented Jan 16, 2024

Wait, what? Aren't them all string?

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Jan 16, 2024

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
Copy link
Contributor

Jim8y commented Jan 16, 2024

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
Copy link
Member Author

cschuchardt88 commented Jan 16, 2024

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
Copy link
Contributor

Hecate2 commented Jan 16, 2024

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

@cschuchardt88
Copy link
Member Author

How are you calling the method? In `RPC?

@Hecate2
Copy link
Contributor

Hecate2 commented Jan 16, 2024

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
Copy link
Contributor

Hecate2 commented Jan 16, 2024

How are you calling the method? In `RPC?

Yes, in neo-fairy-client.

@cschuchardt88
Copy link
Member Author

You using neon-js or RpcClient?

@Jim8y
Copy link
Contributor

Jim8y commented Jan 16, 2024

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

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Jan 16, 2024

@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
Copy link
Contributor

Jim8y commented Jan 16, 2024

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

@cschuchardt88
Copy link
Member Author

@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
Copy link
Contributor

Hecate2 commented Jan 16, 2024

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
Copy link
Member Author

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

@roman-khimov
Copy link

Hello, neo-project/neo#1891.

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

No branches or pull requests

4 participants