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

Callback APIs for NeoContract #284

Closed
erikzhang opened this issue Jun 25, 2018 · 12 comments · Fixed by #1629
Closed

Callback APIs for NeoContract #284

erikzhang opened this issue Jun 25, 2018 · 12 comments · Fixed by #1629
Assignees
Labels
Compiler Module - Issues that are related or influence the behavior of our C# compiler Design Issue state - Feature accepted but the solution requires a design before being implemented Feature Type: Large changes or new features VM New features that affect the Neo Virtual Machine or the Interop layer

Comments

@erikzhang
Copy link
Member

erikzhang commented Jun 25, 2018

Create callback functions for smart contracts or SYSCALL, and allow other contracts to perform tasks through callback functions, which can work with iterators.

Several possible forms:

  • System.Runtime.CreateCallback(int offset, int paramCount, bool retValue)
  • System.Callback.GetFromSyscall(string name)
  • System.Callback.Invoke

Use with enumerators and iterators:

  • Neo.Enumerator.Filter
  • Neo.Enumerator.Select

Require neo-project/neo-vm#190

@erikzhang erikzhang added the Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. label Jun 25, 2018
@vncoelho
Copy link
Member

vncoelho commented Jun 25, 2018

I will call you back after I understand this...aehauheaea
Kidding.

This will be an useful tool for several applications, @igormcoelho.
This is a component for the Dynamic invoke, right?

@igormcoelho
Copy link
Contributor

igormcoelho commented Jun 26, 2018

How (which format) will these callbacks will be passed Erik, as raw opcodes?
In fact, it would be great to have some "Dive" function, to execute a given opcode list (that could just do that again in a inception way :D). That could have many usages for metaprogramming... just wondering the format you are thinking on these callbacks :) the idea is very good!

@erikzhang erikzhang added this to the NEO 3.0 milestone Jul 9, 2018
@erikzhang erikzhang added Discussion Initial issue state - proposed but not yet accepted and removed Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. labels Oct 9, 2018
@igormcoelho
Copy link
Contributor

Nice to come back here, it was the callback you left on neo-vm :)
I think lambda stackitems can handle this easily... its a similar idea, but I guess the 'offset' mentioned on CreateCallback is much more dangerous... if it can point anywhere on script, and it is configurable by parameter, it could skip important jumps and execute "unreachable" codes. Its really better to have lambda code isolated.

Lambdas could easily be Neo interop objects, but honestly Erik, I think that native vm objects will be so much better. Two opcodes we need, one to create, other to execute.

After that,filtering will be finally easily doable 👍

@igormcoelho
Copy link
Contributor

I think there's another problem with doing using syscalls... a hard one. Script passed should be hardcoded, for security, never coming directly from stack. This effectively rules out the possibility of doing on application layer. We should really consider doing that on vm.

@erikzhang
Copy link
Member Author

The callback within the contract is meaningless. This is a callback between contracts. InteroInterface is immutable, so there is no security issue.

@igormcoelho
Copy link
Contributor

igormcoelho commented Aug 6, 2019

The security issue I mention is on callback (offset) creation... it shouldnt be able to input jmp offset via stack parameter. But we can hardcode it. Tomorrow I may have more time to draw an example,with two distinct implementations.
I get your point on usage,I think callback and lambda are two distinct features.

@igormcoelho
Copy link
Contributor

igormcoelho commented Aug 9, 2019

@erikzhang what I mention here is following problem.
Usual Interops (all in fact) take parameters from stack. Right now, NeoVM doesn't have a way to jump to arbitrary positions on Script (like a stack-based address JMP, which is very bad for optimizations, security and everything else!!). I'm just emphasizing that we cannot allow this to happen here, by using callback.

One example:

PUSH OFFSET TO CALLBACK JUMP
PUSH0 # param count
PUSH0 # no return (void)
System.Runtime.CreateCallback  # (int offset, int paramCount, bool retValue)

Is it the idea? Fine. Now, suppose the dapp user finds a way to pass a different offset there, of perhaps the dapp developer intentionally gets first offset from an array, or somewhere else. This would allow this callback to jump at an arbitrary position on code.

I wonder if we could allow Interop hardcoded parameters, like this:

System.Runtime.CreateCallback 0x01 (requires one parameter) 0xOFFSET

Could we do this?

This way, first parameter would always be passed in mandatory script format, never coming from stack. If this is possible, it will be easier to design this callback interop feature.
On C#, I don't know the "best" syntax, perhaps another annotation passing hardcoded parameters (?):

[HardcodedParameter("offset")] // <- how to put this here... a function reference?
InteropObject CreateCallback(int paramc, bool hasReturn);

In my understanding here, this call would generate a CallbackContext object (interop), that when "invoked" (by someone else), would redirect us back to this script, at this position. Right? Result would be kept on stack by previous invoker to consume it.

@lock9 lock9 added Compiler Module - Issues that are related or influence the behavior of our C# compiler Design Issue state - Feature accepted but the solution requires a design before being implemented Feature Type: Large changes or new features VM New features that affect the Neo Virtual Machine or the Interop layer and removed Discussion Initial issue state - proposed but not yet accepted labels Aug 11, 2019
@lock9
Copy link
Contributor

lock9 commented Sep 12, 2019

Hello @neo-project/core, what are the use cases for this? What is not possible to do today, that this will enable? I gave it a heart, but I'm discussing with @belane and @shargon and maybe this doesn't have many usages.

How the callback relates with these features:

  • Neo.Enumerator.Filter
  • Neo.Enumerator.Select

?

Thanks

@erikzhang
Copy link
Member Author

Filter(p => p.Index > 100);

The statement p => p.Index > 100 is a callback function. Without callback APIs, it is impossible to implement Filter or Select.

@igormcoelho
Copy link
Contributor

igormcoelho commented Sep 18, 2019

@erikzhang for this proposal to work securely, I think we need Direct Syscall parameters (#1025).

Suppose you want to create a "lambda" callback p => p.Index > 100. This function is registered at position 20 of Script. So, you want to System.Runtime.CreateCallback(int offset, int paramCount, bool retValue), with parameters offset=20, paramCount=1, retValue=true.

Without direct parameters, you could do this:
PUSH20
PUSH1
PUSHT (or PUSH1)
SYSCALL System.Runtime.CreateCallback

This is fine... the only problem is that somehow these parameters start coming from a local variable:
FROMALTSTACK
DUP
...
PICKITEM
SYSCALL System.Runtime.CreateCallback

Now, we cannot easily know in advance which will be the offset called by this callback, which is dangerous and strongly harms code optimization (that's why relative jump offsets are banned everywhere). If somehow someone manages to change this value, we could fall within script parts that are critical, and allow crazy situations, such as jumping to the middle of a function script (which doesnt makes any sense). This simple thing can break many stack protection guarantees that we can think of.

So, a simple solution, which is also much more readable on nvm side, is passing these parameters directly here:
`SYSCALL System.Runtime.CreateCallback 03 020120 020101 0101

I've used serialization table for this (#1027), which is good for static and runtime serialization, in a organized and clear format.

020120 -> int 1 byte 0x20
020101 -> int 1 byte 0x01
0101 -> bool true

ref table (NEP-3 inspired):

0x00 - signature
0x01 - bool
0x02 - int
0x03 - hash160
0x04 - hash256
0x05 - bytearray
0x06 - pubkey 
0x07 - string
0x10 - array
0xf0 - interop
0xff - void

Obviously, this is a waste of space (020120 -> int 1 byte 0x20), but we are enforcing type int here. This is not always necessary, so option B is to allow few byte pushes to be done directly as bytes (by mimic neovm opcodes, we can use 51 to push 1 byte, etc, until 127 perhaps, and the rest done explicitly...) so, number two could be 5102 (but not enforcing type information here, that is parsed by syscall itself).

We can also use PUSHBYTES01 (code 01) to push1 byte, and perhaps redefine NEP-3 serialization prefix as something like e0 + NEP-3... reserving e0, e1, e2 ... until f0 to ff, and using the smaller ones for other purposes.

The point is: we can benefit of having a explicit serialization standard here, without incurring too many loss of bytes... and still be able to use it directly on syscalls, when necessary.

@erikzhang
Copy link
Member Author

This is fine... the only problem is that somehow these parameters start coming from a local variable

If we allow the parameters come from a local variable, how do you convert them to direct parameters? If you can convert them to direct parameters, of course you can convert them to PUSH parameters.

@igormcoelho
Copy link
Contributor

igormcoelho commented Sep 22, 2019

If you can convert them to direct parameters, of course you can convert them to PUSH parameters.

Direct parameter won't be able to reference stack items, they are constant values. It's like constexpr on C++17, a constant value evaluated on compile time only. This will be a decision on each SYSCALL specification. We could inform the number of direct parameters by passing the parameter count byte, after SYSCALL name.
Some will require stack parameters, for example: Storage.Get. First parameter is an Interop stack item, and this cannot/shouldn't be serialized, so it will require stack parameters. It's also necessary to have runtime parameters for Storage.Get, so it's unnecessary for it to have direct parameters.

On the other hand, think about Contract.Create Erik. If we pass the contract name, email, and script directly, not via stack, we will be able to parse it on evaluation time, and make sure that only valid opcodes exist. Right now, none of these is possible.

@erikzhang erikzhang removed this from the NEO 3.0 milestone Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compiler Module - Issues that are related or influence the behavior of our C# compiler Design Issue state - Feature accepted but the solution requires a design before being implemented Feature Type: Large changes or new features VM New features that affect the Neo Virtual Machine or the Interop layer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants