neo-project / neo-devpack-dotnet

NEO Development Pack
MIT License
79 stars 100 forks source link

GAS.Hash compiled to a CALLT when compiled project uses old Neo3.Compiler.CSharp.Dev and Neo.SmartContract.Framework #1083

Closed Hecate2 closed 1 month ago

Hecate2 commented 1 month ago

It could be arduous work to reproduce the problem.

  1. In neo-devpack-dotnet, git checkout 428b91835d9651bc2e8e2b4f67700847e32509fe (the latest commit) and make the following changes (otherwise compilation would fail, because I used new methods):
    -                if (_methodsExported.Any(u => u.Name == method.Name && u.Parameters.Length == method.Parameters.Length))
    -                    throw new CompilationException(symbol, DiagnosticId.MethodNameConflict, $"Duplicate method key: {method.Name},{method.Parameters.Length}.");
    +                _methodsExported.RemoveAll(u => u.Name == method.Name && u.Parameters.Length == method.Parameters.Length);
  2. git clone https://github.com/Hecate2/NFTLoan.git
  3. nccs NFTLoan/NFTLoan --debug --optimize=All --assembly (You may also use --optimize=Basic)
  4. Open NFTLoan/NFTLoan/bin/sc/NFTFlashLoan.nef.txt and find CALLT 05-00 # GasToken.hash token call
  5. You would find text like this:
    # Code NFTLoan.cs line 357: "Contract.Call(GAS.Hash, "transfer", CallFlags.All, tenant, renter, totalPrice, TRANSACTION_DATA)"
    4714 PACK
    # Code NFTLoan.cs line 357: "CallFlags.All"
    4715 PUSH15
    # Code NFTLoan.cs line 357: ""transfer""
    4716 PUSHDATA1 74-72-61-6E-73-66-65-72 # as text: "transfer"
    # Code GAS.cs line 18: "Contract("0xd2a4cff31913016155e38e474a2c06d08be276cf")"
    4726 CALLT 05-00 # GasToken.hash token call

    This is incorrect. If the code is executed, we would call the Hash method of GAS contract (which does not exist). GAS.Hash should be compiled as

    PUSHDATA1 CF-76-E2-8B-D0-06-2C-4A-47-8E-E3-55-61-01-13-19-F3-CF-A4-D2
    # as script hash: 0xd2a4cff31913016155e38e474a2c06d08be276cf

    However I failed to reproduce the problem in small contracts like this:

    
    using System;
    using System.ComponentModel;
    using Neo.SmartContract.Framework;
    using Neo.SmartContract.Framework.Attributes;
    using Neo;
    using Neo.SmartContract.Framework.Services;
    using Neo.SmartContract.Framework.Native;

namespace txHash { [DisplayName("txHash")] [ManifestExtra("Author", "NEO")] [ManifestExtra("Email", "developer@neo.org")] [ManifestExtra("Description", "This is a txHash")] public class txHash : SmartContract { public static UInt256 Main() { UInt160 zero = UInt160.Zero; ExecutionEngine.Assert((bool)Contract.Call(GAS.Hash, "transfer", CallFlags.All, zero, zero, 0, null), "Failed to pay GAS"); } } }

Hecate2 commented 1 month ago

In order to debug the compiler for NFTLoan, you can set a conditional breakpoint token == 5 at neo-devpack-dotnet/src/Neo.Compiler.CSharp/MethodConvert/CallHelpers.cs:

        return AddInstruction(new Instruction
        {
            OpCode = OpCode.CALLT,
            Operand = BitConverter.GetBytes(token)
        });

We should have gone into the branch at https://github.com/neo-project/neo-devpack-dotnet/blob/428b91835d9651bc2e8e2b4f67700847e32509fe/src/Neo.Compiler.CSharp/MethodConvert/ExternConvert.cs#L91-L92

It seems we can fix the problem if we change Symbol.AssociatedSymbol! to Symbol only, at the following line https://github.com/neo-project/neo-devpack-dotnet/blob/428b91835d9651bc2e8e2b4f67700847e32509fe/src/Neo.Compiler.CSharp/MethodConvert/ExternConvert.cs#L88 But this does not work for small contracts that makes no compilation error. For small contracts, Symbol.GetAttributes() has Length=0.

Hecate2 commented 1 month ago

My fault. I should have used recent versions of Neo3.Compiler.CSharp.Dev and Neo.SmartContract.Framework. Yet I was using 3.1.0. Maybe we can ask the compiler to check contract dependencies.