add the pendingList to TXNpool #214
Conversation
1.add the pendingList to TXNPool 2.make some optimization of TXNPool 3.delete some meaningless code 4.fixed some bugs. Signed-off-by: luodanwg <[email protected]>
| if err := va.VerifyTransactionWithLedger(tx, ledger.DefaultLedger); err != nil { | ||
| return errors.New("[VerifyTransactionWithLedger] error") | ||
| } | ||
| if !node.LocalNode().ExistedID(msg.txn.Hash()) { | ||
| node.LocalNode().AppendTxnPool(&(msg.txn)) |
There was a problem hiding this comment.
The verification functions shouldn't wrap into the appendTxnPool which have no relationship by watching the function name
There was a problem hiding this comment.
this is the old code which i have removed out , could you check it again ,thanks.
| txnCnt uint64 | ||
| list map[common.Uint256]*transaction.Transaction | ||
| txProcessList // transaction which have been verifyed will put into this map | ||
| tXNPendingList // transaction which didn't pass the verify will put into this map |
There was a problem hiding this comment.
Why this List is added and what is used for?
There was a problem hiding this comment.
- txProcessList is the origin txlist.
- PendingList is used to store all the tx which didn't pass the validation check. which can tell the user(sdk) failed reason and which need to redo or need other operations.
- assetIssueSummary is used to store all the issued amount in transaction pool , so the new tx only need to check the delta amount instead of check all the tx in pool. it can reduce the process time.
- inputUTXOList like the Implement Transaction Verification #3 ,instead of check all the tx in pool ,only need to check the delta of new tx.
| txProcessList // transaction which have been verifyed will put into this map | ||
| tXNPendingList // transaction which didn't pass the verify will put into this map | ||
| assetIssueSummary // transaction which pass the verify will summary the amout to this map | ||
| inputUTXOList // transaction which pass the verify will add the UTXO to this map |
| hash := txn.Hash() | ||
| // TODO: Call VerifyTransactionWithTxPool to verify tx | ||
| //verify transaction with Concurrency | ||
| if err, errCode := va.VerifyTransactionCanConcurrency(txn); err != nil { |
There was a problem hiding this comment.
It better move out the verification process from the AppendTxbPool process
| txnPool.txnCnt++ | ||
| txnPool.Unlock() | ||
| defer txnPool.Unlock() | ||
| if ok := txnPool.verifyTransactionWithTxPool(txn); !ok { |
There was a problem hiding this comment.
Rename the function in a meaningful and compact ways.
| if cleanPool == true { | ||
| txnPool.init() | ||
| //Attention: clean the trasaction Pool with committed transactions. | ||
| func (txnPool *TXNPool) CleanSubmittedTransactions(block *ledger.Block) error { |
There was a problem hiding this comment.
The parameter here should be the transaction list instead of the the block, what's your opinion
| return txnPool.getTransactionPendingReason(hash) | ||
| } | ||
|
|
||
| func (node *node) SynchronizeTxnPool() { |
There was a problem hiding this comment.
The function name and the behavior not 100% matching, it better using the get the neighbor node list and then request the txnpool one by one
There was a problem hiding this comment.
This is the origin func SynchronizeTxnPool() which i haven't changed yet.(diff tool's Highlight miss ).
I also think should picked out the nonexistent tx and request it individually . maybe the next step to handle transactions Synchronize.
| } | ||
|
|
||
| func (txnPool *TXNPool) verifyTransactionWithTxPool(txn *transaction.Transaction) bool { | ||
| //check weather have duplicate UTXO input |
There was a problem hiding this comment.
What's the benefit compare with verification transaction in one by one Vs verification the TXN in three different map. Any obvious performance improving in currently case?
|
resubmit the bug fix in another patch set, the multi queues design need further review |
1.add the pendingList to TXNPool
2.make some optimization of TXNPool
3.delete some meaningless code
4.fixed some bugs.
Signed-off-by: luodanwg [email protected]