Fix Oracle PostPersist#2074
Conversation
There was a problem hiding this comment.
Isn't it solved by
neo/src/neo/Network/P2P/Payloads/OracleResponse.cs
Lines 71 to 72 in a1590b0
| if (oracle != null && | ||
| (!oracleRequests.TryGetValue(oracle.Id, out var hash) || hash != tx.Hash)) | ||
| { | ||
| return false; |
There was a problem hiding this comment.
BTW, this can be replaced with generic "Conflicts" mechanism (#1991) that would also allow for proper transaction to have higher priority over fallback.
vncoelho
left a comment
There was a problem hiding this comment.
Regarding the description of checking if the transaction was sucesfull you removed that part, right?
As it is right now it is only improving the caching mechanism for avoiding duplicate txs?
|
UT failed |
| /// </summary> | ||
| private readonly Dictionary<ulong, UInt256> oracleResponses = new Dictionary<ulong, UInt256>(); | ||
|
|
||
| public void AddTransaction(Transaction tx) |
There was a problem hiding this comment.
Why don't remove CheckTransaction and Change Add to bool TryAdd (thread-safe)?
There was a problem hiding this comment.
Because CheckTransaction is called in Transaction.Verify().
|
Merge? |
When processing an oracle’s response transaction, it is not verified that the transaction is successful which allows a group of oracle’s nodes to sign a transaction that will throw an exception in the
PostPersistmethod, causing a Denial of Service.Found by @Red4Sec