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.

Sign up for GitHub

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

Jump to bottom

Extend CalculateNetworkFee with contract verification #3385

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Extend CalculateNetworkFee with contract verification #3385

Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
099f7be
Calculate fee
shargon Jul 2, 2024
3a332fa
change values
shargon Jul 2, 2024
ad3d477
remove using
shargon Jul 2, 2024
7b6ccb9
fix using
shargon Jul 2, 2024
78ddbb1
Remove comments
shargon Jul 2, 2024
eccec43
Merge branch 'master' into core-calculate-fee
cschuchardt88 Jul 4, 2024
8a8af47
Remove map
shargon Jul 5, 2024
20935fd
Merge branch 'master' into core-calculate-fee
shargon Jul 9, 2024
58cab0d
Anna's feedback
shargon Jul 11, 2024
e2cc49f
ensure only one result
shargon Jul 11, 2024
1d071a5
Merge branch 'master' into core-calculate-fee
cschuchardt88 Jul 11, 2024
dabcd58
Merge branch 'master' into core-calculate-fee
Jim8y Jul 15, 2024
45588a9
Merge branch 'master' into core-calculate-fee
Jim8y Jul 22, 2024
373e1d0
Merge branch 'master' into core-calculate-fee
shargon Jul 23, 2024
e4fa517
Merge branch 'master' into core-calculate-fee
cschuchardt88 Jul 23, 2024
56f39ed
Merge branch 'master' into core-calculate-fee
Jim8y Jul 30, 2024
3812a45
Update src/Neo/Wallets/Helper.cs
shargon Jul 30, 2024
c0f03b1
Merge branch 'master' into core-calculate-fee
shargon Jul 30, 2024
15a563d
Merge branch 'master' into core-calculate-fee
shargon Jul 30, 2024
8ad1acf
Remove error
shargon Jul 31, 2024
6fa6ef9
Merge branch 'master' into core-calculate-fee
shargon Aug 1, 2024
2745757
Merge branch 'master' into core-calculate-fee
shargon Aug 6, 2024
149f910
Merge branch 'master' into core-calculate-fee
shargon Aug 9, 2024
590e83d
Merge branch 'master' into core-calculate-fee
Jim8y Sep 5, 2024
ea69990
Merge branch 'master' into core-calculate-fee
shargon Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 60 additions & 16 deletions src/Neo/Wallets/Helper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Neo.SmartContract;
using Neo.SmartContract.Native;
using Neo.VM;
using Neo.VM.Types;
using System;
using static Neo.SmartContract.Helper;

Expand Down Expand Up @@ -117,6 +118,7 @@ public static long CalculateNetworkFee(this Transaction tx, DataCache snapshot,

if (witnessScript is null || witnessScript.Length == 0)
{
// Contract-based verification
var contract = NativeContract.ContractManagement.GetContract(snapshot, hash);
if (contract is null)
throw new ArgumentException($"The smart contract or address {hash} is not found");
Expand All @@ -126,35 +128,77 @@ public static long CalculateNetworkFee(this Transaction tx, DataCache snapshot,
if (md.ReturnType != ContractParameterType.Boolean)
throw new ArgumentException("The verify method doesn't return boolean value.");
if (md.Parameters.Length > 0 && invocationScript is null)
throw new ArgumentException("The verify method requires parameters that need to be passed via the witness' invocation script.");
{
var script = new ScriptBuilder();
foreach (var par in md.Parameters)
{
switch (par.Type)
{
case ContractParameterType.Any:
case ContractParameterType.Signature:
case ContractParameterType.String:
case ContractParameterType.ByteArray:
script.EmitPush(new byte[64]);
break;
case ContractParameterType.Boolean:
script.EmitPush(true);
break;
case ContractParameterType.Integer:
script.Emit(OpCode.PUSHINT256, new byte[Integer.MaxSize]);
break;
case ContractParameterType.Hash160:
script.EmitPush(new byte[UInt160.Length]);
break;
case ContractParameterType.Hash256:
script.EmitPush(new byte[UInt256.Length]);
break;
case ContractParameterType.PublicKey:
script.EmitPush(new byte[33]);
break;
case ContractParameterType.Array:
script.Emit(OpCode.PUSHINT256, new byte[Integer.MaxSize]);
script.Emit(OpCode.NEWARRAY);
shargon marked this conversation as resolved.
Show resolved Hide resolved
shargon marked this conversation as resolved.
Show resolved Hide resolved
break;
default:
throw new ArgumentException("The verify method requires parameters that need to be passed via the witness' invocation script.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It differs slightly from NeoGo behaviour, we need to discuss it: NeoGo doesn't return error on unknown type; instead it tries to do the best and emit as much parameters as possible, skipping the others. Then "verify" method is processed in any case, and an error is returned only if "verify" doesn't return boolean. I think that this approach is good because it tries to do as much as possible without extra user's data.

Copy link
Member

@cschuchardt88 cschuchardt88 Jul 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you need to support contract as signer with verify function with custom parameters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that is missing is Map and InteropInterface currently. For maps some zero value can be added (empty map), but for interop interfaces it's harder to say, maybe NULL.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that is missing is Map and InteropInterface currently. For maps some zero value can be added (empty map), but for interop interfaces it's harder to say, maybe NULL.

#3385 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but then throwing an error is not the best way to handle this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roman-khimov done

}
}
invocationScript = script.ToArray();
}

// Empty verification and non-empty invocation scripts
var invSize = invocationScript?.GetVarSize() ?? Array.Empty<byte>().GetVarSize();
size += Array.Empty<byte>().GetVarSize() + invSize;
var invSize = invocationScript?.GetVarSize() ?? System.Array.Empty<byte>().GetVarSize();
size += System.Array.Empty<byte>().GetVarSize() + invSize;

// Check verify cost
using ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, tx, snapshot.CloneCache(), settings: settings, gas: maxExecutionCost);
engine.LoadContract(contract, md, CallFlags.ReadOnly);
if (invocationScript != null) engine.LoadScript(invocationScript, configureState: p => p.CallFlags = CallFlags.None);
if (engine.Execute() == VMState.FAULT) throw new ArgumentException($"Smart contract {contract.Hash} verification fault.");
if (!engine.ResultStack.Pop().GetBoolean()) throw new ArgumentException($"Smart contract {contract.Hash} returns false.");

if (engine.Execute() == VMState.HALT)
shargon marked this conversation as resolved.
Show resolved Hide resolved
{
// https://github.com/neo-project/neo/issues/2805
if (engine.ResultStack.Count != 1) throw new ArgumentException($"Smart contract {contract.Hash} verification fault.");
_ = engine.ResultStack.Pop().GetBoolean(); // Ensure that the result is boolean
}
maxExecutionCost -= engine.FeeConsumed;
if (maxExecutionCost <= 0) throw new InvalidOperationException("Insufficient GAS.");
networkFee += engine.FeeConsumed;
}
else if (IsSignatureContract(witnessScript))
else
{
size += 67 + witnessScript.GetVarSize();
networkFee += exec_fee_factor * SignatureContractCost();
}
else if (IsMultiSigContract(witnessScript, out int m, out int n))
{
int size_inv = 66 * m;
size += IO.Helper.GetVarSize(size_inv) + size_inv + witnessScript.GetVarSize();
networkFee += exec_fee_factor * MultiSignatureContractCost(m, n);
// Regular signature verification.
if (IsSignatureContract(witnessScript))
{
size += 67 + witnessScript.GetVarSize();
networkFee += exec_fee_factor * SignatureContractCost();
}
else if (IsMultiSigContract(witnessScript, out int m, out int n))
{
int size_inv = 66 * m;
size += IO.Helper.GetVarSize(size_inv) + size_inv + witnessScript.GetVarSize();
networkFee += exec_fee_factor * MultiSignatureContractCost(m, n);
}
}
// We can support more contract types in the future.
}
networkFee += size * NativeContract.Policy.GetFeePerByte(snapshot);
foreach (TransactionAttribute attr in tx.Attributes)
Expand Down