Oracle: Remove some TODOs (#1574)

* Remove some todos

* Remove one method

* Reuse ManualWitness

* dotnet format

* Allow both tx in the same block without change order or fee

* Fix p2p message serialization

* Fix p2p

* Fixes and IInventory

* Fix akka message

* Fix ut

src/neo/Ledger/Blockchain.cs

@@ -62,7 +62,7 @@ public class RelayResult { public IInventory Inventory; public VerifyResult Resu
         private uint stored_header_count = 0;
         private readonly Dictionary<UInt256, Block> block_cache = new Dictionary<UInt256, Block>();
         private readonly Dictionary<uint, LinkedList<Block>> block_cache_unverified = new Dictionary<uint, LinkedList<Block>>();
-        internal readonly RelayCache ConsensusRelayCache = new RelayCache(100);
+        internal readonly RelayCache RelayCache = new RelayCache(100);
         private SnapshotView currentSnapshot;
 
         public IStore Store { get; }
@@ -299,6 +299,7 @@ private void OnInventory(IInventory inventory, bool relay = true)
                     Block block => OnNewBlock(block),
                     Transaction transaction => OnNewTransaction(transaction),
                     ConsensusPayload payload => OnNewConsensus(payload),
+                    OraclePayload payload => OnNewOracle(payload),
                     _ => VerifyResult.Unknown
                 }
             };
@@ -390,7 +391,15 @@ private VerifyResult OnNewConsensus(ConsensusPayload payload)
         {
             if (!payload.Verify(currentSnapshot)) return VerifyResult.Invalid;
             system.Consensus?.Tell(payload);
-            ConsensusRelayCache.Add(payload);
+            RelayCache.Add(payload);
+            return VerifyResult.Succeed;
+        }
+
+        private VerifyResult OnNewOracle(OraclePayload payload)
+        {
+            if (!payload.Verify(currentSnapshot)) return VerifyResult.Invalid;
+            system.Oracle?.Tell(payload);
+            RelayCache.Add(payload);
             return VerifyResult.Succeed;
         }
 
@@ -468,6 +477,9 @@ protected override void OnReceive(object message)
                 case ConsensusPayload payload:
                     OnInventory(payload);
                     break;
+                case OraclePayload oracle:
+                    OnInventory(oracle);
+                    break;
                 case Idle _:
                     if (MemPool.ReVerifyTopUnverifiedTransactionsIfNeeded(MaxTxToReverifyPerIdle, currentSnapshot))
                         Self.Tell(Idle.Instance, ActorRefs.NoSender);

src/neo/Ledger/MemoryPool.cs

@@ -170,11 +170,13 @@ public IEnumerator<Transaction> GetEnumerator()
 
         public IEnumerable<Transaction> GetVerifiedTransactions(StoreView snapshot)
         {
+            Transaction[] ret;
+            var oracle = new PoolItem.DelayState();
+
             _txRwLock.EnterReadLock();
             try
             {
-                var oracle = new PoolItem.OracleState();
-                return _unsortedTransactions
+                ret = _unsortedTransactions
                     .Where(u => u.Value.IsReady(snapshot, oracle))
                     .Select(p => p.Value.Tx)
                     .ToArray();
@@ -183,6 +185,18 @@ public IEnumerable<Transaction> GetVerifiedTransactions(StoreView snapshot)
             {
                 _txRwLock.ExitReadLock();
             }
+
+            // Fetch transactions
+
+            foreach (var tx in ret)
+            {
+                yield return tx;
+            }
+            foreach (var delayed in oracle.Delayed)
+            {
+                if (oracle.Allowed.Contains(delayed.Hash))
+                    yield return delayed;
+            }
         }
 
         public void GetVerifiedAndUnverifiedTransactions(out IEnumerable<Transaction> verifiedTransactions,
@@ -202,11 +216,13 @@ public IEnumerable<Transaction> GetVerifiedTransactions(StoreView snapshot)
 
         public IEnumerable<Transaction> GetSortedVerifiedTransactions(StoreView snapshot)
         {
+            Transaction[] ret;
+            var oracle = new PoolItem.DelayState();
+
             _txRwLock.EnterReadLock();
             try
             {
-                var oracle = new PoolItem.OracleState();
-                return _sortedTransactions
+                ret = _sortedTransactions
                     .Reverse()
                     .Where(u => u.IsReady(snapshot, oracle))
                     .Select(p => p.Tx)
@@ -216,6 +232,18 @@ public IEnumerable<Transaction> GetSortedVerifiedTransactions(StoreView snapshot
             {
                 _txRwLock.ExitReadLock();
             }
+
+            // Fetch transactions
+
+            foreach (var tx in ret)
+            {
+                yield return tx;
+            }
+            foreach (var delayed in oracle.Delayed)
+            {
+                if (oracle.Allowed.Contains(delayed.Hash))
+                    yield return delayed;
+            }
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

src/neo/Ledger/PoolItem.cs

@@ -36,18 +36,19 @@ internal PoolItem(Transaction tx)
             LastBroadcastTimestamp = Timestamp;
         }
 
-        internal class OracleState
+        internal class DelayState
         {
-            public HashSet<UInt256> AllowedRequests = new HashSet<UInt256>();
+            public HashSet<UInt256> Allowed = new HashSet<UInt256>();
+            public HashSet<Transaction> Delayed = new HashSet<Transaction>();
         }
 
-        public bool IsReady(StoreView snapshot, OracleState oracle)
+        public bool IsReady(StoreView snapshot, DelayState state)
         {
             switch (Tx.Version)
             {
                 case TransactionVersion.OracleRequest:
                     {
-                        if (oracle.AllowedRequests.Contains(Tx.Hash))
+                        if (state.Allowed.Remove(Tx.Hash))
                         {
                             // The response was already fetched, we can put request and response in the same block
 
@@ -64,15 +65,16 @@ public bool IsReady(StoreView snapshot, OracleState oracle)
                             else
                             {
                                 // If the response it's in the pool it's located after the request
-                                // TODO: We can order the pool first for OracleResponses
+                                // We save the request in order to put after the response
 
+                                state.Delayed.Add(Tx);
                                 return false;
                             }
                         }
                     }
                 case TransactionVersion.OracleResponse:
                     {
-                        oracle.AllowedRequests.Add(Tx.OracleRequestTx);
+                        state.Allowed.Add(Tx.OracleRequestTx);
                         break;
                     }
             }

src/neo/Network/P2P/Payloads/InventoryType.cs

@@ -4,6 +4,7 @@ public enum InventoryType : byte
     {
         TX = MessageCommand.Transaction,
         Block = MessageCommand.Block,
-        Consensus = MessageCommand.Consensus
+        Consensus = MessageCommand.Consensus,
+        Oracle = MessageCommand.Oracle,
     }
 }

src/neo/Network/P2P/Payloads/OraclePayload.cs

@@ -1,16 +1,16 @@
-using Neo.IO;
-using System.IO;
 using Neo.Cryptography;
 using Neo.Cryptography.ECC;
+using Neo.IO;
 using Neo.Persistence;
-using System;
-using System.Linq;
 using Neo.SmartContract;
 using Neo.SmartContract.Native;
+using System;
+using System.IO;
+using System.Linq;
 
 namespace Neo.Network.P2P.Payloads
 {
-    public class OraclePayload : IVerifiable
+    public class OraclePayload : IInventory
     {
         private byte[] _data;
         public byte[] Data
@@ -48,14 +48,14 @@ public int Size
             }
         }
 
-        private UInt160 _hash = null;
-        public UInt160 Hash
+        private UInt256 _hash = null;
+        public UInt256 Hash
         {
             get
             {
                 if (_hash == null)
                 {
-                    _hash = new UInt160(Crypto.Hash160(this.GetHashData()));
+                    _hash = new UInt256(Crypto.Hash256(this.GetHashData()));
                 }
                 return _hash;
             }
@@ -90,6 +90,8 @@ internal set
             }
         }
 
+        public InventoryType InventoryType => InventoryType.Oracle;
+
         public OracleResponseSignature GetDeserializedOracleSignature()
         {
             return OracleSignature;
@@ -98,7 +100,10 @@ public OracleResponseSignature GetDeserializedOracleSignature()
         void ISerializable.Deserialize(BinaryReader reader)
         {
             ((IVerifiable)this).DeserializeUnsigned(reader);
-            Witness = reader.ReadSerializable<Witness>();
+
+            var witness = reader.ReadSerializableArray<Witness>(1);
+            if (witness.Length != 1) throw new FormatException();
+            Witness = witness[0];
         }
         void IVerifiable.DeserializeUnsigned(BinaryReader reader)
         {
@@ -114,7 +119,7 @@ public virtual void Serialize(BinaryWriter writer)
 
         void IVerifiable.SerializeUnsigned(BinaryWriter writer)
         {
-            writer.Write(Data);
+            writer.WriteVarBytes(Data);
             writer.Write(OraclePub);
         }
 

src/neo/Network/P2P/ProtocolHandler.cs

@@ -94,6 +94,9 @@ private void OnMessage(Message msg)
                 case MessageCommand.Consensus:
                     OnInventoryReceived((ConsensusPayload)msg.Payload);
                     break;
+                case MessageCommand.Oracle:
+                    OnInventoryReceived((OraclePayload)msg.Payload);
+                    break;
                 case MessageCommand.FilterAdd:
                     OnFilterAddMessageReceived((FilterAddPayload)msg.Payload);
                     break;
@@ -137,9 +140,6 @@ private void OnMessage(Message msg)
                     if (msg.Payload.Size <= Transaction.MaxTransactionSize)
                         OnInventoryReceived((Transaction)msg.Payload);
                     break;
-                case MessageCommand.Oracle:
-                    OnOracleReceived((OraclePayload)msg.Payload);
-                    break;
                 case MessageCommand.Verack:
                 case MessageCommand.Version:
                     throw new ProtocolViolationException();
@@ -151,19 +151,6 @@ private void OnMessage(Message msg)
             }
         }
 
-        private void OnOracleReceived(OraclePayload payload)
-        {
-            // Verify
-
-            using var snapshot = Blockchain.Singleton.GetSnapshot();
-            if (payload.Verify(snapshot)) return;
-
-            // Send to oracle and relay it
-
-            system.Oracle?.Tell(payload);
-            Context.Parent.Tell(Message.Create(MessageCommand.Oracle, payload));
-        }
-
         private void OnAddrMessageReceived(AddrPayload payload)
         {
             system.LocalNode.Tell(new Peer.Peers
@@ -289,9 +276,13 @@ private void OnGetDataMessageReceived(InvPayload payload)
                         }
                         break;
                     case InventoryType.Consensus:
-                        if (Blockchain.Singleton.ConsensusRelayCache.TryGet(hash, out IInventory inventoryConsensus))
+                        if (Blockchain.Singleton.RelayCache.TryGet(hash, out IInventory inventoryConsensus))
                             Context.Parent.Tell(Message.Create(MessageCommand.Consensus, inventoryConsensus));
                         break;
+                    case InventoryType.Oracle:
+                        if (Blockchain.Singleton.RelayCache.TryGet(hash, out IInventory inventoryOracle))
+                            Context.Parent.Tell(Message.Create(MessageCommand.Oracle, inventoryOracle));
+                        break;
                 }
             }
         }

src/neo/Network/P2P/TaskManager.cs

@@ -292,7 +292,9 @@ internal protected override bool IsHighPriority(object message)
                 case TaskManager.RestartTasks _:
                     return true;
                 case TaskManager.NewTasks tasks:
-                    if (tasks.Payload.Type == InventoryType.Block || tasks.Payload.Type == InventoryType.Consensus)
+                    if (tasks.Payload.Type == InventoryType.Block ||
+                        tasks.Payload.Type == InventoryType.Oracle ||
+                        tasks.Payload.Type == InventoryType.Consensus)
                         return true;
                     return false;
                 default:

src/neo/Oracle/ManualWitness.cs

@@ -0,0 +1,33 @@
+using Neo.Network.P2P.Payloads;
+using Neo.Persistence;
+using System;
+using System.IO;
+
+namespace Neo.Oracle
+{
+    internal class ManualWitness : IVerifiable
+    {
+        public Witness[] Witnesses
+        {
+            get => throw new NotImplementedException();
+            set => throw new NotImplementedException();
+        }
+        public int Size => throw new NotImplementedException();
+        public void Deserialize(BinaryReader reader) => throw new NotImplementedException();
+        public void DeserializeUnsigned(BinaryReader reader) => throw new NotImplementedException();
+        public void Serialize(BinaryWriter writer) => throw new NotImplementedException();
+        public void SerializeUnsigned(BinaryWriter writer) => throw new NotImplementedException();
+
+        private readonly UInt160[] _hashes;
+
+        public ManualWitness(params UInt160[] hashes)
+        {
+            _hashes = hashes ?? Array.Empty<UInt160>();
+        }
+
+        public UInt160[] GetScriptHashesForVerifying(StoreView snapshot)
+        {
+            return _hashes;
+        }
+    }
+}