bitshares / bitshares-core

BitShares Blockchain node and command-line wallet
https://bitshares.github.io/
Other
1.17k stars 648 forks source link

client rejected message sent by peer #978

Open abitmore opened 6 years ago

abitmore commented 6 years ago

After firstly synced, when witness_node is in normal running mode, I noticed lots of messages in p2p.log (with log level warn):

p2p:message read_loop process_ordinary_mes ] client rejected message sent by peer xxx.xxx.xxx.xxx {"code":10,"name":"assert_exception","message":"Assert Exception","stack":[{"context":{"level":"error","file":"db_block.cpp","line":579,"method":"_apply_transaction","hostname":"","thread_name":"th_a","timestamp":"x"},"format":"(skip & skip_transaction_dupe_check) || trx_idx.indices().get().find(trx_id) == trx_idx.indices().get().end():

This occurs when p2p failed to push a transaction to object database.

Possible optimization:

Thoughts?

abitmore commented 6 years ago

By the way, the biggest p2p log file on my test node is 35 MB, which is for a hour. That means we need around 35 MB * 24 * 7 ~= 6 GB of disk space to store p2p log files (which IMO is fine).

btsabc commented 6 years ago

I think there are 2 possible ways this can happen:

  1. transaction message hash in p2p layer includes all contents of transaction , but blockchain layer transaction's id (or hash) not includes the signature, so p2p layer cannot prevent receiving the same transaction (same id) with different signatures.
  2. node.cpp::on_item_ids_inventory_message does not check messages received from inactive connections infact.

so,we should add new check code.

related:

https://github.com/bitshares/bitshares-core/blob/3293a95face10dd6b0716a596987deab66278f54/libraries/net/node.cpp#L4950-L4957

https://github.com/bitshares/bitshares-core/blob/3293a95face10dd6b0716a596987deab66278f54/libraries/net/node.cpp#L2856-L2883

abitmore commented 5 years ago

fill_or_kill is another thing that is spamming the p2p log heavily.

https://github.com/bitshares/bitshares-core/blob/a0e7dff00cfe7d2d8a7b257d8fdc95cf668108d7/libraries/net/node.cpp#L3398-L3404

It's better to only log useful exceptions.

abitmore commented 5 years ago

Suppressed some p2p logs in #1875, but didn't deal with the duplicate transaction pushing issue, so I'd like to keep this issue open.