-
Notifications
You must be signed in to change notification settings - Fork 1k
[N4] Whitelist of Free Contracts #4201
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
67e4975
d3b943a
39b37b5
9599679
8403891
4bd86c7
4f0c588
a6585bb
0d1ebb0
0a231b7
0a99f61
7181c93
452f9a5
6c55def
a2cd7ef
2378848
2bb8bda
9071acc
142b713
ed1cf40
fb95b4a
ea5a56f
e2e62bb
5c8874d
bed8c57
c8d1549
3e08ff8
de16170
f4e4706
538e5b4
14a7c22
5a01a56
f864e8a
0dad015
1ebd85d
6d59406
6d711c0
67915b2
7036374
6c7f6c4
cb56508
4790594
e5229fe
6c35bf8
852771d
0c268ea
37d3b3b
31dea06
c52eb34
e74abfd
2943818
1328bb4
cfba43d
7991b5e
3e63b2f
e37e142
c330319
a0f5ed0
5bdb4c4
5cbbcd5
6b4468d
c3dd619
aa39868
6126b4d
e29b885
a3eeb56
be1449d
ea94531
f95e3d5
7a84ada
e2fab41
bccd875
01d4305
d4f04ba
02f4ee9
a61ea05
216a17b
356deee
ac1e7fc
69f7d7a
1758d29
bfbee17
e1141a4
42ad70f
c8aa869
ec27ab8
a6d5b66
3c63828
1baf431
62c8cf0
257756e
06ace04
9a7f8d7
96d2c80
045d042
cbf5650
58051c3
78e3618
715bb20
89ba23b
8b2a9ea
0931394
a29922e
e95dca4
bc6f34e
bdfcd66
e3fc3d0
3ba7b14
d06a07d
d42392b
e8e410a
fe2e37c
cc946e6
daf5993
a998ad0
f5bedb3
efcfe3c
657cac5
8afce40
a7bd974
9b85382
07a208a
9004da9
5cc2cfd
e818d88
1c5d3b8
d38f847
d7ec803
5dd874c
2e10d41
1b1d563
5670d9d
d180196
bf73d6e
0938348
363d730
3692bd3
ae044ef
5321dfe
ce4894e
b7b387e
4febfb2
d6be6e0
d0bcc5e
3e401e7
07ea7fa
9f148d6
28e5ab3
451c5e9
2bb4090
d9b80e2
4ba1439
6ffece3
58125f2
146e81e
8fb2bd4
a70d254
ee6ed70
e4e15f1
d118892
0f853fc
4246cc0
ba34663
6043033
6293230
73fd379
d5517eb
db6dd64
272af62
55b7c94
a5b47d2
fe5c745
b4d69e0
a3ef31f
f83b9f1
1b8560e
72eab3b
d7c87fa
455e17a
7345a68
8fd4361
d7e9a6b
26345ca
e491c3d
98e3c63
394bca5
423fc99
d5f54ed
3923ad6
12e634b
b08d5b6
30aaa45
e23feeb
aef6f4d
4e4b25d
4d8e19b
7b58eb7
b9c6438
bee0a76
557b6c9
f3b8170
ceb7cda
426c81a
1b5bb68
4ab5033
17a0c32
c5c3eb6
10229de
f5fb8b2
2093d06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -299,6 +299,14 @@ protected static void OnSysCall(ExecutionEngine engine, Instruction instruction) | |
| /// <param name="datoshi">The amount of GAS, in the unit of datoshi, 1 datoshi = 1e-8 GAS, to be added.</param> | ||
| protected internal void AddFee(long datoshi) | ||
| { | ||
| // Check whitelist | ||
|
|
||
| if (CurrentContext?.GetState<ExecutionContextState>()?.WhiteListed == true) | ||
| { | ||
| // The execution is whitelisted | ||
| return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we still need to AddFee here, to make sure we still get correct maximum GAS fee per block.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if it's free why we need to check the max fee per block? you propose to have two counters? free and paid?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shargon , casue if we dont count the actual gas cost, attacker may use whitelist contract as the DOS attack entry point. Free tranasction should still be counted how much resource it uses, to avoid too much trasnaction being added. Its just a suggestion.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Jim8y I have been thinking about this because it seemed like a good idea to me, but .... I think it cannot be done since the consensus calculated the transaction fee based on signed transaction attributes and the consensus does not execute the transaction to know how much free fee it had, so at the time of deciding which transactions go in the block, that value is not available even if we had it in the application engine. |
||
| } | ||
|
|
||
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| FeeConsumed = GasConsumed = checked(FeeConsumed + datoshi); | ||
| #pragma warning restore CS0618 // Type or member is obsolete | ||
|
|
@@ -341,12 +349,21 @@ private ExecutionContext CallContractInternal(ContractState contract, ContractMe | |
| else | ||
| { | ||
| var executingContract = IsHardforkEnabled(Hardfork.HF_Domovoi) | ||
| ? state.Contract // use executing contract state to avoid possible contract update/destroy side-effects, ref. https://github.com/neo-project/neo/pull/3290. | ||
| : NativeContract.ContractManagement.GetContract(SnapshotCache, CurrentScriptHash); | ||
| ? state.Contract // use executing contract state to avoid possible contract update/destroy side-effects, ref. https://github.com/neo-project/neo/pull/3290. | ||
| : NativeContract.ContractManagement.GetContract(SnapshotCache, CurrentScriptHash); | ||
| if (executingContract?.CanCall(contract, method.Name) == false) | ||
| throw new InvalidOperationException($"Cannot Call Method {method.Name} Of Contract {contract.Hash} From Contract {CurrentScriptHash}"); | ||
| } | ||
|
|
||
| // Check whitelist | ||
|
|
||
| if (IsHardforkEnabled(Hardfork.HF_Faun) && | ||
| NativeContract.Policy.IsWhitelistFeeContract(SnapshotCache, contract.Hash, method.Name, method.Parameters.Length, out var fixedFee)) | ||
| { | ||
| AddFee(fixedFee.Value); | ||
| state.WhiteListed = true; | ||
shargon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if (invocationCounter.TryGetValue(contract.Hash, out var counter)) | ||
| { | ||
| invocationCounter[contract.Hash] = counter + 1; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -397,6 +397,9 @@ protected static void AssertCommittee(ApplicationEngine engine) | |
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| private protected StorageKey CreateStorageKey(byte prefix, UInt256 hash, UInt160 signer) => StorageKey.Create(Id, prefix, hash, signer); | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| private protected StorageKey CreateStorageKey(byte prefix, UInt160 hash, string methodName, int bigEndianKey) => StorageKey.Create(Id, prefix, hash, methodName, bigEndianKey); | ||
|
|
||
| #endregion | ||
|
|
||
| /// <summary> | ||
|
|
@@ -435,8 +438,17 @@ internal async void Invoke(ApplicationEngine engine, byte version) | |
| var state = context.GetState<ExecutionContextState>(); | ||
| if (!state.CallFlags.HasFlag(method.RequiredCallFlags)) | ||
| throw new InvalidOperationException($"Cannot call this method with the flag {state.CallFlags}."); | ||
| // In the unit of datoshi, 1 datoshi = 1e-8 GAS | ||
| engine.AddFee(method.CpuFee * engine.ExecFeeFactor + method.StorageFee * engine.StoragePrice); | ||
| // Check native-whitelist | ||
| if (engine.IsHardforkEnabled(Hardfork.HF_Faun) && Policy.IsWhitelistFeeContract(engine.SnapshotCache, Hash, method.Name, method.Parameters.Length, out var fixedFee)) | ||
| { | ||
| // Whitelisted In the unit of datoshi, 1 datoshi = 1e-8 GAS | ||
| engine.AddFee(fixedFee.Value); | ||
erikzhang marked this conversation as resolved.
Show resolved
Hide resolved
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this implementation might be wrong. |
||
| } | ||
| else | ||
| { | ||
| // In the unit of datoshi, 1 datoshi = 1e-8 GAS | ||
| engine.AddFee(method.CpuFee * engine.ExecFeeFactor + method.StorageFee * engine.StoragePrice); | ||
| } | ||
| List<object> parameters = new(); | ||
| if (method.NeedApplicationEngine) parameters.Add(engine); | ||
| if (method.NeedSnapshot) parameters.Add(engine.SnapshotCache); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,10 +11,13 @@ | |
|
|
||
| #pragma warning disable IDE0051 | ||
|
|
||
| using Neo.Extensions; | ||
| using Neo.Network.P2P.Payloads; | ||
| using Neo.Persistence; | ||
| using Neo.SmartContract.Iterators; | ||
| using System; | ||
| using System.Buffers.Binary; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Numerics; | ||
|
|
||
| namespace Neo.SmartContract.Native | ||
|
|
@@ -83,6 +86,7 @@ public sealed class PolicyContract : NativeContract | |
| public const uint MaxMaxTraceableBlocks = 2102400; | ||
|
|
||
| private const byte Prefix_BlockedAccount = 15; | ||
| internal const byte Prefix_WhitelistedFeeContracts = 16; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why internal?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it's used by UT
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be |
||
| private const byte Prefix_FeePerByte = 10; | ||
| private const byte Prefix_ExecFeeFactor = 18; | ||
| private const byte Prefix_StoragePrice = 19; | ||
|
|
@@ -103,10 +107,18 @@ public sealed class PolicyContract : NativeContract | |
| /// </summary> | ||
| private const string MillisecondsPerBlockChangedEventName = "MillisecondsPerBlockChanged"; | ||
|
|
||
| private const string WhitelistChangedEventName = "WhitelistChanged"; | ||
|
|
||
| [ContractEvent(Hardfork.HF_Echidna, 0, name: MillisecondsPerBlockChangedEventName, | ||
| "old", ContractParameterType.Integer, | ||
| "new", ContractParameterType.Integer | ||
| )] | ||
| [ContractEvent(Hardfork.HF_Faun, 1, name: WhitelistChangedEventName, | ||
| "contract", ContractParameterType.Hash160, | ||
| "method", ContractParameterType.String, | ||
| "argCount", ContractParameterType.Integer, | ||
| "fee", ContractParameterType.Any | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it can be null in |
||
| )] | ||
| internal PolicyContract() : base() | ||
| { | ||
| _feePerByte = CreateStorageKey(Prefix_FeePerByte); | ||
|
|
@@ -258,6 +270,114 @@ public bool IsBlocked(IReadOnlyStore snapshot, UInt160 account) | |
| return snapshot.Contains(CreateStorageKey(Prefix_BlockedAccount, account)); | ||
| } | ||
|
|
||
| internal bool IsWhitelistFeeContract(DataCache snapshot, UInt160 contractHash, string method, int argCount, [NotNullWhen(true)] out long? fixedFee) | ||
| { | ||
| // Check contract existence | ||
|
|
||
| var currentContract = ContractManagement.GetContract(snapshot, contractHash); | ||
|
|
||
| if (currentContract != null) | ||
| { | ||
| // Check state existence | ||
|
|
||
| var item = snapshot.TryGet(CreateStorageKey(Prefix_WhitelistedFeeContracts, contractHash, method, argCount)); | ||
|
|
||
| if (item != null) | ||
| { | ||
| fixedFee = (long)(BigInteger)item; | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| fixedFee = null; | ||
| return false; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Remove whitelisted Fee contracts | ||
| /// </summary> | ||
| /// <param name="engine">The execution engine.</param> | ||
| /// <param name="contractHash">The contract to set the whitelist</param> | ||
| /// <param name="method">Method</param> | ||
| /// <param name="argCount">Argument count</param> | ||
| [ContractMethod(Hardfork.HF_Faun, CpuFee = 1 << 15, RequiredCallFlags = CallFlags.States | CallFlags.AllowNotify)] | ||
| private void RemoveWhitelistFeeContract(ApplicationEngine engine, UInt160 contractHash, string method, int argCount) | ||
| { | ||
| if (!CheckCommittee(engine)) throw new InvalidOperationException("Invalid committee signature"); | ||
|
|
||
| var key = CreateStorageKey(Prefix_WhitelistedFeeContracts, contractHash, method, argCount); | ||
|
|
||
| if (!engine.SnapshotCache.Contains(key)) throw new InvalidOperationException("Whitelist not found"); | ||
|
|
||
| engine.SnapshotCache.Delete(key); | ||
|
|
||
| // Emit event | ||
| engine.SendNotification(Hash, WhitelistChangedEventName, | ||
| [new VM.Types.ByteString(contractHash.ToArray()), new VM.Types.ByteString(method.ToStrictUtf8Bytes()), | ||
| new VM.Types.Integer(argCount), VM.Types.StackItem.Null]); | ||
| } | ||
|
|
||
| internal int CleanWhitelist(ApplicationEngine engine, UInt160 contractHash) | ||
| { | ||
| var count = 0; | ||
| var searchKey = CreateStorageKey(Prefix_WhitelistedFeeContracts, contractHash); | ||
|
|
||
| foreach ((var key, _) in engine.SnapshotCache.Find(searchKey, SeekDirection.Forward)) | ||
| { | ||
| engine.SnapshotCache.Delete(key); | ||
| count++; | ||
|
|
||
| // Emit event recovering the values from the Key | ||
|
|
||
| var keyData = key.ToArray().AsSpan(); | ||
| var argCount = BinaryPrimitives.ReadInt32BigEndian(keyData.Slice(StorageKey.UInt160Length, 4)); | ||
| var method = keyData[(StorageKey.UInt160Length + 4)..]; | ||
|
|
||
| engine.SendNotification(Hash, WhitelistChangedEventName, | ||
| [new VM.Types.ByteString(contractHash.ToArray()), new VM.Types.ByteString(method.ToArray()), | ||
| new VM.Types.Integer(argCount), VM.Types.StackItem.Null]); | ||
| } | ||
|
|
||
| return count; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Set whitelisted Fee contracts | ||
| /// </summary> | ||
| /// <param name="engine">The execution engine.</param> | ||
| /// <param name="contractHash">The contract to set the whitelist</param> | ||
| /// <param name="method">Method</param> | ||
| /// <param name="argCount">Argument count</param> | ||
| /// <param name="fixedFee">Fixed execution fee</param> | ||
| [ContractMethod(Hardfork.HF_Faun, CpuFee = 1 << 15, RequiredCallFlags = CallFlags.States | CallFlags.AllowNotify)] | ||
| internal void SetWhitelistFeeContract(ApplicationEngine engine, UInt160 contractHash, string method, int argCount, long fixedFee) | ||
| { | ||
| ArgumentOutOfRangeException.ThrowIfNegative(fixedFee, nameof(fixedFee)); | ||
|
|
||
shargon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!CheckCommittee(engine)) throw new InvalidOperationException("Invalid committee signature"); | ||
|
|
||
| var key = CreateStorageKey(Prefix_WhitelistedFeeContracts, contractHash, method, argCount); | ||
|
|
||
| // Validate methods | ||
| var contract = ContractManagement.GetContract(engine.SnapshotCache, contractHash) | ||
| ?? throw new InvalidOperationException("Is not a valid contract"); | ||
|
|
||
| if (contract.Manifest.Abi.GetMethod(method, argCount) == null) | ||
| throw new InvalidOperationException($"{method} with {argCount} args is not a valid method of {contractHash}"); | ||
|
|
||
| // Set | ||
| var entry = engine.SnapshotCache | ||
| .GetAndChange(key, () => new StorageItem(fixedFee)); | ||
|
|
||
| entry.Set(fixedFee); | ||
|
|
||
| // Emit event | ||
|
|
||
| engine.SendNotification(Hash, WhitelistChangedEventName, | ||
| [new VM.Types.ByteString(contractHash.ToArray()), new VM.Types.ByteString(method.ToStrictUtf8Bytes()), | ||
| new VM.Types.Integer(argCount), new VM.Types.Integer(fixedFee)]); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sets the block generation time in milliseconds. | ||
| /// </summary> | ||
|
|
@@ -438,5 +558,16 @@ private StorageIterator GetBlockedAccounts(DataCache snapshot) | |
| .GetEnumerator(); | ||
| return new StorageIterator(enumerator, 1, options); | ||
| } | ||
|
|
||
| [ContractMethod(Hardfork.HF_Faun, CpuFee = 1 << 15, RequiredCallFlags = CallFlags.ReadStates)] | ||
| internal StorageIterator GetWhitelistFeeContracts(DataCache snapshot) | ||
Wi1l-B0t marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| const FindOptions options = FindOptions.RemovePrefix | FindOptions.KeysOnly; | ||
| var enumerator = snapshot | ||
| .Find(CreateStorageKey(Prefix_WhitelistedFeeContracts), SeekDirection.Forward) | ||
| .GetEnumerator(); | ||
|
|
||
| return new StorageIterator(enumerator, 1, options); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| using System.Diagnostics; | ||
| using System.Linq; | ||
| using System.Runtime.CompilerServices; | ||
| using System.Text; | ||
|
|
||
| namespace Neo.SmartContract | ||
| { | ||
|
|
@@ -76,7 +77,7 @@ public int Length | |
| private const int ByteLength = PrefixLength + sizeof(byte); | ||
| private const int Int32Length = PrefixLength + sizeof(int); | ||
| private const int Int64Length = PrefixLength + sizeof(long); | ||
| private const int UInt160Length = PrefixLength + UInt160.Length; | ||
| internal const int UInt160Length = PrefixLength + UInt160.Length; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why internal? |
||
| private const int UInt256Length = PrefixLength + UInt256.Length; | ||
| private const int UInt256UInt160Length = PrefixLength + UInt256.Length + UInt160.Length; | ||
|
|
||
|
|
@@ -183,6 +184,30 @@ public static StorageKey Create(int id, byte prefix, UInt256 hash, UInt160 signe | |
| return new(id, data); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Create StorageKey | ||
| /// </summary> | ||
| /// <param name="id">The id of the contract.</param> | ||
| /// <param name="prefix">The prefix of the key.</param> | ||
| /// <param name="hash">Hash</param> | ||
| /// <param name="methodName">Method Name</param> | ||
| /// <param name="bigEndian">Big Endian key.</param> | ||
| /// <returns>The <see cref="StorageKey"/> class</returns> | ||
| public static StorageKey Create(int id, byte prefix, UInt160 hash, string methodName, int bigEndian) | ||
| { | ||
| const int HashAndInt = UInt160Length + sizeof(int); | ||
|
|
||
| var methodData = Encoding.UTF8.GetBytes(methodName); | ||
| var data = new byte[HashAndInt + methodData.Length]; | ||
|
|
||
| FillHeader(data, id, prefix); | ||
| hash.Serialize(data.AsSpan(PrefixLength..)); | ||
| BinaryPrimitives.WriteInt32BigEndian(data.AsSpan(UInt160Length..), bigEndian); | ||
| Array.Copy(methodData, 0, data, HashAndInt, methodData.Length); | ||
Wi1l-B0t marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return new(id, data); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Create StorageKey | ||
| /// </summary> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.