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

Keep track of sender fee #1183

Merged
merged 27 commits into from
Nov 10, 2019
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
36c1d16
Merge pull request #1 from neo-project/master
Qiao-Jin Oct 22, 2019
a1af198
Merge pull request #3 from neo-project/master
Qiao-Jin Oct 23, 2019
21e49d5
Keep track of sender fee to avoid duplicate computation
Oct 23, 2019
512cc20
Code optimization
Oct 23, 2019
fd6e49c
Optimize
shargon Oct 23, 2019
ff11bb7
Optimize
shargon Oct 23, 2019
e12e5d4
Optimize
shargon Oct 23, 2019
fd2fa7c
Code optimization
Oct 23, 2019
bd99e8d
Correction
Oct 23, 2019
1e38c63
Merge branch 'master' into keep-track-of-sender-fee
vncoelho Oct 23, 2019
d3f1501
Merge branch 'master' into keep-track-of-sender-fee
Qiao-Jin Oct 24, 2019
34019b5
Merge branch 'master' into keep-track-of-sender-fee
vncoelho Oct 29, 2019
7ec1f03
Merge branch 'master' into keep-track-of-sender-fee
vncoelho Oct 31, 2019
8549942
Renaming currentFee to totalSenderFeeFromPool
vncoelho Oct 31, 2019
5d7d8e9
Renaming on Verify as well
vncoelho Oct 31, 2019
ae7cd58
Add consideration for null Transactions
Nov 1, 2019
73dfe47
Move sender fee recording systems to class SendersMonitor
Nov 4, 2019
ef84a44
Code optimization
Nov 5, 2019
a3b6dd5
Capitalize public items
Nov 5, 2019
9e5c80e
Code optimization
Nov 5, 2019
c0bd129
Code optimization
Nov 5, 2019
9b9e85f
Merge branch 'master' into keep-track-of-sender-fee
Qiao-Jin Nov 5, 2019
ee5d901
Merge branch 'master' into keep-track-of-sender-fee
shargon Nov 6, 2019
0f60099
Optimization
shargon Nov 6, 2019
bb3a0f5
Code optimization
Nov 7, 2019
8da1c33
Merge branch 'master' into keep-track-of-sender-fee
shargon Nov 7, 2019
d2cb4a9
Merge branch 'master' into keep-track-of-sender-fee
vncoelho Nov 10, 2019
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
5 changes: 3 additions & 2 deletions neo.UnitTests/Ledger/UT_MemoryPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics;

namespace Neo.UnitTests.Ledger
{
Expand Down Expand Up @@ -73,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.Reverify(It.IsAny<Snapshot>(), It.IsAny<IEnumerable<Transaction>>())).Returns(true);
mock.Setup(p => p.Verify(It.IsAny<Snapshot>(), It.IsAny<IEnumerable<Transaction>>())).Returns(true);
mock.Setup(p => p.Reverify(It.IsAny<Snapshot>(), It.IsAny<BigInteger>())).Returns(true);
mock.Setup(p => p.Verify(It.IsAny<Snapshot>(), It.IsAny<BigInteger>())).Returns(true);
mock.Object.Script = randomBytes;
mock.Object.Sender = UInt160.Zero;
mock.Object.NetworkFee = fee;
Expand Down
2 changes: 1 addition & 1 deletion neo.UnitTests/Network/P2P/Payloads/UT_Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ public void Transaction_Reverify_Hashes_Length_Unequal_To_Witnesses_Length()
};
UInt160[] hashes = txSimple.GetScriptHashesForVerifying(snapshot);
Assert.AreEqual(2, hashes.Length);
Assert.IsFalse(txSimple.Reverify(snapshot, new Transaction[0]));
Assert.IsFalse(txSimple.Reverify(snapshot, BigInteger.Zero));
}

[TestMethod]
Expand Down
23 changes: 23 additions & 0 deletions neo/Consensus/ConsensusContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Numerics;
using System.Runtime.CompilerServices;

namespace Neo.Consensus
Expand All @@ -29,6 +30,7 @@ internal class ConsensusContext : IDisposable, ISerializable
public int MyIndex;
public UInt256[] TransactionHashes;
public Dictionary<UInt256, Transaction> Transactions;
public Dictionary<UInt160, BigInteger> SenderFee;
public ConsensusPayload[] PreparationPayloads;
public ConsensusPayload[] CommitPayloads;
public ConsensusPayload[] ChangeViewPayloads;
Expand Down Expand Up @@ -110,6 +112,25 @@ public void Deserialize(BinaryReader reader)
if (TransactionHashes.Length == 0 && !RequestSentOrReceived)
TransactionHashes = null;
Transactions = transactions.Length == 0 && !RequestSentOrReceived ? null : transactions.ToDictionary(p => p.Hash);
SenderFee = new Dictionary<UInt160, BigInteger>();
foreach (Transaction tx in Transactions.Values)
AddSenderFee(tx);
}

public void AddSenderFee(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);
}

public BigInteger GetSenderFee(UInt160 sender)
{
if (SenderFee.TryGetValue(sender, out var value))
return value;
else
return BigInteger.Zero;
}

public void Dispose()
Expand Down Expand Up @@ -245,6 +266,7 @@ internal void EnsureMaxBlockSize(IEnumerable<Transaction> txs)
txs = txs.Take((int)maxTransactionsPerBlock);
List<UInt256> hashes = new List<UInt256>();
Transactions = new Dictionary<UInt256, Transaction>();
SenderFee = new Dictionary<UInt160, BigInteger>();

// Expected block size
var blockSize = GetExpectedBlockSizeWithoutTransactions(txs.Count());
Expand All @@ -258,6 +280,7 @@ internal void EnsureMaxBlockSize(IEnumerable<Transaction> txs)

hashes.Add(tx.Hash);
Transactions.Add(tx.Hash, tx);
AddSenderFee(tx);
}

TransactionHashes = hashes.ToArray();
Expand Down
4 changes: 3 additions & 1 deletion 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.Transactions.Values))
if (verify && !tx.Verify(context.Snapshot, context.GetSenderFee(tx.Sender)))
{
Log($"Invalid transaction: {tx.Hash}{Environment.NewLine}{tx.ToArray().ToHexString()}", LogLevel.Warning);
RequestChangeView(ChangeViewReason.TxInvalid);
Expand All @@ -74,6 +74,7 @@ private bool AddTransaction(Transaction tx, bool verify)
return false;
}
context.Transactions[tx.Hash] = tx;
context.AddSenderFee(tx);
return CheckPrepareResponse();
}

Expand Down Expand Up @@ -423,6 +424,7 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
context.Block.ConsensusData.Nonce = message.Nonce;
context.TransactionHashes = message.TransactionHashes;
context.Transactions = new Dictionary<UInt256, Transaction>();
context.SenderFee = new Dictionary<UInt160, System.Numerics.BigInteger>();
for (int i = 0; i < context.PreparationPayloads.Length; i++)
if (context.PreparationPayloads[i] != null)
if (!context.PreparationPayloads[i].GetDeserializedMessage<PrepareResponse>().PreparationHash.Equals(payload.Hash))
Expand Down
4 changes: 2 additions & 2 deletions neo/Ledger/Blockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,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.GetVerifiedTransactions()))
if (!tx.Verify(currentSnapshot, MemPool.GetSenderFee(tx.Sender)))
continue;
// Add to the memory pool
MemPool.TryAdd(tx.Hash, tx);
Expand Down Expand Up @@ -370,7 +370,7 @@ private RelayResultReason OnNewTransaction(Transaction transaction, bool relay)
return RelayResultReason.AlreadyExists;
if (!MemPool.CanTransactionFitInPool(transaction))
return RelayResultReason.OutOfMemory;
if (!transaction.Verify(currentSnapshot, MemPool.GetVerifiedTransactions()))
if (!transaction.Verify(currentSnapshot, MemPool.GetSenderFee(transaction.Sender)))
return RelayResultReason.Invalid;
if (!NativeContract.Policy.CheckPolicy(transaction, currentSnapshot))
return RelayResultReason.PolicyFail;
Expand Down
37 changes: 36 additions & 1 deletion neo/Ledger/MemoryPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Threading;

Expand Down Expand Up @@ -38,6 +39,11 @@ public class MemoryPool : IReadOnlyCollection<Transaction>
/// </summary>
private readonly ReaderWriterLockSlim _txRwLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);

/// <summary>
/// Store all verified unsorted transactions' senders' fee currently in the pool.
/// </summary>
private readonly Dictionary<UInt160, BigInteger> _senderFee = new Dictionary<UInt160, BigInteger>();

/// <summary>
/// Store all verified unsorted transactions currently in the pool.
/// </summary>
Expand Down Expand Up @@ -165,6 +171,31 @@ public IEnumerator<Transaction> GetEnumerator()

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

public BigInteger GetSenderFee(UInt160 sender)
{
_txRwLock.EnterReadLock();
bool recorded = _senderFee.TryGetValue(sender, out var value);
_txRwLock.ExitReadLock();
if (recorded)
return value;
else
return BigInteger.Zero;
}

private void AddSenderFee(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);
}

private void RemoveSenderFee(Transaction tx)
{
_senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee;
if (_senderFee[tx.Sender] == 0) _senderFee.Remove(tx.Sender);
}

public IEnumerable<Transaction> GetVerifiedTransactions()
{
_txRwLock.EnterReadLock();
Expand Down Expand Up @@ -268,6 +299,7 @@ internal bool TryAdd(UInt256 hash, Transaction tx)
try
{
_unsortedTransactions.Add(hash, poolItem);
AddSenderFee(tx);
_sortedTransactions.Add(poolItem);

if (Count > Capacity)
Expand Down Expand Up @@ -310,6 +342,7 @@ private bool TryRemoveVerified(UInt256 hash, out PoolItem item)
return false;

_unsortedTransactions.Remove(hash);
RemoveSenderFee(item.Tx);
_sortedTransactions.Remove(item);

return true;
Expand Down Expand Up @@ -337,6 +370,7 @@ internal void InvalidateVerifiedTransactions()

// Clear the verified transactions now, since they all must be reverified.
_unsortedTransactions.Clear();
_senderFee.Clear();
_sortedTransactions.Clear();
}

Expand Down Expand Up @@ -409,7 +443,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.Reverify(snapshot, _unsortedTransactions.Select(p => p.Value.Tx)))
if (item.Tx.Reverify(snapshot, GetSenderFee(item.Tx.Sender)))
reverifiedItems.Add(item);
else // Transaction no longer valid -- it will be removed from unverifiedTxPool.
invalidItems.Add(item);
Expand All @@ -432,6 +466,7 @@ private int ReverifyTransactions(SortedSet<PoolItem> verifiedSortedTxPool,
{
if (_unsortedTransactions.TryAdd(item.Tx.Hash, item))
{
AddSenderFee(item.Tx);
verifiedSortedTxPool.Add(item);

if (item.LastBroadcastTimestamp < rebroadcastCutOffTime)
Expand Down
12 changes: 5 additions & 7 deletions neo/Network/P2P/Payloads/Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,14 @@ public UInt160[] GetScriptHashesForVerifying(Snapshot snapshot)
return hashes.OrderBy(p => p).ToArray();
}

public virtual bool Reverify(Snapshot snapshot, IEnumerable<Transaction> mempool)
public virtual bool Reverify(Snapshot snapshot, BigInteger currentFee)
Copy link
Member

@vncoelho vncoelho Oct 23, 2019

Choose a reason for hiding this comment

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

currentFee -> currentSenderTotalFees / currentPoolTotalFees?

I like pool because this method is generic not only to mempool

Copy link
Member

Choose a reason for hiding this comment

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

But currentFee looks good as well, however, I prefer a little bit more explanatory here.
When I looked I needed to double check, with a better naming it will be quite clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

current fee means sum of systemFee & networkFee of all transactions in mempool from a specified sender, maybe there is better name for that:)

Copy link
Member

Choose a reason for hiding this comment

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

It is good but not so good...aehauheuaea
Because it gives the idea that it is the currentFee of the transaction.

I will rename it to totalSenderFeesFromPool

Pool looks good because it is more generic to any set we want to compare against.

{
if (ValidUntilBlock <= snapshot.Height || ValidUntilBlock > snapshot.Height + MaxValidUntilBlockIncrement)
return false;
if (NativeContract.Policy.GetBlockedAccounts(snapshot).Intersect(GetScriptHashesForVerifying(snapshot)).Count() > 0)
return false;
BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, Sender);
BigInteger fee = SystemFee + NetworkFee;
if (balance < fee) return false;
fee += mempool.Where(p => p != this && p.Sender.Equals(Sender)).Select(p => (BigInteger)(p.SystemFee + p.NetworkFee)).Sum();
BigInteger fee = SystemFee + NetworkFee + currentFee;
if (balance < fee) return false;
UInt160[] hashes = GetScriptHashesForVerifying(snapshot);
if (hashes.Length != Witnesses.Length) return false;
Expand Down Expand Up @@ -206,12 +204,12 @@ public static Transaction FromJson(JObject json)

bool IInventory.Verify(Snapshot snapshot)
{
return Verify(snapshot, Enumerable.Empty<Transaction>());
return Verify(snapshot, BigInteger.Zero);
}

public virtual bool Verify(Snapshot snapshot, IEnumerable<Transaction> mempool)
public virtual bool Verify(Snapshot snapshot, BigInteger currentFee)
{
if (!Reverify(snapshot, mempool)) return false;
if (!Reverify(snapshot, currentFee)) return false;
int size = Size;
if (size > MaxTransactionSize) return false;
long net_fee = NetworkFee - size * NativeContract.Policy.GetFeePerByte(snapshot);
Expand Down