Allow Solid state transfers #2013

shargon · 2020-10-22T16:48:20Z

Close neo-project/proposals#97
Original Idea from: @igormcoelho

TODO: test

TODO: Sign the transaction and preserve the script

igormcoelho · 2020-10-22T19:56:07Z

src/neo/SmartContract/Helper.cs

+            if (BitConverter.ToUInt32(script, index) != ApplicationEngine.System_Contract_Call)
+                return false;
+            index += 4;
+            if (script[index++] != (byte)OpCode.EQUAL) return false;


I think we can let both "==" OR ">=", as in comment neo-project/proposals#97 (comment) , OpCode.EQUAL or OpCode.GEQ (this allows massive parallel executions for exchanges).

igormcoelho · 2020-10-22T19:57:25Z

src/neo/SmartContract/Helper.cs

+            }
+            else
+            {
+                // Other pushes


Maybe we can try to simplify here, by restricting to some general PushInt operation... do we have that?

The problem is that script builder generate the most optimized one

igormcoelho · 2020-10-22T19:59:28Z

src/neo/Wallets/Wallet.cs

+                                    byte[] solidTransferScript;
+                                    using (ScriptBuilder sbSolid = new ScriptBuilder())
+                                    {
+                                        sbSolid.EmitPush(value);


Do we have some "default" guarantee for EmitPush value? (just accepting large values opcode is enough) If we can make a standard here, then it simplifies above multiple "numeric matches". Maybe just adopt some large push alternative, which is enough for most people. Witnesses can be pruned on Neo3, so this won't cause big damage.

igormcoelho · 2020-10-22T20:02:18Z

src/neo/Wallets/Wallet.cs

+                                    {
+                                        sbSolid.EmitPush(value);
+                                        sbSolid.EmitAppCall(output.AssetId, "balanceOf", account);
+                                        sbSolid.Emit(OpCode.EQUAL);


Should we default this to .EQUAL or to .GEQ?
Maybe, .GEQ is better, because parallel transfers would happen on Neo Wallet "by default". I think that, other user wallets (maybe Neon, etc), could focus on .EQUAL standard here, because use case is very targetted for non-automatic transfers (which is not true for Neo, as the "core Neo library").

igormcoelho · 2020-10-22T20:04:38Z

tests/neo.UnitTests/SmartContract/UT_Helper.cs

+            {
+                sb.EmitPush(12);
+                sb.EmitAppCall(UInt160.Zero, "balanceOf", UInt160.Parse("01ff00ff00ff00ff00ff00ff00ff00ff00ff00a4"));
+                sb.Emit(OpCode.EQUAL);


Could we try some .EQUAL and some .GEQ? It's good to have both tested.

igormcoelho

Amazing and quick progress @shargon, congratulations!
The general comments are towards two things:

We should adopt .GEQ by default (instead of .EQUAL), because this allows parallel transactions (balance "at least larger than" X)
Is it possible to make a numeric standard using larger numbers during EmitPush, to prevent that many "small cases"? This may be slightly worse from space efficiency perspective, but so much better from code maintainance perspective. I favor code maintainance in this case (so small difference in byte push is not worth it)
Congratulations again!

igormcoelho · 2020-10-22T20:12:58Z

src/neo/Wallets/Wallet.cs

@@ -298,6 +301,32 @@ public Transaction MakeTransaction(TransferOutput[] outputs, UInt160 from = null
                                }
                                sb.EmitAppCall(output.AssetId, "transfer", account, output.ScriptHash, value);


Note that "value" has two meanings here:

The "value" being transferred, which is different from balance (good for OpCode.GEQ operations)

The actual balance from the account, that must be previously taken from the wallet system (good for OpCode.EQUAL operations).

Maybe it's nice to proceed from this point with two values: "transferValue" and "balanceValue".

igormcoelho · 2020-10-22T20:14:55Z

tests/neo.UnitTests/SmartContract/UT_Helper.cs

+            using (ScriptBuilder sb = new ScriptBuilder())
+            {
+                sb.EmitPush(-1);
+                sb.EmitAppCall(UInt160.Zero, "balanceOf", UInt160.Parse("01ff00ff00ff00ff00ff00ff00ff00ff00ff00a4"));


Do we have some test where Balance was not Zero before? I mean, guy has 10 tokens, is sending 6... thus, balance should be 4 (in case of OpCode.EQUAL); and assertion should be ">=6", if using OpCode.GEQ.
Let's call these two strategies "strict" (for OpCode.EQUAL) and "flexible" (for OpCode.GEQ), just to clarify between them.

Tommo-L · 2020-10-23T02:26:43Z

src/neo/Wallets/Wallet.cs

+                                    using (ScriptBuilder sbSolid = new ScriptBuilder())
+                                    {
+                                        sbSolid.EmitPush(value);
+                                        sbSolid.EmitAppCall(output.AssetId, "balanceOf", account);


If so, we also need to update here, CallFlags.AllowStates -> CallFlags.ReadOnly.

neo/src/neo/SmartContract/Helper.cs

Lines 177 to 179 in 22f80a7

using (ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, verifiable, snapshot?.Clone(), gas))

{

CallFlags callFlags = verifiable.Witnesses[i].StateDependent ? CallFlags.AllowStates : CallFlags.None;

IMO it's much easier to unconditionally do callFlags = CallFlags.ReadOnly and allow any verification scripts to access storage. This would then allow this feature to be a wallet feature not requiring anything special support from the ledger and allowing to remove IsSolidTransfer helper completely (except fee calculation?). It would also of course allow to have other interesting state-dependent verification scripts.

Witness StateDependent flag could then be set based on IsStandardContract(), keeping this optimization in place for the most common case.

Completely agree

Also agree... from the perspective on Neo, there are so many applications that require reading from storage, and it's not good to make exceptions to each of them. Best approach is to allow storage reads, and deal with the fact that transaction ordering will affect future verifications.

erikzhang · 2020-10-24T08:06:49Z

What problem does this mechanism solve?

igormcoelho · 2020-10-24T13:59:05Z

What problem does this mechanism solve?

This mechanism ensures that nodes in wrong versions or with poor implementations of Neo protocol, will fail to verify blocks on network, and will be interrupted (since verifications cannot fail). This is complementary to MPT technology, but much easier to track on wallets/explorers, since it doesnt depend on storage implementation prefixes, etc.

igormcoelho · 2020-10-24T15:49:28Z

@shargon I've been thinking, if we want stateless verifications (I need to check code again to refresh my mind on currently adopted state limitations), then we should just drop this PR and NEP, at least for Neo3. So maybe move for master 2, and leave just MPT to do the job on Neo3, do you agree @shargon ?

shargon · 2020-10-26T09:07:20Z

@igormcoelho If we have stateless verification, how can you change the owner of a regular smart contract?

Init commit

ba21da2

shargon requested review from igormcoelho and erikzhang October 22, 2020 16:48

shargon added 5 commits October 22, 2020 18:49

Remove TODO

ea007ba

Add verification script

1a2676f

TODO: Sign the transaction and preserve the script

Sign

0196e89

Fix balance

8279f51

Empty invocation script

811452b

shargon marked this pull request as ready for review October 22, 2020 17:18

igormcoelho reviewed Oct 22, 2020

View reviewed changes

igormcoelho requested changes Oct 22, 2020

View reviewed changes

igormcoelho reviewed Oct 22, 2020

View reviewed changes

Tommo-L reviewed Oct 23, 2020

View reviewed changes

shargon closed this Feb 8, 2022

shargon deleted the solid-transfer branch February 8, 2022 10:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Solid state transfers #2013

Allow Solid state transfers #2013

shargon commented Oct 22, 2020 •

edited

Loading

igormcoelho Oct 22, 2020 •

edited

Loading

igormcoelho Oct 22, 2020

shargon Oct 23, 2020

igormcoelho Oct 22, 2020 •

edited

Loading

igormcoelho Oct 22, 2020

igormcoelho Oct 22, 2020

igormcoelho left a comment

igormcoelho Oct 22, 2020

igormcoelho Oct 22, 2020 •

edited

Loading

Tommo-L Oct 23, 2020

roman-khimov Oct 23, 2020

shargon Oct 23, 2020

igormcoelho Oct 23, 2020

erikzhang commented Oct 24, 2020

igormcoelho commented Oct 24, 2020

igormcoelho commented Oct 24, 2020 •

edited

Loading

shargon commented Oct 26, 2020 •

edited

Loading

		@@ -298,6 +301,32 @@ public Transaction MakeTransaction(TransferOutput[] outputs, UInt160 from = null
		}
		sb.EmitAppCall(output.AssetId, "transfer", account, output.ScriptHash, value);

	using (ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, verifiable, snapshot?.Clone(), gas))
	{
	CallFlags callFlags = verifiable.Witnesses[i].StateDependent ? CallFlags.AllowStates : CallFlags.None;

Allow Solid state transfers #2013

Allow Solid state transfers #2013

Conversation

shargon commented Oct 22, 2020 • edited Loading

igormcoelho Oct 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igormcoelho Oct 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igormcoelho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igormcoelho Oct 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikzhang commented Oct 24, 2020

igormcoelho commented Oct 24, 2020

igormcoelho commented Oct 24, 2020 • edited Loading

shargon commented Oct 26, 2020 • edited Loading

shargon commented Oct 22, 2020 •

edited

Loading

igormcoelho Oct 22, 2020 •

edited

Loading

igormcoelho Oct 22, 2020 •

edited

Loading

igormcoelho Oct 22, 2020 •

edited

Loading

igormcoelho commented Oct 24, 2020 •

edited

Loading

shargon commented Oct 26, 2020 •

edited

Loading