Allow array of single value to pass CHECKSIG

[igormcoelho]

Guys, do you think that's ok to change line

neo-vm/src/neo-vm/ExecutionEngine.cs

Line 588 in ab5f939

byte[] signature = context.EvaluationStack.Pop().GetByteArray();

to accept a array of single value (single signature) to be accepted by CHECKSIG?
The idea is that we could use the exactly same mechanism for single sigs and multi sigs, that can simplify a few things. Something like this:

https://github.com/neo-project/neo-vm/blob/master/src/neo-vm/ExecutionEngine.cs#L618-L619

I can make the PR, just wondering if that's too crazy :)

[shargon]

Do you want to unify CHEKSIG and CHECKMULTISIG?

[ixje]

I think thats the idea, but that would increase execution time for all regular CHECKSIG calls as there are far more sanity checks going on in CHECKMULTISIG and also more stack manipulations. @igormcoelho what exact benefits do you foresee this would have?

[igormcoelho]

@shargon in fact, I don't want to unify, because the "canonical" (or standard) structure of a CHECKSIG is more simple than CHECKMULTISIG, and this expects more parameters. What I want to do is to allow CHECKSIG to succeed if user passes the following invocation script:

PUSH40 ... SIGNATURE
PUSH1 
PACK

@ixje The advantage I see is that all standard verification scripts could naturally expect arrays as input. The benefits could be seen easily in our NEO-SMACCO composer (that creates smart accounts based on simple json): http://neoresearch.io/smacco-demo/
Currently, we need two basis (single input and array input), but if CHECKSIG accepts single array, all this logic will be drastically simplified.

[igormcoelho]

Ok, I implemented the feature hahaha easier to discuss with an implementation proposal ;)
Do you see any downsides related to this? Any attacks? I don't see any...

[ixje]

the example implementation was helpful to clarify the intention. I left my comments there.

[erikzhang]

Agree with @ixje.

On the other hand, I have plans to remove CHECKSIG and CHECKMULTISIG from NeoVM in NEO 3.0.

[igormcoelho]

@ixje thanks a lot for the comments.
@erikzhang, I fully agree that CHECKSIG could be in a higher layer, because it depends on GetMessage, and that always confused me (I took a long time to understand what was going on). On the other hand, this change is more related to a standard view than CHECKSIG itself... even if CHECKSIG changes to anything else, I guess the logic of submitting signature and validating will be kept intact, am I correct?
And in terms of signature validation, it would be nice to have a standard... this change here allows an implicit standard to happen:
bool Main(Signature[] sigs) { ... }
It's similar to the general Application standard (with string, object[]), but for Signature Redeem Scripts on general. By the way, I guess we could have a Signature object on devpack, to enforce byte[] with a fixed number of bytes... that makes things even more clear ;)

[shargon]

This opcodes will be syscalls? @erikzhang

I agree with the weird dependence of GetMessage, should be moved outside

[erikzhang]

We can use VERIFY instead.

neo-vm/src/neo-vm/OpCode.cs

Line 382 in 94a91ae

VERIFY = 0xAD,

According to my plan, in NEO 3.0, all contracts will have a uniform entry point for all triggers:

object Main(string operation, object[] args)

For Verification trigger, the operation should be "verify", the args[0] should be the transaction itself, the args[1] should be a byte array that contains the result from old GetMessage(), and the args[2] is the signature.

So the system will trigger the contract like this:

verify(tx, message, signature);

The verify method can be implemented with the VERIFY opcode:

bool verify(Transaction tx, byte[] message, byte[] signature)
{
    byte[] pubkey = {};
    return OpCodes.VERIFY(message, signature, pubkey);
}

[igormcoelho]

The plan is good @erikzhang ! I just think we don't need to pass the transaction or the message itself by parameters, as they could be accessible already on ApplicationEngine. May I suggest the operation System.Transaction.GetMessage? I was already sure that existed hahaha but it doesn't... using this operation I could build GetMessage(), and it's easy to build Neo 3.0 validation script this way (without interfering on user parameters):

object Main(string operation, object[] args) {
    return (operation=="verify") && (args.Length==1) && verify((byte[])args[0], GetMessage(), pubkey_x);
}

So, if you allow me to go even deeper, I guess CHECKSIG should start accepting this format too, so wallets will be fully prepared for the future.

Using this logic, any user should be able to submit a "future checksig" this way:

PUSHBYTES40: 0x00aabb..00aabb  #signature
PUSH1
PACK
PUSHBYTES6: 0x766572696679    #verify

So, we could support it already by just checking if signature bytearray is in fact 0x766572696679, and if it is, we would need to Peek the next array from stack and verify against passed signature. So even if the system passes tx and message as the first two parameters, the proposed validation will work anyway, because I chose to only take the last parameter as signature.

[igormcoelho]

I created a proposed "verify" validation on CHECKSIG that would work in your current proposal Erik. I will take last parameter from args as if it was the signature, so even if things change regarding the first two system parameters (tx and message), it will work anyway. This way, wallets can already start building Neo 3.0 compatible invocations.

neo-vm/src/neo-vm/ExecutionEngine.cs

Lines 600 to 613 in 9033a97

	// preparing CHECKSIG for Neo 3.0 format: expects "verify" (0x766572696679)
	if(signature.Equals(new byte[]{0x76,0x65,0x72,0x69,0x66,0x79}) && context.EvaluationStack.Length > 0)
	{
	StackItem item2 = context.EvaluationStack.Peek();
	if (item2 is VMArray array2)
	{
	byte[][] parameters = array2.Select(p => p.GetByteArray()).ToArray();
	if (parameters.Length > 0)
	{
	signature = parameters[parameters.Length-1];
	context.EvaluationStack.Pop();
	}
	}
	}

[erikzhang]

Let's refine this proposal first. Please don't start writing code for it.

[erikzhang]

Let me organize the steps:

Step 1, the wallet provides a signature or other parameters in Witness.InvocationScript:

PUSH <signature> //stack: <signature>

Step 2, when other nodes verify the transaction, they need to perform an additional packaging operation on the items in the stack and push "verify" into the stack:

DEPTH         //stack: <signature>, 1
PACK          //stack: [<signature>]
PUSH "verify" //stack: [<signature>], "verify"

At this point, the parameters in the form of a unified entry point are ready.

Step 3, after that, the node should execute Witness.VerificationScript, so it should be:

DROP                                      //stack: [<signature>]
UNPACK                                    //stack: <signature>, 1
DROP                                      //stack: <signature>
SYSCALL System.Runtime.GetScriptContainer //stack: <signature>, <tx>
SYSCALL System.ScriptContainer.GetMessage //stack: <signature>, <message>
SWAP                                      //stack: <message>, <signature>
PUSH <pubkey>                             //stack: <message>, <signature>, <pubkey>
VERIFY                                    //stack: <true|false>

It may seem a bit verbose and may need to be optimized.

The first optimization scheme is to combine System.Runtime.GetScriptContainer and System.ScriptContainer.GetMessage into a single SYSCALL:

SYSCALL System.Runtime.GetMessage

The second optimization scheme is to remove the two SYSCALLs and instead automatically pass in the parameter message in step 2:

DEPTH                                     //stack: <signature>, 1
PACK                                      //stack: [<signature>]
DUP                                       //stack: [<signature>], [<signature>]
SYSCALL System.Runtime.GetScriptContainer //stack: [<signature>], [<signature>], <tx>
SYSCALL System.ScriptContainer.GetMessage //stack: [<signature>], [<signature>], <message>
APPEND                                    //stack: [<signature>, <message>]
PUSH "verify"                             //stack: [<signature>, <message>], "verify"

Then the Witness.VerificationScript would be:

DROP          //stack: [<signature>, <message>]
UNPACK        //stack: <message>, <signature>, 2
DROP          //stack: <message>, <signature>
PUSH <pubkey> //stack: <message>, <signature>, <pubkey>
VERIFY        //stack: <true|false>

Which way is better?

[igormcoelho]

Sorry about the rush coding Erik hahaha I was just giving an example to clarify the discussion and show how it could be easily integrated even on current CHECKSIG for smooth transition :)
I think both ways are good Erik, but if you prefer to do that in the future, we can take that easy for now, and focus on more urgent things ;)
In my opinion, the challenging decision is to decide if we want to have scripts not appearing on Invocation or Verification, because this middle (automatic) part won't be recorded to the blockchain, right? I really like this format where Invocation+Verification forms a complete script, so it would be nice to keep this. If you want to enforce the canonical format using Main(string, object[]), then I would prefer to have user sending this invocation:

PUSH <signature> //stack: <signature>
DEPTH         //stack: <signature>, 1
PACK          //stack: [<signature>]
PUSH "verify" //stack: [<signature>], "verify"

And nodes verify that top stack items are: bytearray and array, respectively, otherwise it fails execution automatically. Then the rest could be done in verification script itself. It would be also nice to have a unified SYSCALL System.Runtime.GetMessage.

[erikzhang]

Invocation:

PUSH <signature> //stack: <signature>
DEPTH            //stack: <signature>, 1
PACK             //stack: [<signature>]
PUSH "verify"    //stack: [<signature>], "verify"

Verification:

DROP                              //stack: [<signature>]
UNPACK                            //stack: <signature>, 1
DROP                              //stack: <signature>
SYSCALL System.Runtime.GetMessage //stack: <signature>, <message>
SWAP                              //stack: <message>, <signature>
PUSH <pubkey>                     //stack: <message>, <signature>, <pubkey>
VERIFY                            //stack: <true|false>

This looks perfect except that the VerificationScript is much longer than it used to be. 😄

[igormcoelho]

Very good Erik! This is a nice format! Some time ago I did some proposal for us to allow hashing the names of interop functions.... if we adopt 8 bytes it could cut some space in many applications.
Example: System.Runtime.GetMessage has 25 bytes: 53797374656d2e52756e74696d652e4765744d657373616765
First 8 bytes of single SHA256 is: a59207ee04ce1a23, so we could do: SYSCALL a59207ee04ce1a23. It's a strange solution tough, but it compress data ;)

[erikzhang]

Maybe we should implement this proposal first: neo-project/neo#264

[igormcoelho]

Now it's neo-project/neo related.