protected OnSysCall and OnCallT methods

[Hecate2]

Description

For the testability of contracts, could we change the accessibility of OnSysCall and OnCallT to protected?

For example, we may need to use a fixed random number or a given timestamp in the runtime of contracts, in order to test contracts. Consequently we need to modify the InteropServices and the OnSysCall method in order to achieve it. I have posted an example at Hecate2/neo-fairy-test@ac22feb that utilizes the protected OnCallT. If OnCallT was private, I would have to re-implement the whole OnCallT by myself, with more dependent methods still being private.

Type of change

There is almost no change...

• NO Bug fix (non-breaking change which fixes an issue)
• NO New feature (non-breaking change which adds functionality)
• NO Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change DOES NOT require a documentation update

How Has This Been Tested?

No need for new tests. There seems to be no problem in all the existing tests.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[cschuchardt88]

Use IApplicationEngineProvider interface. However my example outdated. It has JumpTable now at the end, but the same idea. neo-express ues this.

Example: https://github.com/neo-project/neo-express/blob/e811bafeb120c82c68f010ee959e33c26b509a30/src/neoxp/Node/Modules/ExpressApplicationEngineProvider.cs#L22

https://github.com/neo-project/neo-express/blob/e811bafeb120c82c68f010ee959e33c26b509a30/src/bctklib/smart-contract/TraceApplicationEngine.cs#L22

no need to have this PR

[Hecate2]

Use IApplicationEngineProvider interface. However my example outdated. It has JumpTable now at the end, but the same idea. neo-express ues this.

But in this way I still need to re-write a whole OnCallT. What I need is to "override" not only the JumpTable, but also OnSysCall.

[cschuchardt88]

I'll write you an example give me an minute.

[cschuchardt88]

If you want normal calling just add base.OnSysCall, etc

// Copyright (C) 2015-2024 The Neo Project.
//
// MyApplicationEngine.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
// for more details.
//
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using Neo.Network.P2P.Payloads;
using Neo.Persistence;
using Neo.SmartContract;
using Neo.VM;

namespace MyApp
{
    public class MyApplicationEngine : ApplicationEngine
    {
        protected MyApplicationEngine(
            TriggerType trigger,
            IVerifiable container,
            DataCache snapshot,
            Block persistingBlock,
            ProtocolSettings settings,
            long gas,
            IDiagnostic diagnostic,
            JumpTable jumpTable = null)
            : base(trigger, container, snapshot, persistingBlock, settings, gas, diagnostic, jumpTable)
        {
        }

        protected override void OnSysCall(InteropDescriptor descriptor)
        {
            // TODO: Implement OnSysCall
        }
    }

    public class MyApplicationEngineProvider : IApplicationEngineProvider
    {
        public ApplicationEngine Create(TriggerType trigger, IVerifiable container, DataCache snapshot, Block persistingBlock, ProtocolSettings settings, long gas, IDiagnostic diagnostic, JumpTable jumpTable)
        {
            return new MyApplicationEngine(trigger, container, snapshot, persistingBlock, settings, gas, diagnostic, new MyJumpTable());
        }
    }

    public class MyJumpTable : JumpTable
    {
        public MyJumpTable() : base()
        {
            this[OpCode.SYSCALL] = Syscall;
            this[OpCode.CALLT] = CallT;
        }

        public override void Syscall(ExecutionEngine engine, Instruction instruction)
        {
            // TODO: Implement Syscall
        }

        public override void CallT(ExecutionEngine engine, Instruction instruction)
        {
            // TODO: Implement CallT
        }
    }
}

[cschuchardt88]

@Hecate2 Just fyi you just do

ApplicationEngine.Provider = new MyApplicationEngineProvider();
ApplicationEngine.Run(script, snapshot, tx, settings: neoSystem.Settings);

[Hecate2]

@Hecate2 Just fyi you just do

ApplicationEngine.Provider = new MyApplicationEngineProvider();
ApplicationEngine.Run(script, snapshot, tx, settings: neoSystem.Settings);

Does it still require re-writing a whole OnCallT?

[cschuchardt88]

@Hecate2 Just fyi you just do

ApplicationEngine.Provider = new MyApplicationEngineProvider();
ApplicationEngine.Run(script, snapshot, tx, settings: neoSystem.Settings);

Does it still require re-writing a whole OnCallT?

no, If you still want to call the OnCallT just do

// MyJumpTable Class ....
        public override void CallT(ExecutionEngine engine, Instruction instruction)
        {
            // TODO: Add work before
            base.CallT(engine, instruction);
            // TODO: Add work After
        }

[Hecate2]

But base.CallT(engine, instruction) in public class MyJumpTable : JumpTable leads to InvalidOperationException in the following codes?

neo/src/Neo.VM/JumpTable/JumpTable.Control.cs

Lines 394 to 398 in 1d95792

	[MethodImpl(MethodImplOptions.AggressiveInlining)]
	public virtual void CallT(ExecutionEngine engine, Instruction instruction)
	{
	throw new InvalidOperationException($"Token not found: {instruction.TokenU16}");
	}

[cschuchardt88]

Yes, if you dont want it to, then do this

public class MyApplicationEngineProvider : IApplicationEngineProvider
{
    public static Action<ExecutionEngine, Instruction> Original_OnCallT { get; private set; };

    public ApplicationEngine Create(TriggerType trigger, IVerifiable container, DataCache snapshot, Block persistingBlock, ProtocolSettings settings, long gas, IDiagnostic diagnostic, JumpTable jumpTable)
    {
        Original_OnCallT = jumpTable.CallT;

        return new MyApplicationEngine(trigger, container, snapshot, persistingBlock, settings, gas, diagnostic, new MyJumpTable());
    }
}

public class MyJumpTable : JumpTable
{
    public MyJumpTable() : base()
    {
    }

    public override void Syscall(ExecutionEngine engine, Instruction instruction)
    {
        // TODO: Implement Syscall
    }

    public override void CallT(ExecutionEngine engine, Instruction instruction)
    {
        // TODO: Implement CallT
        MyApplicationEngineProvider.Original_OnCallT(engine, instruction);
    }
}

[Hecate2]

Then we have to rewrite OnCallT again...
Alternatively, can we have protected static JumpTable ComposeDefaultJumpTable or protected static readonly JumpTable DefaultJumpTable in ApplicationEngine.cs?

[cschuchardt88]

@Hecate2
Use code above

[Hecate2]

@Hecate2 Use code above

But with the codes above, I cannot construct a jumpTable with a modified OnSysCall and the original OnCallT method in the ApplicationEngine, because the constructor ComposeDefaultJumpTable and the resulting tables are both private.

[cschuchardt88]

@Hecate2 Use code above

But with the codes above, I cannot construct a jumpTable with a modified OnSysCall and the original OnCallT method in the ApplicationEngine, because the constructor ComposeDefaultJumpTable and the resulting tables are both private.

JumpTable equals the ComposeDefaultJumpTable() method before your provider gets called. So you save it and then use it in the CallT method. Maybe original_OnCallT is a bad name. Original_OnCallT is the that static one from ApplicationEngine.ComposeDefaultJumpTable(). Yes, I know its silly that you have to do it this way.

Edit:
@Hecate2
ill make an issue about it. they all should be protected, so change the static methods. And ill make an issue about it. Because I can steal any method anyways; private or not.

[cschuchardt88]

@Hecate2 make DefaultJumpTable protected as well.

[Jim8y]

So your discussion result is keeping this pr?

[cschuchardt88]

@Jim8y yes, just needs to make changes I requested in #3225 (comment)

[shargon]

Close #3226?