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.

Sign up for GitHub

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

Jump to bottom

null checks for #3364 and fix some bugs #1

Closed
wants to merge 1 commit into from
Closed

null checks for #3364 and fix some bugs #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 21 additions & 17 deletions src/Neo/Ledger/MemoryPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ namespace Neo.Ledger
/// </summary>
public class MemoryPool : IReadOnlyCollection<Transaction>
{
public event EventHandler<Transaction> TransactionAdded;
public event EventHandler<TransactionRemovedEventArgs> TransactionRemoved;
public event EventHandler<Transaction>? TransactionAdded;
public event EventHandler<TransactionRemovedEventArgs>? TransactionRemoved;

// Allow a reverified transaction to be rebroadcast if it has been this many block times since last broadcast.
private const int BlocksTillRebroadcast = 10;
Expand Down Expand Up @@ -162,14 +162,14 @@ public bool ContainsKey(UInt256 hash)
/// <param name="hash">The hash of the <see cref="Transaction"/> to get.</param>
/// <param name="tx">When this method returns, contains the <see cref="Transaction"/> associated with the specified hash, if the hash is found; otherwise, <see langword="null"/>.</param>
/// <returns><see langword="true"/> if the <see cref="MemoryPool"/> contains a <see cref="Transaction"/> with the specified hash; otherwise, <see langword="false"/>.</returns>
public bool TryGetValue(UInt256 hash, out Transaction tx)
public bool TryGetValue(UInt256 hash, out Transaction? tx)
{
_txRwLock.EnterReadLock();
try
{
bool ret = _unsortedTransactions.TryGetValue(hash, out PoolItem item)
var ret = _unsortedTransactions.TryGetValue(hash, out var item)
|| _unverifiedTransactions.TryGetValue(hash, out item);
tx = ret ? item.Tx : null;
tx = ret ? item!.Tx : null;
return ret;
}
finally
Expand Down Expand Up @@ -252,13 +252,13 @@ public IEnumerable<Transaction> GetSortedVerifiedTransactions()
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static PoolItem GetLowestFeeTransaction(SortedSet<PoolItem> verifiedTxSorted,
SortedSet<PoolItem> unverifiedTxSorted, out SortedSet<PoolItem> sortedPool)
private static PoolItem? GetLowestFeeTransaction(SortedSet<PoolItem> verifiedTxSorted,
SortedSet<PoolItem> unverifiedTxSorted, out SortedSet<PoolItem>? sortedPool)
{
PoolItem minItem = unverifiedTxSorted.Min;
var minItem = unverifiedTxSorted.Min;
sortedPool = minItem != null ? unverifiedTxSorted : null;

PoolItem verifiedMin = verifiedTxSorted.Min;
var verifiedMin = verifiedTxSorted.Min;
if (verifiedMin == null) return minItem;

if (minItem != null && verifiedMin.CompareTo(minItem) >= 0)
Expand All @@ -270,7 +270,7 @@ private static PoolItem GetLowestFeeTransaction(SortedSet<PoolItem> verifiedTxSo
return minItem;
}

private PoolItem GetLowestFeeTransaction(out Dictionary<UInt256, PoolItem> unsortedTxPool, out SortedSet<PoolItem> sortedPool)
private PoolItem? GetLowestFeeTransaction(out Dictionary<UInt256, PoolItem> unsortedTxPool, out SortedSet<PoolItem>? sortedPool)
{
sortedPool = null;

Expand All @@ -291,7 +291,10 @@ internal bool CanTransactionFitInPool(Transaction tx)
{
if (Count < Capacity) return true;

return GetLowestFeeTransaction(out _, out _).CompareTo(tx) <= 0;
var item = GetLowestFeeTransaction(out _, out _);
if (item == null) return false;
Copy link
Author

Choose a reason for hiding this comment

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

This can fail if it's null


return item.CompareTo(tx) <= 0;
}

internal VerifyResult TryAdd(Transaction tx, DataCache snapshot)
Expand All @@ -305,7 +308,7 @@ internal VerifyResult TryAdd(Transaction tx, DataCache snapshot)

if (_unsortedTransactions.ContainsKey(tx.Hash)) return VerifyResult.AlreadyInPool;

List<Transaction> removedTransactions = null;
List<Transaction>? removedTransactions = null;
_txRwLock.EnterWriteLock();
try
{
Expand Down Expand Up @@ -378,7 +381,7 @@ private bool CheckConflicts(Transaction tx, out List<PoolItem> conflictsList)
// Step 2: check if unsorted transactions were in `tx`'s Conflicts attributes.
foreach (var hash in tx.GetAttributes<Conflicts>().Select(p => p.Hash))
{
if (_unsortedTransactions.TryGetValue(hash, out PoolItem unsortedTx))
if (_unsortedTransactions.TryGetValue(hash, out var unsortedTx))
{
if (!tx.Signers.Select(p => p.Account).Intersect(unsortedTx.Tx.Signers.Select(p => p.Account)).Any()) return false;
conflictsFeeSum += unsortedTx.Tx.NetworkFee;
Expand All @@ -400,7 +403,8 @@ private List<Transaction> RemoveOverCapacity()
List<Transaction> removedTransactions = new();
do
{
PoolItem minItem = GetLowestFeeTransaction(out var unsortedPool, out var sortedPool);
var minItem = GetLowestFeeTransaction(out var unsortedPool, out var sortedPool);
if (minItem == null || sortedPool == null) break;
Copy link
Author

Choose a reason for hiding this comment

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

This can fail if it's null, because lime 259 and 262


unsortedPool.Remove(minItem.Tx.Hash);
sortedPool.Remove(minItem);
Expand All @@ -417,7 +421,7 @@ private List<Transaction> RemoveOverCapacity()
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool TryRemoveVerified(UInt256 hash, out PoolItem item)
private bool TryRemoveVerified(UInt256 hash, out PoolItem? item)
{
if (!_unsortedTransactions.TryGetValue(hash, out item))
return false;
Expand All @@ -435,7 +439,7 @@ private void RemoveConflictsOfVerified(PoolItem item)
{
foreach (var h in item.Tx.GetAttributes<Conflicts>().Select(attr => attr.Hash))
{
if (_conflicts.TryGetValue(h, out HashSet<UInt256> conflicts))
if (_conflicts.TryGetValue(h, out var conflicts))
{
conflicts.Remove(item.Tx.Hash);
if (conflicts.Count() == 0)
Expand All @@ -447,7 +451,7 @@ private void RemoveConflictsOfVerified(PoolItem item)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool TryRemoveUnVerified(UInt256 hash, out PoolItem item)
internal bool TryRemoveUnVerified(UInt256 hash, out PoolItem? item)
{
if (!_unverifiedTransactions.TryGetValue(hash, out item))
return false;
Expand Down
Loading