From 18054538d1d77816db44580bfdaf4bd8d43324e2 Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 19 Nov 2020 16:19:29 +0100 Subject: [PATCH 01/13] Fix Oracle PostPersist --- src/neo/SmartContract/Native/Oracle/OracleContract.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/neo/SmartContract/Native/Oracle/OracleContract.cs b/src/neo/SmartContract/Native/Oracle/OracleContract.cs index 347803d076..2a5c9ce07e 100644 --- a/src/neo/SmartContract/Native/Oracle/OracleContract.cs +++ b/src/neo/SmartContract/Native/Oracle/OracleContract.cs @@ -145,6 +145,9 @@ protected override void PostPersist(ApplicationEngine engine) OracleResponse response = tx.GetAttribute(); if (response is null) continue; + // Check tx state + if (engine.Snapshot.Transactions.TryGet(tx.Hash)?.VMState != VMState.HALT) continue; + //Remove the request from storage StorageKey key = CreateStorageKey(Prefix_Request).Add(response.Id); OracleRequest request = engine.Snapshot.Storages[key].GetInteroperable(); @@ -153,11 +156,11 @@ protected override void PostPersist(ApplicationEngine engine) //Remove the id from IdList key = CreateStorageKey(Prefix_IdList).Add(GetUrlHash(request.Url)); IdList list = engine.Snapshot.Storages.GetAndChange(key).GetInteroperable(); - if (!list.Remove(response.Id)) throw new InvalidOperationException(); + if (!list.Remove(response.Id)) continue; if (list.Count == 0) engine.Snapshot.Storages.Delete(key); //Mint GAS for oracle nodes - nodes ??= NativeContract.Designate.GetDesignatedByRole(engine.Snapshot, Role.Oracle, engine.Snapshot.PersistingBlock.Index).Select(p => (Contract.CreateSignatureRedeemScript(p).ToScriptHash(), BigInteger.Zero)).ToArray(); + nodes ??= Designate.GetDesignatedByRole(engine.Snapshot, Role.Oracle, engine.Snapshot.PersistingBlock.Index).Select(p => (Contract.CreateSignatureRedeemScript(p).ToScriptHash(), BigInteger.Zero)).ToArray(); if (nodes.Length > 0) { int index = (int)(response.Id % (ulong)nodes.Length); From 74c7a1adbe4744216db7d7a2c4c78ca61e40e58c Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 19 Nov 2020 16:24:37 +0100 Subject: [PATCH 02/13] Add other check --- src/neo/SmartContract/Native/Oracle/OracleContract.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/neo/SmartContract/Native/Oracle/OracleContract.cs b/src/neo/SmartContract/Native/Oracle/OracleContract.cs index 2a5c9ce07e..87af50a285 100644 --- a/src/neo/SmartContract/Native/Oracle/OracleContract.cs +++ b/src/neo/SmartContract/Native/Oracle/OracleContract.cs @@ -150,7 +150,8 @@ protected override void PostPersist(ApplicationEngine engine) //Remove the request from storage StorageKey key = CreateStorageKey(Prefix_Request).Add(response.Id); - OracleRequest request = engine.Snapshot.Storages[key].GetInteroperable(); + OracleRequest request = engine.Snapshot.Storages.TryGet(key)?.GetInteroperable(); + if (request == null) continue; engine.Snapshot.Storages.Delete(key); //Remove the id from IdList From 1da5b7f98c96bb1014c6fe7145ae8a2a36193eee Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 19 Nov 2020 17:54:34 +0100 Subject: [PATCH 03/13] Prevent two same responses in the same block --- .../Ledger/TransactionVerificationContext.cs | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index a2bae45b0b..dabf45f4c6 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -13,12 +13,20 @@ public class TransactionVerificationContext /// private readonly Dictionary senderFee = new Dictionary(); + /// + /// Store oracle requests + /// + private readonly Dictionary oracleRequests = new Dictionary(); + public void AddTransaction(Transaction tx) { if (senderFee.TryGetValue(tx.Sender, out var value)) senderFee[tx.Sender] = value + tx.SystemFee + tx.NetworkFee; else senderFee.Add(tx.Sender, tx.SystemFee + tx.NetworkFee); + + var oracle = tx.GetAttribute(); + if (oracle != null) oracleRequests.TryAdd(oracle.Id, tx.Hash); } public bool CheckTransaction(Transaction tx, StoreView snapshot) @@ -26,12 +34,27 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot) BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, tx.Sender); senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool); BigInteger fee = tx.SystemFee + tx.NetworkFee + totalSenderFeeFromPool; - return balance >= fee; + if (balance < fee) return false; + + var oracle = tx.GetAttribute(); + if (oracle != null && + (!oracleRequests.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash)) + { + return false; + } + + return true; } public void RemoveTransaction(Transaction tx) { if ((senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee) == 0) senderFee.Remove(tx.Sender); + + var oracle = tx.GetAttribute(); + if (oracle != null && oracleRequests.TryGetValue(oracle.Id, out var hash) && hash == tx.Hash) + { + oracleRequests.Remove(oracle.Id); + } } } } From db3eb1e287fcf5f08c8fffa325150abc50012b0f Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 19 Nov 2020 17:59:56 +0100 Subject: [PATCH 04/13] Remove check --- src/neo/SmartContract/Native/Oracle/OracleContract.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/neo/SmartContract/Native/Oracle/OracleContract.cs b/src/neo/SmartContract/Native/Oracle/OracleContract.cs index 87af50a285..028d44a4bc 100644 --- a/src/neo/SmartContract/Native/Oracle/OracleContract.cs +++ b/src/neo/SmartContract/Native/Oracle/OracleContract.cs @@ -145,9 +145,6 @@ protected override void PostPersist(ApplicationEngine engine) OracleResponse response = tx.GetAttribute(); if (response is null) continue; - // Check tx state - if (engine.Snapshot.Transactions.TryGet(tx.Hash)?.VMState != VMState.HALT) continue; - //Remove the request from storage StorageKey key = CreateStorageKey(Prefix_Request).Add(response.Id); OracleRequest request = engine.Snapshot.Storages.TryGet(key)?.GetInteroperable(); From 5715b86ea2f41f3618637ff57d7be90596bbb1cb Mon Sep 17 00:00:00 2001 From: Shargon Date: Tue, 24 Nov 2020 09:36:43 +0100 Subject: [PATCH 05/13] Revert one condition --- src/neo/SmartContract/Native/Oracle/OracleContract.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/neo/SmartContract/Native/Oracle/OracleContract.cs b/src/neo/SmartContract/Native/Oracle/OracleContract.cs index 028d44a4bc..f7661fff2b 100644 --- a/src/neo/SmartContract/Native/Oracle/OracleContract.cs +++ b/src/neo/SmartContract/Native/Oracle/OracleContract.cs @@ -154,7 +154,7 @@ protected override void PostPersist(ApplicationEngine engine) //Remove the id from IdList key = CreateStorageKey(Prefix_IdList).Add(GetUrlHash(request.Url)); IdList list = engine.Snapshot.Storages.GetAndChange(key).GetInteroperable(); - if (!list.Remove(response.Id)) continue; + if (!list.Remove(response.Id)) throw new InvalidOperationException(); if (list.Count == 0) engine.Snapshot.Storages.Delete(key); //Mint GAS for oracle nodes From 423386774ac4ac2e46394937c9317383e9ff6598 Mon Sep 17 00:00:00 2001 From: Shargon Date: Tue, 24 Nov 2020 09:57:18 +0100 Subject: [PATCH 06/13] Rename --- src/neo/Ledger/TransactionVerificationContext.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index dabf45f4c6..e410dddcd5 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -14,9 +14,9 @@ public class TransactionVerificationContext private readonly Dictionary senderFee = new Dictionary(); /// - /// Store oracle requests + /// Store oracle responses /// - private readonly Dictionary oracleRequests = new Dictionary(); + private readonly Dictionary oracleResponses = new Dictionary(); public void AddTransaction(Transaction tx) { @@ -26,7 +26,7 @@ public void AddTransaction(Transaction tx) senderFee.Add(tx.Sender, tx.SystemFee + tx.NetworkFee); var oracle = tx.GetAttribute(); - if (oracle != null) oracleRequests.TryAdd(oracle.Id, tx.Hash); + if (oracle != null) oracleResponses.TryAdd(oracle.Id, tx.Hash); } public bool CheckTransaction(Transaction tx, StoreView snapshot) @@ -38,7 +38,7 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot) var oracle = tx.GetAttribute(); if (oracle != null && - (!oracleRequests.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash)) + (!oracleResponses.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash)) { return false; } @@ -51,9 +51,9 @@ public void RemoveTransaction(Transaction tx) if ((senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee) == 0) senderFee.Remove(tx.Sender); var oracle = tx.GetAttribute(); - if (oracle != null && oracleRequests.TryGetValue(oracle.Id, out var hash) && hash == tx.Hash) + if (oracle != null && oracleResponses.TryGetValue(oracle.Id, out var hash) && hash == tx.Hash) { - oracleRequests.Remove(oracle.Id); + oracleResponses.Remove(oracle.Id); } } } From bcb6585e45537d4709b834b8f9195dd258ae14c2 Mon Sep 17 00:00:00 2001 From: Shargon Date: Tue, 24 Nov 2020 21:45:36 +0100 Subject: [PATCH 07/13] Prevent adding tx if oracle it's duplicated --- .../Ledger/TransactionVerificationContext.cs | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index e410dddcd5..9de523c2a2 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -18,21 +18,39 @@ public class TransactionVerificationContext /// private readonly Dictionary oracleResponses = new Dictionary(); + /// + /// Contains unverified transactions during adding + /// + private readonly HashSet unverifiedTx = new HashSet(); + public void AddTransaction(Transaction tx) { + var oracle = tx.GetAttribute(); + if (oracle != null) + { + if (!oracleResponses.TryAdd(oracle.Id, tx.Hash)) + { + unverifiedTx.Add(tx.Hash); + return; + } + } + if (senderFee.TryGetValue(tx.Sender, out var value)) senderFee[tx.Sender] = value + tx.SystemFee + tx.NetworkFee; else senderFee.Add(tx.Sender, tx.SystemFee + tx.NetworkFee); - - var oracle = tx.GetAttribute(); - if (oracle != null) oracleResponses.TryAdd(oracle.Id, tx.Hash); } public bool CheckTransaction(Transaction tx, StoreView snapshot) { + if (unverifiedTx.Contains(tx.Hash)) return false; + BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, tx.Sender); - senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool); + if (!senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool)) + { + return false; + } + BigInteger fee = tx.SystemFee + tx.NetworkFee + totalSenderFeeFromPool; if (balance < fee) return false; @@ -48,6 +66,7 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot) public void RemoveTransaction(Transaction tx) { + if (unverifiedTx.Remove(tx.Hash)) return; if ((senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee) == 0) senderFee.Remove(tx.Sender); var oracle = tx.GetAttribute(); From 9a2e1d165b71eb7b11cbf498b59e4ea77b50b4aa Mon Sep 17 00:00:00 2001 From: Shargon Date: Tue, 24 Nov 2020 21:47:08 +0100 Subject: [PATCH 08/13] Optimize --- src/neo/Ledger/TransactionVerificationContext.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index 9de523c2a2..774092b811 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -26,13 +26,10 @@ public class TransactionVerificationContext public void AddTransaction(Transaction tx) { var oracle = tx.GetAttribute(); - if (oracle != null) + if (oracle != null && !oracleResponses.TryAdd(oracle.Id, tx.Hash)) { - if (!oracleResponses.TryAdd(oracle.Id, tx.Hash)) - { - unverifiedTx.Add(tx.Hash); - return; - } + unverifiedTx.Add(tx.Hash); + return; } if (senderFee.TryGetValue(tx.Sender, out var value)) @@ -43,8 +40,6 @@ public void AddTransaction(Transaction tx) public bool CheckTransaction(Transaction tx, StoreView snapshot) { - if (unverifiedTx.Contains(tx.Hash)) return false; - BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, tx.Sender); if (!senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool)) { @@ -56,7 +51,7 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot) var oracle = tx.GetAttribute(); if (oracle != null && - (!oracleResponses.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash)) + (!oracleResponses.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash || unverifiedTx.Contains(tx.Hash))) { return false; } From 95674642512035b0a9cf7ce7370575a403ece2bf Mon Sep 17 00:00:00 2001 From: Shargon Date: Wed, 25 Nov 2020 12:05:44 +0100 Subject: [PATCH 09/13] Revert --- src/neo/Ledger/TransactionVerificationContext.cs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index 774092b811..fa115b34e8 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -18,17 +18,11 @@ public class TransactionVerificationContext /// private readonly Dictionary oracleResponses = new Dictionary(); - /// - /// Contains unverified transactions during adding - /// - private readonly HashSet unverifiedTx = new HashSet(); - public void AddTransaction(Transaction tx) { var oracle = tx.GetAttribute(); if (oracle != null && !oracleResponses.TryAdd(oracle.Id, tx.Hash)) { - unverifiedTx.Add(tx.Hash); return; } @@ -41,17 +35,14 @@ public void AddTransaction(Transaction tx) public bool CheckTransaction(Transaction tx, StoreView snapshot) { BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, tx.Sender); - if (!senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool)) - { - return false; - } + senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool); BigInteger fee = tx.SystemFee + tx.NetworkFee + totalSenderFeeFromPool; if (balance < fee) return false; var oracle = tx.GetAttribute(); if (oracle != null && - (!oracleResponses.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash || unverifiedTx.Contains(tx.Hash))) + (!oracleResponses.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash)) { return false; } @@ -61,7 +52,6 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot) public void RemoveTransaction(Transaction tx) { - if (unverifiedTx.Remove(tx.Hash)) return; if ((senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee) == 0) senderFee.Remove(tx.Sender); var oracle = tx.GetAttribute(); From 979639b1a75881bfa83ee9e317a82016706bc004 Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 26 Nov 2020 09:47:53 +0100 Subject: [PATCH 10/13] Fix duplicate --- .../Ledger/TransactionVerificationContext.cs | 3 +-- .../UT_TransactionVerificationContext.cs | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index fa115b34e8..3ddb4fe456 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -41,8 +41,7 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot) if (balance < fee) return false; var oracle = tx.GetAttribute(); - if (oracle != null && - (!oracleResponses.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash)) + if (oracle != null && (oracleResponses.ContainsKey(oracle.Id))) { return false; } diff --git a/tests/neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs b/tests/neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs index a34a8cfb91..311cf7fd51 100644 --- a/tests/neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs +++ b/tests/neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs @@ -47,6 +47,29 @@ private Transaction CreateTransactionWithFee(long networkFee, long systemFee) return mock.Object; } + [TestMethod] + public void TestDuplicateOracle() + { + // Fake balance + var snapshot = Blockchain.Singleton.GetSnapshot(); + + ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, long.MaxValue); + BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, UInt160.Zero); + NativeContract.GAS.Burn(engine, UInt160.Zero, balance); + NativeContract.GAS.Mint(engine, UInt160.Zero, 8); + + // Test + TransactionVerificationContext verificationContext = new TransactionVerificationContext(); + var tx = CreateTransactionWithFee(1, 2); + tx.Attributes = new TransactionAttribute[] { new OracleResponse() { Code = OracleResponseCode.ConsensusUnreachable, Id = 1, Result = new byte[0] } }; + verificationContext.CheckTransaction(tx, snapshot).Should().BeTrue(); + verificationContext.AddTransaction(tx); + + tx = CreateTransactionWithFee(2, 1); + tx.Attributes = new TransactionAttribute[] { new OracleResponse() { Code = OracleResponseCode.ConsensusUnreachable, Id = 1, Result = new byte[0] } }; + verificationContext.CheckTransaction(tx, snapshot).Should().BeFalse(); + } + [TestMethod] public void TestTransactionSenderFee() { From 034c49ba5b8ea1ec19f4192489b0ed39c72908ac Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Thu, 26 Nov 2020 16:59:37 +0800 Subject: [PATCH 11/13] Remove unnecessary parentheses. --- src/neo/Ledger/TransactionVerificationContext.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index 3ddb4fe456..1594cde59e 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -41,10 +41,8 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot) if (balance < fee) return false; var oracle = tx.GetAttribute(); - if (oracle != null && (oracleResponses.ContainsKey(oracle.Id))) - { + if (oracle != null && oracleResponses.ContainsKey(oracle.Id)) return false; - } return true; } From 14c5c042de2e812f07a8b14bf639f6d09bdc8131 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Thu, 26 Nov 2020 17:02:21 +0800 Subject: [PATCH 12/13] Simplify AddTransaction --- src/neo/Ledger/TransactionVerificationContext.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index 1594cde59e..5504677bda 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -21,10 +21,7 @@ public class TransactionVerificationContext public void AddTransaction(Transaction tx) { var oracle = tx.GetAttribute(); - if (oracle != null && !oracleResponses.TryAdd(oracle.Id, tx.Hash)) - { - return; - } + if (oracle != null) oracleResponses.Add(oracle.Id, tx.Hash); if (senderFee.TryGetValue(tx.Sender, out var value)) senderFee[tx.Sender] = value + tx.SystemFee + tx.NetworkFee; From 00804bd96c6bd17708290987f6829ba63c3474ed Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Thu, 26 Nov 2020 17:05:14 +0800 Subject: [PATCH 13/13] Simplify RemoveTransaction --- src/neo/Ledger/TransactionVerificationContext.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/neo/Ledger/TransactionVerificationContext.cs b/src/neo/Ledger/TransactionVerificationContext.cs index 5504677bda..e291974137 100644 --- a/src/neo/Ledger/TransactionVerificationContext.cs +++ b/src/neo/Ledger/TransactionVerificationContext.cs @@ -49,10 +49,7 @@ public void RemoveTransaction(Transaction tx) if ((senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee) == 0) senderFee.Remove(tx.Sender); var oracle = tx.GetAttribute(); - if (oracle != null && oracleResponses.TryGetValue(oracle.Id, out var hash) && hash == tx.Hash) - { - oracleResponses.Remove(oracle.Id); - } + if (oracle != null) oracleResponses.Remove(oracle.Id); } } }