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

Fix transaction route #2116

Closed
wants to merge 12 commits into from
Closed

Fix transaction route #2116

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7e3bff7
Fix transaction route
Dec 3, 2020
ac882ca
optimize
Dec 3, 2020
12c333e
Fix
shargon Dec 3, 2020
7f5b579
Update RemoteNode.ProtocolHandler.cs
shargon Dec 3, 2020
ed109e6
move knownHashes logic to func top
Dec 4, 2020
58143de
Merge branch 'master' into fix_tx_route
Qiao-Jin Dec 7, 2020
b1ec598
Merge branch 'master' into fix_tx_route
Qiao-Jin Dec 8, 2020
67a2569
Merge branch 'master' into fix_tx_route
Qiao-Jin Dec 9, 2020
059e4ce
Merge branch 'master' into fix_tx_route
Qiao-Jin Dec 10, 2020
f019952
Merge branch 'master' into fix_tx_route
Qiao-Jin Dec 10, 2020
5ed24e8
Merge branch 'master' into fix_tx_route
Qiao-Jin Dec 14, 2020
a11df81
Merge branch 'master' into fix_tx_route
Qiao-Jin Dec 16, 2020
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
15 changes: 1 addition & 14 deletions src/neo/Ledger/Blockchain.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public class RelayResult { public IInventory Inventory; public VerifyResult Resu
private const int MaxTxToReverifyPerIdle = 10;
private static readonly object lockObj = new object();
private readonly NeoSystem system;
private readonly IActorRef txrouter;
private readonly List<UInt256> header_index = new List<UInt256>();
private uint stored_header_count = 0;
private readonly ConcurrentDictionary<UInt256, Block> block_cache = new ConcurrentDictionary<UInt256, Block>();
Expand Down Expand Up @@ -113,7 +112,6 @@ static Blockchain()
public Blockchain(NeoSystem system, IStore store)
{
this.system = system;
this.txrouter = Context.ActorOf(TransactionRouter.Props(system));
this.MemPool = new MemoryPool(system, ProtocolSettings.Default.MemoryPoolMaxTransactions);
this.Store = store;
this.View = new ReadOnlyView(store);
Expand Down Expand Up @@ -398,12 +396,9 @@ protected override void OnReceive(object message)
case Block block:
OnInventory(block, false);
break;
case Transaction tx:
OnTransaction(tx, true);
break;
case Transaction[] transactions:
// This message comes from a mempool's revalidation, already relayed
foreach (var tx in transactions) OnTransaction(tx, false);
foreach (var tx in transactions) OnNewTransaction(tx);
Copy link
Member

Choose a reason for hiding this comment

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

The policy changed. You need to re-verify the transactions. And if you revert this change, tps will be reduced to the same as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose Func OnNewTransaction will call func VerifyStateDependent and policy is checked there?

Copy link
Member

Choose a reason for hiding this comment

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

Previously, if the policy was changed, the transactions needed to be completely re-verified, including VerifyStateIndependent. I'm not sure if it is unnecessary now.

Copy link
Contributor Author

@Qiao-Jin Qiao-Jin Dec 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

VerifyStateIndependent will check only the witness correctness now. Its price will be calculated in VerifyDependent together with others like transfer output, etc. Besides I observed that func MaxVerificationGas is untouched in #2045. Is this correct?

Copy link
Member

Choose a reason for hiding this comment

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

Can you fix this in another pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2107 should have solved this problem

Copy link
Member

Choose a reason for hiding this comment

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

#2107 moves TxRouter to NeoSystem too. Can you create a pr for fixing OnNewTransaction only?

break;
case IInventory inventory:
OnInventory(inventory);
Expand All @@ -418,14 +413,6 @@ protected override void OnReceive(object message)
}
}

private void OnTransaction(Transaction tx, bool relay)
{
if (ContainsTransaction(tx.Hash))
SendRelayResult(tx, VerifyResult.AlreadyExists);
else
txrouter.Tell(new TransactionRouter.Task { Transaction = tx, Relay = relay }, Sender);
}

private void Persist(Block block)
{
using (SnapshotView snapshot = GetSnapshot())
Expand Down
2 changes: 2 additions & 0 deletions src/neo/NeoSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public class NeoSystem : IDisposable
public IActorRef LocalNode { get; }
internal IActorRef TaskManager { get; }
public IActorRef Consensus { get; private set; }
public IActorRef TxRouter { get; }

private readonly IStore store;
private ChannelsConfig start_message = null;
Expand All @@ -41,6 +42,7 @@ public NeoSystem(string storageEngine = null)
this.Blockchain = ActorSystem.ActorOf(Ledger.Blockchain.Props(this, store));
this.LocalNode = ActorSystem.ActorOf(Network.P2P.LocalNode.Props(this));
this.TaskManager = ActorSystem.ActorOf(Network.P2P.TaskManager.Props(this));
this.TxRouter = ActorSystem.ActorOf(TransactionRouter.Props(this));
foreach (var plugin in Plugin.Plugins)
plugin.OnPluginsLoaded();
}
Expand Down
6 changes: 5 additions & 1 deletion src/neo/Network/P2P/RemoteNode.ProtocolHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,15 +294,19 @@ private void OnInventoryReceived(IInventory inventory)
{
case Transaction transaction:
system.Consensus?.Tell(transaction);
system.TxRouter.Tell(inventory);
break;
case Block block:
if (block.Index > Blockchain.Singleton.Height + InvPayload.MaxHashesCount) return;
UpdateLastBlockIndex(block.Index, false);
system.Blockchain.Tell(inventory);
break;
default:
system.Blockchain.Tell(inventory);
break;
}
knownHashes.Add(inventory.Hash);
erikzhang marked this conversation as resolved.
Show resolved Hide resolved
system.TaskManager.Tell(inventory);
system.Blockchain.Tell(inventory);
}

private void OnInvMessageReceived(InvPayload payload)
Expand Down