change network fee calculation (#178)

* change network fee calculation

* update to CI00847

* fix fee issue

* add UT for network fee

src/RpcClient/RpcClient.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <Version>3.0.0-CI00825</Version>
+    <Version>3.0.0-CI00847</Version>
     <TargetFramework>netstandard2.1</TargetFramework>
     <RootNamespace>Neo.Network.RPC</RootNamespace>
     <Authors>The Neo Project</Authors>
@@ -14,7 +14,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Neo" Version="3.0.0-CI00846" />
+    <PackageReference Include="Neo" Version="3.0.0-CI00847" />
   </ItemGroup>
 
 </Project>

src/RpcClient/TransactionManager.cs

@@ -4,9 +4,9 @@
 using Neo.Network.RPC.Models;
 using Neo.SmartContract;
 using Neo.SmartContract.Native;
-using Neo.VM;
 using Neo.Wallets;
 using System;
+using System.Linq;
 
 namespace Neo.Network.RPC
 {
@@ -62,9 +62,9 @@ public TransactionManager MakeTransaction(byte[] script, TransactionAttribute[]
                 Script = script,
                 Sender = sender,
                 ValidUntilBlock = height + Transaction.MaxValidUntilBlockIncrement,
-                Attributes = attributes ?? new TransactionAttribute[0],
-                Cosigners = cosigners ?? new Cosigner[0],
-                Witnesses = new Witness[0]
+                Attributes = attributes ?? Array.Empty<TransactionAttribute>(),
+                Cosigners = cosigners ?? Array.Empty<Cosigner>(),
+                Witnesses = Array.Empty<Witness>()
             };
 
             // Add witness hashes parameter to pass CheckWitness
@@ -84,63 +84,49 @@ public TransactionManager MakeTransaction(byte[] script, TransactionAttribute[]
             context = new ContractParametersContext(Tx);
 
             // set networkfee to estimate value when networkFee is 0
-            Tx.NetworkFee = networkFee == 0 ? EstimateNetworkFee() : networkFee;
+            Tx.NetworkFee = networkFee == 0 ? CalculateNetworkFee(true) : networkFee;
 
             var gasBalance = nep5API.BalanceOf(NativeContract.GAS.Hash, sender);
             if (gasBalance >= Tx.SystemFee + Tx.NetworkFee) return this;
             throw new InvalidOperationException($"Insufficient GAS in address: {sender.ToAddress()}");
         }
 
         /// <summary>
-        /// Estimate NetworkFee, assuming the witnesses are basic Signature Contract
+        /// Calculate NetworkFee
         /// </summary>
-        private long EstimateNetworkFee()
+        /// <param name="isEstimate">assuming the witnesses are basic Signature Contract if set to true</param>
+        /// <returns></returns>
+        private long CalculateNetworkFee(bool isEstimate = false)
         {
             long networkFee = 0;
             UInt160[] hashes = Tx.GetScriptHashesForVerifying(null);
-            int size = Transaction.HeaderSize + Tx.Attributes.GetVarSize() + Tx.Script.GetVarSize() + IO.Helper.GetVarSize(hashes.Length);
-
-            // assume the hashes are single Signature
-            foreach (var hash in hashes)
+            int size = Transaction.HeaderSize + Tx.Attributes.GetVarSize() + Tx.Cosigners.GetVarSize() + Tx.Script.GetVarSize() + IO.Helper.GetVarSize(hashes.Length);
+            foreach (UInt160 hash in hashes)
             {
-                size += 166;
-                networkFee += ApplicationEngine.OpCodePrices[OpCode.PUSHNULL]; // message
-                using (ScriptBuilder sb = new ScriptBuilder())
+                byte[] witness_script = null;
+                if (isEstimate)
                 {
-                    networkFee += ApplicationEngine.OpCodePrices[(OpCode)sb.EmitPush(64).ToArray()[0]]; // signature
-                    networkFee += ApplicationEngine.OpCodePrices[(OpCode)sb.EmitPush(33).ToArray()[0]]; // pubKey
-                    size += sb.ToArray().Length;
+                    // assuming the witnesses are basic Signature Contract
+                    KeyPair one = new KeyPair(new byte[31].Concat(new byte[] { 1 }).ToArray());
+                    witness_script = Contract.CreateSignatureRedeemScript(one.PublicKey);
                 }
-                networkFee += InteropService.GetPrice(InteropService.Crypto.ECDsaVerify, null);
-            }
-
-            networkFee += size * policyAPI.GetFeePerByte();
-            return networkFee;
-        }
-
-        /// <summary>
-        /// Calculate NetworkFee with context items
-        /// </summary>
-        private long CalculateNetworkFee()
-        {
-            long networkFee = 0;
-            UInt160[] hashes = Tx.GetScriptHashesForVerifying(null);
-            int size = Transaction.HeaderSize + Tx.Attributes.GetVarSize() + Tx.Script.GetVarSize() + IO.Helper.GetVarSize(hashes.Length);
-            foreach (UInt160 hash in hashes)
-            {
-                byte[] witness_script = context.GetScript(hash);
-                if (witness_script is null || witness_script.Length == 0)
+                else
                 {
-                    try
+                    // calculate NetworkFee with context items
+                    witness_script = context.GetScript(hash);
+                    if (witness_script is null || witness_script.Length == 0)
                     {
-                        witness_script = rpcClient.GetContractState(hash.ToString())?.Script;
+                        try
+                        {
+                            witness_script = rpcClient.GetContractState(hash.ToString())?.Script;
+                        }
+                        catch { }
                     }
-                    catch { }
-                }
 
-                if (witness_script is null) continue;
+                    if (witness_script is null) continue;
+                }
 
-                networkFee += Wallet.CalculateNetWorkFee(witness_script, ref size);
+                networkFee += Wallet.CalculateNetworkFee(witness_script, ref size);
             }
             networkFee += size * policyAPI.GetFeePerByte();
             return networkFee;

tests/Neo.Network.RPC.Tests/UT_TransactionManager.cs

@@ -112,16 +112,26 @@ public void TestSign()
                 }
             };
 
+            Cosigner[] cosigners = new Cosigner[1] {
+                new Cosigner{
+                    Account  =  sender,
+                    Scopes = WitnessScope.Global
+                }
+            };
+
             byte[] script = new byte[1];
-            txManager.MakeTransaction(script, attributes)
+            txManager.MakeTransaction(script, attributes, cosigners)
                 .AddSignature(keyPair1)
                 .Sign();
 
             // get signature from Witnesses
             var tx = txManager.Tx;
-            byte[] signature = tx.Witnesses[0].InvocationScript.Skip(1).ToArray();
+            byte[] signature = tx.Witnesses[0].InvocationScript.Skip(2).ToArray();
 
             Assert.IsTrue(Crypto.VerifySignature(tx.GetHashData(), signature, keyPair1.PublicKey.EncodePoint(false).Skip(1).ToArray()));
+            // verify network fee
+            long networkFee = tx.Size * (long)1000 + ApplicationEngine.OpCodePrices[OpCode.PUSHDATA1] + ApplicationEngine.OpCodePrices[OpCode.PUSHDATA1] + ApplicationEngine.OpCodePrices[OpCode.PUSHNULL] + InteropService.GetPrice(InteropService.Crypto.ECDsaVerify, null);
+            Assert.AreEqual(networkFee, tx.NetworkFee);
 
             // duplicate sign should not add new witness
             txManager.AddSignature(keyPair1).Sign();