Skip to content
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

Send RelayResult to event stream #1495

Merged
merged 3 commits into from
Mar 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/neo/Consensus/ConsensusService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ internal ConsensusService(IActorRef localNode, IActorRef taskManager, ConsensusC

private bool AddTransaction(Transaction tx, bool verify)
{
if (verify && tx.Verify(context.Snapshot, context.SendersFeeMonitor.GetSenderFee(tx.Sender)) != RelayResultReason.Succeed)
if (verify && tx.Verify(context.Snapshot, context.SendersFeeMonitor.GetSenderFee(tx.Sender)) != VerifyResult.Succeed)
{
Log($"Invalid transaction: {tx.Hash}{Environment.NewLine}{tx.ToArray().ToHexString()}", LogLevel.Warning);
RequestChangeView(ChangeViewReason.TxInvalid);
Expand Down
76 changes: 41 additions & 35 deletions src/neo/Ledger/Blockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace Neo.Ledger
{
Expand All @@ -27,6 +26,7 @@ public class Import { public IEnumerable<Block> Blocks; public bool Verify = tru
public class ImportCompleted { }
public class FillMemoryPool { public IEnumerable<Transaction> Transactions; }
public class FillCompleted { }
public class RelayResult { public IInventory Inventory; public VerifyResult Result; }
shargon marked this conversation as resolved.
Show resolved Hide resolved

public static readonly uint MillisecondsPerBlock = ProtocolSettings.Default.MillisecondsPerBlock;
public const uint DecrementInterval = 2000000;
Expand Down Expand Up @@ -279,7 +279,7 @@ private void OnFillMemoryPool(IEnumerable<Transaction> transactions)
// First remove the tx if it is unverified in the pool.
MemPool.TryRemoveUnVerified(tx.Hash, out _);
// Verify the the transaction
if (tx.Verify(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(tx.Sender)) != RelayResultReason.Succeed)
if (tx.Verify(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(tx.Sender)) != VerifyResult.Succeed)
continue;
// Add to the memory pool
MemPool.TryAdd(tx.Hash, tx);
Expand All @@ -289,26 +289,44 @@ private void OnFillMemoryPool(IEnumerable<Transaction> transactions)
Sender.Tell(new FillCompleted());
}

private RelayResultReason OnNewBlock(Block block)
private void OnInventory(IInventory inventory, bool relay = true)
{
RelayResult rr = new RelayResult
{
Inventory = inventory,
Result = inventory switch
vncoelho marked this conversation as resolved.
Show resolved Hide resolved
{
Block block => OnNewBlock(block),
Transaction transaction => OnNewTransaction(transaction),
ConsensusPayload payload => OnNewConsensus(payload),
_ => VerifyResult.Unknown
}
};
if (relay && rr.Result == VerifyResult.Succeed)
system.LocalNode.Tell(new LocalNode.RelayDirectly { Inventory = inventory });
shargon marked this conversation as resolved.
Show resolved Hide resolved
Context.System.EventStream.Publish(rr);
}

private VerifyResult OnNewBlock(Block block)
{
if (block.Index <= Height)
return RelayResultReason.AlreadyExists;
return VerifyResult.AlreadyExists;
if (block_cache.ContainsKey(block.Hash))
return RelayResultReason.AlreadyExists;
return VerifyResult.AlreadyExists;
if (block.Index - 1 >= header_index.Count)
{
AddUnverifiedBlockToCache(block);
return RelayResultReason.UnableToVerify;
return VerifyResult.UnableToVerify;
}
if (block.Index == header_index.Count)
{
if (!block.Verify(currentSnapshot))
return RelayResultReason.Invalid;
return VerifyResult.Invalid;
}
else
{
if (!block.Hash.Equals(header_index[(int)block.Index]))
return RelayResultReason.Invalid;
return VerifyResult.Invalid;
}
if (block.Index == Height + 1)
{
Expand Down Expand Up @@ -365,16 +383,15 @@ private RelayResultReason OnNewBlock(Block block)
UpdateCurrentSnapshot();
}
}
return RelayResultReason.Succeed;
return VerifyResult.Succeed;
}

private RelayResultReason OnNewConsensus(ConsensusPayload payload)
private VerifyResult OnNewConsensus(ConsensusPayload payload)
vncoelho marked this conversation as resolved.
Show resolved Hide resolved
{
if (!payload.Verify(currentSnapshot)) return RelayResultReason.Invalid;
if (!payload.Verify(currentSnapshot)) return VerifyResult.Invalid;
system.Consensus?.Tell(payload);
ConsensusRelayCache.Add(payload);
system.LocalNode.Tell(new LocalNode.RelayDirectly { Inventory = payload });
return RelayResultReason.Succeed;
return VerifyResult.Succeed;
}

private void OnNewHeaders(Header[] headers)
Expand All @@ -398,25 +415,14 @@ private void OnNewHeaders(Header[] headers)
system.TaskManager.Tell(new TaskManager.HeaderTaskCompleted(), Sender);
}

private void OnNewTransaction(Transaction transaction, bool relay)
private VerifyResult OnNewTransaction(Transaction transaction)
{
RelayResultReason reason;
if (ContainsTransaction(transaction.Hash))
reason = RelayResultReason.AlreadyExists;
else if (!MemPool.CanTransactionFitInPool(transaction))
reason = RelayResultReason.OutOfMemory;
else
reason = transaction.Verify(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(transaction.Sender));

if (reason == RelayResultReason.Succeed)
{
if (!MemPool.TryAdd(transaction.Hash, transaction))
reason = RelayResultReason.OutOfMemory;
else if (relay)
system.LocalNode.Tell(new LocalNode.RelayDirectly { Inventory = transaction });
}

Sender.Tell(reason);
if (ContainsTransaction(transaction.Hash)) return VerifyResult.AlreadyExists;
if (!MemPool.CanTransactionFitInPool(transaction)) return VerifyResult.OutOfMemory;
Copy link
Member

@vncoelho vncoelho Mar 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if we could manipulate this order of verification, @erikzhang.

In moments which the mempool has plenty of pending txs we verify this and line 424 first, otherwise, they are the last checks to be verified.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L424 is an atomic operation and you cannot separate it.

VerifyResult reason = transaction.Verify(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(transaction.Sender));
if (reason != VerifyResult.Succeed) return reason;
if (!MemPool.TryAdd(transaction.Hash, transaction)) return VerifyResult.OutOfMemory;
return VerifyResult.Succeed;
}

private void OnPersistCompleted(Block block)
Expand All @@ -440,19 +446,19 @@ protected override void OnReceive(object message)
OnNewHeaders(headers);
break;
case Block block:
Sender.Tell(OnNewBlock(block));
OnInventory(block, false);
vncoelho marked this conversation as resolved.
Show resolved Hide resolved
break;
case Transaction[] transactions:
{
// This message comes from a mempool's revalidation, already relayed
foreach (var tx in transactions) OnNewTransaction(tx, false);
foreach (var tx in transactions) OnInventory(tx, false);
break;
}
case Transaction transaction:
OnNewTransaction(transaction, true);
OnInventory(transaction);
break;
case ConsensusPayload payload:
Sender.Tell(OnNewConsensus(payload));
OnInventory(payload);
break;
case Idle _:
if (MemPool.ReVerifyTopUnverifiedTransactionsIfNeeded(MaxTxToReverifyPerIdle, currentSnapshot))
Expand Down
2 changes: 1 addition & 1 deletion src/neo/Ledger/MemoryPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ private int ReverifyTransactions(SortedSet<PoolItem> verifiedSortedTxPool,
// Since unverifiedSortedTxPool is ordered in an ascending manner, we take from the end.
foreach (PoolItem item in unverifiedSortedTxPool.Reverse().Take(count))
{
if (item.Tx.VerifyForEachBlock(snapshot, SendersFeeMonitor.GetSenderFee(item.Tx.Sender)) == RelayResultReason.Succeed)
if (item.Tx.VerifyForEachBlock(snapshot, SendersFeeMonitor.GetSenderFee(item.Tx.Sender)) == VerifyResult.Succeed)
{
reverifiedItems.Add(item);
SendersFeeMonitor.AddSenderFee(item.Tx);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
namespace Neo.Ledger
{
public enum RelayResultReason : byte
public enum VerifyResult : byte
{
Succeed,
AlreadyExists,
Expand Down
2 changes: 0 additions & 2 deletions src/neo/Network/P2P/LocalNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,6 @@ protected override void OnReceive(object message)
case SendDirectly send:
OnSendDirectly(send.Inventory);
break;
case RelayResultReason _:
break;
}
}

Expand Down
30 changes: 15 additions & 15 deletions src/neo/Network/P2P/Payloads/Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -263,38 +263,38 @@ public static Transaction FromJson(JObject json)

bool IInventory.Verify(StoreView snapshot)
{
return Verify(snapshot, BigInteger.Zero) == RelayResultReason.Succeed;
return Verify(snapshot, BigInteger.Zero) == VerifyResult.Succeed;
}

public virtual RelayResultReason VerifyForEachBlock(StoreView snapshot, BigInteger totalSenderFeeFromPool)
public virtual VerifyResult VerifyForEachBlock(StoreView snapshot, BigInteger totalSenderFeeFromPool)
{
if (ValidUntilBlock <= snapshot.Height || ValidUntilBlock > snapshot.Height + MaxValidUntilBlockIncrement)
return RelayResultReason.Expired;
return VerifyResult.Expired;
UInt160[] hashes = GetScriptHashesForVerifying(snapshot);
if (NativeContract.Policy.GetBlockedAccounts(snapshot).Intersect(hashes).Any())
return RelayResultReason.PolicyFail;
return VerifyResult.PolicyFail;
BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, Sender);
BigInteger fee = SystemFee + NetworkFee + totalSenderFeeFromPool;
if (balance < fee) return RelayResultReason.InsufficientFunds;
if (hashes.Length != Witnesses.Length) return RelayResultReason.Invalid;
if (balance < fee) return VerifyResult.InsufficientFunds;
if (hashes.Length != Witnesses.Length) return VerifyResult.Invalid;
for (int i = 0; i < hashes.Length; i++)
{
if (Witnesses[i].VerificationScript.Length > 0) continue;
if (snapshot.Contracts.TryGet(hashes[i]) is null) return RelayResultReason.Invalid;
if (snapshot.Contracts.TryGet(hashes[i]) is null) return VerifyResult.Invalid;
}
return RelayResultReason.Succeed;
return VerifyResult.Succeed;
}

public virtual RelayResultReason Verify(StoreView snapshot, BigInteger totalSenderFeeFromPool)
public virtual VerifyResult Verify(StoreView snapshot, BigInteger totalSenderFeeFromPool)
{
RelayResultReason result = VerifyForEachBlock(snapshot, totalSenderFeeFromPool);
if (result != RelayResultReason.Succeed) return result;
VerifyResult result = VerifyForEachBlock(snapshot, totalSenderFeeFromPool);
if (result != VerifyResult.Succeed) return result;
int size = Size;
if (size > MaxTransactionSize) return RelayResultReason.Invalid;
if (size > MaxTransactionSize) return VerifyResult.Invalid;
long net_fee = NetworkFee - size * NativeContract.Policy.GetFeePerByte(snapshot);
if (net_fee < 0) return RelayResultReason.InsufficientFunds;
if (!this.VerifyWitnesses(snapshot, net_fee)) return RelayResultReason.Invalid;
return RelayResultReason.Succeed;
if (net_fee < 0) return VerifyResult.InsufficientFunds;
if (!this.VerifyWitnesses(snapshot, net_fee)) return VerifyResult.Invalid;
return VerifyResult.Succeed;
}

public StackItem ToStackItem(ReferenceCounter referenceCounter)
Expand Down
6 changes: 4 additions & 2 deletions tests/neo.UnitTests/Ledger/UT_Blockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,13 @@ public void TestValidTransaction()

var tx = CreateValidTx(walletA, acc.ScriptHash, 0);

system.ActorSystem.EventStream.Subscribe(senderProbe, typeof(Blockchain.RelayResult));

senderProbe.Send(system.Blockchain, tx);
senderProbe.ExpectMsg(RelayResultReason.Succeed);
senderProbe.ExpectMsg<Blockchain.RelayResult>(p => p.Result == VerifyResult.Succeed);

senderProbe.Send(system.Blockchain, tx);
senderProbe.ExpectMsg(RelayResultReason.AlreadyExists);
senderProbe.ExpectMsg<Blockchain.RelayResult>(p => p.Result == VerifyResult.AlreadyExists);
}
}

Expand Down
8 changes: 4 additions & 4 deletions tests/neo.UnitTests/Ledger/UT_MemoryPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ private Transaction CreateTransactionWithFee(long fee)
var randomBytes = new byte[16];
random.NextBytes(randomBytes);
Mock<Transaction> mock = new Mock<Transaction>();
mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(RelayResultReason.Succeed);
mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(RelayResultReason.Succeed);
mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
mock.Object.Script = randomBytes;
mock.Object.Sender = UInt160.Zero;
mock.Object.NetworkFee = fee;
Expand All @@ -99,8 +99,8 @@ private Transaction CreateTransactionWithFeeAndBalanceVerify(long fee)
random.NextBytes(randomBytes);
Mock<Transaction> mock = new Mock<Transaction>();
UInt160 sender = UInt160.Zero;
mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns((StoreView snapshot, BigInteger amount) => NativeContract.GAS.BalanceOf(snapshot, sender) >= amount + fee ? RelayResultReason.Succeed : RelayResultReason.InsufficientFunds);
mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(RelayResultReason.Succeed);
mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns((StoreView snapshot, BigInteger amount) => NativeContract.GAS.BalanceOf(snapshot, sender) >= amount + fee ? VerifyResult.Succeed : VerifyResult.InsufficientFunds);
mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
mock.Object.Script = randomBytes;
mock.Object.Sender = sender;
mock.Object.NetworkFee = fee;
Expand Down
4 changes: 2 additions & 2 deletions tests/neo.UnitTests/Ledger/UT_SendersFeeMonitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ private Transaction CreateTransactionWithFee(long networkFee, long systemFee)
var randomBytes = new byte[16];
random.NextBytes(randomBytes);
Mock<Transaction> mock = new Mock<Transaction>();
mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(RelayResultReason.Succeed);
mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(RelayResultReason.Succeed);
mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
mock.Object.Script = randomBytes;
mock.Object.Sender = UInt160.Zero;
mock.Object.NetworkFee = networkFee;
Expand Down
2 changes: 1 addition & 1 deletion tests/neo.UnitTests/Network/P2P/Payloads/UT_Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ public void Transaction_Reverify_Hashes_Length_Unequal_To_Witnesses_Length()
};
UInt160[] hashes = txSimple.GetScriptHashesForVerifying(snapshot);
Assert.AreEqual(2, hashes.Length);
Assert.AreNotEqual(RelayResultReason.Succeed, txSimple.VerifyForEachBlock(snapshot, BigInteger.Zero));
Assert.AreNotEqual(VerifyResult.Succeed, txSimple.VerifyForEachBlock(snapshot, BigInteger.Zero));
}

[TestMethod]
Expand Down