Skip to content

Commit

Permalink
Always return a value for public methods (neo-project#1706)
Browse files Browse the repository at this point in the history
  • Loading branch information
erikzhang authored and Tommo-L committed Jun 22, 2020
1 parent 4a7e36e commit 33e93b0
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 94 deletions.
5 changes: 4 additions & 1 deletion src/neo/Ledger/Blockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ static Blockchain()
using (ScriptBuilder sb = new ScriptBuilder())
{
foreach (NativeContract contract in contracts)
sb.EmitAppCall(contract.Hash, ContractParameterType.Boolean, "onPersist");
{
sb.EmitAppCall(contract.Hash, "onPersist");
sb.Emit(OpCode.DROP);
}

onPersistNativeContractScript = sb.ToArray();
}
Expand Down
15 changes: 5 additions & 10 deletions src/neo/SmartContract/ApplicationEngine.Contract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,19 @@ internal void DestroyContract()
Snapshot.Storages.Delete(key);
}

internal void CallContract(UInt160 contractHash, ContractParameterType returnType, string method, Array args)
internal void CallContract(UInt160 contractHash, string method, Array args)
{
CallContractInternal(contractHash, returnType, method, args, CallFlags.All);
CallContractInternal(contractHash, method, args, CallFlags.All);
}

internal void CallContractEx(UInt160 contractHash, ContractParameterType returnType, string method, Array args, CallFlags callFlags)
internal void CallContractEx(UInt160 contractHash, string method, Array args, CallFlags callFlags)
{
if ((callFlags & ~CallFlags.All) != 0)
throw new ArgumentOutOfRangeException(nameof(callFlags));
CallContractInternal(contractHash, returnType, method, args, callFlags);
CallContractInternal(contractHash, method, args, callFlags);
}

private void CallContractInternal(UInt160 contractHash, ContractParameterType returnType, string method, Array args, CallFlags flags)
private void CallContractInternal(UInt160 contractHash, string method, Array args, CallFlags flags)
{
if (method.StartsWith('_')) throw new ArgumentException();

Expand Down Expand Up @@ -140,15 +140,10 @@ private void CallContractInternal(UInt160 contractHash, ContractParameterType re
ContractMethodDescriptor md = contract.Manifest.Abi.GetMethod(method);
if (md is null) throw new InvalidOperationException();
if (args.Count != md.Parameters.Length) throw new InvalidOperationException();
if (returnType == ContractParameterType.Void && md.ReturnType != ContractParameterType.Void)
throw new InvalidOperationException();
if (returnType != ContractParameterType.Any && md.ReturnType != returnType)
throw new InvalidOperationException();
ExecutionContext context_new = LoadScript(contract.Script);
state = context_new.GetState<ExecutionContextState>();
state.CallingScriptHash = callingScriptHash;
state.CallFlags = flags & callingFlags;
state.RVCount = returnType == ContractParameterType.Void ? 0 : 1;

if (NativeContract.IsNative(contractHash))
{
Expand Down
7 changes: 3 additions & 4 deletions src/neo/SmartContract/ApplicationEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,9 @@ internal bool AddGas(long gas)
protected override void ContextUnloaded(ExecutionContext context)
{
base.ContextUnloaded(context);
if (context.EvaluationStack == CurrentContext?.EvaluationStack) return;
int rvcount = context.GetState<ExecutionContextState>().RVCount;
if (rvcount != -1 && rvcount != context.EvaluationStack.Count)
throw new InvalidOperationException();
if (CurrentContext != null && context.EvaluationStack != CurrentContext.EvaluationStack)
if (context.EvaluationStack.Count == 0)
Push(StackItem.Null);
}

protected override void LoadContext(ExecutionContext context)
Expand Down
2 changes: 0 additions & 2 deletions src/neo/SmartContract/ExecutionContextState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,5 @@ internal class ExecutionContextState
/// Execution context rights
/// </summary>
public CallFlags CallFlags { get; set; } = CallFlags.All;

public int RVCount { get; set; } = -1;
}
}
2 changes: 1 addition & 1 deletion src/neo/SmartContract/Native/NativeContract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public ApplicationEngine TestCall(string operation, params object[] args)
{
using (ScriptBuilder sb = new ScriptBuilder())
{
sb.EmitAppCall(Hash, ContractParameterType.Any, operation, args);
sb.EmitAppCall(Hash, operation, args);
return ApplicationEngine.Run(sb.ToArray(), testMode: true);
}
}
Expand Down
15 changes: 6 additions & 9 deletions src/neo/VM/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,38 +23,35 @@ public static ScriptBuilder Emit(this ScriptBuilder sb, params OpCode[] ops)
return sb;
}

public static ScriptBuilder EmitAppCall(this ScriptBuilder sb, UInt160 scriptHash, ContractParameterType returnType, string operation)
public static ScriptBuilder EmitAppCall(this ScriptBuilder sb, UInt160 scriptHash, string operation)
{
sb.EmitPush(0);
sb.Emit(OpCode.NEWARRAY);
sb.EmitPush(operation);
sb.EmitPush(returnType);
sb.EmitPush(scriptHash);
sb.EmitSysCall(ApplicationEngine.System_Contract_Call);
return sb;
}

public static ScriptBuilder EmitAppCall(this ScriptBuilder sb, UInt160 scriptHash, ContractParameterType returnType, string operation, params ContractParameter[] args)
public static ScriptBuilder EmitAppCall(this ScriptBuilder sb, UInt160 scriptHash, string operation, params ContractParameter[] args)
{
for (int i = args.Length - 1; i >= 0; i--)
sb.EmitPush(args[i]);
sb.EmitPush(args.Length);
sb.Emit(OpCode.PACK);
sb.EmitPush(operation);
sb.EmitPush(returnType);
sb.EmitPush(scriptHash);
sb.EmitSysCall(ApplicationEngine.System_Contract_Call);
return sb;
}

public static ScriptBuilder EmitAppCall(this ScriptBuilder sb, UInt160 scriptHash, ContractParameterType returnType, string operation, params object[] args)
public static ScriptBuilder EmitAppCall(this ScriptBuilder sb, UInt160 scriptHash, string operation, params object[] args)
{
for (int i = args.Length - 1; i >= 0; i--)
sb.EmitPush(args[i]);
sb.EmitPush(args.Length);
sb.Emit(OpCode.PACK);
sb.EmitPush(operation);
sb.EmitPush(returnType);
sb.EmitPush(scriptHash);
sb.EmitSysCall(ApplicationEngine.System_Contract_Call);
return sb;
Expand Down Expand Up @@ -211,14 +208,14 @@ public static string GetString(this StackItem item)
/// <param name="operation">contract operation</param>
/// <param name="args">operation arguments</param>
/// <returns></returns>
public static byte[] MakeScript(this UInt160 scriptHash, ContractParameterType returnType, string operation, params object[] args)
public static byte[] MakeScript(this UInt160 scriptHash, string operation, params object[] args)
{
using (ScriptBuilder sb = new ScriptBuilder())
{
if (args.Length > 0)
sb.EmitAppCall(scriptHash, returnType, operation, args);
sb.EmitAppCall(scriptHash, operation, args);
else
sb.EmitAppCall(scriptHash, returnType, operation);
sb.EmitAppCall(scriptHash, operation);
return sb.ToArray();
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/neo/Wallets/AssetDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ public AssetDescriptor(UInt160 asset_id)
byte[] script;
using (ScriptBuilder sb = new ScriptBuilder())
{
sb.EmitAppCall(asset_id, ContractParameterType.Integer, "decimals");
sb.EmitAppCall(asset_id, ContractParameterType.String, "name");
sb.EmitAppCall(asset_id, "decimals");
sb.EmitAppCall(asset_id, "name");
script = sb.ToArray();
}
using ApplicationEngine engine = ApplicationEngine.Run(script, extraGAS: 3_000_000);
Expand Down
8 changes: 4 additions & 4 deletions src/neo/Wallets/Wallet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@ public BigDecimal GetBalance(UInt160 asset_id, params UInt160[] accounts)
sb.EmitPush(0);
foreach (UInt160 account in accounts)
{
sb.EmitAppCall(asset_id, ContractParameterType.Integer, "balanceOf", account);
sb.EmitAppCall(asset_id, "balanceOf", account);
sb.Emit(OpCode.ADD);
}
sb.EmitAppCall(asset_id, ContractParameterType.Integer, "decimals");
sb.EmitAppCall(asset_id, "decimals");
script = sb.ToArray();
}
using ApplicationEngine engine = ApplicationEngine.Run(script, extraGAS: 20000000L * accounts.Length);
Expand Down Expand Up @@ -246,7 +246,7 @@ public Transaction MakeTransaction(TransferOutput[] outputs, UInt160 from = null
foreach (UInt160 account in accounts)
using (ScriptBuilder sb2 = new ScriptBuilder())
{
sb2.EmitAppCall(assetId, ContractParameterType.Integer, "balanceOf", account);
sb2.EmitAppCall(assetId, "balanceOf", account);
using (ApplicationEngine engine = ApplicationEngine.Run(sb2.ToArray(), snapshot, testMode: true))
{
if (engine.State.HasFlag(VMState.FAULT))
Expand All @@ -265,7 +265,7 @@ public Transaction MakeTransaction(TransferOutput[] outputs, UInt160 from = null
cosignerList.UnionWith(balances_used.Select(p => p.Account));
foreach (var (account, value) in balances_used)
{
sb.EmitAppCall(output.AssetId, ContractParameterType.Boolean, "transfer", account, output.ScriptHash, value);
sb.EmitAppCall(output.AssetId, "transfer", account, output.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
}
}
Expand Down
40 changes: 20 additions & 20 deletions tests/neo.UnitTests/Network/P2P/Payloads/UT_Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ public void FeeIsMultiSigContract()

var sizeGas = tx.Size * NativeContract.Policy.GetFeePerByte(snapshot);
Assert.AreEqual(2000810, verificationGas);
Assert.AreEqual(368000, sizeGas);
Assert.AreEqual(2368810, tx.NetworkFee);
Assert.AreEqual(367000, sizeGas);
Assert.AreEqual(2367810, tx.NetworkFee);
}
}

Expand Down Expand Up @@ -200,7 +200,7 @@ public void FeeIsSignatureContractDetailed()
Assert.IsNull(tx.Witnesses);

// check pre-computed network fee (already guessing signature sizes)
tx.NetworkFee.Should().Be(1265390);
tx.NetworkFee.Should().Be(1264390);

// ----
// Sign
Expand Down Expand Up @@ -242,7 +242,7 @@ public void FeeIsSignatureContractDetailed()
// ------------------
// check tx_size cost
// ------------------
Assert.AreEqual(265, tx.Size);
Assert.AreEqual(264, tx.Size);

// will verify tx size, step by step

Expand All @@ -256,18 +256,18 @@ public void FeeIsSignatureContractDetailed()
// Note that Data size and Usage size are different (because of first byte on GetVarSize())
Assert.AreEqual(22, tx.Cosigners.Values.First().Size);
// Part III
Assert.AreEqual(87, tx.Script.GetVarSize());
Assert.AreEqual(86, tx.Script.GetVarSize());
// Part IV
Assert.AreEqual(110, tx.Witnesses.GetVarSize());
// I + II + III + IV
Assert.AreEqual(45 + 23 + 87 + 110, tx.Size);
Assert.AreEqual(45 + 23 + 86 + 110, tx.Size);

Assert.AreEqual(1000, NativeContract.Policy.GetFeePerByte(snapshot));
var sizeGas = tx.Size * NativeContract.Policy.GetFeePerByte(snapshot);
Assert.AreEqual(265000, sizeGas);
Assert.AreEqual(264000, sizeGas);

// final check on sum: verification_cost + tx_size
Assert.AreEqual(1265390, verificationGas + sizeGas);
Assert.AreEqual(1264390, verificationGas + sizeGas);
// final assert
Assert.AreEqual(tx.NetworkFee, verificationGas + sizeGas);
}
Expand Down Expand Up @@ -302,7 +302,7 @@ public void FeeIsSignatureContract_TestScope_Global()
{
// self-transfer of 1e-8 GAS
System.Numerics.BigInteger value = (new BigDecimal(1, 8)).Value;
sb.EmitAppCall(NativeContract.GAS.Hash, ContractParameterType.Boolean, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.EmitAppCall(NativeContract.GAS.Hash, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
script = sb.ToArray();
}
Expand Down Expand Up @@ -353,7 +353,7 @@ public void FeeIsSignatureContract_TestScope_Global()
// get sizeGas
var sizeGas = tx.Size * NativeContract.Policy.GetFeePerByte(snapshot);
// final check on sum: verification_cost + tx_size
Assert.AreEqual(1265390, verificationGas + sizeGas);
Assert.AreEqual(1264390, verificationGas + sizeGas);
// final assert
Assert.AreEqual(tx.NetworkFee, verificationGas + sizeGas);
}
Expand Down Expand Up @@ -388,7 +388,7 @@ public void FeeIsSignatureContract_TestScope_CurrentHash_GAS()
{
// self-transfer of 1e-8 GAS
System.Numerics.BigInteger value = (new BigDecimal(1, 8)).Value;
sb.EmitAppCall(NativeContract.GAS.Hash, ContractParameterType.Boolean, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.EmitAppCall(NativeContract.GAS.Hash, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
script = sb.ToArray();
}
Expand Down Expand Up @@ -440,7 +440,7 @@ public void FeeIsSignatureContract_TestScope_CurrentHash_GAS()
// get sizeGas
var sizeGas = tx.Size * NativeContract.Policy.GetFeePerByte(snapshot);
// final check on sum: verification_cost + tx_size
Assert.AreEqual(1286390, verificationGas + sizeGas);
Assert.AreEqual(1285390, verificationGas + sizeGas);
// final assert
Assert.AreEqual(tx.NetworkFee, verificationGas + sizeGas);
}
Expand Down Expand Up @@ -475,7 +475,7 @@ public void FeeIsSignatureContract_TestScope_CalledByEntry_Plus_GAS()
{
// self-transfer of 1e-8 GAS
System.Numerics.BigInteger value = (new BigDecimal(1, 8)).Value;
sb.EmitAppCall(NativeContract.GAS.Hash, ContractParameterType.Boolean, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.EmitAppCall(NativeContract.GAS.Hash, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
script = sb.ToArray();
}
Expand Down Expand Up @@ -530,7 +530,7 @@ public void FeeIsSignatureContract_TestScope_CalledByEntry_Plus_GAS()
// get sizeGas
var sizeGas = tx.Size * NativeContract.Policy.GetFeePerByte(snapshot);
// final check on sum: verification_cost + tx_size
Assert.AreEqual(1286390, verificationGas + sizeGas);
Assert.AreEqual(1285390, verificationGas + sizeGas);
// final assert
Assert.AreEqual(tx.NetworkFee, verificationGas + sizeGas);
}
Expand Down Expand Up @@ -563,7 +563,7 @@ public void FeeIsSignatureContract_TestScope_CurrentHash_NEO_FAULT()
{
// self-transfer of 1e-8 GAS
System.Numerics.BigInteger value = (new BigDecimal(1, 8)).Value;
sb.EmitAppCall(NativeContract.GAS.Hash, ContractParameterType.Boolean, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.EmitAppCall(NativeContract.GAS.Hash, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
script = sb.ToArray();
}
Expand Down Expand Up @@ -615,7 +615,7 @@ public void FeeIsSignatureContract_TestScope_CurrentHash_NEO_GAS()
{
// self-transfer of 1e-8 GAS
System.Numerics.BigInteger value = (new BigDecimal(1, 8)).Value;
sb.EmitAppCall(NativeContract.GAS.Hash, ContractParameterType.Boolean, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.EmitAppCall(NativeContract.GAS.Hash, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
script = sb.ToArray();
}
Expand Down Expand Up @@ -672,7 +672,7 @@ public void FeeIsSignatureContract_TestScope_CurrentHash_NEO_GAS()
// get sizeGas
var sizeGas = tx.Size * NativeContract.Policy.GetFeePerByte(snapshot);
// final check on sum: verification_cost + tx_size
Assert.AreEqual(1306390, verificationGas + sizeGas);
Assert.AreEqual(1305390, verificationGas + sizeGas);
// final assert
Assert.AreEqual(tx.NetworkFee, verificationGas + sizeGas);
}
Expand Down Expand Up @@ -705,7 +705,7 @@ public void FeeIsSignatureContract_TestScope_NoScopeFAULT()
{
// self-transfer of 1e-8 GAS
System.Numerics.BigInteger value = (new BigDecimal(1, 8)).Value;
sb.EmitAppCall(NativeContract.GAS.Hash, ContractParameterType.Boolean, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.EmitAppCall(NativeContract.GAS.Hash, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
script = sb.ToArray();
}
Expand Down Expand Up @@ -953,7 +953,7 @@ public void FeeIsSignatureContract_TestScope_Global_Default()
{
// self-transfer of 1e-8 GAS
System.Numerics.BigInteger value = (new BigDecimal(1, 8)).Value;
sb.EmitAppCall(NativeContract.GAS.Hash, ContractParameterType.Boolean, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.EmitAppCall(NativeContract.GAS.Hash, "transfer", acc.ScriptHash, acc.ScriptHash, value);
sb.Emit(OpCode.ASSERT);
script = sb.ToArray();
}
Expand Down Expand Up @@ -1003,7 +1003,7 @@ public void FeeIsSignatureContract_TestScope_Global_Default()
// get sizeGas
var sizeGas = tx.Size * NativeContract.Policy.GetFeePerByte(snapshot);
// final check on sum: verification_cost + tx_size
Assert.AreEqual(1265390, verificationGas + sizeGas);
Assert.AreEqual(1264390, verificationGas + sizeGas);
// final assert
Assert.AreEqual(tx.NetworkFee, verificationGas + sizeGas);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ public void TestVote()
}));
}

sb.EmitAppCall(NativeContract.NEO.Hash, ContractParameterType.Boolean, "transfer", from, UInt160.Zero, amount);
sb.EmitAppCall(NativeContract.NEO.Hash, "transfer", from, UInt160.Zero, amount);
engine.LoadScript(sb.ToArray());
engine.Execute();
var result = engine.ResultStack.Peek();
Expand Down
Loading

0 comments on commit 33e93b0

Please sign in to comment.