Policy inputs checks (neo-project#1938)

* Policy contract's SetMaxTransactionsPerBlock() doesn't check for Block.MaxTransactionsPerBlock * add more * Clean change * Fix ut * throw exceptions * ArgumentOutOfRangeException * Update src/neo/SmartContract/Native/PolicyContract.cs Co-authored-by: Erik Zhang <erik@neo.org> * Fix UT * Update NativeContractExtensions.cs Co-authored-by: erikzhang <erik@neo.org>
cloud8little · Jan 24, 2021 · dd2a968 · dd2a968
1 parent 05c30fb
commit dd2a968
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 10 deletions.
diff --git a/src/neo/SmartContract/Native/PolicyContract.cs b/src/neo/SmartContract/Native/PolicyContract.cs
@@ -1,6 +1,8 @@
 #pragma warning disable IDE0051
 
 using Neo.Ledger;
+using Neo.Network.P2P;
+using Neo.Network.P2P.Payloads;
 using Neo.Persistence;
 using Neo.SmartContract.Manifest;
 using System;
@@ -87,8 +89,8 @@ public bool IsAnyAccountBlocked(StoreView snapshot, params UInt160[] hashes)
         [ContractMethod(0_03000000, CallFlags.AllowModifyStates)]
         private bool SetMaxBlockSize(ApplicationEngine engine, uint value)
         {
+            if (value > Message.PayloadMaxSize) throw new ArgumentOutOfRangeException(nameof(value));
             if (!CheckCommittee(engine)) return false;
-            if (Network.P2P.Message.PayloadMaxSize <= value) return false;
             StorageItem storage = engine.Snapshot.Storages.GetAndChange(CreateStorageKey(Prefix_MaxBlockSize), () => new StorageItem());
             storage.Set(value);
             return true;
@@ -97,6 +99,7 @@ private bool SetMaxBlockSize(ApplicationEngine engine, uint value)
         [ContractMethod(0_03000000, CallFlags.AllowModifyStates)]
         private bool SetMaxTransactionsPerBlock(ApplicationEngine engine, uint value)
         {
+            if (value > Block.MaxTransactionsPerBlock) throw new ArgumentOutOfRangeException(nameof(value));
             if (!CheckCommittee(engine)) return false;
             StorageItem storage = engine.Snapshot.Storages.GetAndChange(CreateStorageKey(Prefix_MaxTransactionsPerBlock), () => new StorageItem());
             storage.Set(value);
@@ -106,8 +109,8 @@ private bool SetMaxTransactionsPerBlock(ApplicationEngine engine, uint value)
         [ContractMethod(0_03000000, CallFlags.AllowModifyStates)]
         private bool SetMaxBlockSystemFee(ApplicationEngine engine, long value)
         {
+            if (value <= 4007600) throw new ArgumentOutOfRangeException(nameof(value));
             if (!CheckCommittee(engine)) return false;
-            if (value <= 4007600) return false;
             StorageItem storage = engine.Snapshot.Storages.GetAndChange(CreateStorageKey(Prefix_MaxBlockSystemFee), () => new StorageItem());
             storage.Set(value);
             return true;
@@ -116,6 +119,7 @@ private bool SetMaxBlockSystemFee(ApplicationEngine engine, long value)
         [ContractMethod(0_03000000, CallFlags.AllowModifyStates)]
         private bool SetFeePerByte(ApplicationEngine engine, long value)
         {
+            if (value < 0 || value > 1_00000000) throw new ArgumentOutOfRangeException(nameof(value));
             if (!CheckCommittee(engine)) return false;
             StorageItem storage = engine.Snapshot.Storages.GetAndChange(CreateStorageKey(Prefix_FeePerByte), () => new StorageItem());
             storage.Set(value);

diff --git a/tests/neo.UnitTests/Extensions/NativeContractExtensions.cs b/tests/neo.UnitTests/Extensions/NativeContractExtensions.cs
@@ -33,7 +33,9 @@ public static StackItem Call(this NativeContract contract, StoreView snapshot, I
 
             if (engine.Execute() != VMState.HALT)
             {
-                throw new InvalidOperationException();
+                Exception exception = engine.FaultException;
+                while (exception?.InnerException != null) exception = exception.InnerException;
+                throw exception ?? new InvalidOperationException();
             }
 
             return engine.ResultStack.Pop();

diff --git a/tests/neo.UnitTests/SmartContract/Native/UT_PolicyContract.cs b/tests/neo.UnitTests/SmartContract/Native/UT_PolicyContract.cs
@@ -6,6 +6,7 @@
 using Neo.SmartContract;
 using Neo.SmartContract.Native;
 using Neo.UnitTests.Extensions;
+using System;
 using System.Linq;
 
 namespace Neo.UnitTests.SmartContract.Native
@@ -72,10 +73,11 @@ public void Check_SetMaxBlockSize()
 
             // More than expected
 
-            ret = NativeContract.Policy.Call(snapshot, new Nep5NativeContractExtensions.ManualWitness(committeeMultiSigAddr),
-                 "setMaxBlockSize", new ContractParameter(ContractParameterType.Integer) { Value = Neo.Network.P2P.Message.PayloadMaxSize });
-            ret.Should().BeOfType<VM.Types.Boolean>();
-            ret.GetBoolean().Should().BeFalse();
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() =>
+            {
+                NativeContract.Policy.Call(snapshot, new Nep5NativeContractExtensions.ManualWitness(committeeMultiSigAddr),
+                 "setMaxBlockSize", new ContractParameter(ContractParameterType.Integer) { Value = Neo.Network.P2P.Message.PayloadMaxSize + 1 });
+            });
 
             ret = NativeContract.Policy.Call(snapshot, "getMaxBlockSize");
             ret.Should().BeOfType<VM.Types.Integer>();
@@ -120,10 +122,11 @@ public void Check_SetMaxBlockSystemFee()
 
             // Less than expected
 
-            ret = NativeContract.Policy.Call(snapshot, new Nep5NativeContractExtensions.ManualWitness(committeeMultiSigAddr),
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() =>
+            {
+                NativeContract.Policy.Call(snapshot, new Nep5NativeContractExtensions.ManualWitness(committeeMultiSigAddr),
                  "setMaxBlockSystemFee", new ContractParameter(ContractParameterType.Integer) { Value = -1000 });
-            ret.Should().BeOfType<VM.Types.Boolean>();
-            ret.GetBoolean().Should().BeFalse();
+            });
 
             ret = NativeContract.Policy.Call(snapshot, "getMaxBlockSystemFee");
             ret.Should().BeOfType<VM.Types.Integer>();